From 9c436ab7dca9898d013eef321f5b51feb56feb56 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Wed, 8 Jun 2011 15:59:23 -0700 Subject: 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 --- src/pkg/http/request.go | 11 ++++++++--- src/pkg/http/requestwrite_test.go | 36 ++++++++++++++++++++++++++++++++++-- src/pkg/http/transfer.go | 8 +++++++- 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"] -- cgit v1.2.3-54-g00ecf