From b9d934ff8be7f55330a62ff87851f00609acd875 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Fri, 20 Jun 2025 15:55:11 +0800 Subject: [PATCH] http3: tighten checks for incorrect use of RequestStream (#5231) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The RequestStream is a low-level API that’s used by WebTransport, CONNECT-UDP and CONNECT-IP. Methods on the RequestStream must be called in a certain order, and we should detect misuse of the API. --- http3/client.go | 2 +- http3/stream.go | 19 +++++++++++++++++-- http3/stream_test.go | 21 ++++++++++++++++++--- 3 files changed, 36 insertions(+), 6 deletions(-) diff --git a/http3/client.go b/http3/client.go index 5e2e2612d..7cd02bc32 100644 --- a/http3/client.go +++ b/http3/client.go @@ -318,7 +318,7 @@ func (c *ClientConn) sendRequestBody(str *RequestStream, body io.ReadCloser, con func (c *ClientConn) doRequest(req *http.Request, str *RequestStream) (*http.Response, error) { trace := httptrace.ContextClientTrace(req.Context()) - if err := str.SendRequestHeader(req); err != nil { + if err := str.sendRequestHeader(req); err != nil { traceWroteRequest(trace, err) return nil, err } diff --git a/http3/stream.go b/http3/stream.go index b2a644753..a1dafa11d 100644 --- a/http3/stream.go +++ b/http3/stream.go @@ -188,7 +188,7 @@ func newRequestStream( // and the response has been consumed (using ReadResponse). func (s *RequestStream) Read(b []byte) (int, error) { if s.responseBody == nil { - return 0, errors.New("http3: invalid use of RequestStream.Read: need to call ReadResponse first") + return 0, errors.New("http3: invalid use of RequestStream.Read before ReadResponse") } return s.responseBody.Read(b) } @@ -202,6 +202,9 @@ func (s *RequestStream) StreamID() protocol.StreamID { // // It can only be used after the request has been sent (using SendRequestHeader). func (s *RequestStream) Write(b []byte) (int, error) { + if !s.sentRequest { + return 0, errors.New("http3: invalid use of RequestStream.Write before SendRequestHeader") + } return s.str.Write(b) } @@ -264,11 +267,19 @@ func (s *RequestStream) ReceiveDatagram(ctx context.Context) ([]byte, error) { // SendRequestHeader sends the HTTP request. // +// It can only used for requests that don't have a request body. // It is invalid to call it more than once. // It is invalid to call it after Write has been called. func (s *RequestStream) SendRequestHeader(req *http.Request) error { + if req.Body != nil && req.Body != http.NoBody { + return errors.New("http3: invalid use of RequestStream.SendRequestHeader with a request that has a request body") + } + return s.sendRequestHeader(req) +} + +func (s *RequestStream) sendRequestHeader(req *http.Request) error { if s.sentRequest { - return errors.New("http3: invalid duplicate use of SendRequestHeader") + return errors.New("http3: invalid duplicate use of RequestStream.SendRequestHeader") } if !s.disableCompression && req.Method != http.MethodHead && req.Header.Get("Accept-Encoding") == "" && req.Header.Get("Range") == "" { @@ -281,10 +292,14 @@ func (s *RequestStream) SendRequestHeader(req *http.Request) error { // ReadResponse reads the HTTP response from the stream. // +// It must be called after sending the request (using SendRequestHeader). // It is invalid to call it more than once. // It doesn't set Response.Request and Response.TLS. // It is invalid to call it after Read has been called. func (s *RequestStream) ReadResponse() (*http.Response, error) { + if !s.sentRequest { + return nil, errors.New("http3: invalid duplicate use of RequestStream.ReadResponse before SendRequestHeader") + } frame, err := s.str.frameParser.ParseNext() if err != nil { s.str.CancelRead(quic.StreamErrorCode(ErrCodeFrameError)) diff --git a/http3/stream_test.go b/http3/stream_test.go index e0b423e3f..a6352a1d4 100644 --- a/http3/stream_test.go +++ b/http3/stream_test.go @@ -8,6 +8,7 @@ import ( "net/http" "net/http/httptest" "net/http/httptrace" + "strings" "testing" "time" @@ -157,14 +158,27 @@ func TestRequestStream(t *testing.T) { &http.Response{}, ) - _, err := str.Read(make([]byte, 100)) - require.EqualError(t, err, "http3: invalid use of RequestStream.Read: need to call ReadResponse first") + _, err := str.Read([]byte{0}) + require.EqualError(t, err, "http3: invalid use of RequestStream.Read before ReadResponse") + _, err = str.Write([]byte{0}) + require.EqualError(t, err, "http3: invalid use of RequestStream.Write before SendRequestHeader") + + // calling ReadResponse before SendRequestHeader is not valid + _, err = str.ReadResponse() + require.EqualError(t, err, "http3: invalid duplicate use of RequestStream.ReadResponse before SendRequestHeader") + // SendRequestHeader can't be used for requests that have a request body + require.EqualError(t, + str.SendRequestHeader( + httptest.NewRequest(http.MethodGet, "https://quic-go.net", strings.NewReader("foobar")), + ), + "http3: invalid use of RequestStream.SendRequestHeader with a request that has a request body", + ) req := httptest.NewRequest(http.MethodGet, "https://quic-go.net", nil) qstr.EXPECT().Write(gomock.Any()).AnyTimes() require.NoError(t, str.SendRequestHeader(req)) // duplicate calls are not allowed - require.EqualError(t, str.SendRequestHeader(req), "http3: invalid duplicate use of SendRequestHeader") + require.EqualError(t, str.SendRequestHeader(req), "http3: invalid duplicate use of RequestStream.SendRequestHeader") buf := bytes.NewBuffer(encodeResponse(t, 200)) buf.Write((&dataFrame{Length: 6}).Append(nil)) @@ -173,6 +187,7 @@ func TestRequestStream(t *testing.T) { rsp, err := str.ReadResponse() require.NoError(t, err) require.Equal(t, http.StatusOK, rsp.StatusCode) + b := make([]byte, 10) n, err := str.Read(b) require.NoError(t, err)