Skip to content

Commit

Permalink
Remove multiple slashes from asset pathing
Browse files Browse the repository at this point in the history
  • Loading branch information
WillTaylorDev committed Nov 21, 2024
1 parent 6f09f23 commit bf42f5e
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 9 deletions.
8 changes: 2 additions & 6 deletions packages/workers-shared/asset-worker/src/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ export const handleRequest = async (
const { pathname, search } = new URL(request.url);

let decodedPathname = decodePath(pathname);
// normalize the path
decodedPathname = decodedPathname.replaceAll('//', '/');
// normalize the path; remove multiple slashes which could lead to same-schema redirects
decodedPathname = decodedPathname.replace(/\/+/g, "/");

const intent = await getIntent(decodedPathname, configuration, exists);

Expand All @@ -45,8 +45,6 @@ export const handleRequest = async (
* We combine this with other redirects (e.g. for html_handling) to avoid multiple redirects.
*/
if (encodedDestination !== pathname || intent.redirect) {
console.log(encodedDestination, pathname);

return new TemporaryRedirectResponse(encodedDestination + search);
}

Expand Down Expand Up @@ -643,8 +641,6 @@ const safeRedirect = async (
return null;
}

console.log(file, destination, configuration);

if (!(await exists(destination))) {
const intent = await getIntent(destination, configuration, exists, true);
// return only if the eTag matches - i.e. not the 404 case
Expand Down
15 changes: 12 additions & 3 deletions packages/workers-shared/asset-worker/tests/handler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,8 @@ describe("[Asset Worker] `handleRequest`", () => {
exists,
getByETag
);

expect(response.status).toBe(307);
expect(response.headers.get('location')).toBe('/abc/def/123/456');
expect(response.headers.get("location")).toBe("/abc/def/123/456");

response = await handleRequest(
new Request("https://example.com/%2fexample.com/"),
Expand All @@ -127,6 +126,16 @@ describe("[Asset Worker] `handleRequest`", () => {
);

expect(response.status).toBe(307);
expect(response.headers.get('location')).toBe('/example.com/');
expect(response.headers.get("location")).toBe("/example.com/");

response = await handleRequest(
new Request("https://example.com/%2f%2f%2fexample.com/%2f/foo"),
configuration,
exists,
getByETag
);

expect(response.status).toBe(307);
expect(response.headers.get("location")).toBe("/example.com/foo");
});
});

0 comments on commit bf42f5e

Please sign in to comment.