perf: single-scan fast-path guard for resolveDotSegments#1458
Merged
Conversation
Co-Authored-By: Claude Fable 5 <[email protected]>
Co-Authored-By: Claude Fable 5 <[email protected]>
Collapse the fast-path guard (2 regex tests + 2 includes scans) into a
single scan of one of four mode-indexed trigger regexes, built from
shared source strings so each trigger pattern still lives in one place.
Each mode's regex contains exactly its own triggers, so a non-match
remains an exact no-op guarantee.
Fast path with { decodeSlashes, mergeSlashes }: 89ns -> 38ns (-58%);
plain mode 37ns -> 29ns; slow path unchanged. Differential fuzz vs the
previous implementation: 966k comparisons across all four modes, no
divergence.
Co-Authored-By: Claude Fable 5 <[email protected]>
📝 WalkthroughWalkthroughChangesPath normalization
v2 planning
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This was referenced Jul 14, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
v2.md (2)
4-4: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winTie fallback removal to concrete migration criteria.
“After a migration window” should be tied to the maximum legacy-cookie/session lifetime and fallback-use telemetry. Otherwise, removing it can invalidate active sessions, while retaining it unnecessarily prolongs weak-KDF compatibility and extra CPU work.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@v2.md` at line 4, Update the migration guidance for removing the iterations: 1 fallback in unsealSession and the legacySealFallback option to require both expiration of the maximum legacy-cookie/session lifetime and confirmation from fallback-use telemetry that no legacy sessions remain active. State these concrete criteria instead of using only an unspecified migration window.
3-3: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winDefine the cap override contract before implementation.
readValidatedBodyanddefineJsonRpcHandlercurrently do not share amaxSizeoption. Also,Infinitywould provide an easy way to reintroduce unbounded buffering. Specify how the option propagates and restrict unlimited reads to an explicit trusted/unsafe path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@v2.md` at line 3, Define the request-body limit contract before implementing the cap: add and consistently propagate a maxSize option through readBody, readValidatedBody, and defineJsonRpcHandler, defaulting to DEFAULT_BODY_LIMIT. Do not allow Infinity through the general option; expose unlimited buffering only via an explicitly named trusted/unsafe API or path, and ensure that path is intentional and clearly distinguishable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@v2.md`:
- Line 4: Update the migration guidance for removing the iterations: 1 fallback
in unsealSession and the legacySealFallback option to require both expiration of
the maximum legacy-cookie/session lifetime and confirmation from fallback-use
telemetry that no legacy sessions remain active. State these concrete criteria
instead of using only an unspecified migration window.
- Line 3: Define the request-body limit contract before implementing the cap:
add and consistently propagate a maxSize option through readBody,
readValidatedBody, and defineJsonRpcHandler, defaulting to DEFAULT_BODY_LIMIT.
Do not allow Infinity through the general option; expose unlimited buffering
only via an explicitly named trusted/unsafe API or path, and ensure that path is
intentional and clearly distinguishable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 56ab266a-ef25-4cb5-897b-34abac063f05
📒 Files selected for processing (3)
src/utils/path.tstest/unit/path.test.tsv2.md
pi0
changed the title
pi0
changed the title
resolveDotSegments
pi0
approved these changes
Jul 14, 2026
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
2 participants
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
Collapses
resolveDotSegments' fast-path guard — previously 2 regex tests + 2includesscans (\\,ENCODED_SEP_RE,//,DOT_SEGMENT_RE) — into a single scan of one of four mode-indexed trigger regexes ((decodeSlashes?1:0) | (mergeSlashes?2:0)).DOT_SEGMENT_SRC,ENCODED_SEP_SRC), so each trigger pattern still lives in exactly one place.hasEncodedSeppre-test becomes an opt-gated global replace — a no-op when nothing matches.Why
resolveDotSegmentsruns per request inserveStaticand downstream security consumers (h3-rules calls it on every matched request for dual-path rule matching), and the overwhelmingly common input is an already-canonical path — the guard is the hot path.Benchmarks (mitata, node 24, x64):
{ decodeSlashes, mergeSlashes }/.well-known/security.txt, both optsVerification
%252e,%25%32%66,\,//,..., dot-prefixed names) — zero divergence./.well-known/…,/a/..b/c,/a%2eb/c,/a/...stay untouched in every mode.Follow-up
Planned next: export the guard as a public
isCanonicalPath(path, opts)predicate (pinned by a property test againstresolveDotSegmentsidentity) so security-adjacent callers like h3-rules can skip canonicalization calls entirely without duplicating decode knowledge.🤖 Generated with Claude Code
Summary by CodeRabbit
isCanonicalPath(path, opts?)utility to let you check whether path resolution would leave the input unchanged../..resolution, slash decoding, and optional slash merging behavior.isCanonicalPath.