fix(parser): collapse a leading // in sanitizeUrl - #203
Merged
Conversation
sanitizeUrl used /([^:])\/\/+/g, which can only collapse a run of slashes that has a non-colon char in front of it. A leading // was therefore left untouched, contradicting the function's own docstring example (//docs//guide/ -> /docs/guide/). This surfaces in workspace builds: buildAbsoluteUrl normalises an empty base to '/' and combines it with an absolute project path like /search, producing //search/. A browser treats a leading // as protocol-relative, so the project-switcher link resolved to https://search/ (a different host) instead of the intended same-site /search/. Collapse every run of slashes, preserving only a genuine leading scheme:// separator. Adds a sanitizeUrl unit test and tightens the workspace switcher assertion to require an exact /search/.
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.
sanitizeUrlcollapses accidental double slashes, but it can't touch a leading//:The regex
/([^:])\/\/+/gonly matches a slash-run that has a non-colon character in front of it, so a run at the very start of the string is never collapsed.That leading
//is not harmless. In a workspace build,buildAbsoluteUrlnormalises an empty base to/and combines it with an absolute project path such as/search, producing//search/. A browser reads a leading//as a protocol-relative URL, i.e.//host/path, so the project-switcher link resolves against a different host:There's actually a test in
tests/cli-contracts/asset-base-url.test.jsthat already captured this//search/output and rationalised it as "equivalent to/search/in absolute terms" — but it isn't;//search/points at the hostsearch. Its assertion used/\/search\/$/, which passes for both forms, so the broken link slipped through.The fix collapses every run of slashes and only preserves a genuine leading
scheme://separator (https://,ws://, ...). I also fixed the misleading comment in that test and tightened its assertion to require an exact/search/.Verification:
packages/parser/test/sanitize-url.test.js— new unit test (leading//, interior/trailing runs,scheme://preservation, and thenew URL(...)cross-host resolution). It fails onmainand passes with the change.resolveHrefdocumented examples and a 300k-iteration fuzz of the href/nav helpers are unchanged (no new crashes, no altered outputs).