fix(proxy): allocation-free host matching in the request hot path#99
Merged
GordonBeeming merged 1 commit intomainfrom Apr 9, 2026
Merged
fix(proxy): allocation-free host matching in the request hot path#99GordonBeeming merged 1 commit intomainfrom
GordonBeeming merged 1 commit intomainfrom
Conversation
The previous normalize_host helper allocated a fresh String for every rule on every request via .to_ascii_lowercase(). In a proxy that sits on the request hot path that meant N + 1 String allocations per incoming connection (one for the request host, one per rule for the .find() closure). Caught by copilot-pull-request-reviewer in two review threads on PR #98. Fix: replace normalize_host with a host_matches helper that does - strip a single trailing '.' on each side via str::strip_suffix (slice operation, no copy) - eq_ignore_ascii_case for the case-fold comparison (byte-by-byte, no copy) Zero allocations per call. Same semantics as before: "API.GITHUB.COM" matches "api.github.com" matches "api.github.com.". Both call sites updated (check_host_allowed and check_request). Refs #98 Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: GitButler <gitbutler@gitbutler.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes per-request hostname normalization allocations in the proxy hot path by switching from a normalize_host(&str) -> String helper to an allocation-free host_matches(&str, &str) -> bool comparison that is case-insensitive and ignores a single trailing dot.
Changes:
- Replaced
normalize_host(allocatingString) withhost_matchesusingstrip_suffix('.')+eq_ignore_ascii_case. - Updated both host-allowlist lookup call sites (
check_host_allowedandcheck_request) to usehost_matches.
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.
Summary
normalize_host -> Stringhelper withhost_matches(&str, &str) -> boolthat usesstr::strip_suffix('.')+eq_ignore_ascii_case.API.GITHUB.COM,api.github.com., andapi.github.comall still match each other.check_host_allowedandcheck_request.Why
normalize_host(&rule.host)allocated a freshStringfor every rule on every incoming request, which is a meaningful amount of allocator churn for a proxy on the request hot path. Caught bycopilot-pull-request-reviewerin two inline threads on #98 (thread 1, thread 2) — both threads went unanswered at #98 merge time because autopilot's idle baseline was anchored tolastCommitinstead ofmax(lastCommit, readyForReviewAt). The skill has been updated to wait the full 15-minute safety cap fromreadyForReviewAt, and this PR is the actual code fix the reviewer asked for.Test plan
cargo build --releasecleanapi.github.comaccepts a request toAPI.GITHUB.COMapi.github.com.(trailing dot)evil.comblocksgood.comRefs #98