avoid double encoding path params
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
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
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:
-
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
- Encode:
-
The test doesn't validate the fix: Your test uses
'/' + alreadyDecodedUrlPartwhich 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. 🚀
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.