diff --git a/.changeset/ninety-snails-shout.md b/.changeset/ninety-snails-shout.md new file mode 100644 index 0000000000..82c0d4bab9 --- /dev/null +++ b/.changeset/ninety-snails-shout.md @@ -0,0 +1,7 @@ +--- +"react-router": patch +--- + +Be defensive against leading double slashes in paths to avoid `Invalid URL` errors from the URL constructor + +- Note we do not sanitize/normalize these paths - we only detect them so we can avoid the error that would be thrown by `new URL("//", window.location.origin)` diff --git a/packages/react-router/__tests__/dom/data-browser-router-test.tsx b/packages/react-router/__tests__/dom/data-browser-router-test.tsx index abe532a8f2..14c7a85706 100644 --- a/packages/react-router/__tests__/dom/data-browser-router-test.tsx +++ b/packages/react-router/__tests__/dom/data-browser-router-test.tsx @@ -1,4 +1,3 @@ -import "@testing-library/jest-dom"; import { act, fireEvent, @@ -7862,6 +7861,129 @@ function testDomRouter( ]); }); }); + + if (name === "") { + describe("DataBrowserRouter-only tests", () => { + it("is defensive against double slash URLs in window.location", async () => { + let testWindow = getWindow("http://localhost//"); + let router = createTestRouter( + [ + { + path: "*", + Component() { + return Go to Page; + }, + }, + { + path: "/page", + Component() { + return

Worked!

; + }, + }, + ], + { + window: testWindow, + } + ); + render(); + expect(testWindow.location.pathname).toBe("//"); + expect(router.state.location.pathname).toBe("//"); + + fireEvent.click(screen.getByText("Go to Page")); + await waitFor(() => screen.getByText("Worked!")); + expect(testWindow.location.pathname).toBe("/page"); + expect(router.state.location.pathname).toBe("/page"); + }); + }); + + it("handles different-origin absolute redirect URLs", async () => { + let testWindow = getWindow("http://localhost/"); + + // jsdom is making more and more properties non-configurable, so we inject + // our own jest-friendly window + testWindow = { + ...testWindow, + addEventListener: testWindow.addEventListener.bind(testWindow), + location: { + ...testWindow.location, + assign: jest.fn(), + replace: jest.fn(), + }, + }; + + let router = createTestRouter( + [ + { + path: "/", + Component() { + return Go to Page; + }, + }, + { + path: "/page", + loader() { + return redirect("http://otherhost/parent"); + }, + Component() { + return null; + }, + }, + ], + { + window: testWindow, + } + ); + + await router.navigate("/page"); + expect(testWindow.location.assign).toHaveBeenCalledWith( + "http://otherhost/parent" + ); + }); + + it("handles different-origin protocol-less absolute redirect URLs", async () => { + let testWindow = getWindow("http://localhost/"); + + // jsdom is making more and more properties non-configurable, so we inject + // our own jest-friendly window + testWindow = { + ...testWindow, + addEventListener: testWindow.addEventListener.bind(testWindow), + location: { + ...testWindow.location, + assign: jest.fn(), + replace: jest.fn(), + }, + }; + + let router = createTestRouter( + [ + { + path: "/", + Component() { + return Go to Page; + }, + }, + { + path: "/page", + loader() { + return redirect("//otherhost/parent"); + }, + Component() { + return null; + }, + }, + ], + { + window: testWindow, + } + ); + + await router.navigate("/page"); + expect(testWindow.location.assign).toHaveBeenCalledWith( + "//otherhost/parent" + ); + }); + } }); } diff --git a/packages/react-router/lib/router/history.ts b/packages/react-router/lib/router/history.ts index e7e07ac28b..caa64b2444 100644 --- a/packages/react-router/lib/router/history.ts +++ b/packages/react-router/lib/router/history.ts @@ -682,24 +682,7 @@ function getUrlBasedHistory( } function createURL(to: To): URL { - // window.location.origin is "null" (the literal string value) in Firefox - // under certain conditions, notably when serving from a local HTML file - // See https://bugzilla.mozilla.org/show_bug.cgi?id=878297 - let base = - window.location.origin !== "null" - ? window.location.origin - : window.location.href; - - let href = typeof to === "string" ? to : createPath(to); - // Treating this as a full URL will strip any trailing spaces so we need to - // pre-encode them since they might be part of a matching splat param from - // an ancestor route - href = href.replace(/ $/, "%20"); - invariant( - base, - `No window.location.(origin|href) available to create URL for href: ${href}` - ); - return new URL(href, base); + return createBrowserURLImpl(to); } let history: History = { @@ -744,4 +727,38 @@ function getUrlBasedHistory( return history; } +export function createBrowserURLImpl(to: To, isAbsolute = false): URL { + let base = "http://localhost"; + if (typeof window !== "undefined") { + // window.location.origin is "null" (the literal string value) in Firefox + // under certain conditions, notably when serving from a local HTML file + // See https://bugzilla.mozilla.org/show_bug.cgi?id=878297 + base = + window.location.origin !== "null" + ? window.location.origin + : window.location.href; + } + + invariant(base, "No window.location.(origin|href) available to create URL"); + + let href = typeof to === "string" ? to : createPath(to); + + // Treating this as a full URL will strip any trailing spaces so we need to + // pre-encode them since they might be part of a matching splat param from + // an ancestor route + href = href.replace(/ $/, "%20"); + + // If this isn't a usage for absolute URLs (currently only for redirects), + // then we need to avoid the URL constructor treating a leading double slash + // as a protocol-less URL. By prepending the base, it forces the double slash + // to be parsed correctly as part of the pathname. + if (!isAbsolute && href.startsWith("//")) { + // new URL('//', 'https://localhost') -> error! + // new URL('https://localhost//', 'https://localhost') -> no error! + href = base + href; + } + + return new URL(href, base); +} + //#endregion diff --git a/packages/react-router/lib/router/router.ts b/packages/react-router/lib/router/router.ts index 72dd18a6ad..6d23352abf 100644 --- a/packages/react-router/lib/router/router.ts +++ b/packages/react-router/lib/router/router.ts @@ -4,6 +4,7 @@ import { Action as NavigationType, createLocation, createPath, + createBrowserURLImpl, invariant, parsePath, warning, @@ -2748,7 +2749,9 @@ export function createRouter(init: RouterInit): Router { // Hard reload if the response contained X-Remix-Reload-Document isDocumentReload = true; } else if (ABSOLUTE_URL_REGEX.test(location)) { - const url = init.history.createURL(location); + // We skip `history.createURL` here for absolute URLs because we don't + // want to inherit the current `window.location` base URL + const url = createBrowserURLImpl(location, true); isDocumentReload = // Hard reload if it's an absolute URL to a new origin url.origin !== routerWindow.location.origin ||