From 9095c1cb86eb20ab69bc26b5484d7ccb4e8c1620 Mon Sep 17 00:00:00 2001 From: "Nicholas S. Husin" Date: Fri, 6 Feb 2026 19:09:43 -0500 Subject: [PATCH] internal/http3: more robust handling of request & response with no body In HTTP/3, zero or more DATA frames can come after a HEADERS frame to represent a request or response body. Our current implementation can behave rather badly when zero DATA frame is sent. ClientConn does not close the write direction of the stream when it has no body to send. As a result, our Server can end up reading the next frame after a HEADERS frame, only to hang infinitely until the timeout is reached. To fix this, when there is no body to send, ClientConn now closes the write direction of the stream as soon as it has finished writing its HEADERS frame. Server will also prevent itself from reading the stream if a Content-Length header with the value 0 is received. In the opposite direction (client reading response from a server), a similar problem also exists, with a slight variant. While our Server reliably closes its write direction of the stream as soon as the server handler exits, a problem can still occur when a client receives an empty response body due to sending a HEAD request. In this case, if the client decides to read the response body, bodyReader might throw an error due to a mismatch between the Content-Length header given by the server and the actual body length. This is fixed by making ClientConn aware that HEAD requests will always result in an empty response body. For golang/go#70914 Change-Id: I1e8970672e7076c9dbf84aec8808632d04bac807 Reviewed-on: https://go-review.googlesource.com/c/net/+/742960 LUCI-TryBot-Result: Go LUCI Reviewed-by: Damien Neil Reviewed-by: Nicholas Husin --- internal/http3/roundtrip.go | 17 +++++++-- internal/http3/roundtrip_test.go | 60 ++++++++++++++++++++++++++++++++ internal/http3/server.go | 23 +++++++++--- internal/http3/server_test.go | 35 +++++++++++++++++++ 4 files changed, 127 insertions(+), 8 deletions(-) diff --git a/internal/http3/roundtrip.go b/internal/http3/roundtrip.go index 58253d0b..7d05686a 100644 --- a/internal/http3/roundtrip.go +++ b/internal/http3/roundtrip.go @@ -26,7 +26,7 @@ type roundTripState struct { reqBodyWriter bodyWriter // Response.Body, provided to the caller. - respBody bodyReader + respBody io.ReadCloser errOnce sync.Once err error @@ -126,6 +126,11 @@ func (cc *ClientConn) RoundTrip(req *http.Request) (_ *http.Response, err error) encr.HasBody = false go copyRequestBody(rt) } + } else { + // If we have no body to send, close the write direction of the stream + // as soon as we have sent our HEADERS. That way, servers will know + // that there are no DATA frames incoming. + rt.st.stream.CloseWrite() } // Read the response headers. @@ -164,8 +169,14 @@ func (cc *ClientConn) RoundTrip(req *http.Request) (_ *http.Response, err error) if err != nil { return nil, err } - rt.respBody.st = st - rt.respBody.remain = contentLength + if contentLength != 0 && req.Method != http.MethodHead { + rt.respBody = &bodyReader{ + st: st, + remain: contentLength, + } + } else { + rt.respBody = http.NoBody + } resp := &http.Response{ Proto: "HTTP/3.0", ProtoMajor: 3, diff --git a/internal/http3/roundtrip_test.go b/internal/http3/roundtrip_test.go index efbe1052..eb7a538f 100644 --- a/internal/http3/roundtrip_test.go +++ b/internal/http3/roundtrip_test.go @@ -419,3 +419,63 @@ func TestRoundTripExpect100ContinueRejected(t *testing.T) { rt.wantBody(serverBody) }) } + +func TestRoundTripNoBodyClosesStream(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + tc := newTestClientConn(t) + tc.greet() + + req, _ := http.NewRequest("PUT", "https://example.tld/", nil) + tc.roundTrip(req) + st := tc.wantStream(streamTypeRequest) + + st.wantHeaders(nil) + st.wantClosed("no DATA frames to send") + }) +} + +func TestRoundTripReadRespWithNoBody(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + tc := newTestClientConn(t) + tc.greet() + + // Case 1: we know response body is empty because the server closes the + // write direction of the stream. + req, _ := http.NewRequest("GET", "https://example.tld/", nil) + rt := tc.roundTrip(req) + st := tc.wantStream(streamTypeRequest) + st.wantHeaders(nil) + st.writeHeaders(http.Header{ + ":status": {"200"}, + }) + st.stream.stream.CloseWrite() + rt.wantStatus(200) + st.wantClosed("request is complete") + + // Case 2: we know response body is empty because the server indicates + // a Content-Length of 0. + req, _ = http.NewRequest("GET", "https://example.tld/", nil) + rt = tc.roundTrip(req) + st = tc.wantStream(streamTypeRequest) + st.wantHeaders(nil) + st.writeHeaders(http.Header{ + ":status": {"200"}, + "Content-Length": {"0"}, + }) + rt.wantStatus(200) + st.wantClosed("request is complete") + + // Case 3: we know response body is empty because we sent a HEAD + // request. + req, _ = http.NewRequest("HEAD", "https://example.tld/", nil) + rt = tc.roundTrip(req) + st = tc.wantStream(streamTypeRequest) + st.wantHeaders(nil) + st.writeHeaders(http.Header{ + ":status": {"200"}, + "Content-Length": {"1000"}, + }) + rt.wantStatus(200) + st.wantClosed("request is complete") + }) +} diff --git a/internal/http3/server.go b/internal/http3/server.go index 5285d4c7..7fe1245d 100644 --- a/internal/http3/server.go +++ b/internal/http3/server.go @@ -6,6 +6,7 @@ package http3 import ( "context" + "io" "net/http" "strconv" "sync" @@ -217,6 +218,21 @@ func (sc *serverConn) handleRequestStream(st *stream) error { message: reqInfo.InvalidReason, } } + + var body io.ReadCloser + contentLength := int64(-1) + if n, err := strconv.Atoi(header.Get("Content-Length")); err == nil { + contentLength = int64(n) + } + if contentLength != 0 { + body = &bodyReader{ + st: st, + remain: contentLength, + } + } else { + body = http.NoBody + } + req := &http.Request{ Proto: "HTTP/3.0", Method: pHeader.method, @@ -226,11 +242,8 @@ func (sc *serverConn) handleRequestStream(st *stream) error { Trailer: reqInfo.Trailer, ProtoMajor: 3, RemoteAddr: sc.qconn.RemoteAddr().String(), - Body: &bodyReader{ - st: st, - remain: -1, - }, - Header: header, + Body: body, + Header: header, } defer req.Body.Close() diff --git a/internal/http3/server_test.go b/internal/http3/server_test.go index 2b6de06d..805a8b3e 100644 --- a/internal/http3/server_test.go +++ b/internal/http3/server_test.go @@ -373,6 +373,41 @@ func TestServerExpect100ContinueRejected(t *testing.T) { }) } +func TestServerHandlerReadReqWithNoBody(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + serverBody := []byte("hello from server!") + ts := newTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if _, err := io.ReadAll(r.Body); err != nil { + t.Errorf("got %v err when reading from an empty request body, want nil", err) + } + w.Write(serverBody) + })) + tc := ts.connect() + tc.greet() + + // Case 1: we know that there is no body / DATA frame because the + // client closes the write direction of the stream. + reqStream := tc.newStream(streamTypeRequest) + reqStream.writeHeaders(requestHeader(nil)) + reqStream.stream.stream.CloseWrite() + synctest.Wait() + reqStream.wantHeaders(http.Header{":status": {"200"}}) + reqStream.wantData(serverBody) + reqStream.wantClosed("request is complete") + + // Case 2: we know that there is no body / DATA frame because the + // client indicates a Content-Length of 0. + reqStream = tc.newStream(streamTypeRequest) + reqStream.writeHeaders(requestHeader(http.Header{ + "Content-Length": {"0"}, + })) + synctest.Wait() + reqStream.wantHeaders(http.Header{":status": {"200"}}) + reqStream.wantData(serverBody) + reqStream.wantClosed("request is complete") + }) +} + type testServer struct { t testing.TB s *Server