net/http: http2 with header `connection:close` causes the failure of rewinding body after connection loss

[leizongmin]

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

$ go version
go version go1.20.4 darwin/amd64

Does this issue reproduce with the latest release?

Run the following code:

package main

import (
	"bytes"
	"fmt"
	"io"
	"log"
	"net/http"
	"sync"
)

func main() {
	// condition 1: request a http2 server
	const u = "https://http2.github.io/"
	const n = 30

	wg := sync.WaitGroup{}
	wg.Add(n)
	counter := 0

	makeRequest := func(u string) (*http.Response, error) {
		// condition 2: body is not nil
		body := io.NopCloser(bytes.NewBuffer([]byte("hello")))
		req, err := http.NewRequest("POST", u, body)
		if err != nil {
			return nil, err
		}

		// condition 3: add header `connection: close`
		req.Header.Set("connection", "close")

		res, err := http.DefaultClient.Do(req)
		return res, err
	}

	// condition 4: concurrent multiple requests
	for i := 0; i < n; i++ {
		go func(i int) {
			defer wg.Done()
			res, err := makeRequest(fmt.Sprintf("%s?i=%d", u, i))
			if err != nil {
				log.Println(err)
				counter++
				return
			}
			defer res.Body.Close()
		}(i)
	}

	wg.Wait()
	log.Println("total", n, "error", counter)
}

What operating system and processor architecture are you using (go env)?

go env Output

$ go env

What did you do?

What did you expect to see?

2023/09/05 16:52:40 total 30 error 0

What did you see instead?

2023/09/05 16:52:40 Post "https://http2.github.io/?i=29": net/http: cannot rewind body after connection loss
2023/09/05 16:52:40 Post "https://http2.github.io/?i=8": net/http: cannot rewind body after connection loss
2023/09/05 16:52:40 Post "https://http2.github.io/?i=16": net/http: cannot rewind body after connection loss
2023/09/05 16:52:40 Post "https://http2.github.io/?i=10": net/http: cannot rewind body after connection loss
2023/09/05 16:52:40 Post "https://http2.github.io/?i=5": net/http: cannot rewind body after connection loss
2023/09/05 16:52:40 Post "https://http2.github.io/?i=20": net/http: cannot rewind body after connection loss
2023/09/05 16:52:40 total 30 error 6

[panjf2000]

Change body := io.NopCloser(bytes.NewBuffer([]byte("hello"))) to body := bytes.NewBuffer([]byte("hello"))

The body can only be replayed after a connection loss when the body passed to net.NewRequest is one of the type *bytes.Buffer, *bytes.Reader, or *strings.Reader.

[panjf2000]

Duplicate of #62453, and that issue is more general for this kind of problem, so move the discussion to #62453.

[leizongmin]

@panjf2000 I think this issue is a bit different from #62453's situation.
The main problem here is that when the request header contains connection: close, since the http/2 connection is multiplexed, the server will close the connection after processing the first request. The error errCannotRewind is returned when there are other requests that have not sent the body.

According to the relevant description of RFC 7540:

8.1.2.2. Connection-Specific Header Fields
HTTP/2 does not use the Connection header field to indicate connection-specific header fields; in this protocol, connection-specific metadata is conveyed by other means. An endpoint MUST NOT generate an HTTP/2 message containing connection-specific header fields; any message containing connection-specific header fields MUST be treated as malformed (Section 8.1.2.6).

The only exception to this is the TE header field, which MAY be present in an HTTP/2 request; when it is, it MUST NOT contain any value other than "trailers".

This means that an intermediary transforming an HTTP/1.x message to HTTP/2 will need to remove any header fields nominated by the Connection header field, along with the Connection header field itself. Such intermediaries SHOULD also remove other connection-specific header fields, such as Keep-Alive, Proxy-Connection, Transfer-Encoding, and Upgrade, even if they are not nominated by the Connection header field.

Note: HTTP/2 purposefully does not support upgrade to another protocol. The handshake methods described in Section 3 are believed sufficient to negotiate the use of alternative protocols.

My question is, when the http client sends a request using the http/2 protocol, should the connection request header be removed?

[panjf2000]

OK, then we need to change the issue title

[panjf2000]

I just went through the code and found this:

go/src/net/http/h2_bundle.go

Lines 6412 to 6423 in 9af74e7

	// "Connection" headers aren't allowed in HTTP/2 (RFC 7540, 8.1.2.2),
	// but respect "Connection" == "close" to mean sending a GOAWAY and tearing
	// down the TCP connection when idle, like we do for HTTP/1.
	// TODO: remove more Connection-specific header fields here, in addition
	// to "Connection".
	if _, ok := rws.snapHeader["Connection"]; ok {
	v := rws.snapHeader.Get("Connection")
	delete(rws.snapHeader, "Connection")
	if v == "close" {
	rws.conn.startGracefulShutdown()
	}
	}

It turns out this behavior is expected, therefore, this is the end of the discussion.