[Bug]: params encoded with encodeURIComponent incorrectly decoded if they contain % and other special characters

[dstaver]

What version of React Router are you using?

6.15.0

Steps to Reproduce

1. With the route /web/:param and the string a#b%c
2. Encode the string with encodeURIComponent and navigate to the path /web/a%23b%25c
3. Get the param with useParams()
4. The value should match the original string, instead it becomes a%23b%c - A partially decoded version of the string

Expected Behavior

Any segment in a pathname correctly encoded with encodeURIComponent should be decoded back the same way

Actual Behavior

The param is decoded twice:

1. First with safelyDecodeURI(pathname) in matchRoutes - This results in a partially decoded value since decodeURI decodes less characters than decodeURIComponent
2. Then the partially decoded value is decoded again with safelyDecodeURIComponent in matchPath causing it to fail as the url segment is malformed because of the previous decoding.

[mmrzena]

We were facing the same issue, here is how we solved it:

1. Patched the package script - we completely removed safelyDecodeURI and it's using undecoded value. From my understanding it's used there because of special characters possibly being used in route paths -> not our use-case, we use only english words in defining route paths. (for patching we use patch-package)
2. We make sure every parametr we use for navigation is encoded by encodeURIComponent method.
3. React-router gives you decoded parameters with decodeURIComponent method in safelyDecodeURIComponent.

Nothing breaks and everything works like we want it.

[labkey-alan]

As I outlined in the duplicate issue that I opened, I believe this can be fixed by using decodeURIComponent instead of decodeURI in matchRoutes.

[brophdawg11]

I've got a PR up to fix this in in #11199 but since path matching and encoding is such a tricky subject I'd love for a few folks to help alpha test the new approach 🙌

I cut an experimental release with those changes - if anyone (OP or other) could test their specific scenario and let me know if it's fixed that would be super helpful in knowing we've got the right fix.

npm install react-router-dom@0.0.0-experimental-114cf0b7

# Outdated experimental (see below comments)
# npm install react-router-dom@0.0.0-experimental-d90c8fb3

I've also added the OP issue and the issue reported in #11179 to this code sandbox using the experimental release and things look good there: https://codesandbox.io/p/sandbox/billowing-fast-hv6v4v?file=%2Fsrc%2Findex.js%3A11%2C18

[RobinClowers]

@brophdawg11 This does appear to fix the route matching issue, however the useParams hook appears to be stripping the trailing newline as well, which breaks some of my other routing.

[brophdawg11]

@RobinClowers Thanks for confirming! And yeah I would expect newlines to still have issues - we can keep #11146 for issues related to newlines 👍

[RobinClowers]

Oh sorry, I meant to say stripping whitespace, not newlines. My test case was a path segment with spaces on either side of it, and it appears that something is trimming off the trailing whitespace, which breaks my routing.

[brophdawg11]

@RobinClowers Could you fork and add your scenario to this sandbox (which is using the above experimental version)? It looks like spaces on both sides is OK so I might not have the right reproduction: https://codesandbox.io/p/sandbox/zealous-dawn-q8xwyt?file=%2Fsrc%2Findex.js%3A44%2C27

[RobinClowers]

Sure thing: https://codesandbox.io/p/sandbox/mutable-framework-4l8645.

It looks like it's Links with relative paths under a splat that have the issue.

[brophdawg11]

Ah ok thank you for the repro 🙌 . I just pushed a fix for that in #11199 and released a new experimental: 0.0.0-experimental-114cf0b7 that fixes the code sandbox you provided

[RobinClowers]

@brophdawg11 thanks again for being so responsive here, the latest fix resolves all my issues other than the newline issue!

[brophdawg11]

Fixed via #11199 - will be included in the next release

[github-actions]

🤖 Hello there,

We just published version 6.22.1-pre.0 which involves this issue. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

[RobinClowers]

Just tried out the pre-release, looks good!

[github-actions]

🤖 Hello there,

We just published version 6.22.1 which involves this issue. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

[marieletellier]

Hi, would it be possible to have the v5 version updated with this fix as well?

[vikrantgupta25]

Any updates on the fix being added to the router v5 ?

[brophdawg11]

AFAIK, we're no longer making changes to v5 (short of critical security vulnerabilities). Is there anything in particular keeping you from upgrading to v6?