Skip to content

Commit

Permalink
fix: pass-through malformed paths better
Browse files Browse the repository at this point in the history
  • Loading branch information
Cherry committed Dec 23, 2024
1 parent 48fdec6 commit 91f2dca
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 18 deletions.
27 changes: 19 additions & 8 deletions packages/workers-shared/asset-worker/src/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,7 @@ export const handleRequest = async (
) => {
const { pathname, search } = new URL(request.url);

let decodedPathname;
try {
decodedPathname = decodePath(pathname);
} catch (err) {
return new NotFoundResponse();
}
let decodedPathname = decodePath(pathname);
// normalize the path; remove multiple slashes which could lead to same-schema redirects
decodedPathname = decodedPathname.replace(/\/+/g, "/");

Expand Down Expand Up @@ -687,7 +682,15 @@ const safeRedirect = async (
export const decodePath = (pathname: string) => {
return pathname
.split("/")
.map((x) => decodeURIComponent(x))
.map((x) => {
let decoded;
try {
decoded = decodeURIComponent(x);
return decoded;
} catch {
return x;
}
})
.join("/");
};
/**
Expand All @@ -697,6 +700,14 @@ export const decodePath = (pathname: string) => {
const encodePath = (pathname: string) => {
return pathname
.split("/")
.map((x) => encodeURIComponent(x))
.map((x) => {
let encoded;
try {
encoded = encodeURIComponent(x);
return encoded;
} catch {
return x;
}
})
.join("/");
};
33 changes: 23 additions & 10 deletions packages/workers-shared/asset-worker/tests/handler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,28 +151,41 @@ describe("[Asset Worker] `handleRequest`", () => {
expect(response.status).toBe(404);
});

it("returns 404 Not Found responses for malformed path", async () => {
it("returns expected responses for malformed path", async () => {
const assets: Record<string, string> = {
"/index.html": "aaaaaaaaaa",
"/%A0%A0.html": "bbbbbbbbbb",
};
const configuration: Required<AssetConfig> = {
html_handling: "none",
html_handling: "drop-trailing-slash",
not_found_handling: "none",
serve_directly: true,
};

const exists = async (pathname: string) => {
return assets[pathname] ?? null;
};
const getByEtag = async (_: string) => ({
readableStream: new ReadableStream(),
contentType: "text/html",
});

// first malformed URL should return 404 as no match above
const response = await handleRequest(
new Request("https://example.com/%A0"),
configuration,
async (pathname: string) => {
return assets[pathname] ?? null;
},
async (_: string) => ({
readableStream: new ReadableStream(),
contentType: "text/html",
})
exists,
getByEtag
);

expect(response.status).toBe(404);

// but second malformed URL should return 307 as it matches and then redirects
const response2 = await handleRequest(
new Request("https://example.com/%A0%A0"),
configuration,
exists,
getByEtag
);
expect(response2.status).toBe(307);
});
});

0 comments on commit 91f2dca

Please sign in to comment.