From 07429bf1817c654201dce4907c36f53094c60d91 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Fri, 15 Dec 2023 10:55:39 -0500 Subject: [PATCH 1/9] WIP --- examples/basic/package-lock.json | 26 ++-- examples/basic/package.json | 2 +- examples/basic/src/App.tsx | 77 ++--------- .../__tests__/special-characters-test.tsx | 127 ++++++++++++------ packages/react-router/lib/hooks.tsx | 1 + packages/router/history.ts | 1 + packages/router/utils.ts | 11 +- 7 files changed, 122 insertions(+), 123 deletions(-) diff --git a/examples/basic/package-lock.json b/examples/basic/package-lock.json index 93e92b728f..b87b22cd02 100644 --- a/examples/basic/package-lock.json +++ b/examples/basic/package-lock.json @@ -8,7 +8,7 @@ "dependencies": { "react": "^18.2.0", "react-dom": "^18.2.0", - "react-router-dom": "^6.15.0" + "react-router-dom": "^6.21.0" }, "devDependencies": { "@rollup/plugin-replace": "^5.0.2", @@ -769,9 +769,9 @@ "dev": true }, "node_modules/@remix-run/router": { - "version": "1.8.0", - "resolved": "https://registry.npmjs.org/@remix-run/router/-/router-1.8.0.tgz", - "integrity": "sha512-mrfKqIHnSZRyIzBcanNJmVQELTnX+qagEDlcKO90RgRBVOZGSGvZKeDihTRfWcqoDn5N/NkUcwWTccnpN18Tfg==", + "version": "1.14.0", + "resolved": "https://registry.npmjs.org/@remix-run/router/-/router-1.14.0.tgz", + "integrity": "sha512-WOHih+ClN7N8oHk9N4JUiMxQJmRVaOxcg8w7F/oHUXzJt920ekASLI/7cYX8XkntDWRhLZtsk6LbGrkgOAvi5A==", "engines": { "node": ">=14.0.0" } @@ -1282,11 +1282,11 @@ } }, "node_modules/react-router": { - "version": "6.15.0", - "resolved": "https://registry.npmjs.org/react-router/-/react-router-6.15.0.tgz", - "integrity": "sha512-NIytlzvzLwJkCQj2HLefmeakxxWHWAP+02EGqWEZy+DgfHHKQMUoBBjUQLOtFInBMhWtb3hiUy6MfFgwLjXhqg==", + "version": "6.21.0", + "resolved": "https://registry.npmjs.org/react-router/-/react-router-6.21.0.tgz", + "integrity": "sha512-hGZ0HXbwz3zw52pLZV3j3+ec+m/PQ9cTpBvqjFQmy2XVUWGn5MD+31oXHb6dVTxYzmAeaiUBYjkoNz66n3RGCg==", "dependencies": { - "@remix-run/router": "1.8.0" + "@remix-run/router": "1.14.0" }, "engines": { "node": ">=14.0.0" @@ -1296,12 +1296,12 @@ } }, "node_modules/react-router-dom": { - "version": "6.15.0", - "resolved": "https://registry.npmjs.org/react-router-dom/-/react-router-dom-6.15.0.tgz", - "integrity": "sha512-aR42t0fs7brintwBGAv2+mGlCtgtFQeOzK0BM1/OiqEzRejOZtpMZepvgkscpMUnKb8YO84G7s3LsHnnDNonbQ==", + "version": "6.21.0", + "resolved": "https://registry.npmjs.org/react-router-dom/-/react-router-dom-6.21.0.tgz", + "integrity": "sha512-1dUdVj3cwc1npzJaf23gulB562ESNvxf7E4x8upNJycqyUm5BRRZ6dd3LrlzhtLaMrwOCO8R0zoiYxdaJx4LlQ==", "dependencies": { - "@remix-run/router": "1.8.0", - "react-router": "6.15.0" + "@remix-run/router": "1.14.0", + "react-router": "6.21.0" }, "engines": { "node": ">=14.0.0" diff --git a/examples/basic/package.json b/examples/basic/package.json index 6613778a1f..b7cc712881 100644 --- a/examples/basic/package.json +++ b/examples/basic/package.json @@ -9,7 +9,7 @@ "dependencies": { "react": "^18.2.0", "react-dom": "^18.2.0", - "react-router-dom": "^6.15.0" + "react-router-dom": "^6.21.0" }, "devDependencies": { "@rollup/plugin-replace": "^5.0.2", diff --git a/examples/basic/src/App.tsx b/examples/basic/src/App.tsx index 0a157ecf82..9c3c101528 100644 --- a/examples/basic/src/App.tsx +++ b/examples/basic/src/App.tsx @@ -1,31 +1,13 @@ -import { Routes, Route, Outlet, Link } from "react-router-dom"; +import { Routes, Route, Outlet, Link, useParams } from "react-router-dom"; export default function App() { return (

Basic Example

- -

- This example demonstrates some of the core features of React Router - including nested <Route>s,{" "} - <Outlet>s, <Link>s, and using a - "*" route (aka "splat route") to render a "not found" page when someone - visits an unrecognized URL. -

- - {/* Routes nest inside one another. Nested route paths build upon - parent route paths, and nested route elements render inside - parent route elements. See the note about below. */} }> } /> - } /> - } /> - - {/* Using path="*"" means "match anything", so this route - acts like a catch-all for URLs that we don't have explicit - routes for. */} - } /> + } />
@@ -33,32 +15,17 @@ export default function App() { } function Layout() { + // let encoded = "a#b%c".replace(/#/g, "%23"); + let encoded = encodeURIComponent("a#b%c"); return (
- {/* A "layout route" is a good place to put markup you want to - share across all the pages on your site, like navigation. */} - - -
- - {/* An renders whatever child route is currently active, - so you can think about this as a placeholder for - the child routes we defined above. */} + Home +
+ /{encoded} +
+ + /bad%20%26%20encoding%20%25%20here +
); @@ -73,28 +40,12 @@ function Home() { } function About() { + let params = useParams(); return (

About

-
- ); -} - -function Dashboard() { - return ( -
-

Dashboard

-
- ); -} - -function NoMatch() { - return ( -
-

Nothing to see here!

-

- Go to the home page -

+

{params.id}

+

{params.id?.replace(/%23/g, "#")}

); } diff --git a/packages/react-router-dom/__tests__/special-characters-test.tsx b/packages/react-router-dom/__tests__/special-characters-test.tsx index 0f6a5ddb6e..ef53e628a2 100644 --- a/packages/react-router-dom/__tests__/special-characters-test.tsx +++ b/packages/react-router-dom/__tests__/special-characters-test.tsx @@ -136,6 +136,22 @@ let specialChars = [ // Add a few multi-char space use cases for good measure { char: "a b", pathChar: "a%20b", searchChar: "a%20b", hashChar: "a%20b" }, { char: "a+b", pathChar: "a+b", searchChar: "a+b", hashChar: "a+b" }, + + // Miscellaneous + { + char: "bad%20%26%20encoding%20%25%20here", + pathChar: "bad%20%26%20encoding%20%25%20here", + searchChar: "bad%20%26%20encoding%20%25%20here", + hashChar: "bad%20%26%20encoding%20%25%20here", + paramChar: "bad & encoding % here", + }, + { + char: "a%23b%25c", + pathChar: "a%23b%25c", + searchChar: "a%23b%25c", + hashChar: "a%23b%25c", + paramChar: "a#b%c", + }, ]; describe("special character tests", () => { @@ -333,7 +349,7 @@ describe("special character tests", () => { it("handles special chars in inline nested param route paths", async () => { for (let charDef of specialChars) { - let { char, pathChar } = charDef; + let { char, pathChar, paramChar } = charDef; await testParamValues( `/inline-param/${char}`, "Inline Nested Param Route", @@ -342,7 +358,7 @@ describe("special character tests", () => { search: "", hash: "", }, - { slug: char } + { slug: paramChar || char } ); await testParamValues( @@ -353,14 +369,14 @@ describe("special character tests", () => { search: "", hash: "", }, - { slug: `foo${char}bar` } + { slug: `foo${paramChar || char}bar` } ); } }); it("handles special chars in parent nested param route paths", async () => { for (let charDef of specialChars) { - let { char, pathChar } = charDef; + let { char, pathChar, paramChar } = charDef; await testParamValues( `/param/${char}`, "Parent Nested Param Route", @@ -369,7 +385,7 @@ describe("special character tests", () => { search: "", hash: "", }, - { slug: char } + { slug: paramChar || char } ); await testParamValues( @@ -380,14 +396,14 @@ describe("special character tests", () => { search: "", hash: "", }, - { slug: `foo${char}bar` } + { slug: `foo${paramChar || char}bar` } ); } }); it("handles special chars in inline nested splat routes", async () => { for (let charDef of specialChars) { - let { char, pathChar } = charDef; + let { char, pathChar, paramChar } = charDef; await testParamValues( `/inline-splat/${char}`, "Inline Nested Splat Route", @@ -396,7 +412,7 @@ describe("special character tests", () => { search: "", hash: "", }, - { "*": char } + { "*": paramChar || char } ); await testParamValues( @@ -407,14 +423,14 @@ describe("special character tests", () => { search: "", hash: "", }, - { "*": `foo${char}bar` } + { "*": `foo${paramChar || char}bar` } ); } }); it("handles special chars in nested splat routes", async () => { for (let charDef of specialChars) { - let { char, pathChar } = charDef; + let { char, pathChar, paramChar } = charDef; await testParamValues( `/splat/${char}`, "Parent Nested Splat Route", @@ -423,7 +439,7 @@ describe("special character tests", () => { search: "", hash: "", }, - { "*": char } + { "*": paramChar || char } ); await testParamValues( @@ -434,14 +450,14 @@ describe("special character tests", () => { search: "", hash: "", }, - { "*": `foo${char}bar` } + { "*": `foo${paramChar || char}bar` } ); } }); it("handles special chars in nested splat routes with separators", async () => { for (let charDef of specialChars) { - let { char, pathChar } = charDef; + let { char, pathChar, paramChar } = charDef; await testParamValues( `/splat/foo/bar${char}`, "Parent Nested Splat Route", @@ -450,14 +466,14 @@ describe("special character tests", () => { search: "", hash: "", }, - { "*": `foo/bar${char}` } + { "*": `foo/bar${paramChar || char}` } ); } }); it("handles special chars in root splat routes", async () => { for (let charDef of specialChars) { - let { char, pathChar } = charDef; + let { char, pathChar, paramChar } = charDef; await testParamValues( `/${char}`, "Root Splat Route", @@ -466,7 +482,7 @@ describe("special character tests", () => { search: "", hash: "", }, - { "*": char } + { "*": paramChar || char } ); await testParamValues( @@ -477,14 +493,14 @@ describe("special character tests", () => { search: "", hash: "", }, - { "*": `foo${char}bar` } + { "*": `foo${paramChar || char}bar` } ); } }); it("handles special chars in root splat routes with separators", async () => { for (let charDef of specialChars) { - let { char, pathChar } = charDef; + let { char, pathChar, paramChar } = charDef; await testParamValues( `/foo/bar${char}`, "Root Splat Route", @@ -493,37 +509,60 @@ describe("special character tests", () => { search: "", hash: "", }, - { "*": `foo/bar${char}` } + { "*": `foo/bar${paramChar || char}` } ); } }); - it("handles special chars in descendant routes paths", async () => { - for (let charDef of specialChars) { - let { char, pathChar } = charDef; - - await testParamValues( - `/descendant/${char}/match`, - "Descendant Route", - { - pathname: `/descendant/${pathChar}/match`, - search: "", - hash: "", - }, - { param: char, "*": "match" } - ); + it.only("handles special chars in descendant routes paths", async () => { + // for (let charDef of specialChars) { + // let { char, pathChar, paramChar } = charDef; + + // await testParamValues( + // `/descendant/${char}/match`, + // "Descendant Route", + // { + // pathname: `/descendant/${pathChar}/match`, + // search: "", + // hash: "", + // }, + // { param: paramChar || char, "*": "match" } + // ); + + // await testParamValues( + // `/descendant/foo${char}bar/match`, + // "Descendant Route", + // { + // pathname: `/descendant/foo${pathChar}bar/match`, + // search: "", + // hash: "", + // }, + // { param: `foo${paramChar || char}bar`, "*": "match" } + // ); + // } + + debugger; + await testParamValues( + `/descendant/bad%20%26%20encoding%20%25%20here/match`, + "Descendant Route", + { + pathname: `/descendant/bad%20%26%20encoding%20%25%20here/match`, + search: "", + hash: "", + }, + { param: "bad & encoding % here", "*": "match" } + ); - await testParamValues( - `/descendant/foo${char}bar/match`, - "Descendant Route", - { - pathname: `/descendant/foo${pathChar}bar/match`, - search: "", - hash: "", - }, - { param: `foo${char}bar`, "*": "match" } - ); - } + // await testParamValues( + // `/descendant/foobad%20%26%20encoding%20%25%20herebar/match`, + // "Descendant Route", + // { + // pathname: `/descendant/foobad%20%26%20encoding%20%25%20herebar/match`, + // search: "", + // hash: "", + // }, + // { param: `foobad & encoding % herebar`, "*": "match" } + // ); }); it("handles special chars in search params", async () => { diff --git a/packages/react-router/lib/hooks.tsx b/packages/react-router/lib/hooks.tsx index 5d705553e7..338347aaf7 100644 --- a/packages/react-router/lib/hooks.tsx +++ b/packages/react-router/lib/hooks.tsx @@ -422,6 +422,7 @@ export function useRoutesImpl( } let pathname = location.pathname || "/"; + debugger; let remainingPathname = parentPathnameBase === "/" ? pathname diff --git a/packages/router/history.ts b/packages/router/history.ts index ad4bd44b09..d068d11646 100644 --- a/packages/router/history.ts +++ b/packages/router/history.ts @@ -723,6 +723,7 @@ function getUrlBasedHistory( encodeLocation(to) { // Encode a Location the same way window.location would let url = createURL(to); + // to.split('/').map(v => encodeURIComponent(v)).join('/') return { pathname: url.pathname, search: url.search, diff --git a/packages/router/utils.ts b/packages/router/utils.ts index c845d0285f..82e7077bde 100644 --- a/packages/router/utils.ts +++ b/packages/router/utils.ts @@ -480,6 +480,7 @@ export function matchRoutes< return null; } + debugger; let branches = flattenRoutes(routes); rankRouteBranches(branches); @@ -763,6 +764,7 @@ function matchRouteBranch< let route = meta.route; + debugger; matches.push({ // TODO: Can this as be avoided? params: matchedParams as Params, @@ -930,7 +932,8 @@ export function matchPath< if (isOptional && !value) { memo[paramName] = undefined; } else { - memo[paramName] = safelyDecodeURIComponent(value || "", paramName); + memo[paramName] = value || ""; + // memo[paramName] = safelyDecodeURIComponent(value || "", paramName); } return memo; }, @@ -1004,7 +1007,11 @@ function compilePath( function safelyDecodeURI(value: string) { try { - return decodeURI(value); + // return decodeURI(value); + return value + .split("/") + .map((v) => decodeURIComponent(v).replace(/\//g, "%2F")) + .join("/"); } catch (error) { warning( false, From 62479e4db4bda19eeaad2cf73fc58acc5925467e Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Fri, 12 Jan 2024 16:27:49 -0500 Subject: [PATCH 2/9] More updates --- .../__tests__/special-characters-test.tsx | 155 ++++++++---------- packages/react-router/lib/hooks.tsx | 26 ++- packages/router/history.ts | 1 - packages/router/utils.ts | 6 +- 4 files changed, 94 insertions(+), 94 deletions(-) diff --git a/packages/react-router-dom/__tests__/special-characters-test.tsx b/packages/react-router-dom/__tests__/special-characters-test.tsx index ef53e628a2..9b55608a56 100644 --- a/packages/react-router-dom/__tests__/special-characters-test.tsx +++ b/packages/react-router-dom/__tests__/special-characters-test.tsx @@ -139,18 +139,25 @@ let specialChars = [ // Miscellaneous { - char: "bad%20%26%20encoding%20%25%20here", - pathChar: "bad%20%26%20encoding%20%25%20here", - searchChar: "bad%20%26%20encoding%20%25%20here", - hashChar: "bad%20%26%20encoding%20%25%20here", - paramChar: "bad & encoding % here", + char: "a%25b", + pathChar: "a%25b", + searchChar: "a%25b", + hashChar: "a%25b", + decodedChar: "a%b", }, { char: "a%23b%25c", pathChar: "a%23b%25c", searchChar: "a%23b%25c", hashChar: "a%23b%25c", - paramChar: "a#b%c", + decodedChar: "a#b%c", + }, + { + char: "a%26b%25c", + pathChar: "a%26b%25c", + searchChar: "a%26b%25c", + hashChar: "a%26b%25c", + decodedChar: "a&b%c", }, ]; @@ -349,7 +356,7 @@ describe("special character tests", () => { it("handles special chars in inline nested param route paths", async () => { for (let charDef of specialChars) { - let { char, pathChar, paramChar } = charDef; + let { char, pathChar, decodedChar } = charDef; await testParamValues( `/inline-param/${char}`, "Inline Nested Param Route", @@ -358,7 +365,7 @@ describe("special character tests", () => { search: "", hash: "", }, - { slug: paramChar || char } + { slug: decodedChar || char } ); await testParamValues( @@ -369,14 +376,14 @@ describe("special character tests", () => { search: "", hash: "", }, - { slug: `foo${paramChar || char}bar` } + { slug: `foo${decodedChar || char}bar` } ); } }); it("handles special chars in parent nested param route paths", async () => { for (let charDef of specialChars) { - let { char, pathChar, paramChar } = charDef; + let { char, pathChar, decodedChar } = charDef; await testParamValues( `/param/${char}`, "Parent Nested Param Route", @@ -385,7 +392,7 @@ describe("special character tests", () => { search: "", hash: "", }, - { slug: paramChar || char } + { slug: decodedChar || char } ); await testParamValues( @@ -396,14 +403,14 @@ describe("special character tests", () => { search: "", hash: "", }, - { slug: `foo${paramChar || char}bar` } + { slug: `foo${decodedChar || char}bar` } ); } }); it("handles special chars in inline nested splat routes", async () => { for (let charDef of specialChars) { - let { char, pathChar, paramChar } = charDef; + let { char, pathChar, decodedChar } = charDef; await testParamValues( `/inline-splat/${char}`, "Inline Nested Splat Route", @@ -412,7 +419,7 @@ describe("special character tests", () => { search: "", hash: "", }, - { "*": paramChar || char } + { "*": decodedChar || char } ); await testParamValues( @@ -423,14 +430,14 @@ describe("special character tests", () => { search: "", hash: "", }, - { "*": `foo${paramChar || char}bar` } + { "*": `foo${decodedChar || char}bar` } ); } }); it("handles special chars in nested splat routes", async () => { for (let charDef of specialChars) { - let { char, pathChar, paramChar } = charDef; + let { char, pathChar, decodedChar } = charDef; await testParamValues( `/splat/${char}`, "Parent Nested Splat Route", @@ -439,7 +446,7 @@ describe("special character tests", () => { search: "", hash: "", }, - { "*": paramChar || char } + { "*": decodedChar || char } ); await testParamValues( @@ -450,14 +457,14 @@ describe("special character tests", () => { search: "", hash: "", }, - { "*": `foo${paramChar || char}bar` } + { "*": `foo${decodedChar || char}bar` } ); } }); it("handles special chars in nested splat routes with separators", async () => { for (let charDef of specialChars) { - let { char, pathChar, paramChar } = charDef; + let { char, pathChar, decodedChar } = charDef; await testParamValues( `/splat/foo/bar${char}`, "Parent Nested Splat Route", @@ -466,14 +473,14 @@ describe("special character tests", () => { search: "", hash: "", }, - { "*": `foo/bar${paramChar || char}` } + { "*": `foo/bar${decodedChar || char}` } ); } }); it("handles special chars in root splat routes", async () => { for (let charDef of specialChars) { - let { char, pathChar, paramChar } = charDef; + let { char, pathChar, decodedChar } = charDef; await testParamValues( `/${char}`, "Root Splat Route", @@ -482,7 +489,7 @@ describe("special character tests", () => { search: "", hash: "", }, - { "*": paramChar || char } + { "*": decodedChar || char } ); await testParamValues( @@ -493,14 +500,14 @@ describe("special character tests", () => { search: "", hash: "", }, - { "*": `foo${paramChar || char}bar` } + { "*": `foo${decodedChar || char}bar` } ); } }); it("handles special chars in root splat routes with separators", async () => { for (let charDef of specialChars) { - let { char, pathChar, paramChar } = charDef; + let { char, pathChar, decodedChar } = charDef; await testParamValues( `/foo/bar${char}`, "Root Splat Route", @@ -509,60 +516,37 @@ describe("special character tests", () => { search: "", hash: "", }, - { "*": `foo/bar${paramChar || char}` } + { "*": `foo/bar${decodedChar || char}` } ); } }); - it.only("handles special chars in descendant routes paths", async () => { - // for (let charDef of specialChars) { - // let { char, pathChar, paramChar } = charDef; - - // await testParamValues( - // `/descendant/${char}/match`, - // "Descendant Route", - // { - // pathname: `/descendant/${pathChar}/match`, - // search: "", - // hash: "", - // }, - // { param: paramChar || char, "*": "match" } - // ); - - // await testParamValues( - // `/descendant/foo${char}bar/match`, - // "Descendant Route", - // { - // pathname: `/descendant/foo${pathChar}bar/match`, - // search: "", - // hash: "", - // }, - // { param: `foo${paramChar || char}bar`, "*": "match" } - // ); - // } - - debugger; - await testParamValues( - `/descendant/bad%20%26%20encoding%20%25%20here/match`, - "Descendant Route", - { - pathname: `/descendant/bad%20%26%20encoding%20%25%20here/match`, - search: "", - hash: "", - }, - { param: "bad & encoding % here", "*": "match" } - ); + it("handles special chars in descendant routes paths", async () => { + for (let charDef of specialChars) { + let { char, pathChar, decodedChar } = charDef; + + await testParamValues( + `/descendant/${char}/match`, + "Descendant Route", + { + pathname: `/descendant/${pathChar}/match`, + search: "", + hash: "", + }, + { param: decodedChar || char, "*": "match" } + ); - // await testParamValues( - // `/descendant/foobad%20%26%20encoding%20%25%20herebar/match`, - // "Descendant Route", - // { - // pathname: `/descendant/foobad%20%26%20encoding%20%25%20herebar/match`, - // search: "", - // hash: "", - // }, - // { param: `foobad & encoding % herebar`, "*": "match" } - // ); + await testParamValues( + `/descendant/foo${char}bar/match`, + "Descendant Route", + { + pathname: `/descendant/foo${pathChar}bar/match`, + search: "", + hash: "", + }, + { param: `foo${decodedChar || char}bar`, "*": "match" } + ); + } }); it("handles special chars in search params", async () => { @@ -736,28 +720,33 @@ describe("special character tests", () => { it("handles special chars in root route paths", async () => { for (let charDef of specialChars) { - let { char, pathChar } = charDef; + let { char, pathChar, decodedChar } = charDef; // Skip * which is just a splat route if (char === "*") { continue; } - await assertRouteMatch(char, `/${char}`, "Matched Root", { - pathname: `/${pathChar}`, - search: "", - hash: "", - }); + await assertRouteMatch( + decodedChar || char, + `/${char}`, + "Matched Root", + { + pathname: `/${pathChar}`, + search: "", + hash: "", + } + ); } }); it("handles special chars in static nested route paths", async () => { for (let charDef of specialChars) { - let { char, pathChar } = charDef; + let { char, pathChar, decodedChar } = charDef; // Skip * which is just a splat route if (char === "*") { continue; } await assertRouteMatch( - char, + decodedChar || char, `/nested/${char}`, "Matched Static Nested", { @@ -771,13 +760,13 @@ describe("special character tests", () => { it("handles special chars in nested param route paths", async () => { for (let charDef of specialChars) { - let { char, pathChar } = charDef; + let { char, pathChar, decodedChar } = charDef; // Skip * which is just a splat route if (char === "*") { continue; } await assertRouteMatch( - char, + decodedChar || char, `/foo/${char}`, "Matched Param Nested", { diff --git a/packages/react-router/lib/hooks.tsx b/packages/react-router/lib/hooks.tsx index 338347aaf7..0feda4720f 100644 --- a/packages/react-router/lib/hooks.tsx +++ b/packages/react-router/lib/hooks.tsx @@ -422,11 +422,27 @@ export function useRoutesImpl( } let pathname = location.pathname || "/"; - debugger; - let remainingPathname = - parentPathnameBase === "/" - ? pathname - : pathname.slice(parentPathnameBase.length) || "/"; + + let remainingPathname = pathname; + if (parentPathnameBase !== "/") { + // Determine the remaining pathname by removing the # of URL segments the + // parentPathnameBase has, instead of removing based on character count. + // This is because we can't guarantee that incoming/outgoing encodings/ + // decodings will match exactly. + // We decode paths before matching on a per-segment basis with + // decodeURIComponent(), but we re-encode pathnames via `new URL()` so they + // match what `window.location.pathname` would reflect. Those don't 100% + // align when it comes to encoded URI characters such as % and &. + // + // So we may end up with: + // pathname: "/descendant/a%25b/match" + // parentPathnameBase: "/descendant/a%b" + // + // And the direct substring removal approach won't work :/ + let parentSegments = parentPathnameBase.replace(/^\//, "").split("/"); + let segments = pathname.replace(/^\//, "").split("/"); + remainingPathname = "/" + segments.slice(parentSegments.length).join("/"); + } let matches = matchRoutes(routes, { pathname: remainingPathname }); diff --git a/packages/router/history.ts b/packages/router/history.ts index d068d11646..ad4bd44b09 100644 --- a/packages/router/history.ts +++ b/packages/router/history.ts @@ -723,7 +723,6 @@ function getUrlBasedHistory( encodeLocation(to) { // Encode a Location the same way window.location would let url = createURL(to); - // to.split('/').map(v => encodeURIComponent(v)).join('/') return { pathname: url.pathname, search: url.search, diff --git a/packages/router/utils.ts b/packages/router/utils.ts index 82e7077bde..0d2024d7cb 100644 --- a/packages/router/utils.ts +++ b/packages/router/utils.ts @@ -480,7 +480,6 @@ export function matchRoutes< return null; } - debugger; let branches = flattenRoutes(routes); rankRouteBranches(branches); @@ -764,7 +763,6 @@ function matchRouteBranch< let route = meta.route; - debugger; matches.push({ // TODO: Can this as be avoided? params: matchedParams as Params, @@ -932,8 +930,7 @@ export function matchPath< if (isOptional && !value) { memo[paramName] = undefined; } else { - memo[paramName] = value || ""; - // memo[paramName] = safelyDecodeURIComponent(value || "", paramName); + memo[paramName] = (value || "").replace(/%2F/g, "/"); } return memo; }, @@ -1007,7 +1004,6 @@ function compilePath( function safelyDecodeURI(value: string) { try { - // return decodeURI(value); return value .split("/") .map((v) => decodeURIComponent(v).replace(/\//g, "%2F")) From 37ed4122a67c46c7c9d6bba57691d373d9f916e5 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Fri, 12 Jan 2024 16:29:12 -0500 Subject: [PATCH 3/9] Reset basic exmaple to dev --- examples/basic/package-lock.json | 26 +++++------ examples/basic/package.json | 2 +- examples/basic/src/App.tsx | 77 ++++++++++++++++++++++++++------ 3 files changed, 77 insertions(+), 28 deletions(-) diff --git a/examples/basic/package-lock.json b/examples/basic/package-lock.json index b87b22cd02..93e92b728f 100644 --- a/examples/basic/package-lock.json +++ b/examples/basic/package-lock.json @@ -8,7 +8,7 @@ "dependencies": { "react": "^18.2.0", "react-dom": "^18.2.0", - "react-router-dom": "^6.21.0" + "react-router-dom": "^6.15.0" }, "devDependencies": { "@rollup/plugin-replace": "^5.0.2", @@ -769,9 +769,9 @@ "dev": true }, "node_modules/@remix-run/router": { - "version": "1.14.0", - "resolved": "https://registry.npmjs.org/@remix-run/router/-/router-1.14.0.tgz", - "integrity": "sha512-WOHih+ClN7N8oHk9N4JUiMxQJmRVaOxcg8w7F/oHUXzJt920ekASLI/7cYX8XkntDWRhLZtsk6LbGrkgOAvi5A==", + "version": "1.8.0", + "resolved": "https://registry.npmjs.org/@remix-run/router/-/router-1.8.0.tgz", + "integrity": "sha512-mrfKqIHnSZRyIzBcanNJmVQELTnX+qagEDlcKO90RgRBVOZGSGvZKeDihTRfWcqoDn5N/NkUcwWTccnpN18Tfg==", "engines": { "node": ">=14.0.0" } @@ -1282,11 +1282,11 @@ } }, "node_modules/react-router": { - "version": "6.21.0", - "resolved": "https://registry.npmjs.org/react-router/-/react-router-6.21.0.tgz", - "integrity": "sha512-hGZ0HXbwz3zw52pLZV3j3+ec+m/PQ9cTpBvqjFQmy2XVUWGn5MD+31oXHb6dVTxYzmAeaiUBYjkoNz66n3RGCg==", + "version": "6.15.0", + "resolved": "https://registry.npmjs.org/react-router/-/react-router-6.15.0.tgz", + "integrity": "sha512-NIytlzvzLwJkCQj2HLefmeakxxWHWAP+02EGqWEZy+DgfHHKQMUoBBjUQLOtFInBMhWtb3hiUy6MfFgwLjXhqg==", "dependencies": { - "@remix-run/router": "1.14.0" + "@remix-run/router": "1.8.0" }, "engines": { "node": ">=14.0.0" @@ -1296,12 +1296,12 @@ } }, "node_modules/react-router-dom": { - "version": "6.21.0", - "resolved": "https://registry.npmjs.org/react-router-dom/-/react-router-dom-6.21.0.tgz", - "integrity": "sha512-1dUdVj3cwc1npzJaf23gulB562ESNvxf7E4x8upNJycqyUm5BRRZ6dd3LrlzhtLaMrwOCO8R0zoiYxdaJx4LlQ==", + "version": "6.15.0", + "resolved": "https://registry.npmjs.org/react-router-dom/-/react-router-dom-6.15.0.tgz", + "integrity": "sha512-aR42t0fs7brintwBGAv2+mGlCtgtFQeOzK0BM1/OiqEzRejOZtpMZepvgkscpMUnKb8YO84G7s3LsHnnDNonbQ==", "dependencies": { - "@remix-run/router": "1.14.0", - "react-router": "6.21.0" + "@remix-run/router": "1.8.0", + "react-router": "6.15.0" }, "engines": { "node": ">=14.0.0" diff --git a/examples/basic/package.json b/examples/basic/package.json index b7cc712881..6613778a1f 100644 --- a/examples/basic/package.json +++ b/examples/basic/package.json @@ -9,7 +9,7 @@ "dependencies": { "react": "^18.2.0", "react-dom": "^18.2.0", - "react-router-dom": "^6.21.0" + "react-router-dom": "^6.15.0" }, "devDependencies": { "@rollup/plugin-replace": "^5.0.2", diff --git a/examples/basic/src/App.tsx b/examples/basic/src/App.tsx index 9c3c101528..0a157ecf82 100644 --- a/examples/basic/src/App.tsx +++ b/examples/basic/src/App.tsx @@ -1,13 +1,31 @@ -import { Routes, Route, Outlet, Link, useParams } from "react-router-dom"; +import { Routes, Route, Outlet, Link } from "react-router-dom"; export default function App() { return (

Basic Example

+ +

+ This example demonstrates some of the core features of React Router + including nested <Route>s,{" "} + <Outlet>s, <Link>s, and using a + "*" route (aka "splat route") to render a "not found" page when someone + visits an unrecognized URL. +

+ + {/* Routes nest inside one another. Nested route paths build upon + parent route paths, and nested route elements render inside + parent route elements. See the note about below. */} }> } /> - } /> + } /> + } /> + + {/* Using path="*"" means "match anything", so this route + acts like a catch-all for URLs that we don't have explicit + routes for. */} + } />
@@ -15,17 +33,32 @@ export default function App() { } function Layout() { - // let encoded = "a#b%c".replace(/#/g, "%23"); - let encoded = encodeURIComponent("a#b%c"); return (
- Home -
- /{encoded} -
- - /bad%20%26%20encoding%20%25%20here - + {/* A "layout route" is a good place to put markup you want to + share across all the pages on your site, like navigation. */} + + +
+ + {/* An renders whatever child route is currently active, + so you can think about this as a placeholder for + the child routes we defined above. */}
); @@ -40,12 +73,28 @@ function Home() { } function About() { - let params = useParams(); return (

About

-

{params.id}

-

{params.id?.replace(/%23/g, "#")}

+
+ ); +} + +function Dashboard() { + return ( +
+

Dashboard

+
+ ); +} + +function NoMatch() { + return ( +
+

Nothing to see here!

+

+ Go to the home page +

); } From 21879e59e3990ea40de944955baa28fdf6ad262b Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Fri, 12 Jan 2024 16:37:22 -0500 Subject: [PATCH 4/9] Add comment --- .../__tests__/special-characters-test.tsx | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/packages/react-router-dom/__tests__/special-characters-test.tsx b/packages/react-router-dom/__tests__/special-characters-test.tsx index 9b55608a56..ff1583a293 100644 --- a/packages/react-router-dom/__tests__/special-characters-test.tsx +++ b/packages/react-router-dom/__tests__/special-characters-test.tsx @@ -33,6 +33,15 @@ import { * maximum accuracy. This is instead of programmatically generating during * these tests where JSDOM or a bad URL polyfill might not be trustworthy. * + * + * | Field | Description | + * |-------------|----------------------------------------------------------------------| + * | char | The (usually decoded) verbatim "character" you put in your | + * | pathChar | The value we expect to receive from location.pathname | + * | searchChar | The value we expect to receive from location.search | + * | hashChar | The value we expect to receive from location.hash | + * | decodedChar | The decoded value we expect to receive from params | + * * function generateCharDef(char) { * return { * char, @@ -137,7 +146,11 @@ let specialChars = [ { char: "a b", pathChar: "a%20b", searchChar: "a%20b", hashChar: "a%20b" }, { char: "a+b", pathChar: "a+b", searchChar: "a+b", hashChar: "a+b" }, - // Miscellaneous + // Edge case scenarios where the incoming `char` (or string) is pre-encoded + // because it contains special characters such as `&`, `%`, or `#`. For these + // we provide a `decodedChar` so we can assert the param value gets decoded + // properly and so we can ensure we can match these decoded values in static + // paths { char: "a%25b", pathChar: "a%25b", From 39638813e3b8917f71d59166a0bf915bad3c050d Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 17 Jan 2024 14:13:10 -0500 Subject: [PATCH 5/9] rRemove optionality of decodedChar in tests --- .../__tests__/special-characters-test.tsx | 133 ++++++++++-------- 1 file changed, 77 insertions(+), 56 deletions(-) diff --git a/packages/react-router-dom/__tests__/special-characters-test.tsx b/packages/react-router-dom/__tests__/special-characters-test.tsx index ff1583a293..a8eeaf7d4b 100644 --- a/packages/react-router-dom/__tests__/special-characters-test.tsx +++ b/packages/react-router-dom/__tests__/special-characters-test.tsx @@ -54,24 +54,24 @@ import { let specialChars = [ // This set of characters never gets encoded by window.location - { char: "x", pathChar: "x", searchChar: "x", hashChar: "x" }, - { char: "X", pathChar: "X", searchChar: "X", hashChar: "X" }, - { char: "~", pathChar: "~", searchChar: "~", hashChar: "~" }, - { char: "!", pathChar: "!", searchChar: "!", hashChar: "!" }, - { char: "@", pathChar: "@", searchChar: "@", hashChar: "@" }, - { char: "$", pathChar: "$", searchChar: "$", hashChar: "$" }, - { char: "*", pathChar: "*", searchChar: "*", hashChar: "*" }, - { char: "(", pathChar: "(", searchChar: "(", hashChar: "(" }, - { char: ")", pathChar: ")", searchChar: ")", hashChar: ")" }, - { char: "_", pathChar: "_", searchChar: "_", hashChar: "_" }, - { char: "-", pathChar: "-", searchChar: "-", hashChar: "-" }, - { char: "+", pathChar: "+", searchChar: "+", hashChar: "+" }, - { char: "=", pathChar: "=", searchChar: "=", hashChar: "=" }, - { char: "[", pathChar: "[", searchChar: "[", hashChar: "[" }, - { char: "]", pathChar: "]", searchChar: "]", hashChar: "]" }, - { char: ":", pathChar: ":", searchChar: ":", hashChar: ":" }, - { char: ";", pathChar: ";", searchChar: ";", hashChar: ";" }, - { char: ",", pathChar: ",", searchChar: ",", hashChar: "," }, + { char: "x", pathChar: "x", searchChar: "x", hashChar: "x", decodedChar: "x" }, + { char: "X", pathChar: "X", searchChar: "X", hashChar: "X", decodedChar: "X" }, + { char: "~", pathChar: "~", searchChar: "~", hashChar: "~", decodedChar: "~" }, + { char: "!", pathChar: "!", searchChar: "!", hashChar: "!", decodedChar: "!" }, + { char: "@", pathChar: "@", searchChar: "@", hashChar: "@", decodedChar: "@" }, + { char: "$", pathChar: "$", searchChar: "$", hashChar: "$", decodedChar: "$" }, + { char: "*", pathChar: "*", searchChar: "*", hashChar: "*", decodedChar: "*" }, + { char: "(", pathChar: "(", searchChar: "(", hashChar: "(", decodedChar: "(" }, + { char: ")", pathChar: ")", searchChar: ")", hashChar: ")", decodedChar: ")" }, + { char: "_", pathChar: "_", searchChar: "_", hashChar: "_", decodedChar: "_" }, + { char: "-", pathChar: "-", searchChar: "-", hashChar: "-", decodedChar: "-" }, + { char: "+", pathChar: "+", searchChar: "+", hashChar: "+", decodedChar: "+" }, + { char: "=", pathChar: "=", searchChar: "=", hashChar: "=", decodedChar: "=" }, + { char: "[", pathChar: "[", searchChar: "[", hashChar: "[", decodedChar: "[" }, + { char: "]", pathChar: "]", searchChar: "]", hashChar: "]", decodedChar: "]" }, + { char: ":", pathChar: ":", searchChar: ":", hashChar: ":", decodedChar: ":" }, + { char: ";", pathChar: ";", searchChar: ";", hashChar: ";", decodedChar: ";" }, + { char: ",", pathChar: ",", searchChar: ",", hashChar: ",", decodedChar: "," }, // These chars should only get encoded when in the pathname, but JSDOM // seems to have a bug as it does not encode them, so don't test this @@ -81,13 +81,13 @@ let specialChars = [ // These chars get conditionally encoded based on what portion of the // URL they occur in - { char: "{", pathChar: "%7B", searchChar: "{", hashChar: "{" }, - { char: "}", pathChar: "%7D", searchChar: "}", hashChar: "}" }, - { char: "`", pathChar: "%60", searchChar: "`", hashChar: "%60" }, - { char: "'", pathChar: "'", searchChar: "%27", hashChar: "'" }, - { char: '"', pathChar: "%22", searchChar: "%22", hashChar: "%22" }, - { char: "<", pathChar: "%3C", searchChar: "%3C", hashChar: "%3C" }, - { char: ">", pathChar: "%3E", searchChar: "%3E", hashChar: "%3E" }, + { char: "{", pathChar: "%7B", searchChar: "{", hashChar: "{", decodedChar: "{" }, + { char: "}", pathChar: "%7D", searchChar: "}", hashChar: "}", decodedChar: "}" }, + { char: "`", pathChar: "%60", searchChar: "`", hashChar: "%60", decodedChar: "`" }, + { char: "'", pathChar: "'", searchChar: "%27", hashChar: "'", decodedChar: "'" }, + { char: '"', pathChar: "%22", searchChar: "%22", hashChar: "%22", decodedChar: '"' }, + { char: "<", pathChar: "%3C", searchChar: "%3C", hashChar: "%3C", decodedChar: "<" }, + { char: ">", pathChar: "%3E", searchChar: "%3E", hashChar: "%3E", decodedChar: ">" }, // These chars get encoded in all portions of the URL { @@ -95,56 +95,82 @@ let specialChars = [ pathChar: "%F0%9F%A4%AF", searchChar: "%F0%9F%A4%AF", hashChar: "%F0%9F%A4%AF", + decodedChar: "🤯", }, { char: "✅", pathChar: "%E2%9C%85", searchChar: "%E2%9C%85", hashChar: "%E2%9C%85", + decodedChar: "✅", }, { char: "🔥", pathChar: "%F0%9F%94%A5", searchChar: "%F0%9F%94%A5", hashChar: "%F0%9F%94%A5", + decodedChar: "🔥", + }, + { + char: "ä", + pathChar: "%C3%A4", + searchChar: "%C3%A4", + hashChar: "%C3%A4", + decodedChar: "ä", + }, + { + char: "Ä", + pathChar: "%C3%84", + searchChar: "%C3%84", + hashChar: "%C3%84", + decodedChar: "Ä", + }, + { + char: "ø", + pathChar: "%C3%B8", + searchChar: "%C3%B8", + hashChar: "%C3%B8", + decodedChar: "ø", }, - { char: "ä", pathChar: "%C3%A4", searchChar: "%C3%A4", hashChar: "%C3%A4" }, - { char: "Ä", pathChar: "%C3%84", searchChar: "%C3%84", hashChar: "%C3%84" }, - { char: "ø", pathChar: "%C3%B8", searchChar: "%C3%B8", hashChar: "%C3%B8" }, { char: "山", pathChar: "%E5%B1%B1", searchChar: "%E5%B1%B1", hashChar: "%E5%B1%B1", + decodedChar: "山", }, { char: "人", pathChar: "%E4%BA%BA", searchChar: "%E4%BA%BA", hashChar: "%E4%BA%BA", + decodedChar: "人", }, { char: "口", pathChar: "%E5%8F%A3", searchChar: "%E5%8F%A3", hashChar: "%E5%8F%A3", + decodedChar: "口", }, { char: "刀", pathChar: "%E5%88%80", searchChar: "%E5%88%80", hashChar: "%E5%88%80", + decodedChar: "刀", }, { char: "木", pathChar: "%E6%9C%A8", searchChar: "%E6%9C%A8", hashChar: "%E6%9C%A8", + decodedChar: "木", }, // Add a few multi-char space use cases for good measure - { char: "a b", pathChar: "a%20b", searchChar: "a%20b", hashChar: "a%20b" }, - { char: "a+b", pathChar: "a+b", searchChar: "a+b", hashChar: "a+b" }, + { char: "a b", pathChar: "a%20b", searchChar: "a%20b", hashChar: "a%20b", decodedChar: "a b" }, + { char: "a+b", pathChar: "a+b", searchChar: "a+b", hashChar: "a+b", decodedChar: "a+b" }, // Edge case scenarios where the incoming `char` (or string) is pre-encoded // because it contains special characters such as `&`, `%`, or `#`. For these @@ -378,7 +404,7 @@ describe("special character tests", () => { search: "", hash: "", }, - { slug: decodedChar || char } + { slug: decodedChar } ); await testParamValues( @@ -389,7 +415,7 @@ describe("special character tests", () => { search: "", hash: "", }, - { slug: `foo${decodedChar || char}bar` } + { slug: `foo${decodedChar}bar` } ); } }); @@ -405,7 +431,7 @@ describe("special character tests", () => { search: "", hash: "", }, - { slug: decodedChar || char } + { slug: decodedChar } ); await testParamValues( @@ -416,7 +442,7 @@ describe("special character tests", () => { search: "", hash: "", }, - { slug: `foo${decodedChar || char}bar` } + { slug: `foo${decodedChar}bar` } ); } }); @@ -432,7 +458,7 @@ describe("special character tests", () => { search: "", hash: "", }, - { "*": decodedChar || char } + { "*": decodedChar } ); await testParamValues( @@ -443,7 +469,7 @@ describe("special character tests", () => { search: "", hash: "", }, - { "*": `foo${decodedChar || char}bar` } + { "*": `foo${decodedChar}bar` } ); } }); @@ -459,7 +485,7 @@ describe("special character tests", () => { search: "", hash: "", }, - { "*": decodedChar || char } + { "*": decodedChar } ); await testParamValues( @@ -470,7 +496,7 @@ describe("special character tests", () => { search: "", hash: "", }, - { "*": `foo${decodedChar || char}bar` } + { "*": `foo${decodedChar}bar` } ); } }); @@ -486,7 +512,7 @@ describe("special character tests", () => { search: "", hash: "", }, - { "*": `foo/bar${decodedChar || char}` } + { "*": `foo/bar${decodedChar}` } ); } }); @@ -502,7 +528,7 @@ describe("special character tests", () => { search: "", hash: "", }, - { "*": decodedChar || char } + { "*": decodedChar } ); await testParamValues( @@ -513,7 +539,7 @@ describe("special character tests", () => { search: "", hash: "", }, - { "*": `foo${decodedChar || char}bar` } + { "*": `foo${decodedChar}bar` } ); } }); @@ -529,7 +555,7 @@ describe("special character tests", () => { search: "", hash: "", }, - { "*": `foo/bar${decodedChar || char}` } + { "*": `foo/bar${decodedChar}` } ); } }); @@ -546,7 +572,7 @@ describe("special character tests", () => { search: "", hash: "", }, - { param: decodedChar || char, "*": "match" } + { param: decodedChar, "*": "match" } ); await testParamValues( @@ -557,7 +583,7 @@ describe("special character tests", () => { search: "", hash: "", }, - { param: `foo${decodedChar || char}bar`, "*": "match" } + { param: `foo${decodedChar}bar`, "*": "match" } ); } }); @@ -738,16 +764,11 @@ describe("special character tests", () => { if (char === "*") { continue; } - await assertRouteMatch( - decodedChar || char, - `/${char}`, - "Matched Root", - { - pathname: `/${pathChar}`, - search: "", - hash: "", - } - ); + await assertRouteMatch(decodedChar, `/${char}`, "Matched Root", { + pathname: `/${pathChar}`, + search: "", + hash: "", + }); } }); @@ -759,7 +780,7 @@ describe("special character tests", () => { continue; } await assertRouteMatch( - decodedChar || char, + decodedChar, `/nested/${char}`, "Matched Static Nested", { @@ -779,7 +800,7 @@ describe("special character tests", () => { continue; } await assertRouteMatch( - decodedChar || char, + decodedChar, `/foo/${char}`, "Matched Param Nested", { From c78d46463f74362e54e3d5ad311865fdbe3af639 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 17 Jan 2024 14:19:44 -0500 Subject: [PATCH 6/9] Add changeset --- .changeset/fifty-pugs-boil.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/fifty-pugs-boil.md diff --git a/.changeset/fifty-pugs-boil.md b/.changeset/fifty-pugs-boil.md new file mode 100644 index 0000000000..59d5dff2f3 --- /dev/null +++ b/.changeset/fifty-pugs-boil.md @@ -0,0 +1,6 @@ +--- +"react-router": patch +"@remix-run/router": patch +--- + +Fix encoding/decoding issues with pre-encoded dynamic parameter values From e5c75fbeb3f3d0c08b041fe959c5732025f1fec6 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 17 Jan 2024 14:34:53 -0500 Subject: [PATCH 7/9] Remove unused function --- packages/router/utils.ts | 35 +++++++++-------------------------- 1 file changed, 9 insertions(+), 26 deletions(-) diff --git a/packages/router/utils.ts b/packages/router/utils.ts index 0d2024d7cb..4bb9f48e1d 100644 --- a/packages/router/utils.ts +++ b/packages/router/utils.ts @@ -485,16 +485,14 @@ export function matchRoutes< let matches = null; for (let i = 0; matches == null && i < branches.length; ++i) { - matches = matchRouteBranch( - branches[i], - // 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) - ); + // 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. + let decoded = decodePath(pathname); + matches = matchRouteBranch(branches[i], decoded); } return matches; @@ -1002,7 +1000,7 @@ function compilePath( return [matcher, params]; } -function safelyDecodeURI(value: string) { +function decodePath(value: string) { try { return value .split("/") @@ -1020,21 +1018,6 @@ function safelyDecodeURI(value: string) { } } -function safelyDecodeURIComponent(value: string, paramName: string) { - try { - return decodeURIComponent(value); - } catch (error) { - warning( - false, - `The value for the URL param "${paramName}" will not be decoded because` + - ` the string "${value}" is a malformed URL segment. This is probably` + - ` due to a bad percent encoding (${error}).` - ); - - return value; - } -} - /** * @private */ From d90c8fb325e08835d20b416fd89a7756916e7d97 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 17 Jan 2024 14:39:20 -0500 Subject: [PATCH 8/9] Bump bundle --- package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 5b8d5f54c1..3143bb0aa2 100644 --- a/package.json +++ b/package.json @@ -119,10 +119,10 @@ "none": "50.4 kB" }, "packages/react-router/dist/react-router.production.min.js": { - "none": "14.7 kB" + "none": "14.73 kB" }, "packages/react-router/dist/umd/react-router.production.min.js": { - "none": "17.1 kB" + "none": "17.2 kB" }, "packages/react-router-dom/dist/react-router-dom.production.min.js": { "none": "16.9 kB" From 114cf0b75df612104a42390291b7c7e707d8820b Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Thu, 25 Jan 2024 11:57:08 -0500 Subject: [PATCH 9/9] Avoid losing trailing spaces on re-encoding for descendant routes --- .../lib/components.tsx | 4 + .../__tests__/special-characters-test.tsx | 187 +++++++++--------- packages/react-router-dom/server.tsx | 4 + packages/router/history.ts | 4 + 4 files changed, 101 insertions(+), 98 deletions(-) diff --git a/packages/react-router-dom-v5-compat/lib/components.tsx b/packages/react-router-dom-v5-compat/lib/components.tsx index 3f68eb2d05..cdf5e7eb1b 100644 --- a/packages/react-router-dom-v5-compat/lib/components.tsx +++ b/packages/react-router-dom-v5-compat/lib/components.tsx @@ -96,6 +96,10 @@ export function StaticRouter({ }, encodeLocation(to: To) { 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"); let encoded = ABSOLUTE_URL_REGEX.test(href) ? new URL(href) : new URL(href, "http://localhost"); diff --git a/packages/react-router-dom/__tests__/special-characters-test.tsx b/packages/react-router-dom/__tests__/special-characters-test.tsx index a8eeaf7d4b..ce0dfcb2c7 100644 --- a/packages/react-router-dom/__tests__/special-characters-test.tsx +++ b/packages/react-router-dom/__tests__/special-characters-test.tsx @@ -26,6 +26,7 @@ import { useNavigate, useParams, } from "react-router-dom"; +import getHtml from "../../react-router/__tests__/utils/getHtml"; /** * Here's all the special characters we want to test against. This list was @@ -52,6 +53,7 @@ import { * } */ +// prettier-ignore let specialChars = [ // This set of characters never gets encoded by window.location { char: "x", pathChar: "x", searchChar: "x", hashChar: "x", decodedChar: "x" }, @@ -90,83 +92,17 @@ let specialChars = [ { char: ">", pathChar: "%3E", searchChar: "%3E", hashChar: "%3E", decodedChar: ">" }, // These chars get encoded in all portions of the URL - { - char: "🤯", - pathChar: "%F0%9F%A4%AF", - searchChar: "%F0%9F%A4%AF", - hashChar: "%F0%9F%A4%AF", - decodedChar: "🤯", - }, - { - char: "✅", - pathChar: "%E2%9C%85", - searchChar: "%E2%9C%85", - hashChar: "%E2%9C%85", - decodedChar: "✅", - }, - { - char: "🔥", - pathChar: "%F0%9F%94%A5", - searchChar: "%F0%9F%94%A5", - hashChar: "%F0%9F%94%A5", - decodedChar: "🔥", - }, - { - char: "ä", - pathChar: "%C3%A4", - searchChar: "%C3%A4", - hashChar: "%C3%A4", - decodedChar: "ä", - }, - { - char: "Ä", - pathChar: "%C3%84", - searchChar: "%C3%84", - hashChar: "%C3%84", - decodedChar: "Ä", - }, - { - char: "ø", - pathChar: "%C3%B8", - searchChar: "%C3%B8", - hashChar: "%C3%B8", - decodedChar: "ø", - }, - { - char: "山", - pathChar: "%E5%B1%B1", - searchChar: "%E5%B1%B1", - hashChar: "%E5%B1%B1", - decodedChar: "山", - }, - { - char: "人", - pathChar: "%E4%BA%BA", - searchChar: "%E4%BA%BA", - hashChar: "%E4%BA%BA", - decodedChar: "人", - }, - { - char: "口", - pathChar: "%E5%8F%A3", - searchChar: "%E5%8F%A3", - hashChar: "%E5%8F%A3", - decodedChar: "口", - }, - { - char: "刀", - pathChar: "%E5%88%80", - searchChar: "%E5%88%80", - hashChar: "%E5%88%80", - decodedChar: "刀", - }, - { - char: "木", - pathChar: "%E6%9C%A8", - searchChar: "%E6%9C%A8", - hashChar: "%E6%9C%A8", - decodedChar: "木", - }, + { char: "🤯", pathChar: "%F0%9F%A4%AF", searchChar: "%F0%9F%A4%AF", hashChar: "%F0%9F%A4%AF", decodedChar: "🤯" }, + { char: "✅", pathChar: "%E2%9C%85", searchChar: "%E2%9C%85", hashChar: "%E2%9C%85", decodedChar: "✅" }, + { char: "🔥", pathChar: "%F0%9F%94%A5", searchChar: "%F0%9F%94%A5", hashChar: "%F0%9F%94%A5", decodedChar: "🔥" }, + { char: "ä", pathChar: "%C3%A4", searchChar: "%C3%A4", hashChar: "%C3%A4", decodedChar: "ä" }, + { char: "Ä", pathChar: "%C3%84", searchChar: "%C3%84", hashChar: "%C3%84", decodedChar: "Ä" }, + { char: "ø", pathChar: "%C3%B8", searchChar: "%C3%B8", hashChar: "%C3%B8", decodedChar: "ø" }, + { char: "山", pathChar: "%E5%B1%B1", searchChar: "%E5%B1%B1", hashChar: "%E5%B1%B1", decodedChar: "山" }, + { char: "人", pathChar: "%E4%BA%BA", searchChar: "%E4%BA%BA", hashChar: "%E4%BA%BA", decodedChar: "人" }, + { char: "口", pathChar: "%E5%8F%A3", searchChar: "%E5%8F%A3", hashChar: "%E5%8F%A3", decodedChar: "口" }, + { char: "刀", pathChar: "%E5%88%80", searchChar: "%E5%88%80", hashChar: "%E5%88%80", decodedChar: "刀" }, + { char: "木", pathChar: "%E6%9C%A8", searchChar: "%E6%9C%A8", hashChar: "%E6%9C%A8", decodedChar: "木" }, // Add a few multi-char space use cases for good measure { char: "a b", pathChar: "a%20b", searchChar: "a%20b", hashChar: "a%20b", decodedChar: "a b" }, @@ -177,27 +113,9 @@ let specialChars = [ // we provide a `decodedChar` so we can assert the param value gets decoded // properly and so we can ensure we can match these decoded values in static // paths - { - char: "a%25b", - pathChar: "a%25b", - searchChar: "a%25b", - hashChar: "a%25b", - decodedChar: "a%b", - }, - { - char: "a%23b%25c", - pathChar: "a%23b%25c", - searchChar: "a%23b%25c", - hashChar: "a%23b%25c", - decodedChar: "a#b%c", - }, - { - char: "a%26b%25c", - pathChar: "a%26b%25c", - searchChar: "a%26b%25c", - hashChar: "a%26b%25c", - decodedChar: "a&b%c", - }, + { char: "a%25b", pathChar: "a%25b", searchChar: "a%25b", hashChar: "a%25b", decodedChar: "a%b" }, + { char: "a%23b%25c", pathChar: "a%23b%25c", searchChar: "a%23b%25c", hashChar: "a%23b%25c", decodedChar: "a#b%c" }, + { char: "a%26b%25c", pathChar: "a%26b%25c", searchChar: "a%26b%25c", hashChar: "a%26b%25c", decodedChar: "a&b%c" }, ]; describe("special character tests", () => { @@ -609,6 +527,79 @@ describe("special character tests", () => { }); } }); + + it("does not trim trailing spaces on ancestor splat route segments", async () => { + let ctx = render( + + + + ); + + expect(getHtml(ctx.container)).toMatchInlineSnapshot(` + "" + `); + + await fireEvent.click(screen.getByText("Link to grandchild")); + await waitFor(() => screen.getByText("Grandchild")); + + expect(getHtml(ctx.container)).toMatchInlineSnapshot(` + "
+ + Link to grandchild + +

+ Grandchild +

+ 
+            {"*":"grandchild","param":"  param  "}
+
+
" + `); + + function App() { + return ( + + } /> + + ); + } + + function Parent() { + return ( + + } /> + + ); + } + + function Child() { + return ( + <> + Link to grandchild + + } /> + + + ); + } + + function Grandchild() { + return ( + <> +

Grandchild

+ 
{JSON.stringify(useParams())}
+ + ); + } + }); }); describe("when matching as part of the defined route path", () => { diff --git a/packages/react-router-dom/server.tsx b/packages/react-router-dom/server.tsx index 6b75be9ed9..0e07d98d35 100644 --- a/packages/react-router-dom/server.tsx +++ b/packages/react-router-dom/server.tsx @@ -389,6 +389,10 @@ function createHref(to: To) { function encodeLocation(to: To): Path { 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"); let encoded = ABSOLUTE_URL_REGEX.test(href) ? new URL(href) : new URL(href, "http://localhost"); diff --git a/packages/router/history.ts b/packages/router/history.ts index ad4bd44b09..335645db1c 100644 --- a/packages/router/history.ts +++ b/packages/router/history.ts @@ -690,6 +690,10 @@ function getUrlBasedHistory( : 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}`