go: x/net/http2: RoundTrip hangs when the response status code > 299

Change https://golang.org/cl/323689 mentions this issue: http2: Transport will close the request body

Hi @fraenkel,

the issue has been reported against x/net/http2 module, however this package version has been integrated into Go 1.15 from the 8th revision (see the release notes and Go 1.16. Hence, it is now mainstream.

This has caused an unfortunate change of behaviour impacting any type of tunnelling relying on the proxy server to acknowledge/decline the transaction before writing the content in the body.

I am providing here below another example using the HTTP CONNECT verb, based on the discussion provided in https://github.com/golang/go/issues/13717.

func TestHTTP2Connect(t *testing.T) {
	testCases := []struct {
		name       string
		statusCode int
	}{
		{
			name:       "200 OK",
			statusCode: http.StatusOK,
		},
		{
			name:       "401 UNAUTHORIZED hangs",
			statusCode: http.StatusUnauthorized,
		},
	}

	for _, tc := range testCases {
		t.Run(tc.name, func(t *testing.T) {
			destinationServer := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
				if r.Method != http.MethodPost {
					w.WriteHeader(http.StatusMethodNotAllowed)
					return
				}

				data, err := ioutil.ReadAll(r.Body)
				if err != nil {
					log.Fatalf("destination request body read failure: %s", err)
				}

				w.WriteHeader(http.StatusOK)
				w.Write(append([]byte("received: "), data...))
			}))
			defer destinationServer.Close()

			destinationServer.EnableHTTP2 = true
			destinationServer.StartTLS()
			log.Print("destinationServer: " + destinationServer.URL)

			proxyServer := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
				if r.Method != http.MethodConnect {
					http.Error(w, http.StatusText(http.StatusMethodNotAllowed), http.StatusMethodNotAllowed)
					return
				}

				if tc.statusCode > 299 {
					http.Error(w, http.StatusText(tc.statusCode), tc.statusCode)
					return
				}

				log.Print(r.Host)
				conn, err := tls.Dial("tcp", r.Host, &tls.Config{
					InsecureSkipVerify: true,
				})
				if err != nil {
					http.Error(w, err.Error(), http.StatusServiceUnavailable)
					return
				}

				w.WriteHeader(tc.statusCode)
				w.(http.Flusher).Flush()

				wg := sync.WaitGroup{}

				wg.Add(1)
				go func() {
					if _, err := io.Copy(conn, r.Body); err != nil {
						log.Fatalf("outbound copy failure: %s", err)
					}
					r.Body.Close()
					conn.CloseWrite()
					wg.Done()
				}()

				wg.Add(1)
				go func() {
					if _, err := io.Copy(w, conn); err != nil {
						log.Fatalf("inbound copy failure: %s", err)
					}
					conn.Close()
					wg.Done()
				}()

				wg.Wait()
			}))
			defer proxyServer.Close()

			proxyServer.EnableHTTP2 = true
			proxyServer.StartTLS()
			log.Print("proxyServer: " + proxyServer.URL)

			pr, pw := io.Pipe()
			req, err := http.NewRequest(http.MethodConnect, proxyServer.URL, pr)
			if err != nil {
				log.Fatal(err)
			}
			req.ContentLength = -1

			destinationURL, err := url.Parse(destinationServer.URL)
			if err != nil {
				log.Fatal(err)
			}
			req.Host = destinationURL.Host

			resp, err := proxyServer.Client().Do(req)
			if err != nil {
				log.Fatalf("request execution failure: %s", err)
			}
			log.Printf("resp: %v", resp)

			if resp.StatusCode != http.StatusOK {
				pw.Close()
				log.Fatalf("want 200 OK, got %s", resp.Status)
			}

			req, _ = http.NewRequest(http.MethodPost, destinationServer.URL, strings.NewReader("payload"))
			if err := req.Write(pw); err != nil {
				log.Fatalf("req write failure: %s", err)
			}
			pw.Close()

			resp, err = http.ReadResponse(bufio.NewReader(resp.Body), req)
			if err != nil {
				log.Fatalf("Response read failure: %s", err)
			}
			defer resp.Body.Close()

			data, err := ioutil.ReadAll(resp.Body)
			if err != nil {
				log.Fatalf("Response body read failure: %s", err)
			}

			if string(data) != "received: payload" {
				log.Fatalf("unexpected response body: %s", data)
			}

			log.Print("success")
		})
	}
}

I’d greatly appreciate your input on this matter. Thanks!