Revert "fix: validate path parameters and prevent traversal/injection in REST transcoder" - #9165
Conversation
… in REST…" This reverts commit 0152a8e.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request removes URI path validation logic and reverts the URL encoding functions (encodeWithSlashes and encodeWithoutSlashes) to a character-by-character split and map implementation, while also deleting associated unit tests. Feedback on this PR highlights a critical issue with this change: splitting strings character-by-character using .split('') splits Unicode surrogate pairs (such as emojis) into unpaired halves, which will cause encodeURIComponent to throw a URIError: URI malformed at runtime. It is recommended to restore the previous robust implementation that safely handles surrogate pairs.
| export function encodeWithSlashes(str: string): string { | ||
| return encodeURIComponent(str).replace( | ||
| /[!'()*]/g, // Characters preserved by encodeURIComponent | ||
| character => '%' + character.charCodeAt(0).toString(16).toUpperCase() | ||
| ); | ||
| return str | ||
| .split('') | ||
| .map(c => (c.match(/[-_.~0-9a-zA-Z]/) ? c : encodeURIComponent(c))) | ||
| .join(''); | ||
| } | ||
|
|
||
| /** | ||
| * Percent-encodes a string according to RFC 3986, preserving unreserved | ||
| * characters (alpha-numeric, '-', '_', '.', and '~') and slashes ('/'). All other | ||
| * characters are percent-encoded. | ||
| * | ||
| * @param {string} str - The input string to encode. | ||
| * @returns {string} The percent-encoded string with slashes preserved. | ||
| */ | ||
| export function encodeWithoutSlashes(str: string): string { | ||
| return str.split('/').map(encodeWithSlashes).join('/'); | ||
| return str | ||
| .split('') | ||
| .map(c => (c.match(/[-_.~0-9a-zA-Z/]/) ? c : encodeURIComponent(c))) | ||
| .join(''); | ||
| } |
There was a problem hiding this comment.
Reverting these functions to use .split('') and mapping over individual characters with encodeURIComponent re-introduces a critical bug. When a string contains Unicode surrogate pairs (such as emojis or non-BMP characters), .split('') splits them into unpaired surrogate halves. Passing an unpaired surrogate to encodeURIComponent throws a URIError: URI malformed at runtime.
We should keep the robust, standard-compliant implementation that uses encodeURIComponent on the whole string and then replaces the preserved characters, which safely handles surrogate pairs.
export function encodeWithSlashes(str: string): string {
return encodeURIComponent(str).replace(
/[!'()*]/g,
character => '%' + character.charCodeAt(0).toString(16).toUpperCase()
);
}
export function encodeWithoutSlashes(str: string): string {
return str.split('/').map(encodeWithSlashes).join('/');
}There was a problem hiding this comment.
That's fine. We are going to reintroduce these changes again to solve the vulnerability.
2 participants
Reverts #9151 to revert changes in the generated libraries in order to get a clean librarian build.