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 <[email protected]>
Reviewed-by: Brad Fitzpatrick <[email protected]>
Run-TryBot: Tom Bergan <[email protected]>

Author: benburkert

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}

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")
+	}
+}