Conversation
RFC 9113 section 6.9 requires a WINDOW_UPDATE with a zero increment to be treated as a PROTOCOL_ERROR. For stream 0 this is a connection error; for any other stream it is a stream error scoped to that stream only. The OkHttp frame reader previously rejected the value while parsing, before either transport's handler had a chance to distinguish those scopes, so any zero-increment WINDOW_UPDATE tore down the whole connection regardless of which scope actually applied. Let the frame reader dispatch the value to the handler instead. The client handler already contained the stream-vs-connection classification; wire it up by having finishStream() send the caller-supplied RST_STREAM error code instead of always sending CANCEL. Add the equivalent classification to the server handler, which previously had none. Add regression coverage for parser dispatch and for client/server stream-vs-connection behavior on both the violating and unaffected paths.
This branch has not been deployed
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
A zero-increment
WINDOW_UPDATEon a stream is currently handled too early by the OkHttp frame reader. It throws while parsing, which turns the error into a connection-level failure before the transport can tell whether it belongs to the connection or to one stream.In practice, that means a bad
WINDOW_UPDATEfor one stream can take down the whole HTTP/2 connection and unrelated RPCs using it.This changes the handling so the transport gets to make that distinction:
0+ zero increment still fails the connection withPROTOCOL_ERRORRST_STREAM(PROTOCOL_ERROR)finishStream()instead of always sendingCANCELTests cover the parser path and both client/server cases for stream- and connection-scoped zero increments.
No API changes, just narrower failure handling for this edge case.