net/http: ignore response body when forbidden by status code

A 1XX, 204, or 304 response may not include a response body according
to RFC 7230, section 3.3.3. If a buggy server returns a 204 or 304
response with a body that is chunked encoded, the invalid body is
currently made readable in the Response. This can lead to data races due
to the transport connection's read loop which does not wait for the body
EOF when the response status is 204 or 304.

The correct behavior is to ignore the body on a 204 or 304 response, and
treat the body data as the beginning of the next request on the
connection.

Updates #22330.

Change-Id: I89a457ceb783b6f66136d5bf9be0a9b0a04fa955
Reviewed-on: https://go-review.googlesource.com/71910
Reviewed-by: Tom Bergan <tombergan@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Tom Bergan <tombergan@google.com>

diff --git a/src/net/http/transfer.go b/src/net/http/transfer.go
index 2087ce5587a5b694664df164d0530c858667420b..a400a6abb1f7db72c976021a75ec2a9e62088ff3 100644 (file)
--- a/src/net/http/transfer.go
+++ b/src/net/http/transfer.go
@@ -497,7 +497,7 @@ func readTransfer(msg interface{}, r *bufio.Reader) (err error) {
        // or close connection when finished, since multipart is not supported yet
        switch {
        case chunked(t.TransferEncoding):
-               if noResponseBodyExpected(t.RequestMethod) {
+               if noResponseBodyExpected(t.RequestMethod) || !bodyAllowedForStatus(t.StatusCode) {
                        t.Body = NoBody
                } else {
                        t.Body = &body{src: internal.NewChunkedReader(r), hdr: msg, r: r, closing: t.Close}

diff --git a/src/net/http/transport_test.go b/src/net/http/transport_test.go
index b838fe90021f402eb32aa36b7919a441680dc137..31cf9170665ee6cb1bf9396e5ad5eaf95663a90e 100644 (file)
--- a/src/net/http/transport_test.go
+++ b/src/net/http/transport_test.go
@@ -4337,3 +4337,25 @@ func doFetchCheckPanic(tr *Transport, req *Request) (res *Response, err error, p
        res, err = tr.RoundTrip(req)
        return
 }
+
+// Issue 22330: do not allow the response body to be read when the status code
+// forbids a response body.
+func TestNoBodyOnChunked304Response(t *testing.T) {
+       defer afterTest(t)
+       cst := newClientServerTest(t, h1Mode, HandlerFunc(func(w ResponseWriter, r *Request) {
+               conn, buf, _ := w.(Hijacker).Hijack()
+               buf.Write([]byte("HTTP/1.1 304 NOT MODIFIED\r\nTransfer-Encoding: chunked\r\n\r\n0\r\n\r\n"))
+               buf.Flush()
+               conn.Close()
+       }))
+       defer cst.close()
+
+       res, err := cst.c.Get(cst.ts.URL)
+       if err != nil {
+               t.Fatal(err)
+       }
+
+       if res.Body != NoBody {
+               t.Errorf("Unexpected body on 304 response")
+       }
+}