net/http: Response.Write should omit writing the body for HEAD requests

[mmatczuk]

What version of Go are you using (go version)?

go version go1.21.0 darwin/arm64

Does this issue reproduce with the latest release?

Yes

What did you do?

I'm writing a response to http HEAD request that contains chunked Transfer-Encoding.
The response as written contains additional "\r\n".

The program below demonstrates the behavior

package main

import (
	"bufio"
	"bytes"
	"io"
	"net"
	"net/http"
	"os"
	"os/exec"
	"testing"
)

func TestWriteHeadResponse(t *testing.T) {
	l, err := net.Listen("tcp", ":0")
	if err != nil {
		t.Fatal(err)
	}
	defer l.Close()

	go func() {
		conn, err := l.Accept()
		if err != nil {
			t.Fatal(err)
		}

		handleConn(t, conn)

		defer conn.Close()
	}()

	var buf bytes.Buffer

	cmd := exec.Command("curl", "-x", l.Addr().String(), "-v", "--head", "http://www.google.com")
	cmd.Stdout = os.Stdout
	cmd.Stderr = io.MultiWriter(&buf, os.Stderr)

	if err := cmd.Run(); err != nil {
		t.Fatal(err)
	}

	if bytes.Contains(buf.Bytes(), []byte("Excess found: excess = 2 url = / (zero-length body)")) {
		t.Fatal("excess found")
	}
}

func handleConn(t *testing.T, conn net.Conn) {
	brw := bufio.NewReadWriter(bufio.NewReader(conn), bufio.NewWriter(conn))
	defer brw.Flush()

	req, err := http.ReadRequest(brw.Reader)
	if err != nil {
		t.Fatal(err)
	}

	if req.Method != "HEAD" {
		t.Errorf("unexpected method: %s", req.Method)
	}

	res, err := http.DefaultTransport.RoundTrip(req)
	if err != nil {
		t.Fatal(err)
	}

	if err := res.Write(brw); err != nil {
		t.Fatal(err)
	}
}

Output:

=== RUN   TestWriteHeadResponse
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0*   Trying [::]:53490...
* Connected to :: (::1) port 53490 (#0)
> HEAD http://www.google.com/ HTTP/1.1
> Host: www.google.com
> User-Agent: curl/8.1.2
> Accept: */*
> Proxy-Connection: Keep-Alive
> 
< HTTP/1.1 200 OK
< Transfer-Encoding: chunked
< Cache-Control: private
< Content-Security-Policy-Report-Only: object-src 'none';base-uri 'self';script-src 'nonce-2fpqee0gttwFKLELsSVWiA' 'strict-dynamic' 'report-sample' 'unsafe-eval' 'unsafe-inline' https: http:;report-uri https://csp.withgoogle.com/csp/gws/other-hp
< Content-Type: text/html; charset=ISO-8859-1
< Date: Mon, 14 Aug 2023 14:28:16 GMT
< Expires: Mon, 14 Aug 2023 14:28:16 GMT
< Server: gws
< Set-Cookie: AEC=Ad49MVFERbRwKC9-DN6KNUJPfviWTjnnekKDlLJgfFbhCWrT6gCW7Ft63OA; expires=Sat, 10-Feb-2024 14:28:16 GMT; path=/; domain=.google.com; Secure; HttpOnly; SameSite=lax
< X-Frame-Options: SAMEORIGIN
< X-Xss-Protection: 0
< 
* Excess found: excess = 2 url = / (zero-length body)
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
HTTP/1.1 200 OK
Transfer-Encoding: chunked
Cache-Control: private
Content-Security-Policy-Report-Only: object-src 'none';base-uri 'self';script-src 'nonce-2fpqee0gttwFKLELsSVWiA' 'strict-dynamic' 'report-sample' 'unsafe-eval' 'unsafe-inline' https: http:;report-uri https://csp.withgoogle.com/csp/gws/other-hp
Content-Type: text/html; charset=ISO-8859-1
Date: Mon, 14 Aug 2023 14:28:16 GMT
Expires: Mon, 14 Aug 2023 14:28:16 GMT
Server: gws
Set-Cookie: AEC=Ad49MVFERbRwKC9-DN6KNUJPfviWTjnnekKDlLJgfFbhCWrT6gCW7Ft63OA; expires=Sat, 10-Feb-2024 14:28:16 GMT; path=/; domain=.google.com; Secure; HttpOnly; SameSite=lax
X-Frame-Options: SAMEORIGIN
X-Xss-Protection: 0

* Connection #0 to host :: left intact
    chunked_test.go:43: excess found
--- FAIL: TestWriteHeadResponse (0.33s)

FAIL

Note the Excess found: excess = 2 url = / (zero-length body)

If you add res.TransferEncoding = nil before writing the response, the test passes but the response comes with Connection: close.

Output:

=== RUN   TestWriteHeadResponse
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0*   Trying [::]:53555...
* Connected to :: (::1) port 53555 (#0)
> HEAD http://www.google.com/ HTTP/1.1
> Host: www.google.com
> User-Agent: curl/8.1.2
> Accept: */*
> Proxy-Connection: Keep-Alive
> 
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0< HTTP/1.1 200 OK
< Connection: close
< Cache-Control: private
< Content-Security-Policy-Report-Only: object-src 'none';base-uri 'self';script-src 'nonce-JT0gk8j00rtWa2ER4JQozg' 'strict-dynamic' 'report-sample' 'unsafe-eval' 'unsafe-inline' https: http:;report-uri https://csp.withgoogle.com/csp/gws/other-hp
< Content-Type: text/html; charset=ISO-8859-1
< Date: Mon, 14 Aug 2023 14:36:33 GMT
< Expires: Mon, 14 Aug 2023 14:36:33 GMT
< Server: gws
< Set-Cookie: AEC=Ad49MVFNMKsQRucydwAuOMpD55wbsCpuL8IRJwy8xM_oGNFHAspeS5ZMBpQ; expires=Sat, 10-Feb-2024 14:36:33 GMT; path=/; domain=.google.com; Secure; HttpOnly; SameSite=lax
< X-Frame-Options: SAMEORIGIN
< X-Xss-Protection: 0
< 
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
* Closing connection 0
HTTP/1.1 200 OK
Connection: close
Cache-Control: private
Content-Security-Policy-Report-Only: object-src 'none';base-uri 'self';script-src 'nonce-JT0gk8j00rtWa2ER4JQozg' 'strict-dynamic' 'report-sample' 'unsafe-eval' 'unsafe-inline' https: http:;report-uri https://csp.withgoogle.com/csp/gws/other-hp
Content-Type: text/html; charset=ISO-8859-1
Date: Mon, 14 Aug 2023 14:36:33 GMT
Expires: Mon, 14 Aug 2023 14:36:33 GMT
Server: gws
Set-Cookie: AEC=Ad49MVFNMKsQRucydwAuOMpD55wbsCpuL8IRJwy8xM_oGNFHAspeS5ZMBpQ; expires=Sat, 10-Feb-2024 14:36:33 GMT; path=/; domain=.google.com; Secure; HttpOnly; SameSite=lax
X-Frame-Options: SAMEORIGIN
X-Xss-Protection: 0

--- PASS: TestWriteHeadResponse (0.36s)
PASS

It makes it impossible to use http.Response::Write to implement

The HEAD method is identical to GET except that the server MUST NOT send content in the response. HEAD is used to obtain metadata about the selected representation without transferring its representation data, often for the sake of testing hypertext links or finding recent modifications.

https://www.rfc-editor.org/rfc/rfc9110.html#section-9.3.2

The code should write the readers and skip the body as specified.

[bcmills]

(CC @neild)

[seankhliao]

Note that the reproducer appears to rely on a specific version of curl (8.9.0 appears to just output { [2 bytes data] in lieu of Excess found: excess = 2 url = / (zero-length body))

[terinjokes]

It seems like this can also be triggered using the new server mux route handling, since HEAD requests will be sent to routes that match GET. I've tested the following with curl 8.8.0 which prints "Re-using existing connection with host 127.0.0.1" when the server isn't misbehaving (eg, HEAD response has no body, or code-as-written with GET requests).

head_test.go

package main

import (
	"bytes"
	"crypto/rand"
	"io"
	"net/http"
	"net/http/httptest"
	"os"
	"os/exec"
	"testing"
)

func TestWriteHeadResponse(t *testing.T) {
	mux := http.NewServeMux()
	mux.HandleFunc("GET /", func(w http.ResponseWriter, r *http.Request) {
		lr := &io.LimitedReader{
			R: rand.Reader,
			N: 1024,
		}
		w.WriteHeader(http.StatusOK)
		io.Copy(w, lr)
	})
	ts := httptest.NewServer(mux)
	defer ts.Close()

	var buf bytes.Buffer

	cmd := exec.Command("curl", "--trace-ascii", "%", "--head", ts.URL, ts.URL)
	cmd.Stdout = os.Stdout
	cmd.Stderr = io.MultiWriter(&buf, os.Stderr)

	if err := cmd.Run(); err != nil {
		t.Fatal(err)
	}

	if bytes.Contains(buf.Bytes(), []byte("Connection 0 seems to be dead")) {
		t.Fatal("curl had to reset connection")
	}
}

$ go test head_test.go

  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0== Info:   Trying 127.0.0.1:33933...
== Info: Connected to 127.0.0.1 (127.0.0.1) port 33933
=> Send header, 79 bytes (0x4f)
0000: HEAD / HTTP/1.1
0011: Host: 127.0.0.1:33933
0028: User-Agent: curl/8.8.0
0040: Accept: */*
004d:
== Info: Request completely sent off
<= Recv header, 17 bytes (0x11)
0000: HTTP/1.1 200 OK
<= Recv header, 37 bytes (0x25)
0000: Date: Fri, 26 Jul 2024 18:54:02 GMT
<= Recv header, 40 bytes (0x28)
0000: Content-Type: application/octet-stream
<= Recv header, 2 bytes (0x2)
0000:
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
== Info: Connection #0 to host 127.0.0.1 left intact
HTTP/1.1 200 OK
Date: Fri, 26 Jul 2024 18:54:02 GMT
Content-Type: application/octet-stream

== Info: Found bundle for host: 0x55cf7a8b16a0 [serially]
== Info: Can not multiplex, even if we wanted to
== Info: Connection 0 seems to be dead
== Info: Closing connection
== Info: Hostname 127.0.0.1 was found in DNS cache
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0== Info:   Trying 127.0.0.1:33933...
== Info: Connected to 127.0.0.1 (127.0.0.1) port 33933
=> Send header, 79 bytes (0x4f)
0000: HEAD / HTTP/1.1
0011: Host: 127.0.0.1:33933
0028: User-Agent: curl/8.8.0
0040: Accept: */*
004d:
== Info: Request completely sent off
<= Recv header, 17 bytes (0x11)
0000: HTTP/1.1 200 OK
<= Recv header, 37 bytes (0x25)
0000: Date: Fri, 26 Jul 2024 18:54:02 GMT
<= Recv header, 40 bytes (0x28)
0000: Content-Type: application/octet-stream
<= Recv header, 2 bytes (0x2)
0000:
<= Recv data, 512 bytes (0x200)
0000: .a..{fwq....M.....^.P5.....?....~-a...............I..'*..%.>(..c
0040: iQ1L.Hx..?..D7....#.Fb|...h..}B./J...........c...p..9.r{...<]v|.
0080: .w~..m?.....h.?"..c....:`....Z. ....5K..........7.{/.....Z.#.j.G
00c0: ...o..#....Q.z...i...n...bS........`7.qk.l.a..5bTX....T,q4......
0100: ..l,.Q.....R.'..Q.E...U.R..e..m...G.}.........~.2u..2A..%..rO...
0140: .....m.......X.U.s....c...k......_....h{O.P.,......Z.\4a.Eqc.F..
0180: .nf.....BC....,..^C.Ep*....5..$q~.k.Z...........2.s.3'C]...pF...
01c0: 9x/.K-...&E.T......F".........f...i......n..J..k2r..3....,.....V
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
== Info: Closing connection
HTTP/1.1 200 OK
Date: Fri, 26 Jul 2024 18:54:02 GMT
Content-Type: application/octet-stream

--- FAIL: TestWriteHeadResponse (0.01s)
    head_test.go:38: curl had to reset connection
FAIL
FAIL	command-line-arguments	0.014s
FAIL

I would expect any body written in a GET route handler for a HEAD method to be discarded.

[neild]

It seems like this can also be triggered using the new server mux route handling

This is #68609, a bug in ResponseWriter.ReadFrom, and unrelated.

[neild]

The original title of this issue is "ResponseWriter should omit writing the body for HEAD requests", but the included example doesn't use an http.Server or a ResponseWriter. It is instead using http.ReadRequest and http.Response.Write to implement a simple HTTP server.

The problem occurs here:

// res is an http.Response
if err := res.Write(brw); err != nil {
	t.Fatal(err)
}

The issue is that when res contains the response to a HEAD request with a Transfer-Encoding: chunked header, Response.Write always writes the terminal \r\n at the end of the chunked-body section even if it didn't write any body chunks:
https://go.googlesource.com/go/+/refs/tags/go1.22.5/src/net/http/transfer.go#403

Response.Write is more than a little underspecified, but I think this is clearly a bug, given that the chunked-body is invalid. (You need at least one chunk.)

[gopherbot]

Change https://go.dev/cl/601238 mentions this issue: net/http: don't write body for HEAD responses in Response.Write