Skip to content

Comments

HTTP/2: validate HEADERS PRIORITY payload length#635

Merged
arturobernalg merged 1 commit intoapache:masterfrom
arturobernalg:HEADERS-PRIORITY
Feb 24, 2026
Merged

HTTP/2: validate HEADERS PRIORITY payload length#635
arturobernalg merged 1 commit intoapache:masterfrom
arturobernalg:HEADERS-PRIORITY

Conversation

@arturobernalg
Copy link
Member

Reject incoming HEADERS frames with the PRIORITY flag set but without the mandatory 5-byte priority fields, mapping the condition to a FRAME_SIZE_ERROR instead of allowing a BufferUnderflowException.

@arturobernalg arturobernalg requested a review from ok2c February 22, 2026 18:13
@ok2c
Copy link
Member

ok2c commented Feb 23, 2026

@arturobernalg The PRIORITY mechanism in HEADERS frame has been deprecated https://www.rfc-editor.org/rfc/rfc9113.html#PriorityHere. Why do we need this check?

@arturobernalg
Copy link
Member Author

arturobernalg commented Feb 24, 2026

@arturobernalg The PRIORITY mechanism in HEADERS frame has been deprecated https://www.rfc-editor.org/rfc/rfc9113.html#PriorityHere. Why do we need this check?

Deprecated priority is about semantics, not framing. If the peer sets HEADERS PRIORITY, the 5-octet fields are still mandatory; otherwise it’s a malformed frame → FRAME_SIZE_ERROR.
We must also consume/skip those 5 bytes even if we ignore prioritization, or we’d mis-parse the header block fragment.
̶A̶l̶s̶o̶,̶ ̶H̶E̶A̶D̶E̶R̶S̶ ̶P̶A̶D̶D̶E̶D̶ ̶d̶o̶e̶s̶n̶’̶t̶ ̶s̶e̶e̶m̶ ̶t̶o̶ ̶b̶e̶ ̶h̶a̶n̶d̶l̶e̶d̶ ̶o̶n̶ ̶t̶h̶i̶s̶ ̶p̶a̶t̶h̶;̶ ̶I̶’̶l̶l̶ ̶o̶p̶e̶n̶ ̶a̶ ̶s̶e̶p̶a̶r̶a̶t̶e̶ ̶P̶R̶ ̶t̶o̶ ̶i̶m̶p̶l̶e̶m̶e̶n̶t̶ ̶P̶A̶D̶D̶E̶D̶ ̶p̶a̶r̶s̶i̶n̶g̶ ̶/̶ ̶p̶a̶d̶d̶i̶n̶g̶ ̶s̶t̶r̶i̶p̶p̶i̶n̶g̶.̶

Reject incoming HEADERS frames with the PRIORITY flag set but without the
mandatory 5-byte priority fields, mapping the condition to a
FRAME_SIZE_ERROR instead of allowing a BufferUnderflowException.
@arturobernalg arturobernalg merged commit 31a846a into apache:master Feb 24, 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