-
Notifications
You must be signed in to change notification settings - Fork 4.6k
test: add test for invalid streamID #6940
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
24816f2
4300406
71b7ee7
9815dbc
3bd77df
bee7003
6b13297
5fff887
ac91174
577a1b7
f25503b
c68ded3
3e863de
4338789
f1855d3
34dd481
14ecbb8
cfa206b
4d3bc72
26c215b
fcff403
ad742c3
7a1a6f8
7bbea1f
fa61215
6b79110
a1d33da
3522f84
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3901,6 +3901,66 @@ func (s) TestClientRequestBodyErrorCloseAfterLength(t *testing.T) { | |
| } | ||
| } | ||
|
|
||
| // Tests gRPC server's behavior when a gRPC client sends a frame with an invalid | ||
| // streamID. Per [HTTP/2 spec]: Streams initiated by a client MUST use | ||
| // odd-numbered stream identifiers. This test sets up a test server and sends a | ||
| // header frame with stream ID of 2. The test asserts that a subsequent read on | ||
| // the transport sends a GoAwayFrame with error code: PROTOCOL_ERROR. | ||
| // | ||
| // [HTTP/2 spec]: https://httpwg.org/specs/rfc7540.html#StreamIdentifiers | ||
| func (s) TestClientInvalidStreamID(t *testing.T) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since we have this test now, and if we don't already have tests for other invalid stream ID conditions (e.g. a lower ID than the previous, or a stream ID of zero for the first stream), then now might be a good time to add related test cases.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will take a look today and add if tests are not present.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For this case, the http2/frame itself throws an error when we write frames of streamID 0 using serverTester. So I am guessing we can't do this test end to end and ask for question if we need this test at all?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would still be nice to have a test for this, but I'm fine with punting for now if it's difficult to write. Presumably this is a different code path on the server (the framer may error reading the frame), so a test would still be a good idea to have one day.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added one more test. PTAL |
||
| lis, err := net.Listen("tcp", "localhost:0") | ||
| if err != nil { | ||
| t.Fatalf("Failed to listen: %v", err) | ||
| } | ||
| defer lis.Close() | ||
| s := grpc.NewServer() | ||
| defer s.Stop() | ||
| go s.Serve(lis) | ||
|
|
||
| conn, err := net.DialTimeout("tcp", lis.Addr().String(), defaultTestTimeout) | ||
| if err != nil { | ||
| t.Fatalf("Failed to dial: %v", err) | ||
| } | ||
| st := newServerTesterFromConn(t, conn) | ||
| st.greet() | ||
| st.writeHeadersGRPC(2, "/grpc.testing.TestService/StreamingInputCall", true) | ||
| goAwayFrame := st.wantGoAway(http2.ErrCodeProtocol) | ||
| want := "received an illegal stream id: 2." | ||
| if got := string(goAwayFrame.DebugData()); !strings.Contains(got, want) { | ||
| t.Fatalf(" Received: %v, Expected error message to contain: %v.", got, want) | ||
| } | ||
| } | ||
|
|
||
| // TestInvalidStreamIDSmallerThanPrevious tests the server sends a GOAWAY frame | ||
| // with error code: PROTOCOL_ERROR when the streamID of the current frame is | ||
| // lower than the previous frames. | ||
| func (s) TestInvalidStreamIDSmallerThanPrevious(t *testing.T) { | ||
| lis, err := net.Listen("tcp", "localhost:0") | ||
| if err != nil { | ||
| t.Fatalf("Failed to listen: %v", err) | ||
| } | ||
| defer lis.Close() | ||
| s := grpc.NewServer() | ||
| defer s.Stop() | ||
| go s.Serve(lis) | ||
|
|
||
| conn, err := net.DialTimeout("tcp", lis.Addr().String(), defaultTestTimeout) | ||
| if err != nil { | ||
| t.Fatalf("Failed to dial: %v", err) | ||
| } | ||
| st := newServerTesterFromConn(t, conn) | ||
| st.greet() | ||
| st.writeHeadersGRPC(3, "/grpc.testing.TestService/StreamingInputCall", true) | ||
| st.wantAnyFrame() | ||
| st.writeHeadersGRPC(1, "/grpc.testing.TestService/StreamingInputCall", true) | ||
| goAwayFrame := st.wantGoAway(http2.ErrCodeProtocol) | ||
| want := "received an illegal stream id: 1" | ||
| if got := string(goAwayFrame.DebugData()); !strings.Contains(got, want) { | ||
| t.Fatalf(" Received: %v, Expected error message to contain: %v.", got, want) | ||
| } | ||
| } | ||
|
|
||
| func testClientRequestBodyErrorCloseAfterLength(t *testing.T, e env) { | ||
| te := newTest(t, e) | ||
| te.declareLogNoise("Server.processUnaryRPC failed to write status") | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.