Skip to content

Commit 034c0ef

Browse files
brophdawg11brophdawg11
authored
Detect and handle leading double slash paths (#13510)
1 parent bbbcd2c commit 034c0ef

File tree

4 files changed

+169
-20
lines changed

4 files changed

+169
-20
lines changed

‎.changeset/ninety-snails-shout.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
"react-router": patch
3+
---
4+
5+
Be defensive against leading double slashes in paths to avoid `Invalid URL` errors from the URL constructor
6+
7+
- 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)`

‎packages/react-router/__tests__/dom/data-browser-router-test.tsx

Lines changed: 123 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import "@testing-library/jest-dom";
21
import {
32
act,
43
fireEvent,
@@ -7862,6 +7861,129 @@ function testDomRouter(
78627861
]);
78637862
});
78647863
});
7864+
7865+
if (name === "<DataBrowserRouter>") {
7866+
describe("DataBrowserRouter-only tests", () => {
7867+
it("is defensive against double slash URLs in window.location", async () => {
7868+
let testWindow = getWindow("http://localhost//");
7869+
let router = createTestRouter(
7870+
[
7871+
{
7872+
path: "*",
7873+
Component() {
7874+
return <Link to="/page">Go to Page</Link>;
7875+
},
7876+
},
7877+
{
7878+
path: "/page",
7879+
Component() {
7880+
return <h1>Worked!</h1>;
7881+
},
7882+
},
7883+
],
7884+
{
7885+
window: testWindow,
7886+
}
7887+
);
7888+
render(<RouterProvider router={router} />);
7889+
expect(testWindow.location.pathname).toBe("//");
7890+
expect(router.state.location.pathname).toBe("//");
7891+
7892+
fireEvent.click(screen.getByText("Go to Page"));
7893+
await waitFor(() => screen.getByText("Worked!"));
7894+
expect(testWindow.location.pathname).toBe("/page");
7895+
expect(router.state.location.pathname).toBe("/page");
7896+
});
7897+
});
7898+
7899+
it("handles different-origin absolute redirect URLs", async () => {
7900+
let testWindow = getWindow("http://localhost/");
7901+
7902+
// jsdom is making more and more properties non-configurable, so we inject
7903+
// our own jest-friendly window
7904+
testWindow = {
7905+
...testWindow,
7906+
addEventListener: testWindow.addEventListener.bind(testWindow),
7907+
location: {
7908+
...testWindow.location,
7909+
assign: jest.fn(),
7910+
replace: jest.fn(),
7911+
},
7912+
};
7913+
7914+
let router = createTestRouter(
7915+
[
7916+
{
7917+
path: "/",
7918+
Component() {
7919+
return <Link to="/page">Go to Page</Link>;
7920+
},
7921+
},
7922+
{
7923+
path: "/page",
7924+
loader() {
7925+
return redirect("http://otherhost/parent");
7926+
},
7927+
Component() {
7928+
return null;
7929+
},
7930+
},
7931+
],
7932+
{
7933+
window: testWindow,
7934+
}
7935+
);
7936+
7937+
await router.navigate("/page");
7938+
expect(testWindow.location.assign).toHaveBeenCalledWith(
7939+
"http://otherhost/parent"
7940+
);
7941+
});
7942+
7943+
it("handles different-origin protocol-less absolute redirect URLs", async () => {
7944+
let testWindow = getWindow("http://localhost/");
7945+
7946+
// jsdom is making more and more properties non-configurable, so we inject
7947+
// our own jest-friendly window
7948+
testWindow = {
7949+
...testWindow,
7950+
addEventListener: testWindow.addEventListener.bind(testWindow),
7951+
location: {
7952+
...testWindow.location,
7953+
assign: jest.fn(),
7954+
replace: jest.fn(),
7955+
},
7956+
};
7957+
7958+
let router = createTestRouter(
7959+
[
7960+
{
7961+
path: "/",
7962+
Component() {
7963+
return <Link to="/page">Go to Page</Link>;
7964+
},
7965+
},
7966+
{
7967+
path: "/page",
7968+
loader() {
7969+
return redirect("//otherhost/parent");
7970+
},
7971+
Component() {
7972+
return null;
7973+
},
7974+
},
7975+
],
7976+
{
7977+
window: testWindow,
7978+
}
7979+
);
7980+
7981+
await router.navigate("/page");
7982+
expect(testWindow.location.assign).toHaveBeenCalledWith(
7983+
"//otherhost/parent"
7984+
);
7985+
});
7986+
}
78657987
});
78667988
}
78677989

‎packages/react-router/lib/router/history.ts

Lines changed: 35 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -682,24 +682,7 @@ function getUrlBasedHistory(
682682
}
683683

684684
function createURL(to: To): URL {
685-
// window.location.origin is "null" (the literal string value) in Firefox
686-
// under certain conditions, notably when serving from a local HTML file
687-
// See https://bugzilla.mozilla.org/show_bug.cgi?id=878297
688-
let base =
689-
window.location.origin !== "null"
690-
? window.location.origin
691-
: window.location.href;
692-
693-
let href = typeof to === "string" ? to : createPath(to);
694-
// Treating this as a full URL will strip any trailing spaces so we need to
695-
// pre-encode them since they might be part of a matching splat param from
696-
// an ancestor route
697-
href = href.replace(/ $/, "%20");
698-
invariant(
699-
base,
700-
`No window.location.(origin|href) available to create URL for href: ${href}`
701-
);
702-
return new URL(href, base);
685+
return createBrowserURLImpl(to);
703686
}
704687

705688
let history: History = {
@@ -744,4 +727,38 @@ function getUrlBasedHistory(
744727
return history;
745728
}
746729

730+
export function createBrowserURLImpl(to: To, isAbsolute = false): URL {
731+
let base = "http://localhost";
732+
if (typeof window !== "undefined") {
733+
// window.location.origin is "null" (the literal string value) in Firefox
734+
// under certain conditions, notably when serving from a local HTML file
735+
// See https://bugzilla.mozilla.org/show_bug.cgi?id=878297
736+
base =
737+
window.location.origin !== "null"
738+
? window.location.origin
739+
: window.location.href;
740+
}
741+
742+
invariant(base, "No window.location.(origin|href) available to create URL");
743+
744+
let href = typeof to === "string" ? to : createPath(to);
745+
746+
// Treating this as a full URL will strip any trailing spaces so we need to
747+
// pre-encode them since they might be part of a matching splat param from
748+
// an ancestor route
749+
href = href.replace(/ $/, "%20");
750+
751+
// If this isn't a usage for absolute URLs (currently only for redirects),
752+
// then we need to avoid the URL constructor treating a leading double slash
753+
// as a protocol-less URL. By prepending the base, it forces the double slash
754+
// to be parsed correctly as part of the pathname.
755+
if (!isAbsolute && href.startsWith("//")) {
756+
// new URL('//', 'https://localhost') -> error!
757+
// new URL('https://localhost//', 'https://localhost') -> no error!
758+
href = base + href;
759+
}
760+
761+
return new URL(href, base);
762+
}
763+
747764
//#endregion

‎packages/react-router/lib/router/router.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import {
44
Action as NavigationType,
55
createLocation,
66
createPath,
7+
createBrowserURLImpl,
78
invariant,
89
parsePath,
910
warning,
@@ -2748,7 +2749,9 @@ export function createRouter(init: RouterInit): Router {
27482749
// Hard reload if the response contained X-Remix-Reload-Document
27492750
isDocumentReload = true;
27502751
} else if (ABSOLUTE_URL_REGEX.test(location)) {
2751-
const url = init.history.createURL(location);
2752+
// We skip `history.createURL` here for absolute URLs because we don't
2753+
// want to inherit the current `window.location` base URL
2754+
const url = createBrowserURLImpl(location, true);
27522755
isDocumentReload =
27532756
// Hard reload if it's an absolute URL to a new origin
27542757
url.origin !== routerWindow.location.origin ||

0 commit comments

Comments
 (0)