feat(http_headers): add typed HTTP header parsing - #760
Martin Taillefer (geeknoid) wants to merge 1 commit into
Conversation
✅ Version increments look sufficient
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
5658df4 to
9584466
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical and moderate findings remain.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds typed HTTP header parsing with borrowed/owned APIs, HTTP/Serde integrations, SIMD scanning, fuzzing, benchmarks, and an Axum example.
Changes:
- Adds
http_headersheader families, source/sink adapters, validation, and integrations. - Adds
http_headers_simdscalar/SIMD scanners with runtime dispatch and differential coverage. - Adds tests, documentation, packaging, benchmarks, and workspace tooling.
Final findings: three critical issues remain in http_adapter.rs, SIMD range scanning, and the x86 URI table; two moderate test issues affect moved_public_api.rs; two nits affect coverage attribution and crate documentation.
File summaries
| File | Change covered |
|---|---|
README.md |
Workspace documentation |
justfiles/extended.just |
All-feature Bolero execution |
crates/http_headers/tests/support_api.rs |
Support API tests |
crates/http_headers/tests/response_failures.rs |
Sink failure tests |
crates/http_headers/tests/raw_source_validation.rs |
Raw-source validation |
crates/http_headers/tests/http_messages.rs |
HTTP message adapters |
crates/http_headers/tests/http_map.rs |
HeaderMap integration |
crates/http_headers/tests/header_families.rs |
Header-family coverage |
crates/http_headers/tests/documentation_examples.rs |
Documentation examples |
crates/http_headers/tests/collection_traits.rs |
Collection trait coverage |
crates/http_headers/tests/axum_example.rs |
Axum integration |
crates/http_headers/tests/__fuzz__/campaign.toml |
Fuzz configuration |
crates/http_headers/src/validate.rs |
Shared validation |
crates/http_headers/src/test_sink.rs |
Test sink support |
crates/http_headers/src/source/mod.rs |
Source exports |
crates/http_headers/src/source/field_source.rs |
Field source abstraction |
crates/http_headers/src/source/delimited_items.rs |
Delimited source items |
crates/http_headers/src/sink/mod.rs |
Sink exports |
crates/http_headers/src/sink/insert_error.rs |
Insert errors |
crates/http_headers/src/sink/field_encoder.rs |
Field encoding |
crates/http_headers/src/headers/websocket/mod.rs |
WebSocket headers |
crates/http_headers/src/headers/security/mod.rs |
Security headers |
crates/http_headers/src/headers/range/shared.rs |
Range helpers |
crates/http_headers/src/headers/range/mod.rs |
Range exports |
crates/http_headers/src/headers/negotiation/vary.rs |
Vary parsing |
crates/http_headers/src/headers/negotiation/mod.rs |
Negotiation exports |
crates/http_headers/src/headers/negotiation/allow.rs |
Allow parsing |
crates/http_headers/src/headers/negotiation/accept_language.rs |
Accept-Language parsing |
crates/http_headers/src/headers/negotiation/accept_encoding.rs |
Accept-Encoding parsing |
crates/http_headers/src/headers/mod.rs |
Header exports |
crates/http_headers/src/headers/extension_value.rs |
Extension values |
crates/http_headers/src/headers/cors/test_support.rs |
CORS test support |
crates/http_headers/src/headers/cors/mod.rs |
CORS exports |
crates/http_headers/src/headers/cors/access_control_request_headers.rs |
CORS request headers |
crates/http_headers/src/headers/cors/access_control_expose_headers.rs |
CORS exposed headers |
crates/http_headers/src/headers/cors/access_control_allow_headers.rs |
CORS allowed headers |
crates/http_headers/src/headers/content_length.rs |
Content-Length parsing |
crates/http_headers/src/headers/conditional/mod.rs |
Conditional exports |
crates/http_headers/src/headers/conditional/last_modified.rs |
Last-Modified parsing |
crates/http_headers/src/headers/conditional/if_unmodified_since.rs |
If-Unmodified-Since parsing |
crates/http_headers/src/headers/conditional/if_none_match.rs |
If-None-Match parsing |
crates/http_headers/src/headers/conditional/if_modified_since.rs |
If-Modified-Since parsing |
crates/http_headers/src/headers/conditional/if_match.rs |
If-Match parsing |
crates/http_headers/src/decode_error.rs |
Decode errors |
crates/http_headers/logo.png |
Documentation branding |
crates/http_headers/favicon.ico |
Documentation branding |
crates/http_headers/examples/axum/app.rs |
Axum example application |
crates/http_headers/examples/axum.rs |
Axum example entry point |
crates/http_headers/docs/TODO.md |
Project notes |
crates/http_headers/docs/PERF.md |
Performance documentation |
crates/http_headers/docs/COMPATIBILITY.md |
Compatibility documentation |
crates/http_headers/CHANGELOG.md |
Crate changelog |
crates/http_headers/Cargo.toml |
Crate metadata and features |
crates/http_headers/benches/operations.rs |
Operation benchmarks |
crates/http_headers/benches/name_recognition.rs |
Name-recognition benchmarks |
crates/http_headers/benches/fixtures.rs |
Benchmark fixtures |
crates/http_headers/benches/common_values.rs |
Common-value benchmarks |
crates/http_headers_simd/tests/bolero_fuzz.rs |
SIMD differential fuzz tests |
crates/http_headers_simd/tests/__fuzz__/public_scanners_match_scalar_oracles/crashes/.gitkeep |
Crash corpus placeholder |
crates/http_headers_simd/tests/__fuzz__/campaign.toml |
Test fuzz configuration |
crates/http_headers_simd/src/uri.rs |
URI scanning |
crates/http_headers_simd/src/scalar.rs |
Scalar scanners |
crates/http_headers_simd/src/lib.rs |
SIMD crate API |
crates/http_headers_simd/src/base64.rs |
Base64 scanning |
crates/http_headers_simd/src/__fuzz__/sse2_matches_scalar/crashes/.gitkeep |
SSE2 crash corpus placeholder |
crates/http_headers_simd/src/__fuzz__/sse2_matches_scalar/corpus/boundaries |
SSE2 boundary corpus |
crates/http_headers_simd/src/__fuzz__/neon_matches_scalar/crashes/.gitkeep |
NEON crash corpus placeholder |
crates/http_headers_simd/src/__fuzz__/neon_matches_scalar/corpus/boundaries |
NEON boundary corpus |
crates/http_headers_simd/src/__fuzz__/differential_properties/crashes/.gitkeep |
Differential crash corpus placeholder |
crates/http_headers_simd/src/__fuzz__/differential_properties/corpus/boundaries |
Differential boundary corpus |
crates/http_headers_simd/src/__fuzz__/campaign.toml |
SIMD fuzz configuration |
crates/http_headers_simd/README.md |
SIMD crate documentation |
crates/http_headers_simd/logo.png |
Documentation branding |
crates/http_headers_simd/favicon.ico |
Documentation branding |
crates/http_headers_simd/CHANGELOG.md |
SIMD crate changelog |
crates/http_headers_simd/Cargo.toml |
SIMD crate metadata and features |
crates/http_headers_simd/benches/no_std_dispatch.rs |
no_std dispatch benchmark |
CHANGELOG.md |
Workspace changelog |
Cargo.toml |
Workspace configuration |
Cargo.lock |
Dependency lockfile |
.spelling |
Spelling dictionary |
Review details
- Files reviewed: 53/147 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🔵 Needs a closer look
One or more issues must be addressed before approval.
Review details
Suppressed comments (5)
crates/http_headers/src/field_name.rs:693
- This top-level test module is missing the repository's
coverage_nightlyexclusion used by the nested module immediately above. Its test-only lines are then included in the 100% coverage gate; add#[cfg_attr(coverage_nightly, coverage(off))]between#[cfg(test)]andmod tests.
crates/http_headers/src/headers/authorization.rs:89 - The owned Authorization documentation has another unreplaced
******placeholder in the Bearer specification reference. Replace it withBearerso the generated API docs identify the supported scheme correctly.
crates/http_headers/src/headers/authorization.rs:60 - This public specification paragraph still contains the
******placeholder, making the Authorization documentation inaccurate. Name the Bearer scheme explicitly here.
crates/http_headers/src/headers/authorization.rs:24 - The public docs contain an unreplaced
******placeholder, so this type is documented as “The ****** scheme” instead of the Bearer scheme. Replace the placeholder with the actual scheme name before publishing the API docs.
crates/http_headers/src/headers/authorization.rs:104 - This example text contains two unreplaced
******placeholders, so consumers cannot tell that the second form is Bearer authorization. Replace both placeholders with explicit Bearer/token terminology.
- Files reviewed: 53/147 changed files
- Comments generated: 0 new
- Review effort level: Lite
9584466 to
0002131
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #760 +/- ##
==========================================
Coverage 100.0% 100.0%
==========================================
Files 640 735 +95
Lines 85528 98328 +12800
==========================================
+ Hits 85528 98328 +12800
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
0002131 to
325d672
Compare
325d672 to
b5597cd
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical findings affect report validation, custom header handling, and SIMD test/benchmark feature gating.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (1)
crates/http_headers/docs/COMPATIBILITY.md:45
- The feature table omits the declared
serdefeature (crates/http_headers/Cargo.toml:40), even though the crate documents Serde serialization/deserialization for owned headers and field values (src/lib.rs:223-279). This makes the compatibility guide incomplete for consumers; add a row describing the feature and its off-by-default status.
- Files reviewed: 53/147 changed files
- Comments generated: 3
- Review effort level: Lite
There was a problem hiding this comment.
🔵 Needs a closer look
A critical HTTP adapter panic path and a moderate fuzz-validation gap remain unresolved, along with documentation nits.
Review details
Suppressed comments (2)
crates/http_headers/docs/COMPATIBILITY.md:45
- The feature table omits the
serdefeature even thoughcrates/http_headers/Cargo.toml:40declares it and the crate-level documentation describes Serde integration. This makes the compatibility contract incomplete for consumers looking here to discover optional features; please add aserderow describing the owned-header/core-type serialization and deserialization.
crates/http_headers/docs/DESIGN.md:222 - This description is not true for the shared scanners on x86/x86-64:
SIMD_THRESHOLDis 16 there and 32 only on other architectures. As written, the design doc tells readers that 16-31 byte inputs stay scalar on x86, contradicting the dispatch contract and the performance rationale; document the architecture-dependent threshold (and keep the separate equality/URI thresholds explicit).
- Files reviewed: 53/147 changed files
- Comments generated: 0 new
- Review effort level: Lite
b5597cd to
ebbb0c0
Compare
ebbb0c0 to
3c5a658
Compare
d8a1b94 to
26ebb01
Compare
7c71765 to
d8053e3
Compare
327d42a to
c5d3956
Compare
martintmk
left a comment
martintmk
left a comment
There was a problem hiding this comment.
Posted by an AI agent
Warning: Incomplete review coverage
Some review areas could not be executed:
- Public contract output (review-public-api): required cargo public-api/rustdoc output was unavailable, and this untrusted PR could not be executed.
- Public API documentation (review-public-docs): required rustdoc JSON was unavailable, and this untrusted PR could not be executed.
No combined verdict is issued. Findings below come only from completed areas.
The eight completed review areas found no actionable issues at commit f198859. Full coverage was not achieved, so this review is comment-only and casts no vote.
martintmk
left a comment
martintmk
left a comment
There was a problem hiding this comment.
Posted by an AI agent
Warning: Incomplete review coverage
Some review areas could not be executed:
- Public contract output (review-public-api): required cargo public-api/rustdoc output was unavailable, and this untrusted PR could not be executed.
- Public API documentation (review-public-docs): required rustdoc JSON was unavailable, and this untrusted PR could not be executed.
No combined verdict is issued. Findings below come only from completed areas.
The eight completed review areas found no actionable issues at commit f198859. Full coverage was not achieved, so this review is comment-only and casts no vote.
martintmk
left a comment
martintmk
left a comment
There was a problem hiding this comment.
Posted by an AI agent
Warning: Incomplete review coverage
Some review areas could not be executed:
- Public contract output (
review-public-api): no matching cargo public-api/rustdoc output was available, and this untrusted PR could not be executed. - Public API documentation (
review-public-docs): no matching rustdoc JSON bundle was available, and this untrusted PR could not be executed.
No combined verdict is issued. Findings below come only from completed areas.
Eight areas completed at commit 74f116f1274b38fe70716dd97dd06209caa4ecc4. One new documentation/configuration contradiction is reported. Existing discussion was checked and duplicate concerns were not reposted. This review is comment-only and casts no verdict.
martintmk
left a comment
martintmk
left a comment
There was a problem hiding this comment.
Posted by an AI agent
Full fresh Review Lens coverage completed at 6a3b21c. The review found eight blocking correctness/API/documentation issues and seven non-blocking design, test, benchmark, and documentation issues. Public API output and public-doc extraction completed; telemetry had no findings.
Limitations
- Miri was unavailable because cargo-miri and required tooling were not installed.
Posted by an AI agent · Non-blocking
CORS duplicates shared token-view abstractions
Problem
CorsMethodView and CorsHeaderNameView duplicate negotiation views and already diverge as to_header_name versus to_field_name.
Why this matters
Consumers receive incompatible public types for identical HTTP concepts, adding conversions and maintenance.
Suggested fix
Hoist the shared views outside negotiation and reuse them in CORS, updating exports/tests/examples.
Posted by an AI agent · Non-blocking
Supported no_std SIMD paths are absent from normal test execution
Problem
Feature unification enables std in combined legs, x86-64-v3 compiles out detector-cache tests, and the isolated AArch64 test lacks the required Vec import.
Why this matters
Regressions in documented compile-time dispatch can pass CI unnoticed.
Suggested fix
Add package-isolated baseline-x86 and AArch64 --no-default-features test legs that require the gated tests to execute.
Posted by an AI agent · Non-blocking
Custom-source budget tests share production constants
Problem
Boundary fixtures and expected values are derived from MAX_CUSTOM_* rather than literal documented limits.
Why this matters
Changing production limits moves both sides together, allowing the 65,536-byte, 128-line, and 1,024-item contracts to drift silently.
Suggested fix
Add literal boundary and overflow assertions independent of exported constants.
martintmk
left a comment
martintmk
left a comment
There was a problem hiding this comment.
Posted by an AI agent
Warning: Incomplete review coverage
Some review areas could not be executed:
- Public contract output (
review-public-api): Isolated worker could not consume the package-comparison artifact; cargo public-api extraction did not run.
No combined verdict is issued. Findings below come only from completed areas.
The completed areas found 4 blocking and 3 non-blocking issues. One non-blocking recovery-classification issue is already represented by existing discussion and is not reposted.
martintmk
left a comment
martintmk
left a comment
There was a problem hiding this comment.
Posted by an AI agent
Full Review Lens coverage completed for 70d9eb0f00feeae2d00b66f10f4db4ace8106267. Two blocking and three non-blocking findings remain. The mutation-testing finding is already represented by existing discussion and is not reposted.
| /// interesting-byte scans. Their 16-byte instruction counts fell by 40-63% | ||
| /// and wall-clock times by 31-65%. Architectures not measured here use a | ||
| /// two-vector crossover. | ||
| pub(super) const SIMD_THRESHOLD: usize = if cfg!(any(target_arch = "x86", target_arch = "x86_64")) { |
There was a problem hiding this comment.
Posted by an AI agent · Non-blocking
The x86 SIMD cutoff regresses 16-byte token validation on Intel
Problem
The universal x86 threshold selects SIMD at 16 bytes from AMD EPYC measurements, while the supplied Criterion evidence on Intel Xeon Platinum 8370C measured dispatched validation at 30.557 ns versus 27.969 ns scalar, about 9.3% slower with non-overlapping intervals.
Why this matters
Every 16-byte token, including field-name validation, pays this per-parse regression on affected Intel systems.
Suggested fix
Keep 16-byte inputs scalar unless representative cross-vendor measurements justify a universal threshold, and retain boundary benchmarks for supported x86 classes.
There was a problem hiding this comment.
The author wants to keep the current cutoff pending reproducible cross-vendor evidence, so I am leaving this conversation open. Please link the cited raw Criterion report and reproduction command, including the benchmark input, source revision, toolchain/target flags, enabled features, selected backend, and confidence intervals for both arms. The existing cutoff records AMD EPYC measurements; it is shared by token, token68, field-value, and delimiter scans, so changing it globally would affect more than this reported token case. Existing 15/16/17-byte boundary workloads remain in place. I have not rerun benchmarks or treated the reported Intel result as independently verified.
martintmk
left a comment
martintmk
left a comment
There was a problem hiding this comment.
Posted by an AI agent
Review Lens completed all ten required areas at d9dd08d56a60a190fee55e755689739c04f86aa1.
Verdict: changes requested
One blocking test/CI gap leaves the documented AArch64 no_std configuration unprotected. I also found six non-blocking API, documentation, fuzzing, and instrumentation issues plus two naming/ergonomics nits. Existing discussions were deduplicated; in particular, the prior InsertError recovery discussion was not reposted.
Coverage included both newly added crates (http_headers, http_headers_simd), their all-features public surface and rustdoc closure, parsers/encoders, source/sink adapters, SIMD dispatch, tests/fuzzing/benchmarks, feature combinations, docs, telemetry, resilience, and workspace integration. Targeted Windows x86-64 tests passed; AArch64 execution and Miri were unavailable, and several pinned-head CI jobs were still running.
martintmk
left a comment
martintmk
left a comment
There was a problem hiding this comment.
Posted by an AI agent
Review Lens completed all ten areas at �31e28f23f6edc42f3373c9aefc522e561c353cd.
Two blocking issues remain: source-limit failures lose their distinct error semantics, and the documented x86/x86-64
o_std detector path is not exercised by CI. Three additional consistency and API-shape issues are noted inline.
Public API and rustdoc extraction completed for the added http_headers and http_headers_simd packages. Focused package tests, doctests, minimal-feature checks, and SIMD tests passed; Miri was unavailable. Previously posted discussion was read and duplicate root causes were omitted.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 11e4f879-cc17-4c84-8590-fca44b4a456b Copilot-Session: 421aaf0a-555e-4b07-998d-1ed192e45d7a
martintmk
left a comment
martintmk
left a comment
There was a problem hiding this comment.
Posted by an AI agent
Warning: Incomplete review coverage
Some review areas could not be executed:
- Public contract and manifests (
eview-api-design): the required specialist entrypoint was unavailable. - Behavioral defects and proof (
eview-correctness): the required specialist entrypoint was unavailable. - Allocations, hot path, and clocks (
eview-perf): the required specialist entrypoint was unavailable. - Metrics, logs, and spans (
eview-telemetry): the required specialist entrypoint was unavailable. - Output-only public contract (
eview-public-api): mandatory isolated documentation filtering was unavailable.
No combined verdict is issued. Completed areas reported no publishable findings.
Summary
http_headers, a typed HTTP header library with borrowed and owned values, composableFieldSource/FieldSinkadapters, optionalhttpand Serde integration, and an Axum examplehttp_headers_simd, the internal scalar/SIMD scanning companion with runtime dispatch and differential coverageValidation
cargo test -p http_headers --all-featurescargo test -p http_headers --no-default-featurescargo test -p http_headers_simd --all-featurescargo test -p http_headers --doc --no-default-featurescargo doc --all-featuresfromcrates/http_headersjust anvil-clippyjust anvil-readme-checkjust anvil-spellcheck