Drop frames after reset #106
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I don't have a small reproducer for this, but I'm seeing that when I'm sending
an ill-formed gRPC request to one of Google's servers (specifically, when I use
a `GET` request instead of a `POST` request), I'm getting a response back from
the server that looks something like this:
```
HyperText Transfer Protocol 2
Stream: HEADERS, Stream ID: 1, Length 91, 200 OK
(..)
Header: :status: 200 OK
Header: content-type: application/grpc
Header: grpc-status: 2
Header: grpc-message: Bad method header
HyperText Transfer Protocol 2
Stream: RST_STREAM, Stream ID: 1, Length 4
Length: 4
Type: RST_STREAM (3)
Flags: 0x00
0... .... .... .... .... .... .... .... = Reserved: 0x0
.000 0000 0000 0000 0000 0000 0000 0001 = Stream Identifier: 1
Error: NO_ERROR (0)
```
_and this then repeats_ a few times (within a single packet). I don't know why
this repeats, but both the Google Python implementation and the the C++
implementation do this. When this happens, `http2` (the lib) throws an
`undefined` exception after it receives another HTTP2 messages on that
now-reset stream. This commit changes this to simply _drop_ such messages
instead.
The spec (https://datatracker.ietf.org/doc/html/rfc7540#section-6.4) is
strangely silent on this topic: it says that when you _receive_ a `RST_STREAM`,
you MUST NOT send any messages on that stream anymore, and when you _send_ a
`RST_STREAM`, you MUST be prepared to receive frames that your counterpart
might have sent before it received the `RST_STREAM`. However, it is silent on
whether or not it's legal to _send_ further frames after _sending_ a
`RST_STREAM`, which is what we are observing here.
kazu-yamamoto
approved these changes
Jan 22, 2024
|
Rebased, merged and released. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I don't have a small reproducer for this, but I'm seeing that when I'm sending an ill-formed gRPC request to one of Google's servers (specifically, when I use a
GETrequest instead of aPOSTrequest), I'm getting a response back from the server that looks something like this:and this then repeats a few times (within a single packet). I don't know why this repeats, but both the Google Python implementation and the the C++ implementation do this. When this happens,
http2(the lib) throws anundefinedexception after it receives another HTTP2 messages on that now-reset stream. This commit changes this to simply drop such messages instead.The spec (https://datatracker.ietf.org/doc/html/rfc7540#section-6.4) is strangely silent on this topic: it says that when you receive a
RST_STREAM, you MUST NOT send any messages on that stream anymore, and when you send aRST_STREAM, you MUST be prepared to receive frames that your counterpart might have sent before it received theRST_STREAM. However, it is silent on whether or not it's legal to send further frames after sending aRST_STREAM, which is what we are observing here.