Skip to content

Commit e3771c7

Browse files
brophdawg11brophdawg11
committed
Detect and handle leading double slash paths
1 parent e2324af commit e3771c7

File tree

4 files changed

+169
-21
lines changed

4 files changed

+169
-21
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 & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,7 @@ export function createMemoryHistory(
286286
},
287287
createHref,
288288
createURL(to) {
289-
return new URL(createHref(to), "http://localhost");
289+
return createURLImpl(to);
290290
},
291291
encodeLocation(to: To) {
292292
let path = typeof to === "string" ? parsePath(to) : to;
@@ -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 createURLImpl(to);
703686
}
704687

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

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