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 2ff3160..bdc3a7e 100644
--- a/src/pkg/http/request.go
+++ b/src/pkg/http/request.go
@@ -238,9 +238,9 @@
// 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 @@
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 2889048..98fbcf4 100644
--- a/src/pkg/http/requestwrite_test.go
+++ b/src/pkg/http/requestwrite_test.go
@@ -274,13 +274,45 @@
// 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 062e7a0..b54508e 100644
--- a/src/pkg/http/transfer.go
+++ b/src/pkg/http/transfer.go
@@ -38,6 +38,9 @@
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 @@
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 @@
// 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"]
