Repository navigation
Merge master (upstream sync) into integration/microscaler-fork - #12
Merged
Merged
Conversation
- examples/techempower.rs: sort_by → sort_by_key (clippy unnecessary_sort_by)
- examples/headers.rs: inline {bind_addr} (clippy uninlined_format_args)
- docker/Dockerfile.test: rust 1.83 → 1.85 so transitive cpufeatures v0.3 (edition2024) can build
* feat(response): allow owned header values (no Box::leak)
* ci: fix clippy + Dockerfile Rust version
- examples/techempower.rs: sort_by → sort_by_key (clippy unnecessary_sort_by)
- examples/headers.rs: inline {bind_addr} (clippy uninlined_format_args)
- docker/Dockerfile.test: rust 1.83 → 1.85 so transitive cpufeatures v0.3 (edition2024) can build
* ci: bump Dockerfile.test to Rust 1.88 for cookie_store/time/icu deps
* perf(request): remove redundant \r\n\r\n pre-scan before httparse
The pre-scan at the top of decode() was added to fix issue Xudong-Huang#18 ("Token"
parsing errors when headers arrive across multiple TCP packets) by
returning Ok(None) whenever the buffer didn't yet contain the end-of-
headers marker.
The pre-check is redundant because httparse::Request::parse already
returns Status::Partial on every form of truncation we care about:
- mid-method, mid-path, mid-version
- mid-header-name, mid-header-value
- buffer ending before the terminating \r\n\r\n
Our Partial arm below already maps that to Ok(None), so the pre-check
only adds an O(n) window scan on every decode() call — one that's
particularly costly in the warm read path where the buffer is typically
a full 8 KB frame.
The "Token" errors from issue Xudong-Huang#18 are unrelated to the end-of-headers
marker: httparse only returns Token / HeaderName / Version on genuinely
malformed bytes (e.g. 'G@T', space in path). Those are permanent
failures regardless of how much more data arrives; surfacing them as
400 at first sight is more correct than stalling the connection until
a \r\n\r\n happens to show up later.
An empirical probe across 15 input shapes (each truncation + each
malformed case) confirms: truncation always yields Partial, malformed
bytes always yield their specific error class. No scenario existed
where the pre-check converted a Token error into a Partial.
Small hot-path win (saves one linear scan per request) with no
behavioural change on valid or invalid traffic.
* test(request_parsing): ignore wall-clock perf smokes + assert correctness
The two tests in the `performance` sub-module (`benchmark_header_counting`,
`benchmark_completion_check`) assert on absolute wall-clock thresholds
(100 ms / 2 s) against test-local helpers (`count_headers`,
`has_complete_headers`). That combination is fundamentally CI-hostile:
- Debug-build timing varies by an order of magnitude under concurrent
test load. On this machine `benchmark_completion_check` runs in
~0.8 s in isolation and ~2.0 s when the whole suite runs in parallel
— straddling the 2 s bound and flaking either way depending on
scheduler luck.
- They benchmark *test-local helpers*, not production parser code. A
regression here would signal the test harness got slower, not that
`decode()` or `httparse` did.
Fix:
- Mark both `#[ignore]` so `cargo test` skips them by default while
`cargo test -- --ignored performance::` still runs them for manual
perf spot-checks.
- Replace the fragile `assert!(duration < X)` with stable correctness
assertions (every iteration returns the same expected count / hit),
guarded by `std::hint::black_box` to prevent the optimiser from
turning the loop into a no-op.
- Add a module-level doc comment explaining the intent and the
--ignored invocation.
Resulting test counts:
- default: 19 passed / 2 ignored (was 21 passed, occasionally 1 FAIL)
- ignored: 2 passed / 0 failed (runs in ~0.5 s)
master is a sync of Xudong-Huang/may_minihttp master. New here: - 826a761 perf(request): drop the \r\n\r\n pre-scan before httparse (httparse reports a truncated head as Partial); the perf smoke tests in request_parsing are #[ignore]d with correctness assertions. - 3eadfd7 dev-dependencies: atoi 3, nanorand 0.8, testcontainers 0.28; unstable sort in the techempower example. - 45d0ac9 WyRand::new_seed in the example and tests. 054c72a, 580447b and 8d82724 were already here (same patches). Conflicts: - src/request.rs: the pre-scan is dropped as upstream does, and the head limit kept: only the first 64 KiB are parsed, and a head still Partial at that size is refused with 431, as before. - src/response.rs, src/lib.rs: ours (the owned-header change upstream is already here, extended with header_kv and the chunked writer). - tests/goose_header_load_test.rs, tests/header_traffic_integration.rs: ours (the server on its own thread, Connection: close probes for Windows). - Cargo.toml: testcontainers 0.28 with our gumdrop. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y3TSRmEZAYU6RBUXhkn7WF
casibbald
added a commit
to microscaler/BRRTRouter
that referenced
this pull request
Oct 27, 2026
test_write_handler_response_403_uses_forbidden_not_ok sent a 13-byte body (`foo=bar&baz=1`) with `Content-Length: 11`. The server read 11 bytes as the body and the remaining `=1` as the start of the next request. may_minihttp used to wait for a `\r\n\r\n` that never came; since the upstream sync (microscaler/may_minihttp#12) it parses those bytes at once and answers 400 on the same connection, which the test then read as part of the 403's body. The request now declares its 13 bytes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y3TSRmEZAYU6RBUXhkn7WF
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
This merges
masterintointegration/microscaler-fork.masteris an exact sync of upstream Xudong-Huang/may_minihttpmaster: there are no commits on either side that the other lacks.masterhas six commits thatintegrationlacks. Three of them are already here as identical patches (git log --cherry-markmarks them=):WyRand::new_seed(0)in the techempower example and tests (nanorand 0.8 API)homepagedropped from Cargo.toml\r\n\r\npre-scan before httparse; perf smoke tests inrequest_parsingbecome#[ignore]with correctness assertionsWhy a merge, not a rebase or cherry-picks
integration/microscaler-forkin lock files and git dependencies. A merge keeps every existing SHA reachable.mastertracks upstream, so merging makesmasteran ancestor ofintegration. Each future upstream sync then shows only what is new. Cherry-picking would leavemasterpermanently "ahead" in comparisons, with duplicate patches to re-skip every time.Conflicts and how they were resolved
src/request.rs. Upstream dropped the pre-scan. Our hardening (fix(server): HTTP/1.1 request framing and limits for internet-facing use #5) had turned that scan into the request-head limit: no end of head within 64 KiB returns 431. The resolution:MAX_REQUEST_HEAD_BYTES) and refusing a head that is stillPartialat that size with 431, as before;tests/http_hardening.rs(431 on an oversized head, smuggling, framing) passes unchanged.src/response.rs,src/lib.rs: ours. Upstream's owned-header change is already here. Ours has since grownheader_kv, the CR/LF guard, overflow beyond 16 headers and the chunked writer.tests/goose_header_load_test.rs,tests/header_traffic_integration.rs: ours. These keep the server on its own thread and theConnection: closeprobes that fixed the Windows CI.Cargo.toml. testcontainers 0.28 (upstream), keeping ourgumdrop.Untouched by the merge:
header_kv,Interceptorand thetracingfeature.Tests and benchmarks
Tests.
cargo fmt --check, clippy (default with-D warnings, and all features with all targets), and the full suite on Rust 1.99.0: all features 320 passed, default features passed.Benchmarks. ms02, five interleaved rounds, medians. Load climbed from 11 to 29 during the run, so req/s varies ±10%.
-c 64 GET /json: p50-c 64 GET /json: req/s-c 64, 4 KiB header: p50-c 64, 4 KiB header: req/sbench_client pool(16 × 3000): p50No regression. The large-header case benefits from the dropped pre-scan.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Y3TSRmEZAYU6RBUXhkn7WF