Skip to content

Fix integer/long overflow in content-length parsing, timestamp ordering, and HTTP/2 flow control#1049

Draft
pjfanning wants to merge 5 commits into
apache:mainfrom
pjfanning:copilot/fix-integer-overflow-issues
Draft

Fix integer/long overflow in content-length parsing, timestamp ordering, and HTTP/2 flow control#1049
pjfanning wants to merge 5 commits into
apache:mainfrom
pjfanning:copilot/fix-integer-overflow-issues

Conversation

@pjfanning
Copy link
Copy Markdown
Member

Silent arithmetic overflow in four places can produce incorrect behavior: bogus content-length acceptance, wrong timestamp ordering, and HTTP/2 flow-control windows that wrap instead of triggering the required protocol errors.

ContentLengthParser — positive-wrapping Long overflow

The result < 0 guard only catches overflow that wraps negative. Values like 1844674407370955163 * 10 wrap to a small positive Long, silently accepting an invalid content-length. Fixed with a pre-multiply bounds check:

// before
result = result * 10 + digit
if (result < 0) fail(...)

// after
if (result > (Long.MaxValue - digit) / 10) fail(...)
result = result * 10 + digit

Timestamp.Ordering.compare — subtraction overflow

math.signum(x.timestampNanos - y.timestampNanos) overflows when comparing timestamps near Long.MIN_VALUE / Long.MAX_VALUE (e.g. the never sentinel). Replaced with java.lang.Long.compare.

HTTP/2 flow-control window overflow (RFC 7540 §6.9.1)

A WINDOW_UPDATE that pushes a window above 2^31 − 1 must be treated as a protocol error — connection error (GOAWAY) at the connection level, stream error (RST_STREAM) at the stream level. Previously both windows were incremented unconditionally with Int arithmetic.

  • Added MaxWindowSize = Int.MaxValue constant to Http2Protocol
  • updateConnectionLevelWindow now returns Boolean; Http2Demux sends GOAWAY(FLOW_CONTROL_ERROR) on false
  • OutStream.increaseWindow now returns Boolean; all four call paths in Http2StreamHandling (Sending, Open, OpenReceivingDataFirst, HalfClosedRemoteWaitingForOutgoingStream) send RST_STREAM(FLOW_CONTROL_ERROR) and transition to Closed on overflow

The overflow check uses a toLong widening cast to avoid the very overflow being detected:

if (outboundWindowLeft.toLong + increment > MaxWindowSize) return false
outboundWindowLeft += increment

@pjfanning pjfanning marked this pull request as draft May 31, 2026 21:48
@pjfanning pjfanning force-pushed the copilot/fix-integer-overflow-issues branch from 23b3014 to 66b3ad5 Compare May 31, 2026 22:30
@pjfanning
Copy link
Copy Markdown
Member Author

I think a few of the changes should be non-controversial but the refactor for RFC 7540 §6.9.1 (MaxWindowSize) might be an edge case that we may not want to worry too much about.
2.0 is a good time to add such a change but I'll defer to the consensus about whether to proceed with that change.

@pjfanning pjfanning requested a review from mdedetrich June 1, 2026 10: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.

2 participants