http: ignore Transfer-Encoding on HEAD responses

Amazon S3 sends Transfer-Encoding "chunked"
on its 404 responses to HEAD requests for
missing objects.

We weren't ignoring the Transfer-Encoding
and were thus interpretting the subsequent
response headers as a chunk header from the
previous responses body (but a HEAD response
can't have a body)

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

diff --git a/src/pkg/http/response_test.go b/src/pkg/http/response_test.go
index bf63ccb9e9655a5c12869e35d61fa3e67184ba80..ef67fdd2dc374ec1d82412b75d4694813616cb95 100644 (file)
--- a/src/pkg/http/response_test.go
+++ b/src/pkg/http/response_test.go
@@ -164,6 +164,28 @@ var respTests = []respTest{
                "Body here\n",
        },
 
+       // Chunked response in response to a HEAD request (the "chunked" should
+       // be ignored, as HEAD responses never have bodies)
+       {
+               "HTTP/1.0 200 OK\r\n" +
+                       "Transfer-Encoding: chunked\r\n" +
+                       "\r\n",
+
+               Response{
+                       Status:        "200 OK",
+                       StatusCode:    200,
+                       Proto:         "HTTP/1.0",
+                       ProtoMajor:    1,
+                       ProtoMinor:    0,
+                       RequestMethod: "HEAD",
+                       Header:        Header{},
+                       Close:         true,
+                       ContentLength: 0,
+               },
+
+               "",
+       },
+
        // Status line without a Reason-Phrase, but trailing space.
        // (permitted by RFC 2616)
        {

diff --git a/src/pkg/http/transfer.go b/src/pkg/http/transfer.go
index 996e2897325384355aba1b1e20f020c3b55fb773..41614f144fe3ac999ba8338a2fdea81e97be0a4b 100644 (file)
--- a/src/pkg/http/transfer.go
+++ b/src/pkg/http/transfer.go
@@ -215,7 +215,7 @@ func readTransfer(msg interface{}, r *bufio.Reader) (err os.Error) {
        }
 
        // Transfer encoding, content length
-       t.TransferEncoding, err = fixTransferEncoding(t.Header)
+       t.TransferEncoding, err = fixTransferEncoding(t.RequestMethod, t.Header)
        if err != nil {
                return err
        }
@@ -289,13 +289,20 @@ func readTransfer(msg interface{}, r *bufio.Reader) (err os.Error) {
 func chunked(te []string) bool { return len(te) > 0 && te[0] == "chunked" }
 
 // Sanitize transfer encoding
-func fixTransferEncoding(header Header) ([]string, os.Error) {
+func fixTransferEncoding(requestMethod string, header Header) ([]string, os.Error) {
        raw, present := header["Transfer-Encoding"]
        if !present {
                return nil, nil
        }
 
        header["Transfer-Encoding"] = nil, false
+
+       // Head responses have no bodies, so the transfer encoding
+       // should be ignored.
+       if requestMethod == "HEAD" {
+               return nil, nil
+       }
+
        encodings := strings.Split(raw[0], ",", -1)
        te := make([]string, 0, len(encodings))
        // TODO: Even though we only support "identity" and "chunked"

diff --git a/src/pkg/http/transport_test.go b/src/pkg/http/transport_test.go
index 8a77a4854978e9c0d3e5a2f19c680fd89fe71fee..e46f830c82841a0bed2f40247671e2aaba8c6c01 100644 (file)
--- a/src/pkg/http/transport_test.go
+++ b/src/pkg/http/transport_test.go
@@ -296,6 +296,35 @@ func TestTransportHeadResponses(t *testing.T) {
        }
 }
 
+// TestTransportHeadChunkedResponse verifies that we ignore chunked transfer-encoding
+// on responses to HEAD requests.
+func TestTransportHeadChunkedResponse(t *testing.T) {
+       ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) {
+               if r.Method != "HEAD" {
+                       panic("expected HEAD; got " + r.Method)
+               }
+               w.Header().Set("Transfer-Encoding", "chunked") // client should ignore
+               w.Header().Set("x-client-ipport", r.RemoteAddr)
+               w.WriteHeader(200)
+       }))
+       defer ts.Close()
+
+       tr := &Transport{DisableKeepAlives: false}
+       c := &Client{Transport: tr}
+
+       res1, err := c.Head(ts.URL)
+       if err != nil {
+               t.Fatalf("request 1 error: %v", err)
+       }
+       res2, err := c.Head(ts.URL)
+       if err != nil {
+               t.Fatalf("request 2 error: %v", err)
+       }
+       if v1, v2 := res1.Header.Get("x-client-ipport"), res2.Header.Get("x-client-ipport"); v1 != v2 {
+               t.Errorf("ip/ports differed between head requests: %q vs %q", v1, v2)
+       }
+}
+
 func TestTransportNilURL(t *testing.T) {
        ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) {
                fmt.Fprintf(w, "Hi")