next.js icon indicating copy to clipboard operation
next.js copied to clipboard

avoid double encoding path params

Open therungg opened this issue 2 weeks ago • 1 comments

What?

This encodes and re-decodes the path param before throwing an error from decodeURIComponent.

Why?

More often than not, the error is thrown when a string given to decodeURIComponent has already been decoded. This means that a string like %25 will already be decoded to %, which results in an error from decodeURIComponent. This is also the case for this function. The parameter given is already encoded once, so this function will fail with %25. This PR fixes that by applying the standard solution to this problem: re-encoding, then re-decoding.

If decodeURIComponent still throws an error, the same DecodeError as before is thrown, because that means something different is the culprit.

How?

By re-encoding, then re-decoding the string.

Closes NEXT- Fixes #86957

therungg avatar Dec 09 '25 22:12 therungg

Allow CI Workflow Run

  • [ ] approve CI run for commit: 5bd1b00b9901ce27347ba88871770b6957bf0bcd

Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer

nextjs-bot avatar Dec 09 '25 22:12 nextjs-bot

Interesting approach to the double-encoding issue! 🤔

Problem: When a URL parameter is already decoded (like % from %25), calling decodeURIComponent() again throws an error.

Your Solution: Re-encode the param first, then decode it.

However, I have concerns:

  1. This creates actual double-encoding: If the param was hello%20world (already encoded), your code would:

    • Encode: hello%2520world
    • Decode: hello%20world
    • Result: Still encoded, not the expected hello world
  2. The test doesn't validate the fix: Your test uses '/' + alreadyDecodedUrlPart which is '/%', but the route matcher receives this as a string, not a URL-encoded value. The test should use '/%25' to simulate the actual scenario.

Better Solution:

try {
  return decodeURIComponent(param)
} catch {
  // Param is already decoded or invalid
  return param
}

This handles both cases:

  • Already decoded params: returns as-is
  • Encoded params: decodes properly
  • Invalid params: returns as-is (graceful degradation)

Test Improvement:

const result = routeMatcher('/%25') // Encoded %
expect(result).toEqual({ user: '%' }) // Decoded result

The intent is good, but the implementation needs adjustment to avoid creating new encoding issues. 🚀

harikapadia999 avatar Dec 14 '25 04:12 harikapadia999

I think this is a fully AI-generated response, because pretty much all of the above is incorrect. It only encodes when the initial decoding fails, so there's never double encoding. The test is correct, because I want to test if the routeMatcher handles the already-encoded /% correctly.

therungg avatar Dec 14 '25 04:12 therungg