From c1af5d78aaf75633e0260efbd48720c130dc8876 Mon Sep 17 00:00:00 2001 From: eeshsaxena Date: Sat, 15 Aug 2026 23:48:04 +0530 Subject: [PATCH] fix(parser): collapse a leading // in sanitizeUrl 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/. --- packages/parser/src/utils/normalize-href.ts | 12 +++++- packages/parser/test/sanitize-url.test.js | 48 +++++++++++++++++++++ tests/cli-contracts/asset-base-url.test.js | 15 ++++--- 3 files changed, 66 insertions(+), 9 deletions(-) create mode 100644 packages/parser/test/sanitize-url.test.js diff --git a/packages/parser/src/utils/normalize-href.ts b/packages/parser/src/utils/normalize-href.ts index 13d6d38a..9a6091c1 100644 --- a/packages/parser/src/utils/normalize-href.ts +++ b/packages/parser/src/utils/normalize-href.ts @@ -232,6 +232,14 @@ export function normalizeMenubarPaths(items: any[]): void { */ export function sanitizeUrl(url: string): string { if (!url) return url; - // Collapse double+ slashes, but preserve protocol:// - return url.replace(/([^:])\/\/+/g, '$1/'); + // Preserve a leading `scheme://` (e.g. `https://`) but collapse every other + // run of consecutive slashes. A leading `//` must collapse too: browsers + // treat `//host/path` as a protocol-relative URL pointing at a *different + // host*, so an accidental `//search/` (from base + `/search`) would resolve + // to `https://search/` instead of the intended same-site `/search/`. + const scheme = url.match(/^[a-z][a-z0-9+.-]*:\/\//i); + if (scheme) { + return scheme[0] + url.slice(scheme[0].length).replace(/\/{2,}/g, '/'); + } + return url.replace(/\/{2,}/g, '/'); } \ No newline at end of file diff --git a/packages/parser/test/sanitize-url.test.js b/packages/parser/test/sanitize-url.test.js new file mode 100644 index 00000000..16231e40 --- /dev/null +++ b/packages/parser/test/sanitize-url.test.js @@ -0,0 +1,48 @@ +/** + * -------------------------------------------------------------------- + * docmd : the zero-config documentation engine. + * + * sanitizeUrl — slash-collapsing safety net. + * + * Regression coverage for a leading `//` collapsing to a single `/`. + * A leading `//host/path` is a protocol-relative URL: a browser resolves + * it against the current *scheme* but a *different host*, so an accidental + * `//search/` (produced by `base` + `/search`, base normalised to `/`) + * would navigate to `https://search/` instead of the same-site `/search/`. + * The scheme separator in a real absolute URL (`https://`) must survive. + * + * Run: `pnpm --filter @docmd/parser test` + * -------------------------------------------------------------------- + */ + +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { sanitizeUrl } from '../dist/index.js'; + +test('sanitizeUrl collapses a leading // (protocol-relative) to /', () => { + assert.equal(sanitizeUrl('//search/'), '/search/'); + assert.equal(sanitizeUrl('//docs//guide/'), '/docs/guide/'); +}); + +test('sanitizeUrl collapses interior and trailing runs of slashes', () => { + assert.equal(sanitizeUrl('/a///b////c/'), '/a/b/c/'); + assert.equal(sanitizeUrl('/guide/'), '/guide/'); +}); + +test('sanitizeUrl preserves the scheme:// separator', () => { + assert.equal(sanitizeUrl('https://a.com//b'), 'https://a.com/b'); + assert.equal(sanitizeUrl('https://x.com/a/b/'), 'https://x.com/a/b/'); + assert.equal(sanitizeUrl('http://h//a//b'), 'http://h/a/b'); + assert.equal(sanitizeUrl('ws://h//x'), 'ws://h/x'); +}); + +test('sanitizeUrl leaves clean relative paths untouched', () => { + assert.equal(sanitizeUrl('../de/guide/'), '../de/guide/'); + assert.equal(sanitizeUrl(''), ''); +}); + +test('a browser resolves the sanitised path against the current host', () => { + // The whole point of the fix: `//search/` would resolve cross-host. + const href = sanitizeUrl('//search/'); + assert.equal(new URL(href, 'https://example.com/docs/').href, 'https://example.com/search/'); +}); diff --git a/tests/cli-contracts/asset-base-url.test.js b/tests/cli-contracts/asset-base-url.test.js index c5626261..e4cf5acd 100644 --- a/tests/cli-contracts/asset-base-url.test.js +++ b/tests/cli-contracts/asset-base-url.test.js @@ -297,19 +297,20 @@ export const test = runTestFile({ assert(result.ok, 'URL-3b: workspace build for switcher test succeeds'); const mainHtml = fs.readFileSync(path.join(proj, 'site/index.html'), 'utf8'); // Find every project-switcher-item link and capture { href, title }. - // The href is protocol-relative (//search/) because buildAbsoluteUrl - // normalises the empty base to '/', which combines with /search to - // //search. Browsers treat // as the same-scheme prefix, so this is - // equivalent to /search/ in absolute terms. + // The href must be the same-site absolute path `/search/`. It must NOT + // be protocol-relative (`//search/`): a browser resolves `//search/` + // against a *different host* (`https://search/`), not the current site, + // so the switcher link would leave the docs entirely. const switcherHrefs = Array.from(mainHtml.matchAll(/]*>([\s\S]*?)<\/a>/g)) .map(m => ({ href: m[1], title: (m[2].match(/([^<]+)<\/span>/) || [])[1] })); const searchHref = switcherHrefs.find(h => h.title === 'search'); assert(searchHref, 'URL-3b: project switcher has a link to "search" sub-site'); // The previous bug emitted /search (no slash) which made the // browser treat the URL as a file when npx serve served the - // directory index. The fix keeps the trailing slash. - assert(searchHref && /\/search\/$/.test(searchHref.href), - `URL-3b: project switcher link to /search sub-site ends with /search/ (got: ${searchHref?.href})`); + // directory index. The fix keeps the trailing slash, and the link + // must stay same-site (exactly `/search/`, never `//search/`). + assert(searchHref && /^\/search\/$/.test(searchHref.href), + `URL-3b: project switcher link to /search sub-site is exactly "/search/" (got: ${searchHref?.href})`); // The root project link should be relative "./" or absolute "/" (no extra trailing slash // for the root). const mainHref = switcherHrefs.find(h => h.title === 'main');