http: fix handling of 0-lengthed http requests

Via Russ Ross' bug report on golang-nuts, it was not possible
to send an HTTP request with a zero length body with either a
Content-Length (it was stripped) or chunking (it wasn't set).

This means Go couldn't upload 0-length objects to Amazon S3.
(which aren't as silly as they might sound, as S3 objects can
have key/values associated with them, set in the headers)

Amazon further doesn't supported chunked uploads. (not Go's
problem, but we should be able to let users set an explicit
Content-Length, even if it's zero.)

To fix the ambiguity of an explicit zero Content-Length and
the Request struct's default zero value, users need to
explicit set TransferEncoding to []string{"identity"} to force
the Request.Write to include a Content-Length: 0.  identity is
in RFC 2616 but is ignored pretty much everywhere.  We don't
even then serialize it on the wire, since it's kinda useless,
except as an internal sentinel value.

The "identity" value is then documented, but most users can
ignore that because NewRequest now sets that.

And adds more tests.

R=golang-dev, rsc
CC=golang-dev
https://golang.org/cl/4603041

3 files changed, 49 insertions, 6 deletions

diff --git a/src/pkg/http/request.go b/src/pkg/http/request.go
index 2ff3160a95..bdc3a7e4fb 100644
--- a/src/pkg/http/request.go
+++ b/src/pkg/http/request.go
@@ -238,9 +238,9 @@ const defaultUserAgent = "Go http package"
 //	TransferEncoding
 //	Body
 //
-// If Body is present but Content-Length is <= 0, Write adds
-// "Transfer-Encoding: chunked" to the header. Body is closed after
-// it is sent.
+// If Body is present, Content-Length is <= 0 and TransferEncoding
+// hasn't been set to "identity", Write adds "Transfer-Encoding:
+// chunked" to the header. Body is closed after it is sent.
 func (req *Request) Write(w io.Writer) os.Error {
 	return req.write(w, false)
 }
@@ -488,6 +488,11 @@ func NewRequest(method, url string, body io.Reader) (*Request, os.Error) {
 		default:
 			req.ContentLength = -1 // chunked
 		}
+		if req.ContentLength == 0 {
+			// To prevent chunking and disambiguate this
+			// from the default ContentLength zero value.
+			req.TransferEncoding = []string{"identity"}
+		}
 	}
 
 	return req, nil
diff --git a/src/pkg/http/requestwrite_test.go b/src/pkg/http/requestwrite_test.go
index 2889048a94..98fbcf459b 100644
--- a/src/pkg/http/requestwrite_test.go
+++ b/src/pkg/http/requestwrite_test.go
@@ -274,13 +274,45 @@ func (rc *closeChecker) Close() os.Error {
 
 // TestRequestWriteClosesBody tests that Request.Write does close its request.Body.
 // It also indirectly tests NewRequest and that it doesn't wrap an existing Closer
-// inside a NopCloser.
+// inside a NopCloser, and that it serializes it correctly.
 func TestRequestWriteClosesBody(t *testing.T) {
 	rc := &closeChecker{Reader: strings.NewReader("my body")}
-	req, _ := NewRequest("GET", "http://foo.com/", rc)
+	req, _ := NewRequest("POST", "http://foo.com/", rc)
+	if g, e := req.ContentLength, int64(-1); g != e {
+		t.Errorf("got req.ContentLength %d, want %d", g, e)
+	}
 	buf := new(bytes.Buffer)
 	req.Write(buf)
 	if !rc.closed {
 		t.Error("body not closed after write")
 	}
+	if g, e := buf.String(), "POST / HTTP/1.1\r\nHost: foo.com\r\nUser-Agent: Go http package\r\nTransfer-Encoding: chunked\r\n\r\n7\r\nmy body\r\n0\r\n\r\n"; g != e {
+		t.Errorf("write:\n got: %s\nwant: %s", g, e)
+	}
+}
+
+func TestZeroLengthNewRequest(t *testing.T) {
+	var buf bytes.Buffer
+
+	// Writing with default identity encoding
+	req, _ := NewRequest("PUT", "http://foo.com/", strings.NewReader(""))
+	if len(req.TransferEncoding) == 0 || req.TransferEncoding[0] != "identity" {
+		t.Fatalf("got req.TransferEncoding of %v, want %v", req.TransferEncoding, []string{"identity"})
+	}
+	if g, e := req.ContentLength, int64(0); g != e {
+		t.Errorf("got req.ContentLength %d, want %d", g, e)
+	}
+	req.Write(&buf)
+	if g, e := buf.String(), "PUT / HTTP/1.1\r\nHost: foo.com\r\nUser-Agent: Go http package\r\nContent-Length: 0\r\n\r\n"; g != e {
+		t.Errorf("identity write:\n got: %s\nwant: %s", g, e)
+	}
+
+	// Overriding identity encoding and forcing chunked.
+	req, _ = NewRequest("PUT", "http://foo.com/", strings.NewReader(""))
+	req.TransferEncoding = nil
+	buf.Reset()
+	req.Write(&buf)
+	if g, e := buf.String(), "PUT / HTTP/1.1\r\nHost: foo.com\r\nUser-Agent: Go http package\r\nTransfer-Encoding: chunked\r\n\r\n0\r\n\r\n"; g != e {
+		t.Errorf("chunked write:\n got: %s\nwant: %s", g, e)
+	}
 }
diff --git a/src/pkg/http/transfer.go b/src/pkg/http/transfer.go
index 062e7a0ff7..b54508e7ad 100644
--- a/src/pkg/http/transfer.go
+++ b/src/pkg/http/transfer.go
@@ -38,6 +38,9 @@ func newTransferWriter(r interface{}) (t *transferWriter, err os.Error) {
 		t.TransferEncoding = rr.TransferEncoding
 		t.Trailer = rr.Trailer
 		atLeastHTTP11 = rr.ProtoAtLeast(1, 1)
+		if t.Body != nil && t.ContentLength <= 0 && len(t.TransferEncoding) == 0 && atLeastHTTP11 {
+			t.TransferEncoding = []string{"chunked"}
+		}
 	case *Response:
 		t.Body = rr.Body
 		t.ContentLength = rr.ContentLength
@@ -95,7 +98,7 @@ func (t *transferWriter) WriteHeader(w io.Writer) (err os.Error) {
 		if err != nil {
 			return
 		}
-	} else if t.ContentLength > 0 || t.ResponseToHEAD {
+	} else if t.ContentLength > 0 || t.ResponseToHEAD || (t.ContentLength == 0 && isIdentity(t.TransferEncoding)) {
 		io.WriteString(w, "Content-Length: ")
 		_, err = io.WriteString(w, strconv.Itoa64(t.ContentLength)+"\r\n")
 		if err != nil {
@@ -289,6 +292,9 @@ func readTransfer(msg interface{}, r *bufio.Reader) (err os.Error) {
 // Checks whether chunked is part of the encodings stack
 func chunked(te []string) bool { return len(te) > 0 && te[0] == "chunked" }
 
+// Checks whether the encoding is explicitly "identity".
+func isIdentity(te []string) bool { return len(te) == 1 && te[0] == "identity" }
+
 // Sanitize transfer encoding
 func fixTransferEncoding(requestMethod string, header Header) ([]string, os.Error) {
 	raw, present := header["Transfer-Encoding"]