Skip to content

Comments

HTTP/2: fix frame header parsing and validate SETTINGS ACK length#628

Merged
arturobernalg merged 1 commit intoapache:masterfrom
arturobernalg:setting-ack
Feb 20, 2026
Merged

HTTP/2: fix frame header parsing and validate SETTINGS ACK length#628
arturobernalg merged 1 commit intoapache:masterfrom
arturobernalg:setting-ack

Conversation

@arturobernalg
Copy link
Member

Parse the 24-bit Length field as an unsigned value, treat flags as an unsigned byte, and mask out the reserved bit from the Stream Identifier.

Also enforce RFC 9113 §6.5: SETTINGS frames with ACK set MUST have an empty payload (Length = 0); otherwise raise FRAME_SIZE_ERROR.

@arturobernalg arturobernalg requested a review from ok2c February 19, 2026 12:07
throw new H2ConnectionException(H2Error.FRAME_SIZE_ERROR, "Frame size exceeds maximum");
}

if (type == FrameType.SETTINGS.getValue() && (flags & FrameFlag.ACK.getValue()) != 0 && payloadLen != 0) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arturobernalg This is the wrong place and not the buffer's job to validate frames. Move this check to the protocol handler.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arturobernalg This is the wrong place and not the buffer's job to validate frames. Move this check to the protocol handler.

@ok2c yeah. The check of the payloadLen > maxFramePayloadSize make me doubt and i put it there. I just moved.

Parse the 24-bit Length field as an unsigned value, treat flags as an
unsigned byte, and mask out the reserved bit from the Stream Identifier.

Also enforce RFC 9113 §6.5: SETTINGS frames with ACK set MUST have an
empty payload (Length = 0); otherwise raise FRAME_SIZE_ERROR.
@arturobernalg arturobernalg merged commit 999a1e9 into apache:master Feb 20, 2026
10 checks passed
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