Repository navigation
fix(url): remove dot segments and fold special-scheme separators - #13195
Merged
Merged
Conversation
`new URL()` kept `.` and `..` path segments in `pathname`/`href` because `parse_url` never had the WHATWG path-state pass: it split the authority off and stored the remaining text verbatim (#12864). Add `normalize_hierarchical_path`, which applies the path-shape rules the standard's path state contributes: single-dot (`.`, `%2e`) segments are dropped, double-dot (`..`, `.%2e`, `%2e.`, `%2e%2e`) segments shorten the path, a dot segment in final position appends an empty segment instead (`/a/..` is `/`, `/a/b/..` is `/a/`), empty segments from a `//` run survive, and the `file:`-only rules hold (a Windows drive letter canonicalizes to `:`, so `C|` becomes `C:`, and a path that is only a drive letter is never shortened). Opaque paths (`mailto:`, `data:`) keep their text. The same parse pass was missing the rest of the special-scheme shape, so it lands with it: `\` is a path separator and authority terminator for special schemes (folded before the host is read, while query/fragment keep theirs and a non-special host may not contain one), the `//` after a special scheme is optional with its separator run skipped (`https:x`, `https:/x`, `https:///x` all host `x`), `file:` keeps its own slash states (extra separators are path, empty host allowed) and now canonicalizes its host like any other special scheme. `is_valid_absolute_url` mirrors all of that instead of requiring a literal `//`, and no longer rejects an opaque URL with nothing after the colon. `js_url_set_pathname` runs the same states, since assignment clears the path and re-parses the value. Validated against the pinned Node oracle: - `RUST_TEST_THREADS=1 cargo test --release -p perry-runtime --lib url::` — 25 passed - `PERRY_SKIP_BUILD=1 ./run_parity_tests.sh --suite node-suite --module url` — 69/69 PASS - new node-parity cases print byte-identical output to Node
|
Caution Review failedThe pull request is closed. Finishing Touches
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 |
Drop the Rust unit tests for the path rules (the node-parity cases already cover them), merge the two path helpers into one entry point, detect the special scheme before the fold so the `scheme_slashes_consumed` flag and its post-fold authority check disappear, and keep only the comments that explain a spec quirk the code cannot show.
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
new URL()kept.and..path segments inpathname/href(https://x/a/b/../creported/a/b/../c, Node reports/a/c), and the same parse pass was missing the rest of the WHATWG special-scheme shape —\separators, the optional//,file:slash states and drive letters. Both land together because they are one rule family: the path/authority states of the standard's parser.Changes
crates/perry-runtime/src/url/parse.rsnormalize_hierarchical_path: WHATWG path-state rules over a path string. Single-dot segments (.,%2e) are dropped, double-dot (..,.%2e,%2e.,%2e%2e) shorten the path, a dot segment in final position appends an empty segment (/a/..→/,/a/b/..→/a/), empty segments from a//run survive (//a/../b→//b), a segment that merely contains a dot is untouched (/a/.../b).file:-only: a Windows drive letter canonicalizes to:(C|→C:) and a path that is only a drive letter is never shortened (file:///C:/..→/C:/).parse_urlapplies it to the hierarchical branches only; opaque paths (mailto:,data:) keep their text.\folds to/before the host is read (query/fragment keep theirs, a non-special path keeps them, a non-special host may not contain one); a special scheme is always hierarchical with an optional//and a skipped separator run (https:x,https:/x,https:///xhostx);file:keeps its own slash states (extra separators are path, empty host allowed) and now canonicalizes its host like any other special scheme (file://SERVER/x→server).is_valid_absolute_urlmirrors the parser instead of requiring a literal//, and no longer rejects an opaque URL with nothing after the colon (mailto:is valid, as in Node).crates/perry-runtime/src/url/url_class.rs:js_url_set_pathnameruns the same states (assignment clears the path and re-parses the value); the duplicatedis_special_url_schemenow lives inparse.rsonly.test-parity/node-suite/url/url/dot-segments.tsand.../backslash-and-separators.ts: new node-parity cases.changelog.d/12864-url-dot-segments.md: changelog fragment.Known divergences found while probing but not addressed here (separate defects, candidates for their own issues): the constructor does not apply the WHATWG path percent-encode set (
/a b<cvs Node/a%20b%3Cc),url.domainToASCII("a/b")returnsa/bwhere Node returnsa, and thepathnamesetter stores a value on an opaque URL where Node ignores the assignment.Related issue
Closes #12864
Test plan
cargo build --release -p perry -p perry-runtime-static -p perry-stdlib-static— clean (theperry compilelink surface used for the runs below, plusscripts/gc_call_effects/sidecars.sh target/release)PERRY_SKIP_BUILD=1 ./run_parity_tests.sh --suite node-suite --module url— 69/69 PASS (67 pre-existing + the 2 new cases), no previously-passing case changednode --experimental-strip-types(diffempty, 64 and 43 lines)perry compileprints0 /a/c /c, node prints0 /a/c /ccargo test --workspace …— not run locally (crate-scoped tests plus the module parity suite instead); CI'scargo-testjob is the backstoptest-parity/node-suite/url/url/— theurlmodule's own suite, which is where Node-core module behavior is pinneddocs/src/— no dedicatednode:urlpage exists to update; the behavior is pinned by the node-parity suitecargo fmt --check -p perry-runtimeclean; no new clippy warnings in the touched filesScreenshots / output
Related URL test corpus (
test-files/*url*.tsplus four adjacent cases) shows no new failures: the only two non-passing entries pre-date this change (test_gap_11519_net_url_short_strings.tscannot compile locally withoutlibperry_ext_net.a, andtest_gap_6375_legacy_url_parse_format.tsdiffers only ondomainToASCII/domainToUnicodelines).Checklist
feat:/fix:/docs:/chore:prefix convention used in the log