Skip to content

Commit bba73da

Browse files
brophdawg11brophdawg11
authored
fix: make url-encoding history-aware (#9496)
* fix: make url-encoding history-aware * add changeset * fix lint warnings * organize code
1 parent 4b4be06 commit bba73da

File tree

5 files changed

+325
-29
lines changed

5 files changed

+325
-29
lines changed

‎.changeset/ninety-countries-cheat.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@remix-run/router": patch
3+
---
4+
5+
make url-encoding history-aware

‎packages/react-router-dom/__tests__/special-characters-test.tsx‎

Lines changed: 273 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,18 @@ import {
1212
import type { Location, Params } from "react-router-dom";
1313
import {
1414
BrowserRouter,
15+
HashRouter,
16+
MemoryRouter,
1517
Link,
1618
Routes,
1719
Route,
1820
RouterProvider,
1921
createBrowserRouter,
22+
createHashRouter,
23+
createMemoryRouter,
2024
createRoutesFromElements,
2125
useLocation,
26+
useNavigate,
2227
useParams,
2328
} from "react-router-dom";
2429

@@ -709,4 +714,272 @@ describe("special character tests", () => {
709714
}
710715
});
711716
});
717+
718+
describe("encodes characters based on history implementation", () => {
719+
function ShowPath() {
720+
let { pathname, search, hash } = useLocation();
721+
return <pre>{JSON.stringify({ pathname, search, hash })}</pre>;
722+
}
723+
724+
describe("memory routers", () => {
725+
it("does not encode characters in MemoryRouter", () => {
726+
let ctx = render(
727+
<MemoryRouter initialEntries={["/with space"]}>
728+
<Routes>
729+
<Route path="/with space" element={<ShowPath />} />
730+
</Routes>
731+
</MemoryRouter>
732+
);
733+
734+
expect(ctx.container.innerHTML).toMatchInlineSnapshot(
735+
`"<pre>{\\"pathname\\":\\"/with space\\",\\"search\\":\\"\\",\\"hash\\":\\"\\"}</pre>"`
736+
);
737+
});
738+
739+
it("does not encode characters in MemoryRouter (navigate)", () => {
740+
function Start() {
741+
let navigate = useNavigate();
742+
// eslint-disable-next-line react-hooks/exhaustive-deps
743+
React.useEffect(() => navigate("/with space"), []);
744+
return null;
745+
}
746+
let ctx = render(
747+
<MemoryRouter>
748+
<Routes>
749+
<Route path="/" element={<Start />} />
750+
<Route path="/with space" element={<ShowPath />} />
751+
</Routes>
752+
</MemoryRouter>
753+
);
754+
755+
expect(ctx.container.innerHTML).toMatchInlineSnapshot(
756+
`"<pre>{\\"pathname\\":\\"/with space\\",\\"search\\":\\"\\",\\"hash\\":\\"\\"}</pre>"`
757+
);
758+
});
759+
760+
it("does not encode characters in createMemoryRouter", () => {
761+
let router = createMemoryRouter(
762+
[{ path: "/with space", element: <ShowPath /> }],
763+
{ initialEntries: ["/with space"] }
764+
);
765+
let ctx = render(<RouterProvider router={router} />);
766+
767+
expect(ctx.container.innerHTML).toMatchInlineSnapshot(
768+
`"<pre>{\\"pathname\\":\\"/with space\\",\\"search\\":\\"\\",\\"hash\\":\\"\\"}</pre>"`
769+
);
770+
});
771+
772+
it("does not encode characters in createMemoryRouter (navigate)", () => {
773+
function Start() {
774+
let navigate = useNavigate();
775+
// eslint-disable-next-line react-hooks/exhaustive-deps
776+
React.useEffect(() => navigate("/with space"), []);
777+
return null;
778+
}
779+
let router = createMemoryRouter([
780+
{ path: "/", element: <Start /> },
781+
{ path: "/with space", element: <ShowPath /> },
782+
]);
783+
let ctx = render(<RouterProvider router={router} />);
784+
785+
expect(ctx.container.innerHTML).toMatchInlineSnapshot(
786+
`"<pre>{\\"pathname\\":\\"/with space\\",\\"search\\":\\"\\",\\"hash\\":\\"\\"}</pre>"`
787+
);
788+
});
789+
});
790+
791+
describe("browser routers", () => {
792+
let testWindow: Window;
793+
794+
beforeEach(() => {
795+
// Need to use our own custom DOM in order to get a working history
796+
const dom = new JSDOM(`<!DOCTYPE html><html><body></body></html>`, {
797+
url: "https://remix.run/",
798+
});
799+
testWindow = dom.window as unknown as Window;
800+
testWindow.history.pushState({}, "", "/");
801+
});
802+
803+
it("encodes characters in BrowserRouter", () => {
804+
testWindow.history.replaceState(null, "", "/with space");
805+
806+
let ctx = render(
807+
<BrowserRouter window={testWindow}>
808+
<Routes>
809+
<Route path="/with space" element={<ShowPath />} />
810+
</Routes>
811+
</BrowserRouter>
812+
);
813+
814+
expect(testWindow.location.pathname).toBe("/with%20space");
815+
expect(ctx.container.innerHTML).toMatchInlineSnapshot(
816+
`"<pre>{\\"pathname\\":\\"/with%20space\\",\\"search\\":\\"\\",\\"hash\\":\\"\\"}</pre>"`
817+
);
818+
});
819+
820+
it("encodes characters in BrowserRouter (navigate)", () => {
821+
testWindow.history.replaceState(null, "", "/");
822+
823+
function Start() {
824+
let navigate = useNavigate();
825+
// eslint-disable-next-line react-hooks/exhaustive-deps
826+
React.useEffect(() => navigate("/with space"), []);
827+
return null;
828+
}
829+
830+
let ctx = render(
831+
<BrowserRouter window={testWindow}>
832+
<Routes>
833+
<Route path="/" element={<Start />} />
834+
<Route path="/with space" element={<ShowPath />} />
835+
</Routes>
836+
</BrowserRouter>
837+
);
838+
839+
expect(testWindow.location.pathname).toBe("/with%20space");
840+
expect(ctx.container.innerHTML).toMatchInlineSnapshot(
841+
`"<pre>{\\"pathname\\":\\"/with%20space\\",\\"search\\":\\"\\",\\"hash\\":\\"\\"}</pre>"`
842+
);
843+
});
844+
845+
it("encodes characters in createBrowserRouter", () => {
846+
testWindow.history.replaceState(null, "", "/with space");
847+
848+
let router = createBrowserRouter(
849+
[{ path: "/with space", element: <ShowPath /> }],
850+
{ window: testWindow }
851+
);
852+
let ctx = render(<RouterProvider router={router} />);
853+
854+
expect(testWindow.location.pathname).toBe("/with%20space");
855+
expect(ctx.container.innerHTML).toMatchInlineSnapshot(
856+
`"<pre>{\\"pathname\\":\\"/with%20space\\",\\"search\\":\\"\\",\\"hash\\":\\"\\"}</pre>"`
857+
);
858+
});
859+
860+
it("encodes characters in createBrowserRouter (navigate)", () => {
861+
testWindow.history.replaceState(null, "", "/with space");
862+
863+
function Start() {
864+
let navigate = useNavigate();
865+
// eslint-disable-next-line react-hooks/exhaustive-deps
866+
React.useEffect(() => navigate("/with space"), []);
867+
return null;
868+
}
869+
870+
let router = createBrowserRouter(
871+
[
872+
{ path: "/", element: <Start /> },
873+
{ path: "/with space", element: <ShowPath /> },
874+
],
875+
{ window: testWindow }
876+
);
877+
let ctx = render(<RouterProvider router={router} />);
878+
879+
expect(testWindow.location.pathname).toBe("/with%20space");
880+
expect(ctx.container.innerHTML).toMatchInlineSnapshot(
881+
`"<pre>{\\"pathname\\":\\"/with%20space\\",\\"search\\":\\"\\",\\"hash\\":\\"\\"}</pre>"`
882+
);
883+
});
884+
});
885+
886+
describe("hash routers", () => {
887+
let testWindow: Window;
888+
889+
beforeEach(() => {
890+
// Need to use our own custom DOM in order to get a working history
891+
const dom = new JSDOM(`<!DOCTYPE html><html><body></body></html>`, {
892+
url: "https://remix.run/",
893+
});
894+
testWindow = dom.window as unknown as Window;
895+
testWindow.history.pushState({}, "", "/");
896+
});
897+
898+
it("encodes characters in HashRouter", () => {
899+
testWindow.history.replaceState(null, "", "/#/with space");
900+
901+
let ctx = render(
902+
<HashRouter window={testWindow}>
903+
<Routes>
904+
<Route path="/with space" element={<ShowPath />} />
905+
</Routes>
906+
</HashRouter>
907+
);
908+
909+
expect(testWindow.location.pathname).toBe("/");
910+
expect(testWindow.location.hash).toBe("#/with%20space");
911+
expect(ctx.container.innerHTML).toMatchInlineSnapshot(
912+
`"<pre>{\\"pathname\\":\\"/with%20space\\",\\"search\\":\\"\\",\\"hash\\":\\"\\"}</pre>"`
913+
);
914+
});
915+
916+
it("encodes characters in HashRouter (navigate)", () => {
917+
testWindow.history.replaceState(null, "", "/");
918+
919+
function Start() {
920+
let navigate = useNavigate();
921+
// eslint-disable-next-line react-hooks/exhaustive-deps
922+
React.useEffect(() => navigate("/with space"), []);
923+
return null;
924+
}
925+
926+
let ctx = render(
927+
<HashRouter window={testWindow}>
928+
<Routes>
929+
<Route path="/" element={<Start />} />
930+
<Route path="/with space" element={<ShowPath />} />
931+
</Routes>
932+
</HashRouter>
933+
);
934+
935+
expect(testWindow.location.pathname).toBe("/");
936+
expect(testWindow.location.hash).toBe("#/with%20space");
937+
expect(ctx.container.innerHTML).toMatchInlineSnapshot(
938+
`"<pre>{\\"pathname\\":\\"/with%20space\\",\\"search\\":\\"\\",\\"hash\\":\\"\\"}</pre>"`
939+
);
940+
});
941+
942+
it("encodes characters in createHashRouter", () => {
943+
testWindow.history.replaceState(null, "", "/#/with space");
944+
945+
let router = createHashRouter(
946+
[{ path: "/with space", element: <ShowPath /> }],
947+
{ window: testWindow }
948+
);
949+
let ctx = render(<RouterProvider router={router} />);
950+
951+
expect(testWindow.location.pathname).toBe("/");
952+
expect(testWindow.location.hash).toBe("#/with%20space");
953+
expect(ctx.container.innerHTML).toMatchInlineSnapshot(
954+
`"<pre>{\\"pathname\\":\\"/with%20space\\",\\"search\\":\\"\\",\\"hash\\":\\"\\"}</pre>"`
955+
);
956+
});
957+
958+
it("encodes characters in createHashRouter (navigate)", () => {
959+
testWindow.history.replaceState(null, "", "/");
960+
961+
function Start() {
962+
let navigate = useNavigate();
963+
// eslint-disable-next-line react-hooks/exhaustive-deps
964+
React.useEffect(() => navigate("/with space"), []);
965+
return null;
966+
}
967+
968+
let router = createHashRouter(
969+
[
970+
{ path: "/", element: <Start /> },
971+
{ path: "/with space", element: <ShowPath /> },
972+
],
973+
{ window: testWindow }
974+
);
975+
let ctx = render(<RouterProvider router={router} />);
976+
977+
expect(testWindow.location.pathname).toBe("/");
978+
expect(testWindow.location.hash).toBe("#/with%20space");
979+
expect(ctx.container.innerHTML).toMatchInlineSnapshot(
980+
`"<pre>{\\"pathname\\":\\"/with%20space\\",\\"search\\":\\"\\",\\"hash\\":\\"\\"}</pre>"`
981+
);
982+
});
983+
});
984+
});
712985
});

‎packages/router/history.ts‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,15 @@ export interface History {
125125
*/
126126
createHref(to: To): string;
127127

128+
/**
129+
* Encode a location the same way window.history would do (no-op for memory
130+
* history) so we ensure our PUSH/REPLAC e navigations for data routers
131+
* behave the same as POP
132+
*
133+
* @param location The incoming location from router.navigate()
134+
*/
135+
encodeLocation(location: Location): Location;
136+
128137
/**
129138
* Pushes a new location onto the history stack, increasing its length by one.
130139
* If there were any entries in the stack after the current one, they are
@@ -259,6 +268,9 @@ export function createMemoryHistory(
259268
createHref(to) {
260269
return typeof to === "string" ? to : createPath(to);
261270
},
271+
encodeLocation(location) {
272+
return location;
273+
},
262274
push(to, state) {
263275
action = Action.Push;
264276
let nextLocation = createMemoryLocation(to, state);
@@ -527,6 +539,15 @@ export function parsePath(path: string): Partial<Path> {
527539
return parsedPath;
528540
}
529541

542+
export function createURL(location: Location | string): URL {
543+
let base =
544+
typeof window !== "undefined" && typeof window.location !== "undefined"
545+
? window.location.origin
546+
: "unknown://unknown";
547+
let href = typeof location === "string" ? location : createPath(location);
548+
return new URL(href, base);
549+
}
550+
530551
export interface UrlHistory extends History {}
531552

532553
export type UrlHistoryOptions = {
@@ -610,6 +631,16 @@ function getUrlBasedHistory(
610631
createHref(to) {
611632
return createHref(window, to);
612633
},
634+
encodeLocation(location) {
635+
// Encode a Location the same way window.location would
636+
let url = createURL(createPath(location));
637+
return {
638+
...location,
639+
pathname: url.pathname,
640+
search: url.search,
641+
hash: url.hash,
642+
};
643+
},
613644
push,
614645
replace,
615646
go(n) {

0 commit comments

Comments
 (0)