http2/h2c: handle errors when reading HTTP/1 request body

When processing an HTTP/1 Upgrade: h2c request, detect errors reading
the request body and fail the request rather than passing off the
partially-read request to the HTTP/2 server.

Correctly handles the case where a MaxBytesHandler has limited the
size of the initial HTTP/1 request body.

Fixes golang/go#56352

Change-Id: I08d60953cea26961cffbab3094cc1b44236f4e37
Reviewed-on: https://go-review.googlesource.com/c/net/+/447396
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: John Howard <howardjohn@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Damien Neil <dneil@google.com>

http2/h2c/h2c.go

@@ -109,6 +109,7 @@ func (s h2cHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 			if http2VerboseLogs {
 				log.Printf("h2c: error h2c upgrade: %v", err)
 			}
+			w.WriteHeader(http.StatusInternalServerError)
 			return
 		}
 		defer conn.Close()
@@ -167,7 +168,10 @@ func h2cUpgrade(w http.ResponseWriter, r *http.Request) (_ net.Conn, settings []
 		return nil, nil, errors.New("h2c: connection does not support Hijack")
 	}
 
-	body, _ := io.ReadAll(r.Body)
+	body, err := io.ReadAll(r.Body)
+	if err != nil {
+		return nil, nil, err
+	}
 	r.Body = io.NopCloser(bytes.NewBuffer(body))
 
 	conn, rw, err := hijacker.Hijack()

http2/h2c/h2c_test.go

@@ -8,6 +8,7 @@ import (
 	"context"
 	"crypto/tls"
 	"fmt"
+	"io"
 	"io/ioutil"
 	"log"
 	"net"
@@ -134,3 +135,38 @@ func TestPropagation(t *testing.T) {
 		t.Fatal("expected server err, got nil")
 	}
 }
+
+func TestMaxBytesHandler(t *testing.T) {
+	const bodyLimit = 10
+	handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		t.Errorf("got request, expected to be blocked by body limit")
+	})
+
+	h2s := &http2.Server{}
+	h1s := httptest.NewUnstartedServer(http.MaxBytesHandler(NewHandler(handler, h2s), bodyLimit))
+	h1s.Start()
+	defer h1s.Close()
+
+	// Wrap the body in a struct{io.Reader} to prevent it being rewound and resent.
+	body := "0123456789abcdef"
+	req, err := http.NewRequest("POST", h1s.URL, struct{ io.Reader }{strings.NewReader(body)})
+	if err != nil {
+		t.Fatal(err)
+	}
+	req.Header.Set("Http2-Settings", "")
+	req.Header.Set("Upgrade", "h2c")
+	req.Header.Set("Connection", "Upgrade, HTTP2-Settings")
+
+	resp, err := h1s.Client().Do(req)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer resp.Body.Close()
+	_, err = ioutil.ReadAll(resp.Body)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if got, want := resp.StatusCode, http.StatusInternalServerError; got != want {
+		t.Errorf("resp.StatusCode = %v, want %v", got, want)
+	}
+}