Skip to content

Fix max_incomplete_event_size for complete oversized headers - #206

Open
Pitchfork-and-Torch wants to merge 1 commit into
python-hyper:masterfrom
Pitchfork-and-Torch:cook/enforce-max-incomplete-on-complete-headers
Open

Pitchfork-and-Torch wants to merge 1 commit into
python-hyper:masterfrom
Pitchfork-and-Torch:cook/enforce-max-incomplete-on-complete-headers

Conversation

@Pitchfork-and-Torch

Copy link
Copy Markdown

Summary

  • Enforce max_incomplete_event_size when a complete header event is parsed, not only on NEED_DATA.
  • Fixes framing-dependent acceptance of oversized request/response headers (one chunk accepted, split rejected).
  • Adds regression coverage for a single-chunk oversized request.

Closes / relates to #182.

Test plan

  • pytest h11/tests/test_connection.py::test_max_incomplete_event_size_countermeasure
  • CI

Previously the size limit was only checked when next_event() returned
NEED_DATA, so oversized request/response headers that arrived in a
single chunk (or otherwise completed without an incomplete overflow)
were accepted while the same bytes split across packets were rejected.
Enforce the limit using bytes consumed by header events as well
(python-hyper#182).

@Wkhanex Wkhanex left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I checked 05972a8b34fa on both the server request path and client response path, at every byte split point and at limits one byte below, equal to, and one byte above the serialized header length. Over-limit cases consistently raised RemoteProtocolError with status hint 431; exact-limit headers remained accepted even when followed in the same receive call by a 400-byte body.

All 1,424 additional assertions and the full project suite (78 tests) passed on CPython 3.12. The coalesced body control is particularly useful for the before - len(buffer) calculation: it confirms body bytes do not count toward the header limit. AI-assisted validation at the pinned head.

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