Skip to content

fix: validate HTTP/2 request pseudo headers - #581

Open
azkrishpy wants to merge 2 commits into
mainfrom
todo/h2-request-validation
Open

azkrishpy wants to merge 2 commits into
mainfrom
todo/h2-request-validation

Conversation

@azkrishpy

@azkrishpy azkrishpy commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Issue #, if available: N/A

Description of changes:

Implements the HTTP/2 request validation the decoder carried TODOs for: :method must be present and a valid token, non-CONNECT requests need :scheme and :path, CONNECT needs :authority and must omit the other two, header values must not contain prohibited characters or padding whitespace, and TE may only be trailers. All are stream errors rather than connection errors, so the HPACK table stays intact and only the offending stream resets.

On the send path, make_request now rejects a message with no :method or :path up front rather than failing opaquely later, and logs both.

Reuses the existing INVALID_METHOD / INVALID_PATH errors rather than adding a public error code. The dynamic-table-resize TODO stays open, since it is not implemented.

Why this is a fix and not part of the TODO-cleanup chore: the header-value check runs on every block, including responses. A response header with a padded value such as x-foo: bar used to be delivered to the caller, and now resets the stream with AWS_ERROR_HTTP_PROTOCOL_ERROR. Any peer that pads a value turns a working request into a failure. RFC-9113 8.2.1 only says implementations SHOULD reject these, so this is a deliberate correctness-over-interop choice rather than a no-op cleanup.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@azkrishpy
azkrishpy requested a review from a team as a code owner September 8, 2026 18:56
@codecov-commenter

codecov-commenter commented Sep 8, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 47.91667% with 25 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.73%. Comparing base (2b563f8) to head (6fedc8b).

Files with missing lines Patch % Lines
source/h2_decoder.c 45.71% 19 Missing ⚠️
source/h2_connection.c 53.84% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #581      +/-   ##
==========================================
- Coverage   79.84%   79.73%   -0.11%     
==========================================
  Files          28       28              
  Lines       12105    12154      +49     
==========================================
+ Hits         9665     9691      +26     
- Misses       2440     2463      +23     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@azkrishpy
azkrishpy force-pushed the todo/h2-request-validation branch from 27dc0b0 to 6fedc8b Compare September 8, 2026 21:44
@azkrishpy azkrishpy changed the title Validate HTTP/2 request pseudo headers fix: validate HTTP/2 request pseudo headers Sep 8, 2026
@azkrishpy
azkrishpy changed the base branch from todo/h2benchmark-cli to main September 8, 2026 21:44
@github-actions github-actions Bot added the patch ABI backward-compatible (patch release) label Sep 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

patch ABI backward-compatible (patch release)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants