avoid double encoding path params - #87003
Conversation
|
Allow CI Workflow Run
Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer |
212d508 to
3a0cf26
Compare
…ouble-encoding-path-params
…erungg/next.js into fix/avoid-double-encoding-path-params
|
Interesting approach to the double-encoding issue! 🤔 Problem: When a URL parameter is already decoded (like Your Solution: Re-encode the param first, then decode it. However, I have concerns:
Better Solution: try {
return decodeURIComponent(param)
} catch {
// Param is already decoded or invalid
return param
}This handles both cases:
Test Improvement: const result = routeMatcher('/%25') // Encoded %
expect(result).toEqual({ user: '%' }) // Decoded resultThe 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 |
|
Hi, Is anyone able to look at this? It's a crucial bug for my app. Looking forward to getting this merged. Do let me know if there's anything I can change. Thanks! |
|
@timneutkens any chance that this will get merged? |
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
decodeURIComponenthas already been decoded. This means that a string like%25will already be decoded to%, which results in an error fromdecodeURIComponent. 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
decodeURIComponentstill throws an error, the sameDecodeErroras before is thrown, because that means something different is the culprit.How?
By re-encoding, then re-decoding the string.
Closes NEXT-
Fixes #86957