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>
diff --git a/http2/h2c/h2c.go b/http2/h2c/h2c.go index 2b77ffd..a72bbed 100644 --- a/http2/h2c/h2c.go +++ b/http2/h2c/h2c.go
@@ -109,6 +109,7 @@ if http2VerboseLogs { log.Printf("h2c: error h2c upgrade: %v", err) } + w.WriteHeader(http.StatusInternalServerError) return } defer conn.Close() @@ -167,7 +168,10 @@ 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()
diff --git a/http2/h2c/h2c_test.go b/http2/h2c/h2c_test.go index 558e597..038cbc3 100644 --- a/http2/h2c/h2c_test.go +++ b/http2/h2c/h2c_test.go
@@ -8,6 +8,7 @@ "context" "crypto/tls" "fmt" + "io" "io/ioutil" "log" "net" @@ -134,3 +135,38 @@ 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) + } +}