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.
Comment From: bcmills
(CC @neild)
Comment From: 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))
Comment From: 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.
Comment From: 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.
Comment From: 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.)
Comment From: gopherbot
Change https://go.dev/cl/601238 mentions this issue: net/http: don't write body for HEAD responses in Response.Write