fix(core): allow encoded reserved chars after a literal % in pathnames#16799
Closed
truffle-dev wants to merge 1 commit into
Closed
fix(core): allow encoded reserved chars after a literal % in pathnames#16799truffle-dev wants to merge 1 commit into
truffle-dev wants to merge 1 commit into
Conversation
…hastro#16781) `validateAndDecodePathname` rejected any pathname where decoding produced a different string that still contained `%XX` sequences. That conflated two distinct shapes: /api/%2561dmin -> %25 + 61 (hex pair) -> decodes to %61dmin /uploads/%25%3F -> %25 + %3F (reserved) -> decodes to %%3F Only the first is multi-level encoding. The second is `encodeURIComponent` emitting an encoded literal `%` next to an encoded reserved character, which is a normal output of e.g. `encodeURIComponent('%?.pdf')`. After v6.3.2 (withastro#16556) any dynamic route handling such a path returned 400. Detect multi-level encoding from the pre-decode signature directly: `%25` followed by two hex digits. A `%25` followed by a non-hex byte (`%` or any literal char) is a legitimate encoded `%`, not double encoding. Drops the `decoded !== pathname` second pass; the regex on the input is sufficient and avoids the false positive. Existing double-encoding-bypass coverage stays green: - `/api/%2561dmin` -> 400 (still) - `/api/%2561dmin/%75sers` -> 400 (still) - `/api/%61dmin` -> 401 via middleware (still) - `/api/us%65rs/list` -> 200 (still) Adds one regression test for the reporter's case: `/api/uploads/${encodeURIComponent('%?.pdf')}` reaches the handler with `params.path === 'uploads/%%3F.pdf'`.
🦋 Changeset detectedLatest commit: 4e06318 The changes in this PR will be included in the next version bump. This PR includes changesets to release 417 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
Bot
added
the
pkg: astro
ematipico
requested changes
May 20, 2026
Member
ematipico
left a comment
ematipico
left a comment
There was a problem hiding this comment.
Thank you. The code is sound, but we can tight the checks
| // next to other encoded content, not multi-level encoding. | ||
| if (/%25[0-9a-fA-F]{2}/.test(pathname)) { | ||
| throw new MultiLevelEncodingError(); | ||
| } |
Member
There was a problem hiding this comment.
The regex is sound, however there's still a small gap that we need to close, so I suggest to following code:
export function validateAndDecodePathname(pathname: string): string {
if (/%25[0-9a-fA-F]{2}/.test(pathname)) {
throw new MultiLevelEncodingError();
}
let decoded: string;
try {
decoded = decodeURI(pathname);
} catch (_e) {
throw new Error('Invalid URL encoding');
}
// Defense-in-depth: catch creative encodings that reassemble
// into %25HH after the first decode pass
if (/%25[0-9a-fA-F]{2}/.test(decoded)) {
throw new MultiLevelEncodingError();
}
return decoded;
}And add against %25%32%3561dmin
Author
Member
|
Closing, as this is a bot. I'll fix it |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #16781.
validateAndDecodePathnamerejected any pathname where decoding produced a different string that still contained%XXsequences. That conflated two distinct shapes:/api/%2561dmin—%25followed by61(hex pair); decodes to%61dminwhich can be further decoded toadmin. Real multi-level encoding./uploads/%25%3F.pdf—%25followed by%3F(an encoded reserved char); decodes to%%3F.pdf. JustencodeURIComponentemitting an encoded literal%next to an encoded?.After v6.3.2 (#16556), any dynamic route receiving the second shape returned 400.
encodeURIComponent('%?.pdf')produces it, so requests like/uploads/${encodeURIComponent('%?.pdf')}stopped reaching their handler.Detect multi-level encoding from the pre-decode signature directly:
%25followed by two hex digits. A%25followed by a non-hex byte — another%, a literal letter outside[a-f], end-of-string — is a legitimate encoded%, not double encoding. Removes thedecoded !== pathnamesecond pass; the regex against the input is sufficient.Test coverage
Existing double-encoding-bypass cases still behave as before:
/api/admin/users/api/%61dmin/usersadmin)/api/%2561dmin/users/api/%2561dmin/%75sers/api/public/data/api/us%65rs/listAdds one regression test for the reporter's case:
/api/uploads/${encodeURIComponent('%?.pdf')}uploads/%%3F.pdfAll eight tests in
packages/astro/test/units/app/double-encoding-bypass.test.tspass locally.