diff --git a/.changeset/ninety-countries-cheat.md b/.changeset/ninety-countries-cheat.md new file mode 100644 index 0000000000..e54b33c6d8 --- /dev/null +++ b/.changeset/ninety-countries-cheat.md @@ -0,0 +1,5 @@ +--- +"@remix-run/router": patch +--- + +make url-encoding history-aware diff --git a/packages/react-router-dom/__tests__/special-characters-test.tsx b/packages/react-router-dom/__tests__/special-characters-test.tsx index 3d84393856..ff66a95585 100644 --- a/packages/react-router-dom/__tests__/special-characters-test.tsx +++ b/packages/react-router-dom/__tests__/special-characters-test.tsx @@ -12,13 +12,18 @@ import { import type { Location, Params } from "react-router-dom"; import { BrowserRouter, + HashRouter, + MemoryRouter, Link, Routes, Route, RouterProvider, createBrowserRouter, + createHashRouter, + createMemoryRouter, createRoutesFromElements, useLocation, + useNavigate, useParams, } from "react-router-dom"; @@ -709,4 +714,272 @@ describe("special character tests", () => { } }); }); + + describe("encodes characters based on history implementation", () => { + function ShowPath() { + let { pathname, search, hash } = useLocation(); + return
{JSON.stringify({ pathname, search, hash })}
; + } + + describe("memory routers", () => { + it("does not encode characters in MemoryRouter", () => { + let ctx = render( + + + } /> + + + ); + + expect(ctx.container.innerHTML).toMatchInlineSnapshot( + `"
{\\"pathname\\":\\"/with space\\",\\"search\\":\\"\\",\\"hash\\":\\"\\"}
"` + ); + }); + + it("does not encode characters in MemoryRouter (navigate)", () => { + function Start() { + let navigate = useNavigate(); + // eslint-disable-next-line react-hooks/exhaustive-deps + React.useEffect(() => navigate("/with space"), []); + return null; + } + let ctx = render( + + + } /> + } /> + + + ); + + expect(ctx.container.innerHTML).toMatchInlineSnapshot( + `"
{\\"pathname\\":\\"/with space\\",\\"search\\":\\"\\",\\"hash\\":\\"\\"}
"` + ); + }); + + it("does not encode characters in createMemoryRouter", () => { + let router = createMemoryRouter( + [{ path: "/with space", element: }], + { initialEntries: ["/with space"] } + ); + let ctx = render(); + + expect(ctx.container.innerHTML).toMatchInlineSnapshot( + `"
{\\"pathname\\":\\"/with space\\",\\"search\\":\\"\\",\\"hash\\":\\"\\"}
"` + ); + }); + + it("does not encode characters in createMemoryRouter (navigate)", () => { + function Start() { + let navigate = useNavigate(); + // eslint-disable-next-line react-hooks/exhaustive-deps + React.useEffect(() => navigate("/with space"), []); + return null; + } + let router = createMemoryRouter([ + { path: "/", element: }, + { path: "/with space", element: }, + ]); + let ctx = render(); + + expect(ctx.container.innerHTML).toMatchInlineSnapshot( + `"
{\\"pathname\\":\\"/with space\\",\\"search\\":\\"\\",\\"hash\\":\\"\\"}
"` + ); + }); + }); + + describe("browser routers", () => { + let testWindow: Window; + + beforeEach(() => { + // Need to use our own custom DOM in order to get a working history + const dom = new JSDOM(``, { + url: "https://remix.run/", + }); + testWindow = dom.window as unknown as Window; + testWindow.history.pushState({}, "", "/"); + }); + + it("encodes characters in BrowserRouter", () => { + testWindow.history.replaceState(null, "", "/with space"); + + let ctx = render( + + + } /> + + + ); + + expect(testWindow.location.pathname).toBe("/with%20space"); + expect(ctx.container.innerHTML).toMatchInlineSnapshot( + `"
{\\"pathname\\":\\"/with%20space\\",\\"search\\":\\"\\",\\"hash\\":\\"\\"}
"` + ); + }); + + it("encodes characters in BrowserRouter (navigate)", () => { + testWindow.history.replaceState(null, "", "/"); + + function Start() { + let navigate = useNavigate(); + // eslint-disable-next-line react-hooks/exhaustive-deps + React.useEffect(() => navigate("/with space"), []); + return null; + } + + let ctx = render( + + + } /> + } /> + + + ); + + expect(testWindow.location.pathname).toBe("/with%20space"); + expect(ctx.container.innerHTML).toMatchInlineSnapshot( + `"
{\\"pathname\\":\\"/with%20space\\",\\"search\\":\\"\\",\\"hash\\":\\"\\"}
"` + ); + }); + + it("encodes characters in createBrowserRouter", () => { + testWindow.history.replaceState(null, "", "/with space"); + + let router = createBrowserRouter( + [{ path: "/with space", element: }], + { window: testWindow } + ); + let ctx = render(); + + expect(testWindow.location.pathname).toBe("/with%20space"); + expect(ctx.container.innerHTML).toMatchInlineSnapshot( + `"
{\\"pathname\\":\\"/with%20space\\",\\"search\\":\\"\\",\\"hash\\":\\"\\"}
"` + ); + }); + + it("encodes characters in createBrowserRouter (navigate)", () => { + testWindow.history.replaceState(null, "", "/with space"); + + function Start() { + let navigate = useNavigate(); + // eslint-disable-next-line react-hooks/exhaustive-deps + React.useEffect(() => navigate("/with space"), []); + return null; + } + + let router = createBrowserRouter( + [ + { path: "/", element: }, + { path: "/with space", element: }, + ], + { window: testWindow } + ); + let ctx = render(); + + expect(testWindow.location.pathname).toBe("/with%20space"); + expect(ctx.container.innerHTML).toMatchInlineSnapshot( + `"
{\\"pathname\\":\\"/with%20space\\",\\"search\\":\\"\\",\\"hash\\":\\"\\"}
"` + ); + }); + }); + + describe("hash routers", () => { + let testWindow: Window; + + beforeEach(() => { + // Need to use our own custom DOM in order to get a working history + const dom = new JSDOM(``, { + url: "https://remix.run/", + }); + testWindow = dom.window as unknown as Window; + testWindow.history.pushState({}, "", "/"); + }); + + it("encodes characters in HashRouter", () => { + testWindow.history.replaceState(null, "", "/#/with space"); + + let ctx = render( + + + } /> + + + ); + + expect(testWindow.location.pathname).toBe("/"); + expect(testWindow.location.hash).toBe("#/with%20space"); + expect(ctx.container.innerHTML).toMatchInlineSnapshot( + `"
{\\"pathname\\":\\"/with%20space\\",\\"search\\":\\"\\",\\"hash\\":\\"\\"}
"` + ); + }); + + it("encodes characters in HashRouter (navigate)", () => { + testWindow.history.replaceState(null, "", "/"); + + function Start() { + let navigate = useNavigate(); + // eslint-disable-next-line react-hooks/exhaustive-deps + React.useEffect(() => navigate("/with space"), []); + return null; + } + + let ctx = render( + + + } /> + } /> + + + ); + + expect(testWindow.location.pathname).toBe("/"); + expect(testWindow.location.hash).toBe("#/with%20space"); + expect(ctx.container.innerHTML).toMatchInlineSnapshot( + `"
{\\"pathname\\":\\"/with%20space\\",\\"search\\":\\"\\",\\"hash\\":\\"\\"}
"` + ); + }); + + it("encodes characters in createHashRouter", () => { + testWindow.history.replaceState(null, "", "/#/with space"); + + let router = createHashRouter( + [{ path: "/with space", element: }], + { window: testWindow } + ); + let ctx = render(); + + expect(testWindow.location.pathname).toBe("/"); + expect(testWindow.location.hash).toBe("#/with%20space"); + expect(ctx.container.innerHTML).toMatchInlineSnapshot( + `"
{\\"pathname\\":\\"/with%20space\\",\\"search\\":\\"\\",\\"hash\\":\\"\\"}
"` + ); + }); + + it("encodes characters in createHashRouter (navigate)", () => { + testWindow.history.replaceState(null, "", "/"); + + function Start() { + let navigate = useNavigate(); + // eslint-disable-next-line react-hooks/exhaustive-deps + React.useEffect(() => navigate("/with space"), []); + return null; + } + + let router = createHashRouter( + [ + { path: "/", element: }, + { path: "/with space", element: }, + ], + { window: testWindow } + ); + let ctx = render(); + + expect(testWindow.location.pathname).toBe("/"); + expect(testWindow.location.hash).toBe("#/with%20space"); + expect(ctx.container.innerHTML).toMatchInlineSnapshot( + `"
{\\"pathname\\":\\"/with%20space\\",\\"search\\":\\"\\",\\"hash\\":\\"\\"}
"` + ); + }); + }); + }); }); diff --git a/packages/router/history.ts b/packages/router/history.ts index 35d0c51344..84fcae3aa3 100644 --- a/packages/router/history.ts +++ b/packages/router/history.ts @@ -125,6 +125,15 @@ export interface History { */ createHref(to: To): string; + /** + * Encode a location the same way window.history would do (no-op for memory + * history) so we ensure our PUSH/REPLAC e navigations for data routers + * behave the same as POP + * + * @param location The incoming location from router.navigate() + */ + encodeLocation(location: Location): Location; + /** * Pushes a new location onto the history stack, increasing its length by one. * If there were any entries in the stack after the current one, they are @@ -259,6 +268,9 @@ export function createMemoryHistory( createHref(to) { return typeof to === "string" ? to : createPath(to); }, + encodeLocation(location) { + return location; + }, push(to, state) { action = Action.Push; let nextLocation = createMemoryLocation(to, state); @@ -527,6 +539,15 @@ export function parsePath(path: string): Partial { return parsedPath; } +export function createURL(location: Location | string): URL { + let base = + typeof window !== "undefined" && typeof window.location !== "undefined" + ? window.location.origin + : "unknown://unknown"; + let href = typeof location === "string" ? location : createPath(location); + return new URL(href, base); +} + export interface UrlHistory extends History {} export type UrlHistoryOptions = { @@ -610,6 +631,16 @@ function getUrlBasedHistory( createHref(to) { return createHref(window, to); }, + encodeLocation(location) { + // Encode a Location the same way window.location would + let url = createURL(createPath(location)); + return { + ...location, + pathname: url.pathname, + search: url.search, + hash: url.hash, + }; + }, push, replace, go(n) { diff --git a/packages/router/router.ts b/packages/router/router.ts index b409ffc38c..700b5c463a 100644 --- a/packages/router/router.ts +++ b/packages/router/router.ts @@ -1,8 +1,9 @@ -import type { History, Location, Path, To } from "./history"; +import type { History, Location, To } from "./history"; import { Action as HistoryAction, createLocation, createPath, + createURL, parsePath, } from "./history"; import type { @@ -769,13 +770,7 @@ export function createRouter(init: RouterInit): Router { // remains the same as POP and non-data-router usages. new URL() does all // the same encoding we'd get from a history.pushState/window.location read // without having to touch history - let url = createURL(createPath(location)); - location = { - ...location, - pathname: url.pathname, - search: url.search, - hash: url.hash, - }; + location = init.history.encodeLocation(location); let historyAction = (opts && opts.replace) === true || submission != null @@ -2038,14 +2033,13 @@ export function unstable_createStaticHandler( ): Promise | Response> { let result: DataResult; if (!actionMatch.route.action) { - let href = createServerHref(new URL(request.url)); if (isRouteRequest) { throw createRouterErrorResponse(null, { status: 405, statusText: "Method Not Allowed", }); } - result = getMethodNotAllowedResult(href); + result = getMethodNotAllowedResult(request.url); } else { result = await callLoaderOrAction( "action", @@ -2288,7 +2282,7 @@ function normalizeNavigateOptions( path, submission: { formMethod: opts.formMethod, - formAction: createServerHref(parsePath(path)), + formAction: stripHashFromPath(path), formEncType: (opts && opts.formEncType) || "application/x-www-form-urlencoded", formData: opts.formData, @@ -2644,7 +2638,7 @@ function createRequest( signal: AbortSignal, submission?: Submission ): Request { - let url = createURL(location).toString(); + let url = createURL(stripHashFromPath(location)).toString(); let init: RequestInit = { signal }; if (submission) { @@ -2894,7 +2888,7 @@ function getMethodNotAllowedMatches(routes: AgnosticDataRouteObject[]) { } function getMethodNotAllowedResult(path: Location | string): ErrorResult { - let href = typeof path === "string" ? path : createServerHref(path); + let href = typeof path === "string" ? path : createPath(path); console.warn( "You're trying to submit to a route that does not have an action. To " + "fix this, please add an `action` function to the route for " + @@ -2916,9 +2910,9 @@ function findRedirect(results: DataResult[]): RedirectResult | undefined { } } -// Create an href to represent a "server" URL without the hash -function createServerHref(location: Partial | Location | URL) { - return (location.pathname || "") + (location.search || ""); +function stripHashFromPath(path: To) { + let parsedPath = typeof path === "string" ? parsePath(path) : path; + return createPath({ ...parsedPath, hash: "" }); } function isHashChangeOnly(a: Location, b: Location): boolean { @@ -3058,14 +3052,4 @@ function getTargetMatch( let pathMatches = getPathContributingMatches(matches); return pathMatches[pathMatches.length - 1]; } - -function createURL(location: Location | string): URL { - let base = - typeof window !== "undefined" && typeof window.location !== "undefined" - ? window.location.origin - : "unknown://unknown"; - let href = - typeof location === "string" ? location : createServerHref(location); - return new URL(href, base); -} //#endregion diff --git a/packages/router/utils.ts b/packages/router/utils.ts index 70761f6146..016e19bf27 100644 --- a/packages/router/utils.ts +++ b/packages/router/utils.ts @@ -331,9 +331,12 @@ export function matchRoutes< for (let i = 0; matches == null && i < branches.length; ++i) { matches = matchRouteBranch( branches[i], - // incoming pathnames are always encoded from either window.location or - // from route.navigate, but we want to match against the unencoded paths - // in the route definitions + // Incoming pathnames are generally encoded from either window.location + // or from router.navigate, but we want to match against the unencoded + // paths in the route definitions. Memory router locations won't be + // encoded here but there also shouldn't be anything to decode so this + // should be a safe operation. This avoids needing matchRoutes to be + // history-aware. safelyDecodeURI(pathname) ); }