Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fix-pathname-allow-encoded-reserved-after-25.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Fixes dynamic route handlers returning 400 for request paths that contain a literal `%` next to a reserved character, such as the output of `encodeURIComponent('%?.pdf')` (`%25%3F.pdf`). Multi-level encoding detection is now scoped to the pre-decode signature `%25` followed by a hex pair, so legitimate `%25%XX` patterns are no longer rejected. Double-encoded paths like `/api/%2561dmin` still return 400 as before.
27 changes: 12 additions & 15 deletions packages/astro/src/core/util/pathname.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,24 +21,21 @@ export class MultiLevelEncodingError extends Error {
* @throws Error if the pathname contains invalid URL encoding
*/
export function validateAndDecodePathname(pathname: string): string {
let decoded: string;
// Multi-level encoding signature: `%25` (encoded `%`) followed by a hex pair.
// After one decode pass, `%25XY` becomes `%XY`, which a downstream decoder
// could turn into the original byte — bypassing middleware checks against
// the final-decoded value (e.g. `%2561dmin` -> `%61dmin` -> `admin`).
//
// A `%25` followed by a non-hex character — including another `%` (as in
// `%25%3F` from `encodeURIComponent('%?')`) — is a legitimate literal `%`
// next to other encoded content, not multi-level encoding.
if (/%25[0-9a-fA-F]{2}/.test(pathname)) {
throw new MultiLevelEncodingError();
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Post-decode check added with the same regex applied to decoded (fac63c7).
  2. Test case /api/%25%32%3561dmin -> 400 added in the same commit (fac63c7).


try {
decoded = decodeURI(pathname);
return decodeURI(pathname);
} catch (_e) {
throw new Error('Invalid URL encoding');
}

// Check if the decoded path is different from the original
// AND still contains URL-encoded sequences.
// This indicates the original had encoding that got partially decoded, suggesting double encoding.
// Example: /%2561dmin -> decodeURI -> /%61dmin (different AND still has %)
const hasDecoding = decoded !== pathname;
const decodedStillHasEncoding = /%[0-9a-fA-F]{2}/.test(decoded);

if (hasDecoding && decodedStillHasEncoding) {
throw new MultiLevelEncodingError();
}

return decoded;
}
14 changes: 14 additions & 0 deletions packages/astro/test/units/app/double-encoding-bypass.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,4 +157,18 @@ describe('URL normalization: double-encoding middleware bypass', () => {
const body = await response.json();
assert.equal(body.path, 'users/list');
});

it('accepts encodeURIComponent output with a literal % next to a reserved char', async () => {
// encodeURIComponent('%?.pdf') -> '%25%3F.pdf'. After decodeURI, %25 -> %
// and %3F stays (reserved), yielding '%%3F.pdf'. The pre-decode pattern
// %25%3F is not multi-level encoding (the byte after %25 is `%`, not hex),
// so it must reach the handler instead of being rejected as a bad request.
const app = createApp(createAuthMiddleware());
const filename = encodeURIComponent('%?.pdf');
const request = new Request(`http://example.com/api/uploads/${filename}`);
const response = await app.render(request);
assert.equal(response.status, 200, `/api/uploads/${filename} should be accessible`);
const body = await response.json();
assert.equal(body.path, 'uploads/%%3F.pdf');
});
});
Loading