Skip to content

Clarify stream flow-control comment; add RST_STREAM regression test - #1133

Open
arturobernalg wants to merge 1 commit into
apache:mainfrom
arturobernalg:rfc9113-data-exceeds-flow-control-window
Open

arturobernalg wants to merge 1 commit into
apache:mainfrom
arturobernalg:rfc9113-data-exceeds-flow-control-window

Conversation

@arturobernalg

@arturobernalg arturobernalg commented Oct 30, 2026 •

Copy link
Copy Markdown
Member

When a DATA frame payload exceeds the remaining capacity of a stream's receive buffer, Http2Parser throws StreamException(FLOW_CONTROL_ERROR), resulting in RST_STREAM. The connection and all other multiplexed streams remain open.

The stream-level inBuffer is sized to the per-stream initial window (SETTINGS_INITIAL_WINDOW_SIZE). A violation of that buffer is a stream-scoped error. RFC 9113, section 6.9, permits either a stream error or a connection error for this case (MAY — not MUST). Tomcat's existing choice of RST_STREAM is correct and avoids unnecessary collateral damage to concurrent streams.

This PR:

  • Corrects a misleading comment in Http2Parser.processFrameData that incorrectly cited a "MUST connection error" requirement.
  • Adds TestHttp2Section_6_9.testDataFrameExceedsFlowControlWindow as a regression guard, verifying that a DATA frame exceeding the stream window produces RST_STREAM(FLOW_CONTROL_ERROR) and leaves the connection open.

@arturobernalg
arturobernalg force-pushed the rfc9113-data-exceeds-flow-control-window branch from bd40850 to 8187cd7 Compare October 30, 2026 17:26
@arturobernalg

Copy link
Copy Markdown
Member Author

The original change (StreamException → ConnectionException) was incorrect. Confirmed regression in TestCancelledUpload.testCancelledRequest: when a servlet rejects a large upload early (reads 128 bytes, returns 403), in-flight DATA frames can exceed the remaining stream buffer, triggering the same code path. Killing the entire connection in that scenario is wrong.

The dest.remaining() < dataLength check is a stream-level buffer capacity check, not a connection-level window check. The inBuffer is scoped to the per-stream initial window. A violation of that window is a stream error; the connection must remain up.

Updated the commit to revert Http2Parser.java to throw StreamException and corrected the test to assert RST_STREAM(FLOW_CONTROL_ERROR) instead of GOAWAY. All 20 tests in TestHttp2Section_6_9 and both tests in TestCancelledUpload pass.

@arturobernalg arturobernalg changed the title Treat DATA frame exceeding flow-control window as connection error Clarify stream flow-control comment; add RST_STREAM regression test Oct 30, 2026
When a DATA frame payload exceeds the remaining capacity of the stream's
receive buffer, Http2Parser throws StreamException(FLOW_CONTROL_ERROR),
resulting in RST_STREAM. This is a stream-level check: the inBuffer is
scoped to the per-stream initial window, so the violation affects only
that stream; the connection and other streams remain open.

RFC 9113, section 6.9, permits either a stream error or a connection
error for this case (MAY). Tomcat's choice of RST_STREAM is correct and
avoids unnecessary collateral damage to multiplexed streams.

Adds TestHttp2Section_6_9.testDataFrameExceedsFlowControlWindow as a
regression guard for this behavior.
@arturobernalg
arturobernalg force-pushed the rfc9113-data-exceeds-flow-control-window branch from 8187cd7 to 217da04 Compare October 30, 2026 19:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant