Skip to content

Commit

Permalink
fix: better handler malformed paths (#7612)
Browse files Browse the repository at this point in the history
* fix: better handler malformed paths

* chore: changeset

* fix: pass-through malformed paths better
  • Loading branch information
Cherry authored Jan 1, 2025
1 parent b9533a3 commit 2e78812
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 2 deletions.
5 changes: 5 additions & 0 deletions .changeset/empty-cars-provide.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@cloudflare/workers-shared": patch
---

fix: resolves an issue where a malformed path such as `https://example.com/%A0` would cause an unhandled error
18 changes: 16 additions & 2 deletions packages/workers-shared/asset-worker/src/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -682,7 +682,14 @@ const safeRedirect = async (
export const decodePath = (pathname: string) => {
return pathname
.split("/")
.map((x) => decodeURIComponent(x))
.map((x) => {
try {
const decoded = decodeURIComponent(x);
return decoded;
} catch {
return x;
}
})
.join("/");
};
/**
Expand All @@ -692,6 +699,13 @@ export const decodePath = (pathname: string) => {
const encodePath = (pathname: string) => {
return pathname
.split("/")
.map((x) => encodeURIComponent(x))
.map((x) => {
try {
const encoded = encodeURIComponent(x);
return encoded;
} catch {
return x;
}
})
.join("/");
};
38 changes: 38 additions & 0 deletions packages/workers-shared/asset-worker/tests/handler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,4 +150,42 @@ describe("[Asset Worker] `handleRequest`", () => {

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

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: "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,
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 2e78812

Please sign in to comment.