http3: tighten checks for incorrect use of RequestStream (#5231)

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

@@ -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
 	}

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))

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)