Skip to content

Bound header field name size before validating#296

Merged
Kludex merged 1 commit into
mainfrom
perf/bound-header-name-size
Jun 4, 2026
Merged

Bound header field name size before validating#296
Kludex merged 1 commit into
mainfrom
perf/bound-header-name-size

Conversation

@Kludex
Copy link
Copy Markdown
Owner

@Kludex Kludex commented Jun 4, 2026

Summary

Fixes a regression introduced in #295.

That PR rewrote HEADER_FIELD to find the colon and validate the whole field-name span at once. The size check (advance_header_size) ran after slicing and translate-validating the span, so a header field name with no colon arriving in one large write() chunk got sliced and scanned in full before the max_header_size limit was applied - the limit no longer bounded the work or temporary allocation for an oversized name.

This moves advance_header_size to before the slice/validate, so an over-limit field name fails fast as it did prior to #295.

For an 8 MiB field-name chunk with the default 4 KiB limit, parser-side temporary allocation drops from ~16 MiB back to ~4 KiB; behaviour for valid input is unchanged.

Correctness

  • All existing tests pass; 100% coverage; mypy + ruff clean.
  • Differentially fuzzed against the pre-Speed up multipart header parsing and callback dispatch #295 implementation: identical callback event streams and identical exception classes across randomized bodies x every chunk-split strategy (including byte-by-byte), at small and default header limits.
  • Starlette 1.2.1's tests/test_formparsers.py (40 tests) pass against this branch.

AI Disclaimer

This PR was developed with the assistance of either Claude or Codex. I've reviewed and verified the changes.

Enforce the per-header size limit before slicing and bulk-validating the
field name, so an over-limit name fails fast instead of copying and scanning
the whole span. Fixes a regression from #295, which moved the size check
after the slice/validate.
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Jun 4, 2026

Merging this PR will not alter performance

✅ 5 untouched benchmarks


Comparing perf/bound-header-name-size (1e4b3f3) with main (e0c4f9d)

Open in CodSpeed

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

No issues found across 1 file

Tip: cubic could auto-approve low-risk PRs like this, if it thinks it's safe to merge. Learn more

Re-trigger cubic

@Kludex Kludex merged commit 6b837d4 into main Jun 4, 2026
15 checks passed
@Kludex Kludex deleted the perf/bound-header-name-size branch June 4, 2026 08:08
@Kludex Kludex mentioned this pull request Jun 4, 2026
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.

1 participant