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

diff --git a/src/pkg/http/request.go b/src/pkg/http/request.go
index 2ff3160a95036a671ee6ecddc81b125ebb3bfb7a..bdc3a7e4fb64ef090efbb2ccc718e991c5ce9e9a 100644 (file)
--- 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 2889048a9404227533310ec304141ad714ba8729..98fbcf459bef36f81752302617a108667d96184e 100644 (file)
--- 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 062e7a0ff7512a79b862655baa1f526485a6bec6..b54508e7add2a14758c23ad0c30d49a1b9227e92 100644 (file)
--- 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"]