Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: better handler malformed paths #7612

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
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/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);
});
});
Loading