Fix issues with pre-encoded param names not being properly decoded #11199
Conversation
🦋 Changeset detectedLatest commit: 114cf0b The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| { char: "]", pathChar: "]", searchChar: "]", hashChar: "]", decodedChar: "]" }, | ||
| { char: ":", pathChar: ":", searchChar: ":", hashChar: ":", decodedChar: ":" }, | ||
| { char: ";", pathChar: ";", searchChar: ";", hashChar: ";", decodedChar: ";" }, | ||
| { char: ",", pathChar: ",", searchChar: ",", hashChar: ",", decodedChar: "," }, |
There was a problem hiding this comment.
Added the decodedChar to all existing setups - even though it's the same as the char since we now assert matched param values against decodedChar
| searchChar: "a%26b%25c", | ||
| hashChar: "a%26b%25c", | ||
| decodedChar: "a&b%c", | ||
| }, |
There was a problem hiding this comment.
These are the net new tests for the bugs being reported, if you pre-encode because that portion of the URL has URL-sensitive characters - you weren't previously getting back the proper decoded value. These are the only tests right now that have a different char versus decodedChar
| @@ -342,7 +404,7 @@ describe("special character tests", () => { | |||
| search: "", | |||
| hash: "", | |||
| }, | |||
| { slug: char } | |||
| { slug: decodedChar } | |||
There was a problem hiding this comment.
Assert params against decodedChar now
| let parentSegments = parentPathnameBase.replace(/^\//, "").split("/"); | ||
| let segments = pathname.replace(/^\//, "").split("/"); | ||
| remainingPathname = "/" + segments.slice(parentSegments.length).join("/"); | ||
| } |
There was a problem hiding this comment.
Needed to ensure we can match down into descendant routes when we get odd encodings coming in
| @@ -930,7 +928,7 @@ export function matchPath< | |||
| if (isOptional && !value) { | |||
| memo[paramName] = undefined; | |||
| } else { | |||
| memo[paramName] = safelyDecodeURIComponent(value || "", paramName); | |||
| memo[paramName] = (value || "").replace(/%2F/g, "/"); | |||
There was a problem hiding this comment.
We don't need to call decodeURIComponent on param values here anymore since we matched against a slash-delimited path of segments that already went through decodeURIComponent
| return value | ||
| .split("/") | ||
| .map((v) => decodeURIComponent(v).replace(/\//g, "%2F")) | ||
| .join("/"); |
There was a problem hiding this comment.
decodeURI here is a bit too aggressive since it assumes a full URI which may have query params and hash values in it. Since we're only operating on a pathname, we should instead decode each component individually in case they have pre-encoded hashes, percent signs, etc. in them.
However, if there was an encoded slash - preserve that as %2F since we can't introduce new segments via this decoding
|
@brophdawg11 It would be awesome to merge this, is it waiting for something specific? |
|
Mostly just giving it a little time to see if other folks would be able to pull it down and confirm it fixes their issues. You and one other user have confirmed it solves it so that's probably all we're going to get so I'll get this merged 👍 |
|
🤖 Hello there, We just published version Thanks! |
|
🤖 Hello there, We just published version Thanks! |
Handle edge cases of pre-ended param values not being properly decoded correctly due to differences in
decodeURI/decodeURIComponent/new URL()Supersedes #11138
Closes #10814