Skip to content

Commit 131a2ea

Browse files
Brooooooklynclaude
andauthored
fix(dispatch): serve static assets with percent-encoded filenames (#225)
* fix(dispatch): serve static assets with percent-encoded filenames Static-asset serving-manifest keys are the decoded on-disk path (a file named `oxc masthead.riv` is keyed with a literal space), but dispatch looked them up with `url.pathname`, which stays percent-encoded (`%20`). Any filename containing a space / `#` / non-ASCII char returned 404 — e.g. the oxc.rs hero `.riv`. Confirmed against the live prod manifest: the key held a literal space and the bytes were present. Decode the pathname before the manifest lookup at both sites (`resolveStaticPath` and the hashed-asset fast path), trying the raw key first then the decoded key. Serve-side, so it fixes already-deployed sites with no redeploy. Decoding is segment-safe: it is rejected when it would change the `/`-segment count, so an encoded slash (`%2F`) can never alias `/assets%2Fx` onto `/assets/x` and slip past the `/__void/*` block, the hashed-prefix gate, or the header/redirect rules (all keyed on the raw pathname). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(dispatch): reject over-encoded aliases in the decoded asset lookup Addresses the P1 review comment. The prior segment-count guard only blocked encoded slashes (`%2F`); it still let over-encoded ordinary path text through — `/%61ssets/app.js` decoded to `/assets/app.js` and `/%5F%5Fvoid/secret.js` to `/__void/secret.js`, serving the manifest entry while the raw-`url.pathname` policy gates (the `/__void/*` reserved-route block, the hashed-asset prefix gate, and the header/redirect rules) never fired. Replace the segment-count check with a canonical round-trip: accept the decoded key only when re-encoding it via the WHATWG URL pathname setter reproduces the exact request path. That admits only the encodings a conformant URL must use (space, `#`, non-ASCII) and rejects every over-encoded spelling of ordinary text (`%2F`, `%61`, `%5F`, `%2e%2e`) a browser would never send. This subsumes the old encoded-slash guard. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(dispatch): accept lower-case escapes and re-encode decoded redirects Addresses two P2 review comments on the canonical decode guard: - Exact string equality was too strict: it rejected valid lower-case percent escapes (the URL encoder emits upper-case, so `%c3%a9` != `%C3%A9`). Compare after normalizing escape letter case, so a lower-case-encoded request for a non-ASCII filename still resolves. (A literal `%` in a filename remains unsupported — the round-trip can't reproduce the double-escaped request; it 404'd before this change too, so it is not a regression.) - A redirect Location produced from a decoded match was emitted decoded, e.g. `force-trailing-slash` on `/docs/a%23b` (backed by `/docs/a#b/index.html`) returned `Location: /docs/a#b/`, which a browser reads as fragment `b/`. Serialize the Location back to a safe URL pathname (`/docs/a%23b/`). Factor the WHATWG path re-encoder (`URL.pathname` setter) into a shared helper used by both the decode guard and the redirect fix. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 18a79ad commit 131a2ea

4 files changed

Lines changed: 258 additions & 9 deletions

File tree

‎packages/dispatch/src/index.ts‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import {
1818
} from './isr.ts';
1919
import { recordRequest, sendUsageEvent, type RequestType } from './metering.ts';
2020
import { applyRoutingRules, mergeQueryIntoLocation, type RedirectRule } from './routing-rules.ts';
21-
import { resolveStaticPath, resolveAssetContentType } from './static.ts';
21+
import { resolveStaticPath, resolveAssetContentType, safeDecodePathname } from './static.ts';
2222
import type { AssetConfig } from './static.ts';
2323
import { buildVariantOriginalUrl, type QueryAllowlist } from './variant-key.ts';
2424

@@ -539,9 +539,20 @@ export default {
539539
cacheTtl: 86400,
540540
});
541541
const assetManifest: Record<string, string> = manifestRaw ? JSON.parse(manifestRaw) : {};
542-
const hash = assetManifest[pathname];
542+
// Manifest keys are decoded on-disk paths, but `pathname` stays
543+
// percent-encoded. Try the raw key, then the decoded key (e.g. a
544+
// hashed file named `oxc masthead.riv` requested as `oxc%20masthead.riv`).
545+
let assetPath = pathname;
546+
let hash = assetManifest[pathname];
547+
if (!hash) {
548+
const decoded = safeDecodePathname(pathname);
549+
if (decoded !== pathname && assetManifest[decoded]) {
550+
assetPath = decoded;
551+
hash = assetManifest[decoded];
552+
}
553+
}
543554
const response = hash
544-
? ((await fetchStaticAsset(env, hash, pathname, routing.projectId)) ??
555+
? ((await fetchStaticAsset(env, hash, assetPath, routing.projectId)) ??
545556
createErrorResponse(request, 404, 'Not Found'))
546557
: createErrorResponse(request, 404, 'Not Found');
547558
if (response.ok && !isEventStreamResponse(response)) {

‎packages/dispatch/src/static.ts‎

Lines changed: 111 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,21 +12,88 @@ export type ResolvedAsset = Readonly<{
1212
}>;
1313

1414
/**
15-
* Resolve a request pathname to a manifest entry using AssetConfig rules.
16-
* Replicates Cloudflare Workers Assets path resolution behavior.
15+
* Percent-decode a request pathname for a manifest lookup, honoring ONLY the
16+
* encodings a conformant URL is required to use. Returns the raw value
17+
* unchanged on a malformed sequence (e.g. a lone `%`) or on any non-canonical /
18+
* over-encoded spelling.
19+
*
20+
* Request pathnames arrive percent-encoded — WHATWG `URL.pathname` never
21+
* decodes — but manifest keys are the asset's decoded on-disk path (a file
22+
* named `oxc masthead.riv` is stored under `/assets/oxc masthead.riv`, not
23+
* `%20`). Callers try the raw key first, then this decoded form.
24+
*
25+
* The decoded form is accepted only when re-encoding it reproduces the request
26+
* path (up to percent-escape letter case) — i.e. the only encoded bytes were
27+
* characters a URL MUST encode (space, `#`, non-ASCII, …). This rejects
28+
* over-encoded spellings of ordinary path text (`%2F`→`/`, `%61`→`a`, `%5F`→`_`,
29+
* `%2e%2e`→`..`), which a browser would never send and which would otherwise let
30+
* a request reconstruct a guarded path (`/assets%2Fx`, `/%61ssets/x`,
31+
* `/%5F%5Fvoid/x`) and serve the manifest entry while the raw-`url.pathname`
32+
* policy gates — the `/__void/*` reserved-route block, the hashed-asset prefix
33+
* gate, and the header/redirect rules — never fire. The URL pathname setter
34+
* applies exactly the WHATWG "path percent-encode set", making it the canonical
35+
* re-encoder.
36+
*
37+
* Known limitation: a file whose name contains a literal `%` (e.g. `logo%20.svg`,
38+
* requested as `logo%2520.svg`) is not resolved — the path encoder leaves `%`
39+
* as-is, so the round-trip can't reproduce the double-escaped request. Such
40+
* names 404'd before this change too, so this is not a regression.
1741
*/
18-
export function resolveStaticPath(
42+
export function safeDecodePathname(pathname: string): string {
43+
let decoded: string;
44+
try {
45+
decoded = decodeURIComponent(pathname);
46+
} catch {
47+
return pathname;
48+
}
49+
if (decoded === pathname) {
50+
return pathname;
51+
}
52+
const canonical = reencodePathname(decoded);
53+
if (canonical === null) {
54+
return pathname;
55+
}
56+
// Compare up to percent-escape letter case: the URL encoder emits upper-case
57+
// (`%C3`), but a client may send lower-case (`%c3`), and both are valid.
58+
return normalizePercentCase(canonical) === normalizePercentCase(pathname) ? decoded : pathname;
59+
}
60+
61+
/** Uppercase the hex digits of every `%xx` escape so encodings compare equal
62+
* regardless of the letter case a client used. */
63+
function normalizePercentCase(pathname: string): string {
64+
return pathname.replace(/%[0-9a-fA-F]{2}/g, (escape) => escape.toUpperCase());
65+
}
66+
67+
/**
68+
* Serialize a decoded path back to a safe URL pathname via the WHATWG "path
69+
* percent-encode set" (the `URL.pathname` setter). Returns null if the value
70+
* can't be encoded. Used to canonicalize the decoded lookup key and to make a
71+
* redirect Location URL-safe.
72+
*/
73+
function reencodePathname(pathname: string): string | null {
74+
try {
75+
const encoder = new URL('https://void.invalid/');
76+
encoder.pathname = pathname;
77+
return encoder.pathname;
78+
} catch {
79+
return null;
80+
}
81+
}
82+
83+
/**
84+
* Match a pathname against the manifest via exact match + html_handling rules
85+
* (no not-found fallback). Returns null when nothing matches.
86+
*/
87+
function matchManifestEntry(
1988
pathname: string,
2089
manifest: Record<string, string>,
21-
config: AssetConfig,
90+
htmlHandling: NonNullable<AssetConfig['html_handling']>,
2291
): ResolvedAsset | null {
2392
// Exact match always wins
2493
if (manifest[pathname]) {
2594
return { path: pathname, hash: manifest[pathname] };
2695
}
2796

28-
const htmlHandling = config.html_handling ?? 'auto-trailing-slash';
29-
3097
switch (htmlHandling) {
3198
case 'auto-trailing-slash': {
3299
// Try appending .html first (preferred over /index.html)
@@ -91,6 +158,44 @@ export function resolveStaticPath(
91158
break;
92159
}
93160

161+
return null;
162+
}
163+
164+
/**
165+
* Resolve a request pathname to a manifest entry using AssetConfig rules.
166+
* Replicates Cloudflare Workers Assets path resolution behavior.
167+
*/
168+
export function resolveStaticPath(
169+
pathname: string,
170+
manifest: Record<string, string>,
171+
config: AssetConfig,
172+
): ResolvedAsset | null {
173+
const htmlHandling = config.html_handling ?? 'auto-trailing-slash';
174+
175+
// Try the raw (percent-encoded) path first — the fast path, and it lets a
176+
// filename that literally contains a `%` win. On a miss, retry with the
177+
// decoded path so a file named e.g. `a b.txt` resolves for an `a%20b.txt`
178+
// request (manifest keys are decoded on-disk paths).
179+
let matched = matchManifestEntry(pathname, manifest, htmlHandling);
180+
if (!matched) {
181+
const decoded = safeDecodePathname(pathname);
182+
if (decoded !== pathname) {
183+
matched = matchManifestEntry(decoded, manifest, htmlHandling);
184+
if (matched?.redirect !== undefined) {
185+
// The Location was derived from the decoded path; serialize it back to
186+
// a safe URL pathname (`/docs/a#b/` → `/docs/a%23b/`) so the client
187+
// doesn't read `#`/space/etc. as URL structure.
188+
const safeRedirect = reencodePathname(matched.redirect);
189+
if (safeRedirect !== null) {
190+
matched = { ...matched, redirect: safeRedirect };
191+
}
192+
}
193+
}
194+
}
195+
if (matched) {
196+
return matched;
197+
}
198+
94199
// Not found handling
95200
switch (config.not_found_handling ?? 'none') {
96201
case 'single-page-application':

‎packages/dispatch/test/unit/index.test.ts‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2925,6 +2925,33 @@ describe('static deployment caching (KV/R2)', () => {
29252925
'public, s-maxage=31536000, max-age=0, must-revalidate',
29262926
);
29272927
});
2928+
2929+
it('serves a hashed asset whose filename contains a space (%20-encoded request)', async () => {
2930+
// Regression: the oxc.rs hero `.riv` 404'd. The manifest key is the decoded
2931+
// on-disk path (literal space), but the browser requests it `%20`-encoded and
2932+
// `url.pathname` never decodes. Routing carries `hashedAssetsPrefix`, so the
2933+
// request hits the hashed-asset fast path — which must decode before the
2934+
// manifest lookup.
2935+
const { env } = createStaticDeployEnv({
2936+
routing: {
2937+
workerName: '',
2938+
projectId: 'proj-1',
2939+
staticDeploymentId: 'deploy-abc',
2940+
version: 'deploy-abc',
2941+
hashedAssetsPrefix: 'assets',
2942+
},
2943+
manifest: { '/assets/640_x_630_oxc masthead_.B7dLtkJB.riv': 'hash_riv' },
2944+
assets: { hash_riv: { body: 'RIVE-BYTES', contentType: 'application/octet-stream' } },
2945+
});
2946+
2947+
const res = await app.fetch(
2948+
new Request('https://test.void.app/assets/640_x_630_oxc%20masthead_.B7dLtkJB.riv'),
2949+
env,
2950+
createCtx(),
2951+
);
2952+
expect(res.status).toBe(200);
2953+
expect(await res.text()).toBe('RIVE-BYTES');
2954+
});
29282955
});
29292956

29302957
describe('static asset content-type normalization', () => {

‎packages/dispatch/test/unit/static.test.ts‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,112 @@ describe('resolveStaticPath', () => {
169169
expect(resolveStaticPath('/unknown', manifest, config)).toBeNull();
170170
});
171171
});
172+
173+
describe('percent-encoded request paths', () => {
174+
// Manifest keys are the asset's decoded on-disk path, but `url.pathname`
175+
// arrives percent-encoded (WHATWG URL never decodes it). A filename with a
176+
// space is stored as a literal space yet requested as `%20`, so the raw
177+
// lookup misses. Regression: the oxc.rs hero `.riv` (a file literally named
178+
// `640_x_630_oxc masthead_.B7dLtkJB.riv`) 404'd because `%20` != ' '.
179+
const m: Record<string, string> = {
180+
'/assets/640_x_630_oxc masthead_.B7dLtkJB.riv': 'hash_riv',
181+
'/files/a#b.txt': 'hash_hash',
182+
};
183+
184+
it('resolves a %20-encoded request to a literal-space manifest key', () => {
185+
expect(
186+
resolveStaticPath('/assets/640_x_630_oxc%20masthead_.B7dLtkJB.riv', m, {
187+
html_handling: 'none',
188+
}),
189+
).toEqual({ path: '/assets/640_x_630_oxc masthead_.B7dLtkJB.riv', hash: 'hash_riv' });
190+
});
191+
192+
it('resolves other percent-encoded characters (e.g. %23 for #)', () => {
193+
expect(resolveStaticPath('/files/a%23b.txt', m, { html_handling: 'none' })).toEqual({
194+
path: '/files/a#b.txt',
195+
hash: 'hash_hash',
196+
});
197+
});
198+
199+
it('still prefers an exact raw match over the decoded key', () => {
200+
// A file whose name literally contains "%20" must win on the raw path.
201+
const literal: Record<string, string> = {
202+
'/x%20y.txt': 'raw_hit',
203+
'/x y.txt': 'decoded_hit',
204+
};
205+
expect(resolveStaticPath('/x%20y.txt', literal, { html_handling: 'none' })).toEqual({
206+
path: '/x%20y.txt',
207+
hash: 'raw_hit',
208+
});
209+
});
210+
211+
it('does not throw on a malformed percent-sequence, just misses', () => {
212+
expect(resolveStaticPath('/bad%2', m, { html_handling: 'none' })).toBeNull();
213+
});
214+
215+
it('only honors decodings a conformant URL must encode (canonical-encoding guard)', () => {
216+
// The decoded fallback must accept ONLY percent-encodings that a conformant
217+
// URL is required to use (space, #, UTF-8, …). An over-encoded spelling of
218+
// ordinary path text — %2F for '/', %61 for 'a', %5F for '_', %2e%2e for
219+
// '..' — must be rejected: otherwise a request could reconstruct a guarded
220+
// path (`/assets%2Fapp.js`, `/%61ssets/app.js`, `/%5F%5Fvoid/secret.js`)
221+
// and serve the manifest entry while the raw-`url.pathname` policy gates
222+
// — the `/__void/*` reserved-route block, the hashed-asset prefix gate,
223+
// and the header/redirect rules — never fire.
224+
const guarded: Record<string, string> = {
225+
'/assets/app.js': 'hash_app',
226+
'/__void/secret.js': 'hash_secret',
227+
};
228+
const cfg = { html_handling: 'none' } as const;
229+
// encoded slash (both cases)
230+
expect(resolveStaticPath('/assets%2Fapp.js', guarded, cfg)).toBeNull();
231+
expect(resolveStaticPath('/assets%2fapp.js', guarded, cfg)).toBeNull();
232+
expect(resolveStaticPath('/__void%2Fsecret.js', guarded, cfg)).toBeNull();
233+
// over-encoded ordinary characters
234+
expect(resolveStaticPath('/%61ssets/app.js', guarded, cfg)).toBeNull();
235+
expect(resolveStaticPath('/%5F%5Fvoid/secret.js', guarded, cfg)).toBeNull();
236+
// reconstructed dot-segments
237+
expect(resolveStaticPath('/assets/%2e%2e/app.js', guarded, cfg)).toBeNull();
238+
});
239+
240+
it('resolves a UTF-8 (non-ASCII) filename requested percent-encoded', () => {
241+
const m: Record<string, string> = { '/assets/café.png': 'hash_cafe' };
242+
// "café" → the é is C3 A9 in UTF-8
243+
expect(resolveStaticPath('/assets/caf%C3%A9.png', m, { html_handling: 'none' })).toEqual({
244+
path: '/assets/café.png',
245+
hash: 'hash_cafe',
246+
});
247+
});
248+
249+
it('accepts lower-case percent escapes (browsers emit upper, but both are valid)', () => {
250+
const m: Record<string, string> = { '/assets/café.png': 'hash_cafe' };
251+
expect(resolveStaticPath('/assets/caf%c3%a9.png', m, { html_handling: 'none' })).toEqual({
252+
path: '/assets/café.png',
253+
hash: 'hash_cafe',
254+
});
255+
});
256+
257+
it('re-encodes a redirect Location produced from a decoded match', () => {
258+
// force-trailing-slash: `/docs/a#b` (requested `%23`) is backed by
259+
// `/docs/a#b/index.html`, so it redirects to the trailing-slash form. The
260+
// Location must be URL-safe (`/docs/a%23b/`), not the decoded `/docs/a#b/`
261+
// — otherwise the browser reads `#b/` as a fragment.
262+
const m: Record<string, string> = { '/docs/a#b/index.html': 'hash_idx' };
263+
expect(
264+
resolveStaticPath('/docs/a%23b', m, { html_handling: 'force-trailing-slash' }),
265+
).toEqual({ path: '/docs/a#b/index.html', hash: 'hash_idx', redirect: '/docs/a%23b/' });
266+
});
267+
268+
it('does not support a literal % in a filename (known limitation, not a regression)', () => {
269+
// A file literally named `logo%20.svg` is requested as `logo%2520.svg`.
270+
// decodeURIComponent yields the key, but re-encoding can't reproduce the
271+
// double-escaped request (the URL path encoder leaves `%` alone), so it
272+
// misses — same as before this change. Locked here so the behavior is
273+
// intentional, not an accidental regression.
274+
const m: Record<string, string> = { '/assets/logo%20.svg': 'hash_logo' };
275+
expect(resolveStaticPath('/assets/logo%2520.svg', m, { html_handling: 'none' })).toBeNull();
276+
});
277+
});
172278
});
173279

174280
describe('getContentType', () => {

0 commit comments

Comments
 (0)