Repository: quic-go/quic-go
Pull request #5145 (2025-05-19 04:09:50)

http3: fix off-by-one error when processing the GOAWAY stream ID

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/quic-go/quic-go/pull/5145?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 83.85%. Comparing base [(`363e0cc`)](https://app.codecov.io/gh/quic-go/quic-go/commit/363e0ccafb9578ab998ccbe51b36a5a9374c24c3?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go) to head [(`cbdccc0`)](https://app.codecov.io/gh/quic-go/quic-go/commit/cbdccc0d83e29a8c7655544aeb71d530f040e6c4?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go).

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master    #5145      +/-   ##
==========================================
- Coverage   83.90%   83.85%   -0.05%     
==========================================
  Files         153      150       -3     
  Lines       17308    16958     -350     
==========================================
- Hits        14521    14219     -302     
+ Misses       2225     2196      -29     
+ Partials      562      543      -19     
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/quic-go/quic-go/pull/5145?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go).

<details><summary> :rocket: New features to boost your workflow: </summary>

- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
</details>

Patch:
diff --git a/http3/conn.go b/http3/conn.go
index 746029e1..a2c8ba61 100644
--- a/http3/conn.go
+++ b/http3/conn.go
@@ -120,12 +120,21 @@ func (c *connection) openRequestStream(
 	disableCompression bool,
 	maxHeaderBytes uint64,
 ) (*requestStream, error) {
-	c.streamMx.Lock()
-	maxStreamID := c.maxStreamID
-	lastStreamID := c.lastStreamID
-	c.streamMx.Unlock()
-	if maxStreamID != protocol.InvalidStreamID && lastStreamID >= maxStreamID {
-		return nil, errGoAway
+	if c.perspective == protocol.PerspectiveClient {
+		c.streamMx.Lock()
+		maxStreamID := c.maxStreamID
+		var nextStreamID quic.StreamID
+		if c.lastStreamID == protocol.InvalidStreamID {
+			nextStreamID = 0
+		} else {
+			nextStreamID = c.lastStreamID + 4
+		}
+		c.streamMx.Unlock()
+		// Streams with stream ID equal to or greater than the stream ID carried in the GOAWAY frame
+		// will be rejected, see section 5.2 of RFC 9114.
+		if maxStreamID != protocol.InvalidStreamID && nextStreamID >= maxStreamID {
+			return nil, errGoAway
+		}
 	}
 
 	str, err := c.OpenStreamSync(ctx)


Test patch:
diff --git a/http3/conn_test.go b/http3/conn_test.go
index 13e23b34..5096c3a9 100644
--- a/http3/conn_test.go
+++ b/http3/conn_test.go
@@ -283,13 +283,13 @@ func testConnGoAway(t *testing.T, withStream bool) {
 	)
 	b := quicvarint.Append(nil, streamTypeControlStream)
 	b = (&settingsFrame{}).Append(b)
-	b = (&goAwayFrame{StreamID: 4}).Append(b)
+	b = (&goAwayFrame{StreamID: 8}).Append(b)
 
 	var mockStr *mockquic.MockStream
 	var str quic.Stream
 	if withStream {
 		mockStr = mockquic.NewMockStream(mockCtrl)
-		mockStr.EXPECT().StreamID().Return(4).AnyTimes()
+		mockStr.EXPECT().StreamID().Return(0).AnyTimes()
 		mockStr.EXPECT().Context().Return(context.Background()).AnyTimes()
 		qconn.EXPECT().OpenStreamSync(gomock.Any()).Return(mockStr, nil)
 		s, err := conn.openRequestStream(context.Background(), nil, nil, true, 1000)
@@ -327,7 +327,20 @@ func testConnGoAway(t *testing.T, withStream bool) {
 		case <-time.After(scaleDuration(10 * time.Millisecond)):
 		}
 
-		_, err := conn.openRequestStream(context.Background(), nil, nil, true, 1000)
+		// The stream ID in the GOAWAY frame is 8, so it's possible to open stream 4.
+		mockStr2 := mockquic.NewMockStream(mockCtrl)
+		mockStr2.EXPECT().StreamID().Return(4).AnyTimes()
+		mockStr2.EXPECT().Context().Return(context.Background()).AnyTimes()
+		qconn.EXPECT().OpenStreamSync(gomock.Any()).Return(mockStr2, nil)
+		str2, err := conn.openRequestStream(context.Background(), nil, nil, true, 1000)
+		require.NoError(t, err)
+		mockStr2.EXPECT().Close()
+		str2.Close()
+		mockStr2.EXPECT().CancelRead(gomock.Any())
+		str2.CancelRead(1337)
+
+		// It's not possible to open stream 8.
+		_, err = conn.openRequestStream(context.Background(), nil, nil, true, 1000)
 		require.ErrorIs(t, err, errGoAway)
 
 		mockStr.EXPECT().Close()
diff --git a/integrationtests/self/http_test.go b/integrationtests/self/http_test.go
index b04aa47c..83535d35 100644
--- a/integrationtests/self/http_test.go
+++ b/integrationtests/self/http_test.go
@@ -1188,14 +1188,12 @@ func testHTTPRequestAfterGracefulShutdown(t *testing.T, setGetBody bool) {
 		require.Nil(t, req.GetBody)
 	}
 
+	// By increasing the RTT, we make sure that the request is sent before the client receives the GOAWAY frame.
 	inShutdown.Store(true)
 	go server1.Shutdown(context.Background())
 	go server2.ServeListener(ln)
 	defer server2.Close()
 
-	// so that graceful shutdown can actually start
-	time.Sleep(scaleDuration(10 * time.Millisecond))
-
 	resp, err = cl.Do(req)
 	if !setGetBody {
 		require.ErrorContains(t, err, "after Request.Body was written; define Request.GetBody to avoid this error")