Skip to content

Commit

Permalink
fix and expand fixture tests
Browse files Browse the repository at this point in the history
  • Loading branch information
emily-shen committed Sep 20, 2024
1 parent 86a255f commit b362754
Show file tree
Hide file tree
Showing 12 changed files with 65 additions and 9 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<p>%255Bboop%255D.html</p>
1 change: 1 addition & 0 deletions fixtures/workers-with-assets-only/public/about/[boop].html
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<p>[boop].html</p>
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
This should work.
26 changes: 26 additions & 0 deletions fixtures/workers-with-assets-only/tests/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,4 +149,30 @@ describe("[Workers + Assets] static-assets only site`", () => {
});
expect(response.status).toBe(404);
});

it("should work with encoded path names", async ({ expect }) => {
let response = await fetch(`http://${ip}:${port}/about/[fünky].txt`);
let text = await response.text();
expect(response.status).toBe(200);
expect(response.url).toBe(
`http://${ip}:${port}/about/%5Bf%C3%BCnky%5D.txt`
);
expect(text).toContain(`This should work.`);

response = await fetch(`http://${ip}:${port}/about/[boop]`);
text = await response.text();
expect(response.status).toBe(200);
expect(response.url).toBe(`http://${ip}:${port}/about/%5Bboop%5D`);
expect(text).toContain(`[boop].html`);

response = await fetch(`http://${ip}:${port}/about/%5Bboop%5D`);
text = await response.text();
expect(response.status).toBe(200);
expect(text).toContain(`[boop].html`);

response = await fetch(`http://${ip}:${port}/about/%255Bboop%255D`);
text = await response.text();
expect(response.status).toBe(200);
expect(text).toContain(`%255Bboop%255D.html`);
});
});
1 change: 1 addition & 0 deletions fixtures/workers-with-assets/public/about/%5Bboop%5D.html
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<p>%5Bboop%5D.html</p>
1 change: 1 addition & 0 deletions fixtures/workers-with-assets/public/about/%5Bwomp%5D.html
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<p>womp</p>
1 change: 1 addition & 0 deletions fixtures/workers-with-assets/public/about/[boop].html
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<p>[boop].html</p>
23 changes: 20 additions & 3 deletions fixtures/workers-with-assets/tests/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,12 +119,29 @@ describe("[Workers + Assets] dynamic site", () => {
});

it("should work with encoded path names", async ({ expect }) => {
let response = await fetch(
`http://${ip}:${port}/about/%5Bf%C3%BCnky%5D.txt`
);
let response = await fetch(`http://${ip}:${port}/about/[fünky].txt`);
let text = await response.text();
expect(response.status).toBe(200);
expect(response.url).toBe(
`http://${ip}:${port}/about/%5Bf%C3%BCnky%5D.txt`
);
expect(text).toContain(`This should work.`);

response = await fetch(`http://${ip}:${port}/about/[boop]`);
text = await response.text();
expect(response.status).toBe(200);
expect(response.url).toBe(`http://${ip}:${port}/about/%5Bboop%5D`);
expect(text).toContain(`[boop].html`);

response = await fetch(`http://${ip}:${port}/about/%5Bboop%5D`);
text = await response.text();
expect(response.status).toBe(200);
expect(text).toContain(`[boop].html`);

response = await fetch(`http://${ip}:${port}/about/%255Bboop%255D`);
text = await response.text();
expect(response.status).toBe(200);
expect(text).toContain(`%5Bboop%5D.html`);
});

it("should forward all request types to the user Worker if there are *not* assets on that route", async ({
Expand Down
9 changes: 8 additions & 1 deletion packages/miniflare/src/workers/assets/assets-kv.worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,14 @@ export default <ExportedHandler<Env>>{
const { filePath, contentType } = entry;
const blobsService = env[SharedBindings.MAYBE_SERVICE_BLOBS];
const response = await blobsService.fetch(
new URL(filePath, "http://placeholder")
new URL(
// somewhere in blobservice I think this is being decoded again
filePath
.split("/")
.map((x) => encodeURIComponent(x))
.join("/"),
"http://placeholder"
)
);
const newResponse = new Response(response.body, response);
// ensure the runtime will return the metadata we need
Expand Down
3 changes: 1 addition & 2 deletions packages/workers-shared/asset-worker/src/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ export const handleRequest = async (
}

const asset = await getByETag(intent.asset.eTag);

const headers = getHeaders(intent.asset.eTag, asset.contentType, request);

const strongETag = `"${intent.asset.eTag}"`;
Expand Down Expand Up @@ -677,7 +676,7 @@ const safeRedirect = async (
/**
* Decode all incoming paths to ensure that we can handle paths with non-ASCII characters.
*/
const decodePath = (pathname: string) => {
export const decodePath = (pathname: string) => {
return pathname
.split("/")
.map((x) => decodeURIComponent(x))
Expand Down
5 changes: 3 additions & 2 deletions packages/workers-shared/asset-worker/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { WorkerEntrypoint } from "cloudflare:workers";
import { setupSentry } from "../../utils/sentry";
import { AssetsManifest } from "./assets-manifest";
import { applyConfigurationDefaults } from "./configuration";
import { getIntent, handleRequest } from "./handler";
import { decodePath, getIntent, handleRequest } from "./handler";
import {
InternalServerErrorResponse,
MethodNotAllowedResponse,
Expand Down Expand Up @@ -75,8 +75,9 @@ export default class extends WorkerEntrypoint<Env> {
async unstable_canFetch(request: Request): Promise<boolean | Response> {
const url = new URL(request.url);
const method = request.method.toUpperCase();
const decodedPathname = decodePath(url.pathname);
const intent = await getIntent(
url.pathname,
decodedPathname,
{
...applyConfigurationDefaults(this.env.CONFIG),
not_found_handling: "none",
Expand Down
2 changes: 1 addition & 1 deletion packages/workers-shared/utils/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { getType } from "mime";

/** normalises sep for windows and prefix with `/` */
export const normalizeFilePath = (relativeFilepath: string) => {
if (!isAbsolute(relativeFilepath)) {
if (isAbsolute(relativeFilepath)) {
throw new Error(`Expected relative path`);
}
const encodedPath = relativeFilepath.split(sep).join("/");
Expand Down

0 comments on commit b362754

Please sign in to comment.