Skip to content

Detect and handle leading double slash paths #13510

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 30, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/ninety-snails-shout.md
Original file line number Diff line number Diff line change
@@ -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)`
124 changes: 123 additions & 1 deletion packages/react-router/__tests__/dom/data-browser-router-test.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import "@testing-library/jest-dom";
import {
act,
fireEvent,
Expand Down Expand Up @@ -7862,6 +7861,129 @@ function testDomRouter(
]);
});
});

if (name === "<DataBrowserRouter>") {
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 <Link to="/page">Go to Page</Link>;
},
},
{
path: "/page",
Component() {
return <h1>Worked!</h1>;
},
},
],
{
window: testWindow,
}
);
render(<RouterProvider router={router} />);
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 <Link to="/page">Go to Page</Link>;
},
},
{
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 <Link to="/page">Go to Page</Link>;
},
},
{
path: "/page",
loader() {
return redirect("//otherhost/parent");
},
Component() {
return null;
},
},
],
{
window: testWindow,
}
);

await router.navigate("/page");
expect(testWindow.location.assign).toHaveBeenCalledWith(
"//otherhost/parent"
);
});
}
});
}

Expand Down
53 changes: 35 additions & 18 deletions packages/react-router/lib/router/history.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -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;
}
Comment on lines +751 to +759
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the new behavior - which we don't want to apply to cases where we want to create an absolute URL, so we extract this utility for direct usage in the redirect case where we have an absolute URL.


return new URL(href, base);
}

//#endregion
5 changes: 4 additions & 1 deletion packages/react-router/lib/router/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
Action as NavigationType,
createLocation,
createPath,
createBrowserURLImpl,
invariant,
parsePath,
warning,
Expand Down Expand Up @@ -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 ||
Expand Down