From 6e4966eb7fd8892265ce14817ed75db86180bf24 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Mon, 4 Apr 2011 19:43:36 -0700 Subject: [PATCH] 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 --- src/pkg/http/response_test.go | 22 ++++++++++++++++++++++ src/pkg/http/transfer.go | 11 +++++++++-- src/pkg/http/transport_test.go | 29 +++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 2 deletions(-) diff --git a/src/pkg/http/response_test.go b/src/pkg/http/response_test.go index bf63ccb9e96..ef67fdd2dc3 100644 --- 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 996e2897325..41614f144fe 100644 --- 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 8a77a485497..e46f830c828 100644 --- 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")