From 7ca7acac8bca6fcb7ff5b7d76d7c9dc891b1ce3c Mon Sep 17 00:00:00 2001 From: James Date: Fri, 22 May 2026 15:41:44 +0100 Subject: [PATCH 1/2] fix(link): warn on repeated forward slashes in href MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #1554 When a `` contains repeated forward-slashes (e.g. `/foo//bar`) or a backslash, Next.js's `resolveHref` emits a `console.error` so the developer notices the invalid URL during dev/CI. vinext's Link shim did not surface that warning, which caused the Next.js Deploy Suite test `test/e2e/repeated-forward-slashes-error` to fail waiting for the message in the CLI output. Add a small `warnOnRepeatedSlashesInHref` helper that mirrors the regex and message used by Next.js, and call it from the Link body after the href is resolved. The href continues to render and navigate unchanged — Next.js does not block, it only warns. Ported from Next.js: packages/next/src/client/resolve-href.ts --- packages/vinext/src/shims/link.tsx | 41 +++++++++++++++++++++ tests/link.test.ts | 57 ++++++++++++++++++++++++++++++ 2 files changed, 98 insertions(+) diff --git a/packages/vinext/src/shims/link.tsx b/packages/vinext/src/shims/link.tsx index f71800d9d..ff0522171 100644 --- a/packages/vinext/src/shims/link.tsx +++ b/packages/vinext/src/shims/link.tsx @@ -152,6 +152,40 @@ function resolveHref(href: LinkProps["href"]): string { return url; } +// Track hrefs we have already warned about so we do not spam the console on +// every re-render. Mirrors React's behaviour for repeated warnings. +const warnedRepeatedSlashHrefs = new Set(); + +/** + * Emit Next.js's "Invalid href" console.error when `href` contains repeated + * forward slashes or backslashes in its path portion. + * + * Ported from Next.js: packages/next/src/client/resolve-href.ts + * https://github.com/vercel/next.js/blob/canary/packages/next/src/client/resolve-href.ts + * + * Matches the message asserted by: + * test/e2e/repeated-forward-slashes-error/repeated-forward-slashes-error.test.ts + */ +function warnOnRepeatedSlashesInHref(urlAsString: string): void { + // Strip any protocol prefix (e.g. "https://") so we do not flag the + // legitimate `//` that separates the scheme from the authority. + const urlProtoMatch = urlAsString.match(/^[a-z][a-z0-9+.-]*:\/\//i); + const urlAsStringNoProto = urlProtoMatch + ? urlAsString.slice(urlProtoMatch[0].length) + : urlAsString; + const urlParts = urlAsStringNoProto.split("?", 1); + if (!(urlParts[0] || "").match(/(\/\/|\\)/)) return; + + if (warnedRepeatedSlashHrefs.has(urlAsString)) return; + warnedRepeatedSlashHrefs.add(urlAsString); + + const pathname = + typeof window !== "undefined" && window.location ? window.location.pathname : "/"; + console.error( + `Invalid href '${urlAsString}' passed to next/router in page: '${pathname}'. Repeated forward-slashes (//) or backslashes \\ are not valid in the href.`, + ); +} + export function resolveLinkPrefetchMode( prefetchProp: LinkProps["prefetch"], isDangerous: boolean, @@ -537,6 +571,13 @@ const Link = forwardRef(function Link( // where href is a route pattern like "/user/[id]" and as is "/user/1") const resolvedHref = as ?? resolveHref(href); + // Mirror Next.js: emit a console.error when the href contains repeated + // forward-slashes (e.g. "/foo//bar") or backslashes. Next.js does not block + // navigation — it only warns. See packages/next/src/client/resolve-href.ts. + if (typeof resolvedHref === "string") { + warnOnRepeatedSlashesInHref(resolvedHref); + } + const isDangerous = typeof resolvedHref === "string" && isDangerousScheme(resolvedHref); // Apply locale prefix if specified (safe even for dangerous hrefs since we diff --git a/tests/link.test.ts b/tests/link.test.ts index 99d480626..13380047b 100644 --- a/tests/link.test.ts +++ b/tests/link.test.ts @@ -138,6 +138,63 @@ describe("Link rendering", () => { }); }); +// ─── Repeated-slash warning (parity with Next.js) ─────────────────────── +// +// Ported from Next.js: test/e2e/repeated-forward-slashes-error/repeated-forward-slashes-error.test.ts +// https://github.com/vercel/next.js/blob/canary/test/e2e/repeated-forward-slashes-error/repeated-forward-slashes-error.test.ts +// +// Next.js's `resolveHref` emits a `console.error` when an href contains +// repeated forward-slashes (e.g. "/hello//world") or backslashes. Navigation +// is not blocked; only a warning is surfaced. + +describe("Link repeated-slash warning", () => { + let consoleSpy: ReturnType; + + beforeEach(() => { + consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + }); + + afterEach(() => { + consoleSpy.mockRestore(); + }); + + it("logs a console.error when href contains repeated forward slashes", () => { + ReactDOMServer.renderToString(React.createElement(Link, { href: "/hello//world" }, "Hello")); + expect(consoleSpy).toHaveBeenCalled(); + const message = consoleSpy.mock.calls[0]?.[0] as string; + expect(message).toContain("Invalid href '/hello//world'"); + expect(message).toContain( + "Repeated forward-slashes (//) or backslashes \\ are not valid in the href.", + ); + }); + + it("logs a console.error when href contains a backslash", () => { + ReactDOMServer.renderToString(React.createElement(Link, { href: "/foo\\bar" }, "Bad")); + expect(consoleSpy).toHaveBeenCalled(); + const message = consoleSpy.mock.calls[0]?.[0] as string; + expect(message).toContain("Invalid href '/foo\\bar'"); + }); + + it("does not warn for absolute URLs whose only '//' is the protocol separator", () => { + ReactDOMServer.renderToString( + React.createElement(Link, { href: "https://example.com/path" }, "Ext"), + ); + expect(consoleSpy).not.toHaveBeenCalled(); + }); + + it("does not warn for hrefs without repeated slashes", () => { + ReactDOMServer.renderToString(React.createElement(Link, { href: "/normal/path" }, "Normal")); + expect(consoleSpy).not.toHaveBeenCalled(); + }); + + it("ignores repeated slashes inside the query string", () => { + // Next.js only checks the path portion (everything before '?'), so a + // query string containing '//' must not trigger the warning. + ReactDOMServer.renderToString(React.createElement(Link, { href: "/ok?next=//foo//bar" }, "Q")); + expect(consoleSpy).not.toHaveBeenCalled(); + }); +}); + // ─── useLinkStatus ────────────────────────────────────────────────────── describe("useLinkStatus", () => { From 074d07ff7db48cbd01bce40d5530f5d577eb9171 Mon Sep 17 00:00:00 2001 From: James Date: Fri, 22 May 2026 15:50:37 +0100 Subject: [PATCH 2/2] fix(link): normalize repeated slashes and drop dedup for Next.js parity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review feedback on #1563: - Remove the module-level `warnedRepeatedSlashHrefs` Set. Next.js fires this warning on every `resolveHref` call, so dedup was an unintended divergence and a latent test-ordering hazard. - Add `normalizeRepeatedSlashes` (port of packages/next/src/shared/lib/utils/normalize-repeated-slashes.ts) and apply it after warning, so `` renders `href="/a/b"` exactly as Next.js does. - Preserve protocol-relative URLs (`//example.com/...`) unchanged — vinext treats those as external via `isAbsoluteOrProtocolRelativeUrl`, so we skip both the warning and the normalization for them to avoid regressing the existing "locale does not mangle protocol-relative URLs" test. - Document the `window.location.pathname` fallback (Next.js uses `router.pathname`; the difference is cosmetic and not asserted by the compat test). - Add tests covering no-dedup behaviour, href normalization (including backslashes, query-string preservation, and protocol-prefixed URLs). --- packages/vinext/src/shims/link.tsx | 66 ++++++++++++++++++++++-------- tests/link.test.ts | 44 ++++++++++++++++++++ 2 files changed, 94 insertions(+), 16 deletions(-) diff --git a/packages/vinext/src/shims/link.tsx b/packages/vinext/src/shims/link.tsx index ff0522171..9406e1c78 100644 --- a/packages/vinext/src/shims/link.tsx +++ b/packages/vinext/src/shims/link.tsx @@ -152,21 +152,53 @@ function resolveHref(href: LinkProps["href"]): string { return url; } -// Track hrefs we have already warned about so we do not spam the console on -// every re-render. Mirrors React's behaviour for repeated warnings. -const warnedRepeatedSlashHrefs = new Set(); +/** + * Collapse repeated forward-slashes (and convert backslashes to forward-slashes) + * in the path portion of a URL, preserving any query string. + * + * Ported from Next.js: packages/next/src/shared/lib/utils/normalize-repeated-slashes.ts + * https://github.com/vercel/next.js/blob/canary/packages/next/src/shared/lib/utils/normalize-repeated-slashes.ts + */ +function normalizeRepeatedSlashes(url: string): string { + const urlParts = url.split("?"); + const urlNoQueryString = urlParts.shift() ?? ""; + const queryString = urlParts.join("?"); + return ( + urlNoQueryString.replace(/\\/g, "/").replace(/\/\/+/g, "/") + + (queryString ? `?${queryString}` : "") + ); +} /** - * Emit Next.js's "Invalid href" console.error when `href` contains repeated - * forward slashes or backslashes in its path portion. + * Emit Next.js's "Invalid href" `console.error` when `href` contains repeated + * forward slashes or backslashes in its path portion, and return the + * normalized URL (with `\\` converted to `/` and runs of `/` collapsed). If + * the href is already well-formed, the original string is returned unchanged. * * Ported from Next.js: packages/next/src/client/resolve-href.ts * https://github.com/vercel/next.js/blob/canary/packages/next/src/client/resolve-href.ts * * Matches the message asserted by: * test/e2e/repeated-forward-slashes-error/repeated-forward-slashes-error.test.ts + * + * Note: Next.js fires this warning unconditionally on every call to + * `resolveHref`. We mirror that behaviour (no dedup) for exact parity. + * + * Note: Next.js uses `router.pathname` (the route pattern, e.g. + * `/posts/[id]`) for the "in page" segment of the message. We do not have + * cheap access to the route pattern from inside the Link shim, so we + * fall back to `window.location.pathname` (or `"/"` during SSR). The text + * is cosmetic and is not asserted by the Next.js compat test. */ -function warnOnRepeatedSlashesInHref(urlAsString: string): void { +function warnAndNormalizeRepeatedSlashesInHref(urlAsString: string): string { + // Protocol-relative URLs (e.g. "//example.com/path") are treated by vinext + // as external — see `isAbsoluteOrProtocolRelativeUrl` in url-utils. We + // intentionally skip the repeated-slash warning and normalization for them + // so that locale prefixing and same-origin detection elsewhere in this + // shim continue to receive the original href. (Next.js itself does flag + // these, but our external-URL handling supersedes that behaviour.) + if (urlAsString.startsWith("//")) return urlAsString; + // Strip any protocol prefix (e.g. "https://") so we do not flag the // legitimate `//` that separates the scheme from the authority. const urlProtoMatch = urlAsString.match(/^[a-z][a-z0-9+.-]*:\/\//i); @@ -174,16 +206,16 @@ function warnOnRepeatedSlashesInHref(urlAsString: string): void { ? urlAsString.slice(urlProtoMatch[0].length) : urlAsString; const urlParts = urlAsStringNoProto.split("?", 1); - if (!(urlParts[0] || "").match(/(\/\/|\\)/)) return; - - if (warnedRepeatedSlashHrefs.has(urlAsString)) return; - warnedRepeatedSlashHrefs.add(urlAsString); + if (!(urlParts[0] || "").match(/(\/\/|\\)/)) return urlAsString; const pathname = typeof window !== "undefined" && window.location ? window.location.pathname : "/"; console.error( `Invalid href '${urlAsString}' passed to next/router in page: '${pathname}'. Repeated forward-slashes (//) or backslashes \\ are not valid in the href.`, ); + + const normalizedNoProto = normalizeRepeatedSlashes(urlAsStringNoProto); + return (urlProtoMatch ? urlProtoMatch[0] : "") + normalizedNoProto; } export function resolveLinkPrefetchMode( @@ -569,14 +601,16 @@ const Link = forwardRef(function Link( // If `as` is provided, use it as the actual URL (legacy Next.js pattern // where href is a route pattern like "/user/[id]" and as is "/user/1") - const resolvedHref = as ?? resolveHref(href); + const rawResolvedHref = as ?? resolveHref(href); // Mirror Next.js: emit a console.error when the href contains repeated - // forward-slashes (e.g. "/foo//bar") or backslashes. Next.js does not block - // navigation — it only warns. See packages/next/src/client/resolve-href.ts. - if (typeof resolvedHref === "string") { - warnOnRepeatedSlashesInHref(resolvedHref); - } + // forward-slashes (e.g. "/foo//bar") or backslashes, and then normalize the + // href so navigation targets the collapsed path rather than the raw one. + // See packages/next/src/client/resolve-href.ts. + const resolvedHref = + typeof rawResolvedHref === "string" + ? warnAndNormalizeRepeatedSlashesInHref(rawResolvedHref) + : rawResolvedHref; const isDangerous = typeof resolvedHref === "string" && isDangerousScheme(resolvedHref); diff --git a/tests/link.test.ts b/tests/link.test.ts index 13380047b..d08152fb2 100644 --- a/tests/link.test.ts +++ b/tests/link.test.ts @@ -193,6 +193,50 @@ describe("Link repeated-slash warning", () => { ReactDOMServer.renderToString(React.createElement(Link, { href: "/ok?next=//foo//bar" }, "Q")); expect(consoleSpy).not.toHaveBeenCalled(); }); + + it("fires on every render (no dedup, matches Next.js behaviour)", () => { + // Next.js's resolve-href.ts does NOT dedupe these warnings — every call + // emits a console.error. Confirm we do the same so repeated renders + // surface every offending href. + const el = React.createElement(Link, { href: "/dup//slash" }, "Dup"); + ReactDOMServer.renderToString(el); + ReactDOMServer.renderToString(el); + expect(consoleSpy).toHaveBeenCalledTimes(2); + }); + + it("normalises repeated forward slashes in the rendered href", () => { + // Next.js mirrors Vercel's gateway behaviour: after warning, the href is + // collapsed so the browser navigates to the canonical path. + const html = ReactDOMServer.renderToString( + React.createElement(Link, { href: "/hello//world" }, "Hello"), + ); + expect(html).toContain('href="/hello/world"'); + expect(html).not.toContain("//world"); + }); + + it("normalises backslashes to forward slashes in the rendered href", () => { + const html = ReactDOMServer.renderToString( + React.createElement(Link, { href: "/foo\\bar" }, "Bad"), + ); + expect(html).toContain('href="/foo/bar"'); + expect(html).not.toContain("\\"); + }); + + it("preserves the query string when normalising repeated slashes", () => { + const html = ReactDOMServer.renderToString( + React.createElement(Link, { href: "/a//b?x=1&y=2" }, "Q"), + ); + expect(html).toContain('href="/a/b?x=1&y=2"'); + }); + + it("preserves the protocol when normalising absolute URLs", () => { + // The "//" between scheme and authority must survive normalisation, but + // a duplicate slash in the *path* portion must still collapse. + const html = ReactDOMServer.renderToString( + React.createElement(Link, { href: "https://example.com//foo//bar" }, "Ext"), + ); + expect(html).toContain('href="https://example.com/foo/bar"'); + }); }); // ─── useLinkStatus ──────────────────────────────────────────────────────