fix(pages-router): normalize repeated URL slashes

Author: james-elicx

packages/vinext/src/shims/internal/normalize-router-href.ts

@@ -0,0 +1,20 @@
+/**
+ * Warn about and normalize repeated path separators like Next.js `resolveHref`.
+ * The protocol separator is preserved and query strings are left untouched.
+ *
+ * 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
+ */
+export function normalizeRouterHref(href: string, routePathname: string): string {
+  const protocol = href.match(/^[a-z][a-z0-9+.-]*:\/\//i)?.[0] ?? "";
+  const withoutProtocol = protocol ? href.slice(protocol.length) : href;
+  if (!/(\/\/|\\)/.test(withoutProtocol.split("?", 1)[0] ?? "")) return href;
+
+  console.error(
+    `Invalid href '${href}' passed to next/router in page: '${routePathname}'. Repeated forward-slashes (//) or backslashes \\ are not valid in the href.`,
+  );
+
+  const [pathname, ...query] = withoutProtocol.split("?");
+  const normalizedPathname = pathname.replace(/\\/g, "/").replace(/\/\/+/g, "/");
+  return protocol + normalizedPathname + (query[0] ? `?${query.join("?")}` : "");
+}

packages/vinext/src/shims/link.tsx

@@ -69,6 +69,7 @@ import {
   type PendingLinkSetter,
 } from "./internal/link-status-registry.js";
 import { getCurrentRoutePathnameForWarning } from "./internal/route-pattern-for-warning.js";
+import { normalizeRouterHref } from "./internal/normalize-router-href.js";
 import { scheduleAppPrefetchFetch } from "./internal/app-prefetch-fetch-queue.js";
 
 type NavigateEvent = {
@@ -258,73 +259,6 @@ function applyPagesNavigationFallback(href: string, replace: boolean): void {
   window.dispatchEvent(new PopStateEvent("popstate"));
 }
 
-/**
- * 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, 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. The Next.js
- * compat test asserts this exact text (`in page: '/my/path/[name]'`), so we
- * source it from the current render's route pattern via
- * `getCurrentRoutePathnameForWarning()`: the Pages Router SSR context's route
- * pattern on the server, `window.location.pathname` on the client, falling
- * back to `"/"`.
- */
-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);
-  const urlAsStringNoProto = urlProtoMatch
-    ? urlAsString.slice(urlProtoMatch[0].length)
-    : urlAsString;
-  const urlParts = urlAsStringNoProto.split("?", 1);
-  if (!(urlParts[0] || "").match(/(\/\/|\\)/)) return urlAsString;
-
-  const pathname = getCurrentRoutePathnameForWarning();
-  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(
   prefetchProp: LinkProps["prefetch"],
   isDangerous: boolean,
@@ -1138,7 +1072,7 @@ const Link = forwardRef<HTMLAnchorElement, LinkProps>(function Link(
   // See packages/next/src/client/resolve-href.ts.
   const resolvedHref =
     typeof rawResolvedHref === "string"
-      ? warnAndNormalizeRepeatedSlashesInHref(rawResolvedHref)
+      ? normalizeRouterHref(rawResolvedHref, getCurrentRoutePathnameForWarning())
       : rawResolvedHref;
 
   const isDangerous = typeof resolvedHref === "string" && isDangerousScheme(resolvedHref);

packages/vinext/src/shims/router.ts

@@ -97,6 +97,8 @@ import {
 } from "./pages-router-runtime.js";
 import { assertSafeNavigationUrl } from "./url-safety.js";
 import { interpolateDynamicRouteHref } from "./internal/interpolate-as.js";
+import { normalizeRouterHref } from "./internal/normalize-router-href.js";
+import { getCurrentRoutePathnameForWarning } from "./internal/route-pattern-for-warning.js";
 import { getCurrentBrowserLocale } from "./client-locale.js";
 import { getDeploymentId, NEXT_DEPLOYMENT_ID_HEADER } from "../utils/deployment-id.js";
 import type { RequestContext } from "../config/config-matchers.js";
@@ -547,6 +549,12 @@ function resolveUrl(url: string | UrlObject): string {
   return result;
 }
 
+function prepareUrl(url: Url): Url {
+  return typeof url === "string"
+    ? normalizeRouterHref(url, getCurrentRoutePathnameForWarning())
+    : url;
+}
+
 /**
  * When `as` is provided, use it as the navigation target. This is a
  * simplification: Next.js keeps `url` and `as` as separate values (url for
@@ -4200,20 +4208,20 @@ const RouterMethods = {
     // (an async function) becomes a rejected Promise that React does not
     // observe from an event handler that does not await it (e.g.
     // `<button onClick={() => router.push(...)}>`).
-    assertSafeNavigationUrl(resolveUrl(url));
-    if (as) {
-      assertSafeNavigationUrl(resolveUrl(as));
-    }
-    return performNavigation(url, as, options, "push");
+    const preparedUrl = prepareUrl(url);
+    const preparedAs = as ? prepareUrl(as) : undefined;
+    assertSafeNavigationUrl(resolveUrl(preparedUrl));
+    if (preparedAs) assertSafeNavigationUrl(resolveUrl(preparedAs));
+    return performNavigation(preparedUrl, preparedAs, options, "push");
   },
   replace: (url: Url, as?: Url, options?: TransitionOptions) => {
     if (typeof window === "undefined") throwNoRouterInstance();
     // See `push` above for the rationale on the synchronous guard.
-    assertSafeNavigationUrl(resolveUrl(url));
-    if (as) {
-      assertSafeNavigationUrl(resolveUrl(as));
-    }
-    return performNavigation(url, as, options, "replace");
+    const preparedUrl = prepareUrl(url);
+    const preparedAs = as ? prepareUrl(as) : undefined;
+    assertSafeNavigationUrl(resolveUrl(preparedUrl));
+    if (preparedAs) assertSafeNavigationUrl(resolveUrl(preparedAs));
+    return performNavigation(preparedUrl, preparedAs, options, "replace");
   },
   back: () => {
     if (typeof window === "undefined") throwNoRouterInstance();

tests/link.test.ts

@@ -953,12 +953,17 @@ describe("Link locale handling", () => {
     expect(html).toContain('href="https://example.com/about"');
   });
 
-  it("locale does not mangle protocol-relative URLs", () => {
-    // //example.com/about should not become /fr///example.com/about
-    const html = ReactDOMServer.renderToString(
-      React.createElement(Link, { href: "//example.com/about", locale: "fr" } as any, "x"),
-    );
-    expect(html).toContain('href="//example.com/about"');
+  it("normalizes protocol-relative URLs before applying locale", () => {
+    const consoleError = vi.spyOn(console, "error").mockImplementation(() => {});
+    try {
+      const html = ReactDOMServer.renderToString(
+        React.createElement(Link, { href: "//example.com/about", locale: "fr" } as any, "x"),
+      );
+      expect(html).toContain('href="/fr/example.com/about"');
+      expect(consoleError).toHaveBeenCalledTimes(1);
+    } finally {
+      consoleError.mockRestore();
+    }
   });
 
   it("locale does not mangle http:// URLs", () => {

tests/shims.test.ts

@@ -2886,6 +2886,48 @@ describe("window.next debug global", () => {
     }
   });
 
+  it("normalizes protocol-relative router hrefs before applying basePath", async () => {
+    const previousWindow = (globalThis as any).window;
+    const previousBasePath = process.env.__NEXT_ROUTER_BASEPATH;
+    const pushState = vi.fn();
+    const consoleError = vi.spyOn(console, "error").mockImplementation(() => {});
+    process.env.__NEXT_ROUTER_BASEPATH = "/docs";
+    (globalThis as any).window = {
+      location: {
+        pathname: "/docs/start",
+        search: "",
+        hash: "",
+        href: "http://localhost/docs/start",
+        origin: "http://localhost",
+      },
+      history: { state: null, pushState, replaceState() {} },
+      addEventListener() {},
+      dispatchEvent() {},
+      scrollTo() {},
+    };
+
+    try {
+      vi.resetModules();
+      const routerModule = await import("../packages/vinext/src/shims/router.js");
+
+      await routerModule.default.push("//localhost/outside", undefined, { shallow: true });
+
+      expect(pushState).toHaveBeenCalledWith(
+        expect.objectContaining({ url: "/localhost/outside", as: "/localhost/outside" }),
+        "",
+        "/docs/localhost/outside",
+      );
+      expect(consoleError).toHaveBeenCalledTimes(1);
+    } finally {
+      consoleError.mockRestore();
+      if (previousBasePath === undefined) delete process.env.__NEXT_ROUTER_BASEPATH;
+      else process.env.__NEXT_ROUTER_BASEPATH = previousBasePath;
+      if (previousWindow === undefined) delete (globalThis as any).window;
+      else (globalThis as any).window = previousWindow;
+      vi.resetModules();
+    }
+  });
+
   it.each([
     [{}, "/rewrite-navigation/[id]/destination", "/rewrite-navigation/[id]/destination"],
     [{ query: {} }, "/rewrite-navigation/[id]/destination", "/rewrite-navigation/[id]/destination"],