From ebf8367f4965bf88643ca4ef47ee77c2db83e898 Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Thu, 19 Sep 2024 21:23:02 +0100 Subject: [PATCH 1/7] remove encoding at upload --- packages/miniflare/src/plugins/assets/index.ts | 2 +- packages/workers-shared/utils/helpers.ts | 17 +++-------------- packages/wrangler/src/__tests__/deploy.test.ts | 4 ++-- packages/wrangler/src/experimental-assets.ts | 13 ++++--------- 4 files changed, 10 insertions(+), 26 deletions(-) diff --git a/packages/miniflare/src/plugins/assets/index.ts b/packages/miniflare/src/plugins/assets/index.ts index e3a868f67bce..540281b70101 100644 --- a/packages/miniflare/src/plugins/assets/index.ts +++ b/packages/miniflare/src/plugins/assets/index.ts @@ -3,12 +3,12 @@ import fs from "node:fs/promises"; import path from "node:path"; import { CONTENT_HASH_OFFSET, - encodeFilePath, ENTRY_SIZE, getContentType, HEADER_SIZE, MAX_ASSET_COUNT, MAX_ASSET_SIZE, + normalizeFilePath, PATH_HASH_OFFSET, PATH_HASH_SIZE, } from "@cloudflare/workers-shared"; diff --git a/packages/workers-shared/utils/helpers.ts b/packages/workers-shared/utils/helpers.ts index 7bd560658dc5..dff7c6d30729 100644 --- a/packages/workers-shared/utils/helpers.ts +++ b/packages/workers-shared/utils/helpers.ts @@ -1,22 +1,11 @@ import { getType } from "mime"; -/** normalises sep for windows, and encodes each segment */ -export const encodeFilePath = (filePath: string, sep: string) => { - const encodedPath = filePath - .split(sep) - .map((segment) => encodeURIComponent(segment)) - .join("/"); +/** normalises sep for windows */ +export const normalizeFilePath = (filePath: string, sep: string) => { + const encodedPath = filePath.split(sep).join("/"); return "/" + encodedPath; }; -/** reverses encodeFilePath for accessing from file system */ -export const decodeFilePath = (filePath: string, sep: string) => { - return filePath - .split("/") - .map((segment) => decodeURIComponent(segment)) - .join(sep); -}; - export const getContentType = (absFilePath: string) => { let contentType = getType(absFilePath) || "application/octet-stream"; if (contentType.startsWith("text/") && !contentType.includes("charset")) { diff --git a/packages/wrangler/src/__tests__/deploy.test.ts b/packages/wrangler/src/__tests__/deploy.test.ts index b586daf1e336..da9f8b92fce1 100644 --- a/packages/wrangler/src/__tests__/deploy.test.ts +++ b/packages/wrangler/src/__tests__/deploy.test.ts @@ -4427,11 +4427,11 @@ addEventListener('fetch', event => {});` expect(manifestBodies.length).toBe(1); expect(manifestBodies[0]).toEqual({ manifest: { - "/b%C3%A9%C3%ABp/boo%5Ep.txt": { + "/béëp/boo^p.txt": { hash: "ff5016e92f039aa743a4ff7abb3180fa", size: 17, }, - "/boop/file%231.txt": { + "/boop/file#1.txt": { hash: "7574a8cd3094a050388ac9663af1c1d6", size: 17, }, diff --git a/packages/wrangler/src/experimental-assets.ts b/packages/wrangler/src/experimental-assets.ts index eecc51ce5a15..273fd7921036 100644 --- a/packages/wrangler/src/experimental-assets.ts +++ b/packages/wrangler/src/experimental-assets.ts @@ -3,11 +3,10 @@ import { existsSync } from "node:fs"; import { readdir, readFile, stat } from "node:fs/promises"; import * as path from "node:path"; import { - decodeFilePath, - encodeFilePath, getContentType, MAX_ASSET_COUNT, MAX_ASSET_SIZE, + normalizeFilePath, } from "@cloudflare/workers-shared"; import chalk from "chalk"; import PQueue from "p-queue"; @@ -101,10 +100,7 @@ export const syncExperimentalAssets = async ( } // just logging file uploads at the moment... // unsure how to log deletion vs unchanged file ignored/if we want to log this - assetLogCount = logAssetUpload( - `+ ${decodeFilePath(manifestEntry[0], path.sep)}`, - assetLogCount - ); + assetLogCount = logAssetUpload(`+ ${manifestEntry[0]}`, assetLogCount); return manifestEntry; }); }); @@ -123,8 +119,7 @@ export const syncExperimentalAssets = async ( // This is so we don't run out of memory trying to upload the files. const payload = new FormData(); for (const manifestEntry of bucket) { - const decodedFilePath = decodeFilePath(manifestEntry[0], path.sep); - const absFilePath = path.join(assetDirectory, decodedFilePath); + const absFilePath = path.join(assetDirectory, manifestEntry[0]); payload.append( manifestEntry[1].hash, new File( @@ -268,7 +263,7 @@ export const buildAssetManifest = async (dir: string) => { `Ensure all assets in your assets directory "${dir}" conform with the Workers maximum size requirement.` ); } - manifest[encodeFilePath(relativeFilepath, path.sep)] = { + manifest[normalizeFilePath(relativeFilepath, path.sep)] = { hash: hashFile(filepath), size: filestat.size, }; From 3f57a439ec6667b370f28469fc8e6a473bbf9f78 Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Thu, 19 Sep 2024 21:24:16 +0100 Subject: [PATCH 2/7] add encoding redirect behaviour --- .../asset-worker/src/handler.ts | 110 ++++++++++++++---- 1 file changed, 89 insertions(+), 21 deletions(-) diff --git a/packages/workers-shared/asset-worker/src/handler.ts b/packages/workers-shared/asset-worker/src/handler.ts index a873f72b8a59..29b6f7864aa3 100644 --- a/packages/workers-shared/asset-worker/src/handler.ts +++ b/packages/workers-shared/asset-worker/src/handler.ts @@ -18,7 +18,12 @@ export const handleRequest = async ( ) => { const { pathname, search } = new URL(request.url); - const intent = await getIntent(pathname, configuration, exists); + const decoded = pathname + .split("/") + .map((x) => decodeURIComponent(x)) + .join("/"); + const intent = await getIntent(decoded, configuration, exists); + if (!intent) { return new NotFoundResponse(); } @@ -30,9 +35,16 @@ export const handleRequest = async ( if (!["GET", "HEAD"].includes(method)) { return new MethodNotAllowedResponse(); } - if (intent.redirect) { - return new TemporaryRedirectResponse(intent.redirect + search); + + const decodedDestination = intent.redirect ?? decoded; + const encodedDestination = decodedDestination + .split("/") + .map((x) => encodeURIComponent(x)) + .join("/"); + if (encodedDestination !== pathname || intent.redirect) { + return new TemporaryRedirectResponse(encodedDestination + search); } + if (!intent.asset) { return new InternalServerErrorResponse(new Error("Unknown action")); } @@ -114,7 +126,10 @@ const htmlHandlingAutoTrailingSlash = async ( if (pathname.endsWith("/index")) { if (exactETag) { // there's a binary /index file - return { asset: { eTag: exactETag, status: 200 }, redirect: null }; + return { + asset: { eTag: exactETag, status: 200 }, + redirect: null, + }; } else { if ( (redirectResult = await safeRedirect( @@ -167,7 +182,10 @@ const htmlHandlingAutoTrailingSlash = async ( } else if (pathname.endsWith("/")) { if ((eTagResult = await exists(`${pathname}index.html`))) { // /foo/index.html exists so serve at /foo/ - return { asset: { eTag: eTagResult, status: 200 }, redirect: null }; + return { + asset: { eTag: eTagResult, status: 200 }, + redirect: null, + }; } else if ( (redirectResult = await safeRedirect( `${pathname.slice(0, -"/".length)}.html`, @@ -208,10 +226,16 @@ const htmlHandlingAutoTrailingSlash = async ( if (exactETag) { // there's a binary /foo file - return { asset: { eTag: exactETag, status: 200 }, redirect: null }; + return { + asset: { eTag: exactETag, status: 200 }, + redirect: null, + }; } else if ((eTagResult = await exists(`${pathname}.html`))) { // foo.html exists so serve at /foo - return { asset: { eTag: eTagResult, status: 200 }, redirect: null }; + return { + asset: { eTag: eTagResult, status: 200 }, + redirect: null, + }; } else if ( (redirectResult = await safeRedirect( `${pathname}/index.html`, @@ -240,7 +264,10 @@ const htmlHandlingForceTrailingSlash = async ( if (pathname.endsWith("/index")) { if (exactETag) { // there's a binary /index file - return { asset: { eTag: exactETag, status: 200 }, redirect: null }; + return { + asset: { eTag: exactETag, status: 200 }, + redirect: null, + }; } else { if ( (redirectResult = await safeRedirect( @@ -293,12 +320,18 @@ const htmlHandlingForceTrailingSlash = async ( } else if (pathname.endsWith("/")) { if ((eTagResult = await exists(`${pathname}index.html`))) { // /foo/index.html exists so serve at /foo/ - return { asset: { eTag: eTagResult, status: 200 }, redirect: null }; + return { + asset: { eTag: eTagResult, status: 200 }, + redirect: null, + }; } else if ( (eTagResult = await exists(`${pathname.slice(0, -"/".length)}.html`)) ) { // /foo.html exists so serve at /foo/ - return { asset: { eTag: eTagResult, status: 200 }, redirect: null }; + return { + asset: { eTag: eTagResult, status: 200 }, + redirect: null, + }; } } else if (pathname.endsWith(".html")) { if ( @@ -314,7 +347,10 @@ const htmlHandlingForceTrailingSlash = async ( return redirectResult; } else if (exactETag) { // there's both /foo.html and /foo/index.html so we serve /foo.html at /foo.html only - return { asset: { eTag: exactETag, status: 200 }, redirect: null }; + return { + asset: { eTag: exactETag, status: 200 }, + redirect: null, + }; } else if ( (redirectResult = await safeRedirect( `${pathname.slice(0, -".html".length)}/index.html`, @@ -331,7 +367,11 @@ const htmlHandlingForceTrailingSlash = async ( if (exactETag) { // there's a binary /foo file - return { asset: { eTag: exactETag, status: 200 }, redirect: null }; + return { + asset: { eTag: exactETag, status: 200 }, + redirect: null, + file: pathname, + }; } else if ( (redirectResult = await safeRedirect( `${pathname}.html`, @@ -371,7 +411,10 @@ const htmlHandlingDropTrailingSlash = async ( if (pathname.endsWith("/index")) { if (exactETag) { // there's a binary /index file - return { asset: { eTag: exactETag, status: 200 }, redirect: null }; + return { + asset: { eTag: exactETag, status: 200 }, + redirect: null, + }; } else { if (pathname === "/index") { if ( @@ -436,7 +479,10 @@ const htmlHandlingDropTrailingSlash = async ( return redirectResult; } else if (exactETag) { // there's both /foo.html and /foo/index.html so we serve /foo/index.html at /foo/index.html only - return { asset: { eTag: exactETag, status: 200 }, redirect: null }; + return { + asset: { eTag: exactETag, status: 200 }, + redirect: null, + }; } else if ( (redirectResult = await safeRedirect( `${pathname.slice(0, -"/index.html".length)}.html`, @@ -453,7 +499,10 @@ const htmlHandlingDropTrailingSlash = async ( if (pathname === "/") { if ((eTagResult = await exists("/index.html"))) { // /index.html exists so serve at / - return { asset: { eTag: eTagResult, status: 200 }, redirect: null }; + return { + asset: { eTag: eTagResult, status: 200 }, + redirect: null, + }; } } else if ( (redirectResult = await safeRedirect( @@ -506,13 +555,22 @@ const htmlHandlingDropTrailingSlash = async ( if (exactETag) { // there's a binary /foo file - return { asset: { eTag: exactETag, status: 200 }, redirect: null }; + return { + asset: { eTag: exactETag, status: 200 }, + redirect: null, + }; } else if ((eTagResult = await exists(`${pathname}.html`))) { // /foo.html exists so serve at /foo - return { asset: { eTag: eTagResult, status: 200 }, redirect: null }; + return { + asset: { eTag: eTagResult, status: 200 }, + redirect: null, + }; } else if ((eTagResult = await exists(`${pathname}/index.html`))) { // /foo/index.html exists so serve at /foo - return { asset: { eTag: eTagResult, status: 200 }, redirect: null }; + return { + asset: { eTag: eTagResult, status: 200 }, + redirect: null, + }; } return notFound(pathname, configuration, exists); @@ -525,7 +583,10 @@ const htmlHandlingNone = async ( ): Promise => { const exactETag = await exists(pathname); if (exactETag) { - return { asset: { eTag: exactETag, status: 200 }, redirect: null }; + return { + asset: { eTag: exactETag, status: 200 }, + redirect: null, + }; } else { return notFound(pathname, configuration, exists); } @@ -540,7 +601,10 @@ const notFound = async ( case "single-page-application": { const eTag = await exists("/index.html"); if (eTag) { - return { asset: { eTag, status: 200 }, redirect: null }; + return { + asset: { eTag, status: 200 }, + redirect: null, + }; } return null; } @@ -550,7 +614,10 @@ const notFound = async ( cwd = cwd.slice(0, cwd.lastIndexOf("/")); const eTag = await exists(`${cwd}/404.html`); if (eTag) { - return { asset: { eTag, status: 404 }, redirect: null }; + return { + asset: { eTag, status: 404 }, + redirect: null, + }; } } return null; @@ -575,6 +642,7 @@ const safeRedirect = async ( if (!(await exists(destination))) { const intent = await getIntent(destination, configuration, exists, true); + // return only if the eTag matches - i.e. not the 404 case if (intent?.asset && intent.asset.eTag === (await exists(file))) { return { asset: null, From 0a7e8c7018966d940203b0191aa27dd4582c95bb Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Thu, 19 Sep 2024 21:25:07 +0100 Subject: [PATCH 3/7] add and move test cases Co-authored-by: Greg Brimble --- fixtures/asset-config/html-handling.test.ts | 757 +---------------- .../test-cases/encoding-test-cases.ts | 631 ++++++++++++++ .../test-cases/html-handling-test-cases.ts | 788 ++++++++++++++++++ 3 files changed, 1429 insertions(+), 747 deletions(-) create mode 100644 fixtures/asset-config/test-cases/encoding-test-cases.ts create mode 100644 fixtures/asset-config/test-cases/html-handling-test-cases.ts diff --git a/fixtures/asset-config/html-handling.test.ts b/fixtures/asset-config/html-handling.test.ts index 274d69b0b574..e024bfd956cc 100644 --- a/fixtures/asset-config/html-handling.test.ts +++ b/fixtures/asset-config/html-handling.test.ts @@ -3,6 +3,8 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { applyConfigurationDefaults } from "../../packages/workers-shared/asset-worker/src/configuration"; import Worker from "../../packages/workers-shared/asset-worker/src/index"; import { getAssetWithMetadataFromKV } from "../../packages/workers-shared/asset-worker/src/utils/kv"; +import { encodingTestCases } from "./test-cases/encoding-test-cases"; +import { htmlHandlingTestCases } from "./test-cases/html-handling-test-cases"; import type { AssetMetadata } from "../../packages/workers-shared/asset-worker/src/utils/kv"; const IncomingRequest = Request; @@ -20,7 +22,7 @@ const existsMock = (fileList: Set) => { }; const BASE_URL = "http://example.com"; -type TestCase = { +export type TestCase = { title: string; files: string[]; requestPath: string; @@ -28,756 +30,18 @@ type TestCase = { finalPath?: string; }; -const testCases: { - html_handling: - | "auto-trailing-slash" - | "drop-trailing-slash" - | "force-trailing-slash" - | "none"; - cases: TestCase[]; -}[] = [ +const testSuites = [ { - html_handling: "auto-trailing-slash", - cases: [ - { - title: "/ -> 200 (with /index.html)", - files: ["/index.html"], - requestPath: "/index.html", - matchedFile: "/index.html", - finalPath: "/", - }, - { - title: "/index -> / 307 (with /index.html)", - files: ["/index.html"], - requestPath: "/index", - matchedFile: "/index.html", - finalPath: "/", - }, - { - title: "/index.html -> / 307 (with /index.html)", - files: ["/index.html"], - requestPath: "/index.html", - matchedFile: "/index.html", - finalPath: "/", - }, - { - title: "/both -> 200 (with /both.html)", - files: ["/both.html", "/both/index.html"], - requestPath: "/both", - matchedFile: "/both.html", - finalPath: "/both", - }, - { - title: "/both.html -> /both 307 (with /both.html)", - files: ["/both.html", "/both/index.html"], - requestPath: "/both.html", - matchedFile: "/both.html", - finalPath: "/both", - }, - { - title: "/both/ -> 200 (with /both/index.html)", - files: ["/both.html", "/both/index.html"], - requestPath: "/both/", - matchedFile: "/both/index.html", - finalPath: "/both/", - }, - { - title: "/both/index.html -> /both/ 307 (with /both/index.html)", - files: ["/both.html", "/both/index.html"], - requestPath: "/both/index.html", - matchedFile: "/both/index.html", - finalPath: "/both/", - }, - { - title: "/both/index -> /both/ 307 (with /both/index.html)", - files: ["/both.html", "/both/index.html"], - requestPath: "/both/index", - matchedFile: "/both/index.html", - finalPath: "/both/", - }, - { - title: "/file -> 200 (with file.html)", - files: ["/file.html"], - requestPath: "/file", - matchedFile: "/file.html", - finalPath: "/file", - }, - { - title: "/file.html -> /file 307 (with file.html)", - files: ["/file.html"], - requestPath: "/file.html", - matchedFile: "/file.html", - finalPath: "/file", - }, - { - title: "/file/ -> /file 307 (with file.html)", - files: ["/file.html"], - requestPath: "/file/", - matchedFile: "/file.html", - finalPath: "/file", - }, - { - title: "/file/index -> /file 307 (with file.html)", - files: ["/file.html"], - requestPath: "/file/index", - matchedFile: "/file.html", - finalPath: "/file", - }, - { - title: "/file/index.html -> /file 307 (with file.html)", - files: ["/file.html"], - requestPath: "/file/index.html", - matchedFile: "/file.html", - finalPath: "/file", - }, - { - title: "/folder -> /folder/ 307 (with /folder/index.html)", - files: ["/folder/index.html"], - requestPath: "/folder", - matchedFile: "/folder/index.html", - finalPath: "/folder/", - }, - { - title: "/folder.html -> /folder/ 307 (with /folder/index.html)", - files: ["/folder/index.html"], - requestPath: "/folder.html", - matchedFile: "/folder/index.html", - finalPath: "/folder/", - }, - { - title: "/folder/ -> 200 (with /folder/index.html)", - files: ["/folder/index.html"], - requestPath: "/folder/", - matchedFile: "/folder/index.html", - finalPath: "/folder/", - }, - { - title: "/folder/index -> /folder/ 307 (with /folder/index.html)", - files: ["/folder/index.html"], - requestPath: "/folder/index", - matchedFile: "/folder/index.html", - finalPath: "/folder/", - }, - { - title: "/folder/index.html -> /folder/ 307 (with /folder/index.html)", - files: ["/folder/index.html"], - requestPath: "/folder/index.html", - matchedFile: "/folder/index.html", - finalPath: "/folder/", - }, - { - title: "/bin -> /bin/ 307 (with /bin/index.html)", - files: ["/bin%2F", "/bin/index.html"], - requestPath: "/bin", - matchedFile: "/bin/index.html", - finalPath: "/bin/", - }, - { - title: "/bin.html -> /bin/ 307 (with /bin/index.html)", - files: ["/bin%2F", "/bin/index.html"], - requestPath: "/bin.html", - matchedFile: "/bin/index.html", - finalPath: "/bin/", - }, - { - title: "/bin%2F -> 200 (with /bin%2F)", - files: ["/bin%2F", "/bin/index.html"], - requestPath: "/bin%2F", - matchedFile: "/bin%2F", - finalPath: "/bin%2F", - }, - { - title: "/bin/ -> 200 (with /bin/index.html not /bin%2F", - files: ["/bin%2F", "/bin/index.html"], - requestPath: "/bin/", - matchedFile: "/bin/index.html", - finalPath: "/bin/", - }, - { - title: "/bin/index -> 307 /bin/ (with /bin/index.html)", - files: ["/bin%2F", "/bin/index.html"], - requestPath: "/bin/index", - matchedFile: "/bin/index.html", - finalPath: "/bin/", - }, - { - title: "/bin/index.html -> 307 /bin/ (with /bin/index.html)", - files: ["/bin%2F", "/bin/index.html"], - requestPath: "/bin/index.html", - matchedFile: "/bin/index.html", - finalPath: "/bin/", - }, - // prefers exact match - { - title: "/file-bin -> 200 ", - files: ["/file-bin", "/file-bin.html"], - requestPath: "/file-bin", - matchedFile: "/file-bin", - finalPath: "/file-bin", - }, - // (doesn't rewrite if resulting path would match another asset) - { - title: "/file-bin.html -> 200 ", - files: ["/file-bin", "/file-bin.html"], - requestPath: "/file-bin.html", - matchedFile: "/file-bin.html", - finalPath: "/file-bin.html", - }, - // (finds file-bin.html --rewrite--> /file-bin, but /file-bin exists) - { - title: "/file-bin/ -> 404 ", - files: ["/file-bin", "/file-bin.html"], - requestPath: "/file-bin/", - }, - { - title: "/file-bin/index -> 404 ", - files: ["/file-bin", "/file-bin.html"], - requestPath: "/file-bin/index", - }, - { - title: "/file-bin/index.html -> 404 ", - files: ["/file-bin", "/file-bin.html"], - requestPath: "/file-bin/index.html", - }, - ], + title: "htmlHanding options", + suite: htmlHandlingTestCases, }, { - html_handling: "drop-trailing-slash", - cases: [ - // note that we don't drop the "/" if that is the only path component - { - title: "/ -> 200 (with /index.html)", - files: ["/index.html"], - requestPath: "/index.html", - matchedFile: "/index.html", - finalPath: "/", - }, - { - title: "/index -> / 307 (with /index.html)", - files: ["/index.html"], - requestPath: "/index", - matchedFile: "/index.html", - finalPath: "/", - }, - { - title: "/index.html -> / 307 (with /index.html)", - files: ["/index.html"], - requestPath: "/index.html", - matchedFile: "/index.html", - finalPath: "/", - }, - { - title: "/both -> 200 (with /both.html)", - files: ["/both.html", "/both/index.html"], - requestPath: "/both", - matchedFile: "/both.html", - finalPath: "/both", - }, - { - title: "/both.html -> /both 307 (with /both.html)", - files: ["/both.html", "/both/index.html"], - requestPath: "/both.html", - matchedFile: "/both.html", - finalPath: "/both", - }, - // drops trailing slash and so it tries /both.html first - { - title: "/both/ -> /both 307 (with /both.html)", - files: ["/both.html", "/both/index.html"], - requestPath: "/both/", - matchedFile: "/both.html", - finalPath: "/both", - }, - { - title: "/both/index -> 307 (with /both.html)", - files: ["/both.html", "/both/index.html"], - requestPath: "/both/index", - matchedFile: "/both.html", - finalPath: "/both", - }, - // can't rewrite /both/index.html: would be /both/ -> /both -> /both.html - // ie can only access /both/index.html by exact match - { - title: "/both/index.html -> 200 (with /both/index.html)", - files: ["/both.html", "/both/index.html"], - requestPath: "/both/index.html", - matchedFile: "/both/index.html", - finalPath: "/both/index.html", - }, - { - title: "/file -> 200 (with file.html)", - files: ["/file.html"], - requestPath: "/file", - matchedFile: "/file.html", - finalPath: "/file", - }, - { - title: "/file.html -> /file 307 (with file.html)", - files: ["/file.html"], - requestPath: "/file.html", - matchedFile: "/file.html", - finalPath: "/file", - }, - { - title: "/file/ -> /file 307 (with file.html)", - files: ["/file.html"], - requestPath: "/file/", - matchedFile: "/file.html", - finalPath: "/file", - }, - { - title: "/file/index -> /file 307 (with file.html)", - files: ["/file.html"], - requestPath: "/file/index", - matchedFile: "/file.html", - finalPath: "/file", - }, - { - title: "/file/index.html -> /file 307 (with file.html)", - files: ["/file.html"], - requestPath: "/file/index.html", - matchedFile: "/file.html", - finalPath: "/file", - }, - { - title: "/folder -> 200 (with /folder/index.html)", - files: ["/folder/index.html"], - requestPath: "/folder", - matchedFile: "/folder/index.html", - finalPath: "/folder", - }, - { - title: "/folder.html -> /folder 307 (with /folder/index.html)", - files: ["/folder/index.html"], - requestPath: "/folder.html", - matchedFile: "/folder/index.html", - finalPath: "/folder", - }, - { - title: "/folder/ -> /folder 307 (with /folder/index.html)", - files: ["/folder/index.html"], - requestPath: "/folder/", - matchedFile: "/folder/index.html", - finalPath: "/folder", - }, - { - title: "/folder/index -> /folder 307 (with /folder/index.html)", - files: ["/folder/index.html"], - requestPath: "/folder/index", - matchedFile: "/folder/index.html", - finalPath: "/folder", - }, - { - title: "/folder/index.html -> /folder 307 (with /folder/index.html)", - files: ["/folder/index.html"], - requestPath: "/folder/index.html", - matchedFile: "/folder/index.html", - finalPath: "/folder", - }, - { - title: "/bin -> 200 (with /bin/index.html)", - files: ["/bin%2F", "/bin/index.html"], - requestPath: "/bin", - matchedFile: "/bin/index.html", - finalPath: "/bin", - }, - { - title: "/bin.html -> /bin 307 (with /bin/index.html)", - files: ["/bin%2F", "/bin/index.html"], - requestPath: "/bin.html", - matchedFile: "/bin/index.html", - finalPath: "/bin", - }, - { - title: "/bin%2F -> 200 (with /bin%2F)", - files: ["/bin%2F", "/bin/index.html"], - requestPath: "/bin%2F", - matchedFile: "/bin%2F", - finalPath: "/bin%2F", - }, - { - title: "/bin/ -> /bin 307 (with /bin/index.html not /bin%2F", - files: ["/bin%2F", "/bin/index.html"], - requestPath: "/bin/", - matchedFile: "/bin/index.html", - finalPath: "/bin", - }, - { - title: "/bin/index -> /bin 307 (with /bin/index.html not /bin%2F", - files: ["/bin%2F", "/bin/index.html"], - requestPath: "/bin/index", - matchedFile: "/bin/index.html", - finalPath: "/bin", - }, - { - title: "/bin/index.html -> /bin 307 (with /bin/index.html not /bin%2F", - files: ["/bin%2F", "/bin/index.html"], - requestPath: "/bin/index.html", - matchedFile: "/bin/index.html", - finalPath: "/bin", - }, - { - title: "/file-bin -> 200", - files: ["/file-bin", "/file-bin.html"], - requestPath: "/file-bin", - matchedFile: "/file-bin", - finalPath: "/file-bin", - }, - // doesn't redirect to /file-bin because that also exists - { - title: "/file-bin.html -> 200 (with /file-bin.html)", - files: ["/file-bin", "/file-bin.html"], - requestPath: "/file-bin.html", - matchedFile: "/file-bin.html", - finalPath: "/file-bin.html", - }, - // 404s because ambiguity between /file-bin or /file-bin.html? - { - title: "/file-bin/ -> 404", - files: ["/file-bin", "/file-bin.html"], - requestPath: "/file-bin/", - }, - { - title: "/file-bin/index -> 404", - files: ["/file-bin", "/file-bin.html"], - requestPath: "/file-bin/index", - }, - { - title: "/file-bin/index.html -> 404", - files: ["/file-bin", "/file-bin.html"], - requestPath: "/file-bin/index.html", - }, - ], - }, - { - html_handling: "force-trailing-slash", - cases: [ - { - title: "/ -> 200 (with /index.html)", - files: ["/index.html"], - requestPath: "/index.html", - matchedFile: "/index.html", - finalPath: "/", - }, - { - title: "/index -> / 307 (with /index.html)", - files: ["/index.html"], - requestPath: "/index", - matchedFile: "/index.html", - finalPath: "/", - }, - { - title: "/index.html -> / 307 (with /index.html)", - files: ["/index.html"], - requestPath: "/index.html", - matchedFile: "/index.html", - finalPath: "/", - }, - // ie tries /both/index.html first - { - title: "/both -> /both/ 307 (with /both/index.html)", - files: ["/both.html", "/both/index.html"], - requestPath: "/both", - matchedFile: "/both/index.html", - finalPath: "/both/", - }, - // can't rewrite /both.html: would be /both -> /both/ -> /both/index.html - // ie can only access /both.html by exact match - { - title: "/both.html -> 200", - files: ["/both.html", "/both/index.html"], - requestPath: "/both.html", - matchedFile: "/both.html", - finalPath: "/both.html", - }, - { - title: "/both/ -> 200 (with /both/index.html)", - files: ["/both.html", "/both/index.html"], - requestPath: "/both/", - matchedFile: "/both/index.html", - finalPath: "/both/", - }, - { - title: "/both/index -> /both/ 307 (with /both/index.html)", - files: ["/both.html", "/both/index.html"], - requestPath: "/both/index", - matchedFile: "/both/index.html", - finalPath: "/both/", - }, - { - title: "/both/index.html -> /both/ 307 (with /both/index.html)", - files: ["/both.html", "/both/index.html"], - requestPath: "/both/index.html", - matchedFile: "/both/index.html", - finalPath: "/both/", - }, - // always ends in a trailing slash - { - title: "/file -> /file/ 307 (with file.html)", - files: ["/file.html"], - requestPath: "/file", - matchedFile: "/file.html", - finalPath: "/file/", - }, - { - title: "/file.html -> /file/ 307 (with file.html)", - files: ["/file.html"], - requestPath: "/file.html", - matchedFile: "/file.html", - finalPath: "/file/", - }, - - { - title: "/file/ -> 200 (with file.html)", - files: ["/file.html"], - requestPath: "/file/", - matchedFile: "/file.html", - finalPath: "/file/", - }, - { - title: "/file/index -> /file/ 307 (with file.html)", - files: ["/file.html"], - requestPath: "/file/index", - matchedFile: "/file.html", - finalPath: "/file/", - }, - { - title: "/file/index.html -> /file/ 307 (with file.html)", - files: ["/file.html"], - requestPath: "/file/index.html", - matchedFile: "/file.html", - finalPath: "/file/", - }, - { - title: "/folder -> /folder/ 307 (with /folder/index.html)", - files: ["/folder/index.html"], - requestPath: "/folder", - matchedFile: "/folder/index.html", - finalPath: "/folder/", - }, - { - title: "/folder.html -> /folder/ 307 (with /folder/index.html)", - files: ["/folder/index.html"], - requestPath: "/folder.html", - matchedFile: "/folder/index.html", - finalPath: "/folder/", - }, - { - title: "/folder/ -> 200 (with /folder/index.html)", - files: ["/folder/index.html"], - requestPath: "/folder/", - matchedFile: "/folder/index.html", - finalPath: "/folder/", - }, - { - title: "/folder/index -> /folder/ 307 (with /folder/index.html)", - files: ["/folder/index.html"], - requestPath: "/folder/index", - matchedFile: "/folder/index.html", - finalPath: "/folder/", - }, - { - title: "/folder/index.html -> /folder/ 307 (with /folder/index.html)", - files: ["/folder/index.html"], - requestPath: "/folder/index.html", - matchedFile: "/folder/index.html", - finalPath: "/folder/", - }, - { - title: "/bin -> /bin/ 307 (with /bin/index.html)", - files: ["/bin%2F", "/bin/index.html"], - requestPath: "/bin", - matchedFile: "/bin/index.html", - finalPath: "/bin/", - }, - { - title: "/bin.html -> /bin/ 307 (with /bin/index.html)", - files: ["/bin%2F", "/bin/index.html"], - requestPath: "/bin.html", - matchedFile: "/bin/index.html", - finalPath: "/bin/", - }, - { - title: "/bin%2F -> 200 (with /bin%2F)", - files: ["/bin%2F", "/bin/index.html"], - requestPath: "/bin%2F", - matchedFile: "/bin%2F", - finalPath: "/bin%2F", - }, - { - title: "/bin/ -> 200 (with /bin/index.html not /bin%2F", - files: ["/bin%2F", "/bin/index.html"], - requestPath: "/bin/", - matchedFile: "/bin/index.html", - finalPath: "/bin/", - }, - { - title: "/bin/index -> /bin/ 307 (with /bin/index.html)", - files: ["/bin%2F", "/bin/index.html"], - requestPath: "/bin/index", - matchedFile: "/bin/index.html", - finalPath: "/bin/", - }, - { - title: "/bin/index.html -> /bin/ 307 (with /bin/index.html)", - files: ["/bin%2F", "/bin/index.html"], - requestPath: "/bin/index.html", - matchedFile: "/bin/index.html", - finalPath: "/bin/", - }, - // doesn't force a trailing slash here because it would redirect to /file-bin.html - { - title: "/file-bin -> 200", - files: ["/file-bin", "/file-bin.html"], - requestPath: "/file-bin", - matchedFile: "/file-bin", - finalPath: "/file-bin", - }, - { - title: "/file-bin.html -> /file-bin/ 307 (with /file-bin.html)", - files: ["/file-bin", "/file-bin.html"], - requestPath: "/file-bin.html", - matchedFile: "/file-bin.html", - finalPath: "/file-bin/", - }, - { - title: "/file-bin/ -> 200", - files: ["/file-bin", "/file-bin.html"], - requestPath: "/file-bin/", - matchedFile: "/file-bin.html", - finalPath: "/file-bin/", - }, - { - title: "/file-bin/index -> /file-bin/ 307 (with /file-bin.html)", - files: ["/file-bin", "/file-bin.html"], - requestPath: "/file-bin/index", - matchedFile: "/file-bin.html", - finalPath: "/file-bin/", - }, - { - title: "/file-bin/index.html -> /file-bin/ 307 (with /file-bin.html)", - files: ["/file-bin", "/file-bin.html"], - requestPath: "/file-bin/index.html", - matchedFile: "/file-bin.html", - finalPath: "/file-bin/", - }, - ], - }, - { - html_handling: "none", - cases: [ - { - title: "/ -> 404", - files: ["/index.html"], - requestPath: "/", - }, - { - title: "/index -> 404", - files: ["/index.html"], - requestPath: "/index", - }, - { - title: "/index.html -> 200", - files: ["/index.html"], - requestPath: "/index.html", - matchedFile: "/index.html", - finalPath: "/index.html", - }, - { - title: "/both -> 404", - files: ["/both.html", "/both/index.html"], - requestPath: "/both", - }, - { - title: "/both.html -> 200", - files: ["/both.html", "/both/index.html"], - requestPath: "/both.html", - matchedFile: "/both.html", - finalPath: "/both.html", - }, - { - title: "/both/ -> 404", - files: ["/both.html", "/both/index.html"], - requestPath: "/both/", - }, - { - title: "/both/index.html -> 200", - files: ["/both.html", "/both/index.html"], - requestPath: "/both/index.html", - matchedFile: "/both/index.html", - finalPath: "/both/index.html", - }, - { - title: "/file/index.html -> 404", - files: ["/file.html"], - requestPath: "/file/index.html", - }, - { - title: "/folder.html -> 404", - files: ["/folder/index.html"], - requestPath: "/folder.html", - }, - { - title: "/bin -> 404", - files: ["/bin%2F", "/bin/index.html"], - requestPath: "/bin", - }, - { - title: "/bin.html -> 404", - files: ["/bin%2F", "/bin/index.html"], - requestPath: "/bin.html", - }, - { - title: "/bin%2F -> 200", - files: ["/bin%2F", "/bin/index.html"], - requestPath: "/bin%2F", - matchedFile: "/bin%2F", - finalPath: "/bin%2F", - }, - { - title: "/bin/ -> 404", - files: ["/bin%2F", "/bin/index.html"], - requestPath: "/bin/", - }, - { - title: "/bin/index -> 404", - files: ["/bin%2F", "/bin/index.html"], - requestPath: "/bin/index", - }, - { - title: "/file-bin -> 200", - files: ["/file-bin", "/file-bin.html"], - requestPath: "/file-bin", - matchedFile: "/file-bin", - finalPath: "/file-bin", - }, - { - title: "/file-bin.html -> 200", - files: ["/file-bin", "/file-bin.html"], - requestPath: "/file-bin.html", - matchedFile: "/file-bin.html", - finalPath: "/file-bin.html", - }, - { - title: "/file-bin/ -> 404", - files: ["/file-bin", "/file-bin.html"], - requestPath: "/file-bin/", - }, - { - title: "/file-bin/index -> 404", - files: ["/file-bin", "/file-bin.html"], - requestPath: "/file-bin/index", - }, - { - title: "/file-bin/index.html -> 404", - files: ["/file-bin", "/file-bin.html"], - requestPath: "/file-bin/index.html", - }, - ], + title: "encoding options", + suite: encodingTestCases, }, ]; -describe("htmlHanding options", () => { +describe.each(testSuites)("$title", ({ title, suite }) => { beforeEach(() => { vi.mocked(getAssetWithMetadataFromKV).mockImplementation( () => @@ -794,7 +58,7 @@ describe("htmlHanding options", () => { afterEach(() => { vi.mocked(getAssetWithMetadataFromKV).mockRestore(); }); - describe.each(testCases)(`$html_handling`, ({ html_handling, cases }) => { + describe.each(suite)(`$html_handling`, ({ html_handling, cases }) => { beforeEach(() => { vi.mocked(applyConfigurationDefaults).mockImplementation(() => { return { @@ -815,7 +79,6 @@ describe("htmlHanding options", () => { undefined, matchedFile ); - console.dir(response.status); expect(response.status).toBe(200); expect(response.url).toBe(BASE_URL + finalPath); // can't check intermediate 307 directly: diff --git a/fixtures/asset-config/test-cases/encoding-test-cases.ts b/fixtures/asset-config/test-cases/encoding-test-cases.ts new file mode 100644 index 000000000000..0da5719f1947 --- /dev/null +++ b/fixtures/asset-config/test-cases/encoding-test-cases.ts @@ -0,0 +1,631 @@ +import { TestCase } from "../html-handling.test"; + +export const encodingTestCases: { + html_handling: + | "auto-trailing-slash" + | "drop-trailing-slash" + | "force-trailing-slash" + | "none"; + cases: TestCase[]; +}[] = [ + { + html_handling: "auto-trailing-slash", + cases: [ + { + title: "/[boop] -> /%5Bboop%5D 307 (with /[boop].html)", + files: ["/[boop].html"], + requestPath: "/[boop]", + matchedFile: "/[boop].html", + finalPath: "/%5Bboop%5D", + }, + { + title: "/%5Bboop%5D -> 200 (with /[boop].html)", + files: ["/[boop].html"], + requestPath: "/%5Bboop%5D", + matchedFile: "/[boop].html", + finalPath: "/%5Bboop%5D", + }, + // auto-trailing-slash html handling still works + { + title: "/%5Bboop%5D.html -> /%5Bboop%5D 307 (with /[boop].html)", + files: ["/[boop].html"], + requestPath: "/%5Bboop%5D.html", + matchedFile: "/[boop].html", + finalPath: "/%5Bboop%5D", + }, + { + title: "/[boop].html -> /%5Bboop%5D 307 (with /[boop].html)", + files: ["/[boop].html"], + requestPath: "/[boop].html", + matchedFile: "/[boop].html", + finalPath: "/%5Bboop%5D", + }, + { + title: "/%5Bboop%5D/ -> /%5Bboop%5D 307 (with /[boop].html)", + files: ["/[boop].html"], + requestPath: "/%5Bboop%5D/", + matchedFile: "/[boop].html", + finalPath: "/%5Bboop%5D", + }, + { + title: "/[boop]/ -> /%5Bboop%5D 307 (with /[boop].html)", + files: ["/[boop].html"], + requestPath: "/[boop]/", + matchedFile: "/[boop].html", + finalPath: "/%5Bboop%5D", + }, + { + title: "/[boop] -> /%5Bboop%5D 307 (with /[boop].html)", + files: ["/[boop].html", "/[boop]/index.html"], + requestPath: "/[boop]", + matchedFile: "/[boop].html", + finalPath: "/%5Bboop%5D", + }, + { + title: "/%5Bboop%5D -> 200 (with /[boop].html)", + files: ["/[boop].html", "/[boop]/index.html"], + requestPath: "/%5Bboop%5D", + matchedFile: "/[boop].html", + finalPath: "/%5Bboop%5D", + }, + { + title: "/[boop].html -> /%5Bboop%5D 307 (with /[boop].html)", + files: ["/[boop].html", "/[boop]/index.html"], + requestPath: "/[boop].html", + matchedFile: "/[boop].html", + finalPath: "/%5Bboop%5D", + }, + { + title: "/%5Bboop%5D.html -> /%5Bboop%5D 307 (with /[boop].html)", + files: ["/[boop].html", "/[boop]/index.html"], + requestPath: "/%5Bboop%5D.html", + matchedFile: "/[boop].html", + finalPath: "/%5Bboop%5D", + }, + { + title: "/[boop]/ -> /%5Bboop%5D/ 307 (with /[boop]/index.html)", + files: ["/[boop].html", "/[boop]/index.html"], + requestPath: "/[boop]/", + matchedFile: "/[boop]/index.html", + finalPath: "/%5Bboop%5D/", + }, + { + title: "/%5Bboop%5D/ -> 200 (with /[boop]/index.html)", + files: ["/[boop].html", "/[boop]/index.html"], + requestPath: "/%5Bboop%5D/", + matchedFile: "/[boop]/index.html", + finalPath: "/%5Bboop%5D/", + }, + { + title: + "/[boop]/index.html -> /%5Bboop%5D/ 307 (with /[boop]/index.html)", + files: ["/[boop].html", "/[boop]/index.html"], + requestPath: "/[boop]/index.html", + matchedFile: "/[boop]/index.html", + finalPath: "/%5Bboop%5D/", + }, + { + title: + "/%5Bboop%5D/index.html -> /%5Bboop%5D/ 307 (with /[boop]/index.html)", + files: ["/[boop].html", "/[boop]/index.html"], + requestPath: "/%5Bboop%5D/index.html", + matchedFile: "/[boop]/index.html", + finalPath: "/%5Bboop%5D/", + }, + // paths with a mix of encoded and unencoded characters + { + title: + "/beep/[b%C3%B2op] -> /beep/%5Bb%C3%B2op%5D 307 (with /beep/[bòop].html)", + files: ["/beep/[bòop].html"], + requestPath: "/beep/[b%C3%B2op]", + matchedFile: "/beep/[bòop].html", + finalPath: "/beep/%5Bb%C3%B2op%5D", + }, + // user-encoded paths should only be accessible at the (double) encoded path + { + title: "/[boop] -> /%5Bboop%5D 307 (with /[boop].html)", + files: ["/%5Bboop%5D.html", "/[boop].html"], + requestPath: "/[boop]", + matchedFile: "/[boop].html", + finalPath: "/%5Bboop%5D", + }, + { + title: "/[boop].html -> /%5Bboop%5D 307 (with /[boop].html)", + files: ["/%5Bboop%5D.html", "/[boop].html"], + requestPath: "/[boop].html", + matchedFile: "/[boop].html", + finalPath: "/%5Bboop%5D", + }, + { + title: "/%5Bboop%5D -> 200 (with /[boop].html)", + files: ["/%5Bboop%5D.html", "/[boop].html"], + requestPath: "/%5Bboop%5D", + matchedFile: "/[boop].html", + finalPath: "/%5Bboop%5D", + }, + { + title: "/%5Bboop%5D.html -> /%5Bboop%5D 307 (with /[boop].html)", + files: ["/%5Bboop%5D.html", "/[boop].html"], + requestPath: "/%5Bboop%5D.html", + matchedFile: "/[boop].html", + finalPath: "/%5Bboop%5D", + }, + { + title: "/%5Bboop%5D -> 200 (with /[boop].html)", + files: ["/%5Bboop%5D.html", "/[boop].html"], + requestPath: "/%5Bboop%5D", + matchedFile: "/[boop].html", + finalPath: "/%5Bboop%5D", + }, + { + title: "/%255Bboop%255D -> 200 (with /%5Bboop%5D.html)", + files: ["/%5Bboop%5D.html", "/[boop].html"], + requestPath: "/%255Bboop%255D", + matchedFile: "/%5Bboop%5D.html", + finalPath: "/%255Bboop%255D", + }, + { + title: + "/%255Bboop%255D.html -> /%255Bboop%255D 307 (with /%5Bboop%5D.html)", + files: ["/%5Bboop%5D.html", "/[boop].html"], + requestPath: "/%255Bboop%255D.html", + matchedFile: "/%5Bboop%5D.html", + finalPath: "/%255Bboop%255D", + }, + { + title: "/beep?boop -> 404", + files: ["/beep?boop.html"], + requestPath: "/beep?boop", + }, + { + title: "/beep%3Fboop -> 200 (with /beep?boop.html)", + files: ["/beep?boop.html"], + requestPath: "/beep%3Fboop", + matchedFile: "/beep?boop.html", + finalPath: "/beep%3Fboop", + }, + { + title: "/beep%3Fboop/ -> /beep%3Fboop 307 (with /beep?boop.html)", + files: ["/beep?boop.html"], + requestPath: "/beep%3Fboop/", + matchedFile: "/beep?boop.html", + finalPath: "/beep%3Fboop", + }, + ], + }, + { + html_handling: "drop-trailing-slash", + cases: [ + { + title: "/[boop] -> /%5Bboop%5D 307 (with /[boop].html)", + files: ["/[boop].html"], + requestPath: "/[boop]", + matchedFile: "/[boop].html", + finalPath: "/%5Bboop%5D", + }, + { + title: "/%5Bboop%5D -> 200 (with /[boop].html)", + files: ["/[boop].html"], + requestPath: "/%5Bboop%5D", + matchedFile: "/[boop].html", + finalPath: "/%5Bboop%5D", + }, + // drop-trailing-slash html handling still works + { + title: "/%5Bboop%5D.html -> /%5Bboop%5D 307 (with /[boop].html)", + files: ["/[boop].html"], + requestPath: "/%5Bboop%5D.html", + matchedFile: "/[boop].html", + finalPath: "/%5Bboop%5D", + }, + { + title: "/[boop].html -> /%5Bboop%5D 307 (with /[boop].html)", + files: ["/[boop].html"], + requestPath: "/[boop].html", + matchedFile: "/[boop].html", + finalPath: "/%5Bboop%5D", + }, + { + title: "/%5Bboop%5D/ -> /%5Bboop%5D 307 (with /[boop].html)", + files: ["/[boop].html"], + requestPath: "/%5Bboop%5D/", + matchedFile: "/[boop].html", + finalPath: "/%5Bboop%5D", + }, + { + title: "/[boop]/ -> /%5Bboop%5D 307 (with /[boop].html)", + files: ["/[boop].html"], + requestPath: "/[boop]/", + matchedFile: "/[boop].html", + finalPath: "/%5Bboop%5D", + }, + { + title: "/[boop] -> /%5Bboop%5D 307 (with /[boop].html)", + files: ["/[boop].html", "/[boop]/index.html"], + requestPath: "/[boop]", + matchedFile: "/[boop].html", + finalPath: "/%5Bboop%5D", + }, + { + title: "/%5Bboop%5D -> 200 (with /[boop].html)", + files: ["/[boop].html", "/[boop]/index.html"], + requestPath: "/%5Bboop%5D", + matchedFile: "/[boop].html", + finalPath: "/%5Bboop%5D", + }, + { + title: "/[boop].html -> /%5Bboop%5D 307 (with /[boop].html)", + files: ["/[boop].html", "/[boop]/index.html"], + requestPath: "/[boop].html", + matchedFile: "/[boop].html", + finalPath: "/%5Bboop%5D", + }, + { + title: "/%5Bboop%5D.html -> /%5Bboop%5D 307 (with /[boop].html)", + files: ["/[boop].html", "/[boop]/index.html"], + requestPath: "/%5Bboop%5D.html", + matchedFile: "/[boop].html", + finalPath: "/%5Bboop%5D", + }, + { + title: "/[boop]/ -> /%5Bboop%5D 307 (with /[boop].html)", + files: ["/[boop].html", "/[boop]/index.html"], + requestPath: "/[boop]/", + matchedFile: "/[boop].html", + finalPath: "/%5Bboop%5D", + }, + { + title: "/%5Bboop%5D/ -> /%5Bboop%5D 307 (with /[boop].html)", + files: ["/[boop].html", "/[boop]/index.html"], + requestPath: "/%5Bboop%5D/", + matchedFile: "/[boop].html", + finalPath: "/%5Bboop%5D", + }, + { + title: + "/[boop]/index.html -> /%5Bboop%5D/index.html 307 (with /[boop]/index.html)", + files: ["/[boop].html", "/[boop]/index.html"], + requestPath: "/[boop]/index.html", + matchedFile: "/[boop]/index.html", + finalPath: "/%5Bboop%5D/index.html", + }, + { + title: "/%5Bboop%5D/index.html -> 200 (with /[boop]/index.html)", + files: ["/[boop].html", "/[boop]/index.html"], + requestPath: "/%5Bboop%5D/index.html", + matchedFile: "/[boop]/index.html", + finalPath: "/%5Bboop%5D/index.html", + }, + // paths with a mix of encoded and unencoded characters + { + title: + "/beep/[b%C3%B2op] -> /beep/%5Bb%C3%B2op%5D 307 (with /beep/[bòop].html)", + files: ["/beep/[bòop].html"], + requestPath: "/beep/[b%C3%B2op]", + matchedFile: "/beep/[bòop].html", + finalPath: "/beep/%5Bb%C3%B2op%5D", + }, + // user-encoded paths should only be accessible at the (double) encoded path + { + title: "/[boop] -> /%5Bboop%5D 307 (with /[boop].html)", + files: ["/%5Bboop%5D.html", "/[boop].html"], + requestPath: "/[boop]", + matchedFile: "/[boop].html", + finalPath: "/%5Bboop%5D", + }, + { + title: "/[boop].html -> /%5Bboop%5D 307 (with /[boop].html)", + files: ["/%5Bboop%5D.html", "/[boop].html"], + requestPath: "/[boop].html", + matchedFile: "/[boop].html", + finalPath: "/%5Bboop%5D", + }, + { + title: "/%5Bboop%5D -> 200 (with /[boop].html)", + files: ["/%5Bboop%5D.html", "/[boop].html"], + requestPath: "/%5Bboop%5D", + matchedFile: "/[boop].html", + finalPath: "/%5Bboop%5D", + }, + { + title: "/%5Bboop%5D.html -> /%5Bboop%5D 307 (with /[boop].html)", + files: ["/%5Bboop%5D.html", "/[boop].html"], + requestPath: "/%5Bboop%5D.html", + matchedFile: "/[boop].html", + finalPath: "/%5Bboop%5D", + }, + { + title: "/%5Bboop%5D -> 200 (with /[boop].html)", + files: ["/%5Bboop%5D.html", "/[boop].html"], + requestPath: "/%5Bboop%5D", + matchedFile: "/[boop].html", + finalPath: "/%5Bboop%5D", + }, + { + title: "/%255Bboop%255D -> 200 (with /%5Bboop%5D.html)", + files: ["/%5Bboop%5D.html", "/[boop].html"], + requestPath: "/%255Bboop%255D", + matchedFile: "/%5Bboop%5D.html", + finalPath: "/%255Bboop%255D", + }, + { + title: + "/%255Bboop%255D.html -> /%255Bboop%255D 307 (with /%5Bboop%5D.html)", + files: ["/%5Bboop%5D.html", "/[boop].html"], + requestPath: "/%255Bboop%255D.html", + matchedFile: "/%5Bboop%5D.html", + finalPath: "/%255Bboop%255D", + }, + { + title: "/beep?boop -> 404", + files: ["/beep?boop.html"], + requestPath: "/beep?boop", + }, + { + title: "/beep%3Fboop -> 200 (with /beep?boop.html)", + files: ["/beep?boop.html"], + requestPath: "/beep%3Fboop", + matchedFile: "/beep?boop.html", + finalPath: "/beep%3Fboop", + }, + { + title: "/beep%3Fboop/ -> /beep%3Fboop 307 (with /beep?boop.html)", + files: ["/beep?boop.html"], + requestPath: "/beep%3Fboop/", + matchedFile: "/beep?boop.html", + finalPath: "/beep%3Fboop", + }, + ], + }, + { + html_handling: "force-trailing-slash", + cases: [ + { + title: "/%5Bboop%5D/ -> 200 (with /[boop].html)", + files: ["/[boop].html"], + requestPath: "/%5Bboop%5D/", + matchedFile: "/[boop].html", + finalPath: "/%5Bboop%5D/", + }, + { + title: "/[boop]/ -> /%5Bboop%5D/ 307 (with /[boop].html)", + files: ["/[boop].html"], + requestPath: "/[boop]/", + matchedFile: "/[boop].html", + finalPath: "/%5Bboop%5D/", + }, + // force-trailing-slash html handling still works + { + title: "/[boop] -> /%5Bboop%5D/ 307 (with /[boop]/index.html)", + files: ["/[boop].html", "/[boop]/index.html"], + requestPath: "/[boop]", + matchedFile: "/[boop]/index.html", + finalPath: "/%5Bboop%5D/", + }, + { + title: "/%5Bboop%5D -> /%5Bboop%5D/ 307 (with /[boop]/index.html)", + files: ["/[boop].html", "/[boop]/index.html"], + requestPath: "/%5Bboop%5D", + matchedFile: "/[boop]/index.html", + finalPath: "/%5Bboop%5D/", + }, + { + title: "/[boop].html -> /%5Bboop%5D.html 307 (with /[boop].html)", + files: ["/[boop].html", "/[boop]/index.html"], + requestPath: "/[boop].html", + matchedFile: "/[boop].html", + finalPath: "/%5Bboop%5D.html", + }, + { + title: "/%5Bboop%5D.html -> 200 (with /[boop].html)", + files: ["/[boop].html", "/[boop]/index.html"], + requestPath: "/%5Bboop%5D.html", + matchedFile: "/[boop].html", + finalPath: "/%5Bboop%5D.html", + }, + { + title: "/[boop]/ -> /%5Bboop%5D/ 307 (with /[boop]/index.html)", + files: ["/[boop].html", "/[boop]/index.html"], + requestPath: "/[boop]/", + matchedFile: "/[boop]/index.html", + finalPath: "/%5Bboop%5D/", + }, + { + title: "/%5Bboop%5D/ -> 200 (with /[boop]/index.html)", + files: ["/[boop].html", "/[boop]/index.html"], + requestPath: "/%5Bboop%5D/", + matchedFile: "/[boop]/index.html", + finalPath: "/%5Bboop%5D/", + }, + { + title: + "/[boop]/index.html -> /%5Bboop%5D/ 307 (with /[boop]/index.html)", + files: ["/[boop].html", "/[boop]/index.html"], + requestPath: "/[boop]/index.html", + matchedFile: "/[boop]/index.html", + finalPath: "/%5Bboop%5D/", + }, + { + title: + "/%5Bboop%5D/index.html -> /%5Bboop%5D/ 307 (with /[boop]/index.html)", + files: ["/[boop].html", "/[boop]/index.html"], + requestPath: "/%5Bboop%5D/index.html", + matchedFile: "/[boop]/index.html", + finalPath: "/%5Bboop%5D/", + }, + // paths with a mix of encoded and unencoded characters + { + title: + "/beep/[b%C3%B2op]/ -> /beep/%5Bb%C3%B2op%5D/ 307 (with /beep/[bòop].html)", + files: ["/beep/[bòop].html"], + requestPath: "/beep/[b%C3%B2op]/", + matchedFile: "/beep/[bòop].html", + finalPath: "/beep/%5Bb%C3%B2op%5D/", + }, + // user-encoded paths should only be accessible at the (double) encoded path + { + title: "/[boop] -> /%5Bboop%5D/ 307 (with /[boop].html)", + files: ["/%5Bboop%5D.html", "/[boop].html"], + requestPath: "/[boop]", + matchedFile: "/[boop].html", + finalPath: "/%5Bboop%5D/", + }, + { + title: "/[boop].html -> /%5Bboop%5D/ 307 (with /[boop].html)", + files: ["/%5Bboop%5D.html", "/[boop].html"], + requestPath: "/[boop].html", + matchedFile: "/[boop].html", + finalPath: "/%5Bboop%5D/", + }, + { + title: "/%5Bboop%5D -> /%5Bboop%5D/ 307 (with /[boop].html)", + files: ["/%5Bboop%5D.html", "/[boop].html"], + requestPath: "/%5Bboop%5D", + matchedFile: "/[boop].html", + finalPath: "/%5Bboop%5D/", + }, + { + title: "/%5Bboop%5D.html -> /%5Bboop%5D/ 307 (with /[boop].html)", + files: ["/%5Bboop%5D.html", "/[boop].html"], + requestPath: "/%5Bboop%5D.html", + matchedFile: "/[boop].html", + finalPath: "/%5Bboop%5D/", + }, + { + title: "/%5Bboop%5D -> /%5Bboop%5D/ 307 (with /[boop].html)", + files: ["/%5Bboop%5D.html", "/[boop].html"], + requestPath: "/%5Bboop%5D", + matchedFile: "/[boop].html", + finalPath: "/%5Bboop%5D/", + }, + { + title: + "/%255Bboop%255D -> /%255Bboop%255D/ 307 (with /%5Bboop%5D.html)", + files: ["/%5Bboop%5D.html", "/[boop].html"], + requestPath: "/%255Bboop%255D", + matchedFile: "/%5Bboop%5D.html", + finalPath: "/%255Bboop%255D/", + }, + { + title: + "/%255Bboop%255D.html -> /%255Bboop%255D/ 307 (with /%5Bboop%5D.html)", + files: ["/%5Bboop%5D.html", "/[boop].html"], + requestPath: "/%255Bboop%255D.html", + matchedFile: "/%5Bboop%5D.html", + finalPath: "/%255Bboop%255D/", + }, + { + title: "/beep?boop/ -> 404", + files: ["/beep?boop.html"], + requestPath: "/beep?boop/", + }, + { + title: "/beep%3Fboop -> /beep%3Fboop/ 307 (with /beep?boop.html)", + files: ["/beep?boop.html"], + requestPath: "/beep%3Fboop", + matchedFile: "/beep?boop.html", + finalPath: "/beep%3Fboop/", + }, + { + title: "/beep%3Fboop/ -> 200 (with /beep?boop.html)", + files: ["/beep?boop.html"], + requestPath: "/beep%3Fboop/", + matchedFile: "/beep?boop.html", + finalPath: "/beep%3Fboop/", + }, + ], + }, + { + html_handling: "none", + cases: [ + { + title: "/[boop] -> 404", + files: ["/[boop].html"], + requestPath: "/[boop]", + }, + { + title: "/%5Bboop%5D -> 404", + files: ["/[boop].html"], + requestPath: "/%5Bboop%5D", + }, + // encoding still operates when html_handling is set to 'none' + { + title: "/[boop].html -> /%5Bboop%5D.html 307 (with /[boop].html)", + files: ["/[boop].html"], + requestPath: "/[boop].html", + matchedFile: "/[boop].html", + finalPath: "/%5Bboop%5D.html", + }, + { + title: "/%5Bboop%5D.html -> 200 (with /[boop].html)", + files: ["/[boop].html"], + requestPath: "/%5Bboop%5D.html", + matchedFile: "/[boop].html", + finalPath: "/%5Bboop%5D.html", + }, + // mix of encoded and unencoded paths + { + title: + "/beep/[b%C3%B2op].html -> /beep/%5Bb%C3%B2op%5D.html 307 (with /beep/[bòop].html)", + files: ["/beep/[bòop].html"], + requestPath: "/beep/[b%C3%B2op].html", + matchedFile: "/beep/[bòop].html", + finalPath: "/beep/%5Bb%C3%B2op%5D.html", + }, + // user-encoded paths should only be accessible at the (double) encoded path + { + title: "/[boop].html -> /%5Bboop%5D.html 307 (with /[boop].html)", + files: ["/%5Bboop%5D.html", "/[boop].html"], + requestPath: "/[boop].html", + matchedFile: "/[boop].html", + finalPath: "/%5Bboop%5D.html", + }, + { + title: "/%5Bboop%5D.html -> 200 (with /[boop].html)", + files: ["/%5Bboop%5D.html", "/[boop].html"], + requestPath: "/%5Bboop%5D.html", + matchedFile: "/[boop].html", + finalPath: "/%5Bboop%5D.html", + }, + { + title: "/[boop] -> 404", + files: ["/%5Bboop%5D.html", "/[boop].html"], + requestPath: "/[boop]", + }, + { + title: "/%5Bboop%5D -> 404", + files: ["/%5Bboop%5D.html", "/[boop].html"], + requestPath: "/%5Bboop%5D", + }, + { + title: "/%255Bboop%255D.html -> 200 (with /%5Bboop%5D.html)", + files: ["/%5Bboop%5D.html", "/[boop].html"], + requestPath: "/%255Bboop%255D.html", + matchedFile: "/%5Bboop%5D.html", + finalPath: "/%255Bboop%255D.html", + }, + { + title: "/%255Bboop%255D -> 404", + files: ["/%5Bboop%5D.html", "/[boop].html"], + requestPath: "/%255Bboop%255D", + }, + { + title: "/beep?boop/ -> 404", + files: ["/beep?boop.html"], + requestPath: "/beep?boop/", + }, + { + title: "/beep%3Fboop.html -> 200 (with /beep?boop.html)", + files: ["/beep?boop.html"], + requestPath: "/beep%3Fboop.html", + matchedFile: "/beep?boop.html", + finalPath: "/beep%3Fboop.html", + }, + { + title: "/beep%3Fboop -> 404", + files: ["/beep?boop.html"], + requestPath: "/beep%3Fboop", + }, + ], + }, +]; diff --git a/fixtures/asset-config/test-cases/html-handling-test-cases.ts b/fixtures/asset-config/test-cases/html-handling-test-cases.ts new file mode 100644 index 000000000000..8c65ebac26aa --- /dev/null +++ b/fixtures/asset-config/test-cases/html-handling-test-cases.ts @@ -0,0 +1,788 @@ +import { TestCase } from "../html-handling.test"; + +export const htmlHandlingTestCases: { + html_handling: + | "auto-trailing-slash" + | "drop-trailing-slash" + | "force-trailing-slash" + | "none"; + cases: TestCase[]; +}[] = [ + { + html_handling: "auto-trailing-slash", + cases: [ + { + title: "/ -> 200 (with /index.html)", + files: ["/index.html"], + requestPath: "/index.html", + matchedFile: "/index.html", + finalPath: "/", + }, + { + title: "/index -> / 307 (with /index.html)", + files: ["/index.html"], + requestPath: "/index", + matchedFile: "/index.html", + finalPath: "/", + }, + { + title: "/index.html -> / 307 (with /index.html)", + files: ["/index.html"], + requestPath: "/index.html", + matchedFile: "/index.html", + finalPath: "/", + }, + { + title: "/both -> 200 (with /both.html)", + files: ["/both.html", "/both/index.html"], + requestPath: "/both", + matchedFile: "/both.html", + finalPath: "/both", + }, + { + title: "/both.html -> /both 307 (with /both.html)", + files: ["/both.html", "/both/index.html"], + requestPath: "/both.html", + matchedFile: "/both.html", + finalPath: "/both", + }, + { + title: "/both/ -> 200 (with /both/index.html)", + files: ["/both.html", "/both/index.html"], + requestPath: "/both/", + matchedFile: "/both/index.html", + finalPath: "/both/", + }, + { + title: "/both/index.html -> /both/ 307 (with /both/index.html)", + files: ["/both.html", "/both/index.html"], + requestPath: "/both/index.html", + matchedFile: "/both/index.html", + finalPath: "/both/", + }, + { + title: "/both/index -> /both/ 307 (with /both/index.html)", + files: ["/both.html", "/both/index.html"], + requestPath: "/both/index", + matchedFile: "/both/index.html", + finalPath: "/both/", + }, + { + title: "/file -> 200 (with file.html)", + files: ["/file.html"], + requestPath: "/file", + matchedFile: "/file.html", + finalPath: "/file", + }, + { + title: "/file.html -> /file 307 (with file.html)", + files: ["/file.html"], + requestPath: "/file.html", + matchedFile: "/file.html", + finalPath: "/file", + }, + { + title: "/file/ -> /file 307 (with file.html)", + files: ["/file.html"], + requestPath: "/file/", + matchedFile: "/file.html", + finalPath: "/file", + }, + { + title: "/file/index -> /file 307 (with file.html)", + files: ["/file.html"], + requestPath: "/file/index", + matchedFile: "/file.html", + finalPath: "/file", + }, + { + title: "/file/index.html -> /file 307 (with file.html)", + files: ["/file.html"], + requestPath: "/file/index.html", + matchedFile: "/file.html", + finalPath: "/file", + }, + { + title: "/folder -> /folder/ 307 (with /folder/index.html)", + files: ["/folder/index.html"], + requestPath: "/folder", + matchedFile: "/folder/index.html", + finalPath: "/folder/", + }, + { + title: "/folder.html -> /folder/ 307 (with /folder/index.html)", + files: ["/folder/index.html"], + requestPath: "/folder.html", + matchedFile: "/folder/index.html", + finalPath: "/folder/", + }, + { + title: "/folder/ -> 200 (with /folder/index.html)", + files: ["/folder/index.html"], + requestPath: "/folder/", + matchedFile: "/folder/index.html", + finalPath: "/folder/", + }, + { + title: "/folder/index -> /folder/ 307 (with /folder/index.html)", + files: ["/folder/index.html"], + requestPath: "/folder/index", + matchedFile: "/folder/index.html", + finalPath: "/folder/", + }, + { + title: "/folder/index.html -> /folder/ 307 (with /folder/index.html)", + files: ["/folder/index.html"], + requestPath: "/folder/index.html", + matchedFile: "/folder/index.html", + finalPath: "/folder/", + }, + // see encoding tests, but tldr only accessible at the (double) encoded path + { + title: "/bin -> /bin/ 307 (with /bin/index.html)", + files: ["/bin%2F", "/bin/index.html"], + requestPath: "/bin", + matchedFile: "/bin/index.html", + finalPath: "/bin/", + }, + { + title: "/bin.html -> /bin/ 307 (with /bin/index.html)", + files: ["/bin%2F", "/bin/index.html"], + requestPath: "/bin.html", + matchedFile: "/bin/index.html", + finalPath: "/bin/", + }, + // This is a bit rogue, but then so is the test case + { + title: "/bin%2F -> /bin 307 (with /bin/index.html)", + files: ["/bin%2F", "/bin/index.html"], + requestPath: "/bin%2F", + matchedFile: "/bin/index.html", + finalPath: "/bin/", + }, + { + title: "/bin%252F -> 200 (with /bin%2F)", + files: ["/bin%2F", "/bin/index.html"], + requestPath: "/bin%252F", + matchedFile: "/bin%2F", + finalPath: "/bin%252F", + }, + { + title: "/bin/ -> 200 (with /bin/index.html not /bin%2F", + files: ["/bin%2F", "/bin/index.html"], + requestPath: "/bin/", + matchedFile: "/bin/index.html", + finalPath: "/bin/", + }, + { + title: "/bin/index -> 307 /bin/ (with /bin/index.html)", + files: ["/bin%2F", "/bin/index.html"], + requestPath: "/bin/index", + matchedFile: "/bin/index.html", + finalPath: "/bin/", + }, + { + title: "/bin/index.html -> 307 /bin/ (with /bin/index.html)", + files: ["/bin%2F", "/bin/index.html"], + requestPath: "/bin/index.html", + matchedFile: "/bin/index.html", + finalPath: "/bin/", + }, + // prefers exact match + { + title: "/file-bin -> 200 ", + files: ["/file-bin", "/file-bin.html"], + requestPath: "/file-bin", + matchedFile: "/file-bin", + finalPath: "/file-bin", + }, + // (doesn't rewrite if resulting path would match another asset) + { + title: "/file-bin.html -> 200 ", + files: ["/file-bin", "/file-bin.html"], + requestPath: "/file-bin.html", + matchedFile: "/file-bin.html", + finalPath: "/file-bin.html", + }, + // (finds file-bin.html --rewrite--> /file-bin, but /file-bin exists) + { + title: "/file-bin/ -> 404 ", + files: ["/file-bin", "/file-bin.html"], + requestPath: "/file-bin/", + }, + { + title: "/file-bin/index -> 404 ", + files: ["/file-bin", "/file-bin.html"], + requestPath: "/file-bin/index", + }, + { + title: "/file-bin/index.html -> 404 ", + files: ["/file-bin", "/file-bin.html"], + requestPath: "/file-bin/index.html", + }, + ], + }, + { + html_handling: "drop-trailing-slash", + cases: [ + // note that we don't drop the "/" if that is the only path component + { + title: "/ -> 200 (with /index.html)", + files: ["/index.html"], + requestPath: "/index.html", + matchedFile: "/index.html", + finalPath: "/", + }, + { + title: "/index -> / 307 (with /index.html)", + files: ["/index.html"], + requestPath: "/index", + matchedFile: "/index.html", + finalPath: "/", + }, + { + title: "/index.html -> / 307 (with /index.html)", + files: ["/index.html"], + requestPath: "/index.html", + matchedFile: "/index.html", + finalPath: "/", + }, + { + title: "/both -> 200 (with /both.html)", + files: ["/both.html", "/both/index.html"], + requestPath: "/both", + matchedFile: "/both.html", + finalPath: "/both", + }, + { + title: "/both.html -> /both 307 (with /both.html)", + files: ["/both.html", "/both/index.html"], + requestPath: "/both.html", + matchedFile: "/both.html", + finalPath: "/both", + }, + // drops trailing slash and so it tries /both.html first + { + title: "/both/ -> /both 307 (with /both.html)", + files: ["/both.html", "/both/index.html"], + requestPath: "/both/", + matchedFile: "/both.html", + finalPath: "/both", + }, + { + title: "/both/index -> 307 (with /both.html)", + files: ["/both.html", "/both/index.html"], + requestPath: "/both/index", + matchedFile: "/both.html", + finalPath: "/both", + }, + // can't rewrite /both/index.html: would be /both/ -> /both -> /both.html + // ie can only access /both/index.html by exact match + { + title: "/both/index.html -> 200 (with /both/index.html)", + files: ["/both.html", "/both/index.html"], + requestPath: "/both/index.html", + matchedFile: "/both/index.html", + finalPath: "/both/index.html", + }, + { + title: "/file -> 200 (with file.html)", + files: ["/file.html"], + requestPath: "/file", + matchedFile: "/file.html", + finalPath: "/file", + }, + { + title: "/file.html -> /file 307 (with file.html)", + files: ["/file.html"], + requestPath: "/file.html", + matchedFile: "/file.html", + finalPath: "/file", + }, + { + title: "/file/ -> /file 307 (with file.html)", + files: ["/file.html"], + requestPath: "/file/", + matchedFile: "/file.html", + finalPath: "/file", + }, + { + title: "/file/index -> /file 307 (with file.html)", + files: ["/file.html"], + requestPath: "/file/index", + matchedFile: "/file.html", + finalPath: "/file", + }, + { + title: "/file/index.html -> /file 307 (with file.html)", + files: ["/file.html"], + requestPath: "/file/index.html", + matchedFile: "/file.html", + finalPath: "/file", + }, + { + title: "/folder -> 200 (with /folder/index.html)", + files: ["/folder/index.html"], + requestPath: "/folder", + matchedFile: "/folder/index.html", + finalPath: "/folder", + }, + { + title: "/folder.html -> /folder 307 (with /folder/index.html)", + files: ["/folder/index.html"], + requestPath: "/folder.html", + matchedFile: "/folder/index.html", + finalPath: "/folder", + }, + { + title: "/folder/ -> /folder 307 (with /folder/index.html)", + files: ["/folder/index.html"], + requestPath: "/folder/", + matchedFile: "/folder/index.html", + finalPath: "/folder", + }, + { + title: "/folder/index -> /folder 307 (with /folder/index.html)", + files: ["/folder/index.html"], + requestPath: "/folder/index", + matchedFile: "/folder/index.html", + finalPath: "/folder", + }, + { + title: "/folder/index.html -> /folder 307 (with /folder/index.html)", + files: ["/folder/index.html"], + requestPath: "/folder/index.html", + matchedFile: "/folder/index.html", + finalPath: "/folder", + }, + { + title: "/bin -> 200 (with /bin/index.html)", + files: ["/bin%2F", "/bin/index.html"], + requestPath: "/bin", + matchedFile: "/bin/index.html", + finalPath: "/bin", + }, + { + title: "/bin.html -> /bin 307 (with /bin/index.html)", + files: ["/bin%2F", "/bin/index.html"], + requestPath: "/bin.html", + matchedFile: "/bin/index.html", + finalPath: "/bin", + }, + // This is a bit rogue, but then so is the test case + { + title: "/bin%2F -> /bin 307 (with /bin/index.html)", + files: ["/bin%2F", "/bin/index.html"], + requestPath: "/bin%2F", + matchedFile: "/bin/index.html", + finalPath: "/bin", + }, + { + title: "/bin%252F -> 200 (with /bin%2F)", + files: ["/bin%2F", "/bin/index.html"], + requestPath: "/bin%252F", + matchedFile: "/bin%2F", + finalPath: "/bin%252F", + }, + { + title: "/bin/ -> /bin 307 (with /bin/index.html not /bin%2F", + files: ["/bin%2F", "/bin/index.html"], + requestPath: "/bin/", + matchedFile: "/bin/index.html", + finalPath: "/bin", + }, + { + title: "/bin/index -> /bin 307 (with /bin/index.html not /bin%2F", + files: ["/bin%2F", "/bin/index.html"], + requestPath: "/bin/index", + matchedFile: "/bin/index.html", + finalPath: "/bin", + }, + { + title: "/bin/index.html -> /bin 307 (with /bin/index.html not /bin%2F", + files: ["/bin%2F", "/bin/index.html"], + requestPath: "/bin/index.html", + matchedFile: "/bin/index.html", + finalPath: "/bin", + }, + { + title: "/file-bin -> 200", + files: ["/file-bin", "/file-bin.html"], + requestPath: "/file-bin", + matchedFile: "/file-bin", + finalPath: "/file-bin", + }, + // doesn't redirect to /file-bin because that also exists + { + title: "/file-bin.html -> 200 (with /file-bin.html)", + files: ["/file-bin", "/file-bin.html"], + requestPath: "/file-bin.html", + matchedFile: "/file-bin.html", + finalPath: "/file-bin.html", + }, + // 404s because ambiguity between /file-bin or /file-bin.html? + { + title: "/file-bin/ -> 404", + files: ["/file-bin", "/file-bin.html"], + requestPath: "/file-bin/", + }, + { + title: "/file-bin/index -> 404", + files: ["/file-bin", "/file-bin.html"], + requestPath: "/file-bin/index", + }, + { + title: "/file-bin/index.html -> 404", + files: ["/file-bin", "/file-bin.html"], + requestPath: "/file-bin/index.html", + }, + ], + }, + { + html_handling: "force-trailing-slash", + cases: [ + { + title: "/ -> 200 (with /index.html)", + files: ["/index.html"], + requestPath: "/index.html", + matchedFile: "/index.html", + finalPath: "/", + }, + { + title: "/index -> / 307 (with /index.html)", + files: ["/index.html"], + requestPath: "/index", + matchedFile: "/index.html", + finalPath: "/", + }, + { + title: "/index.html -> / 307 (with /index.html)", + files: ["/index.html"], + requestPath: "/index.html", + matchedFile: "/index.html", + finalPath: "/", + }, + // ie tries /both/index.html first + { + title: "/both -> /both/ 307 (with /both/index.html)", + files: ["/both.html", "/both/index.html"], + requestPath: "/both", + matchedFile: "/both/index.html", + finalPath: "/both/", + }, + // can't rewrite /both.html: would be /both -> /both/ -> /both/index.html + // ie can only access /both.html by exact match + { + title: "/both.html -> 200", + files: ["/both.html", "/both/index.html"], + requestPath: "/both.html", + matchedFile: "/both.html", + finalPath: "/both.html", + }, + { + title: "/both/ -> 200 (with /both/index.html)", + files: ["/both.html", "/both/index.html"], + requestPath: "/both/", + matchedFile: "/both/index.html", + finalPath: "/both/", + }, + { + title: "/both/index -> /both/ 307 (with /both/index.html)", + files: ["/both.html", "/both/index.html"], + requestPath: "/both/index", + matchedFile: "/both/index.html", + finalPath: "/both/", + }, + { + title: "/both/index.html -> /both/ 307 (with /both/index.html)", + files: ["/both.html", "/both/index.html"], + requestPath: "/both/index.html", + matchedFile: "/both/index.html", + finalPath: "/both/", + }, + // always ends in a trailing slash + { + title: "/file -> /file/ 307 (with file.html)", + files: ["/file.html"], + requestPath: "/file", + matchedFile: "/file.html", + finalPath: "/file/", + }, + { + title: "/file.html -> /file/ 307 (with file.html)", + files: ["/file.html"], + requestPath: "/file.html", + matchedFile: "/file.html", + finalPath: "/file/", + }, + + { + title: "/file/ -> 200 (with file.html)", + files: ["/file.html"], + requestPath: "/file/", + matchedFile: "/file.html", + finalPath: "/file/", + }, + { + title: "/file/index -> /file/ 307 (with file.html)", + files: ["/file.html"], + requestPath: "/file/index", + matchedFile: "/file.html", + finalPath: "/file/", + }, + { + title: "/file/index.html -> /file/ 307 (with file.html)", + files: ["/file.html"], + requestPath: "/file/index.html", + matchedFile: "/file.html", + finalPath: "/file/", + }, + { + title: "/folder -> /folder/ 307 (with /folder/index.html)", + files: ["/folder/index.html"], + requestPath: "/folder", + matchedFile: "/folder/index.html", + finalPath: "/folder/", + }, + { + title: "/folder.html -> /folder/ 307 (with /folder/index.html)", + files: ["/folder/index.html"], + requestPath: "/folder.html", + matchedFile: "/folder/index.html", + finalPath: "/folder/", + }, + { + title: "/folder/ -> 200 (with /folder/index.html)", + files: ["/folder/index.html"], + requestPath: "/folder/", + matchedFile: "/folder/index.html", + finalPath: "/folder/", + }, + { + title: "/folder/index -> /folder/ 307 (with /folder/index.html)", + files: ["/folder/index.html"], + requestPath: "/folder/index", + matchedFile: "/folder/index.html", + finalPath: "/folder/", + }, + { + title: "/folder/index.html -> /folder/ 307 (with /folder/index.html)", + files: ["/folder/index.html"], + requestPath: "/folder/index.html", + matchedFile: "/folder/index.html", + finalPath: "/folder/", + }, + { + title: "/bin -> /bin/ 307 (with /bin/index.html)", + files: ["/bin%2F", "/bin/index.html"], + requestPath: "/bin", + matchedFile: "/bin/index.html", + finalPath: "/bin/", + }, + { + title: "/bin.html -> /bin/ 307 (with /bin/index.html)", + files: ["/bin%2F", "/bin/index.html"], + requestPath: "/bin.html", + matchedFile: "/bin/index.html", + finalPath: "/bin/", + }, + { + title: "/bin%2F -> /bin/ 307 (with /bin%2F)", + files: ["/bin%2F", "/bin/index.html"], + requestPath: "/bin%2F", + matchedFile: "/bin/index.html", + finalPath: "/bin/", + }, + // This is a bit rogue, but then so is the test case + { + title: "/bin%2F -> /bin/ 307 (with /bin/index.html)", + files: ["/bin%2F", "/bin/index.html"], + requestPath: "/bin%2F", + matchedFile: "/bin/index.html", + finalPath: "/bin/", + }, + { + title: "/bin%252F -> 200 (with /bin%2F)", + files: ["/bin%2F", "/bin/index.html"], + requestPath: "/bin%252F", + matchedFile: "/bin%2F", + finalPath: "/bin%252F", + }, + { + title: "/bin/ -> 200 (with /bin/index.html not /bin%2F", + files: ["/bin%2F", "/bin/index.html"], + requestPath: "/bin/", + matchedFile: "/bin/index.html", + finalPath: "/bin/", + }, + { + title: "/bin/index -> /bin/ 307 (with /bin/index.html)", + files: ["/bin%2F", "/bin/index.html"], + requestPath: "/bin/index", + matchedFile: "/bin/index.html", + finalPath: "/bin/", + }, + { + title: "/bin/index.html -> /bin/ 307 (with /bin/index.html)", + files: ["/bin%2F", "/bin/index.html"], + requestPath: "/bin/index.html", + matchedFile: "/bin/index.html", + finalPath: "/bin/", + }, + // doesn't force a trailing slash here because it would redirect to /file-bin.html + { + title: "/file-bin -> 200", + files: ["/file-bin", "/file-bin.html"], + requestPath: "/file-bin", + matchedFile: "/file-bin", + finalPath: "/file-bin", + }, + { + title: "/file-bin.html -> /file-bin/ 307 (with /file-bin.html)", + files: ["/file-bin", "/file-bin.html"], + requestPath: "/file-bin.html", + matchedFile: "/file-bin.html", + finalPath: "/file-bin/", + }, + { + title: "/file-bin/ -> 200", + files: ["/file-bin", "/file-bin.html"], + requestPath: "/file-bin/", + matchedFile: "/file-bin.html", + finalPath: "/file-bin/", + }, + { + title: "/file-bin/index -> /file-bin/ 307 (with /file-bin.html)", + files: ["/file-bin", "/file-bin.html"], + requestPath: "/file-bin/index", + matchedFile: "/file-bin.html", + finalPath: "/file-bin/", + }, + { + title: "/file-bin/index.html -> /file-bin/ 307 (with /file-bin.html)", + files: ["/file-bin", "/file-bin.html"], + requestPath: "/file-bin/index.html", + matchedFile: "/file-bin.html", + finalPath: "/file-bin/", + }, + ], + }, + { + html_handling: "none", + cases: [ + { + title: "/ -> 404", + files: ["/index.html"], + requestPath: "/", + }, + { + title: "/index -> 404", + files: ["/index.html"], + requestPath: "/index", + }, + { + title: "/index.html -> 200", + files: ["/index.html"], + requestPath: "/index.html", + matchedFile: "/index.html", + finalPath: "/index.html", + }, + { + title: "/both -> 404", + files: ["/both.html", "/both/index.html"], + requestPath: "/both", + }, + { + title: "/both.html -> 200", + files: ["/both.html", "/both/index.html"], + requestPath: "/both.html", + matchedFile: "/both.html", + finalPath: "/both.html", + }, + { + title: "/both/ -> 404", + files: ["/both.html", "/both/index.html"], + requestPath: "/both/", + }, + { + title: "/both/index.html -> 200", + files: ["/both.html", "/both/index.html"], + requestPath: "/both/index.html", + matchedFile: "/both/index.html", + finalPath: "/both/index.html", + }, + { + title: "/file/index.html -> 404", + files: ["/file.html"], + requestPath: "/file/index.html", + }, + { + title: "/folder.html -> 404", + files: ["/folder/index.html"], + requestPath: "/folder.html", + }, + { + title: "/bin -> 404", + files: ["/bin%2F", "/bin/index.html"], + requestPath: "/bin", + }, + { + title: "/bin.html -> 404", + files: ["/bin%2F", "/bin/index.html"], + requestPath: "/bin.html", + }, + // encoding is independent of html handling + { + title: "/bin%2F -> 404", + files: ["/bin%2F", "/bin/index.html"], + requestPath: "/bin%2F", // -> /bin/ -> 404 + }, + { + title: "/bin%252F -> 200", + files: ["/bin%2F", "/bin/index.html"], + requestPath: "/bin%252F", + matchedFile: "/bin%2F", + finalPath: "/bin%252F", + }, + { + title: "/bin/ -> 404", + files: ["/bin%2F", "/bin/index.html"], + requestPath: "/bin/", + }, + { + title: "/bin/index -> 404", + files: ["/bin%2F", "/bin/index.html"], + requestPath: "/bin/index", + }, + { + title: "/file-bin -> 200", + files: ["/file-bin", "/file-bin.html"], + requestPath: "/file-bin", + matchedFile: "/file-bin", + finalPath: "/file-bin", + }, + { + title: "/file-bin.html -> 200", + files: ["/file-bin", "/file-bin.html"], + requestPath: "/file-bin.html", + matchedFile: "/file-bin.html", + finalPath: "/file-bin.html", + }, + { + title: "/file-bin/ -> 404", + files: ["/file-bin", "/file-bin.html"], + requestPath: "/file-bin/", + }, + { + title: "/file-bin/index -> 404", + files: ["/file-bin", "/file-bin.html"], + requestPath: "/file-bin/index", + }, + { + title: "/file-bin/index.html -> 404", + files: ["/file-bin", "/file-bin.html"], + requestPath: "/file-bin/index.html", + }, + ], + }, +]; From 2721f86ba79b925187765ce0271851800a4d86cd Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Fri, 20 Sep 2024 13:12:10 +0100 Subject: [PATCH 4/7] pr feedback --- .../miniflare/src/plugins/assets/index.ts | 7 +-- .../asset-worker/src/handler.ts | 62 +++++++++++++++---- packages/workers-shared/utils/helpers.ts | 10 ++- packages/workers-shared/utils/tsconfig.json | 2 +- packages/wrangler/src/experimental-assets.ts | 2 +- pnpm-lock.yaml | 6 ++ 6 files changed, 69 insertions(+), 20 deletions(-) diff --git a/packages/miniflare/src/plugins/assets/index.ts b/packages/miniflare/src/plugins/assets/index.ts index 540281b70101..0f28394ef768 100644 --- a/packages/miniflare/src/plugins/assets/index.ts +++ b/packages/miniflare/src/plugins/assets/index.ts @@ -239,10 +239,9 @@ const walk = async (dir: string) => { */ const [pathHash, contentHash] = await Promise.all([ - hashPath(encodeFilePath(relativeFilepath, path.sep)), - hashPath( - encodeFilePath(filepath, path.sep) + filestat.mtimeMs.toString() - ), + hashPath(normalizeFilePath(relativeFilepath)), + // used absolute filepath here so that changes to the enclosing asset folder will be registered + hashPath(filepath + filestat.mtimeMs.toString()), ]); manifest.push({ pathHash, diff --git a/packages/workers-shared/asset-worker/src/handler.ts b/packages/workers-shared/asset-worker/src/handler.ts index 29b6f7864aa3..fa1556c2db18 100644 --- a/packages/workers-shared/asset-worker/src/handler.ts +++ b/packages/workers-shared/asset-worker/src/handler.ts @@ -18,11 +18,8 @@ export const handleRequest = async ( ) => { const { pathname, search } = new URL(request.url); - const decoded = pathname - .split("/") - .map((x) => decodeURIComponent(x)) - .join("/"); - const intent = await getIntent(decoded, configuration, exists); + const decodedPathname = decodePath(pathname); + const intent = await getIntent(decodedPathname, configuration, exists); if (!intent) { return new NotFoundResponse(); @@ -36,11 +33,14 @@ export const handleRequest = async ( return new MethodNotAllowedResponse(); } - const decodedDestination = intent.redirect ?? decoded; - const encodedDestination = decodedDestination - .split("/") - .map((x) => encodeURIComponent(x)) - .join("/"); + const decodedDestination = intent.redirect ?? decodedPathname; + const encodedDestination = encodePath(decodedDestination); + + /** + * The canonical path we serve an asset at is the decoded and re-encoded version. + * Thus we need to redirect if that is different from the decoded version. + * We combine this with other redirects (e.g. for html_handling) to avoid multiple redirects. + */ if (encodedDestination !== pathname || intent.redirect) { return new TemporaryRedirectResponse(encodedDestination + search); } @@ -370,7 +370,6 @@ const htmlHandlingForceTrailingSlash = async ( return { asset: { eTag: exactETag, status: 200 }, redirect: null, - file: pathname, }; } else if ( (redirectResult = await safeRedirect( @@ -653,3 +652,44 @@ const safeRedirect = async ( return null; }; +/** + * + * +===========================================+===========+======================+ + * | character type | fetch() | encodeURIComponent() | + * +===========================================+===========+======================+ + * | unreserved ASCII e.g. a-z | unchanged | unchanged | + * +-------------------------------------------+-----------+----------------------+ + * | reserved (sometimes encoded) | unchanged | encoded | + * | e.g. [ ] @ $ ! ' ( ) * + , ; = : ? # & % | | | + * +-------------------------------------------+-----------+----------------------+ + * | non-ASCII e.g. ü. and space | encoded | encoded | + * +-------------------------------------------+-----------+----------------------+ + * + * 1. Decode incoming path to handle non-ASCII characters or optionally encoded characters (e.g. square brackets) + * 2. Match decoded path to manifest + * 3. Re-encode the path and redirect if the re-encoded path is different from the original path + * + * If the user uploads a file that is already URL-encoded, that is accessible only at the (double) encoded path. + * e.g. /%5Bboop%5D.html is served at /%255Bboop%255D only + * + * */ + +/** + * Decode all incoming paths to ensure that we can handle paths with non-ASCII characters. + */ +const decodePath = (pathname: string) => { + return pathname + .split("/") + .map((x) => decodeURIComponent(x)) + .join("/"); +}; +/** + * Use the encoded path as the canonical path for sometimes-encoded characters + * e.g. /[boop] -> /%5Bboop%5D 307 + */ +const encodePath = (pathname: string) => { + return pathname + .split("/") + .map((x) => encodeURIComponent(x)) + .join("/"); +}; diff --git a/packages/workers-shared/utils/helpers.ts b/packages/workers-shared/utils/helpers.ts index dff7c6d30729..901726880aa7 100644 --- a/packages/workers-shared/utils/helpers.ts +++ b/packages/workers-shared/utils/helpers.ts @@ -1,8 +1,12 @@ +import { isAbsolute, sep } from "node:path"; import { getType } from "mime"; -/** normalises sep for windows */ -export const normalizeFilePath = (filePath: string, sep: string) => { - const encodedPath = filePath.split(sep).join("/"); +/** normalises sep for windows and prefix with `/` */ +export const normalizeFilePath = (relativeFilepath: string) => { + if (!isAbsolute(relativeFilepath)) { + throw new Error(`Expected relative path`); + } + const encodedPath = relativeFilepath.split(sep).join("/"); return "/" + encodedPath; }; diff --git a/packages/workers-shared/utils/tsconfig.json b/packages/workers-shared/utils/tsconfig.json index 965b1378614d..33c65059b33a 100644 --- a/packages/workers-shared/utils/tsconfig.json +++ b/packages/workers-shared/utils/tsconfig.json @@ -4,7 +4,7 @@ "lib": ["es2021"], "module": "NodeNext", "moduleResolution": "nodenext", - "types": ["@cloudflare/workers-types/experimental"], + "types": ["@cloudflare/workers-types/experimental", "@types/node"], "noEmit": true, "isolatedModules": true, "allowSyntheticDefaultImports": true, diff --git a/packages/wrangler/src/experimental-assets.ts b/packages/wrangler/src/experimental-assets.ts index 273fd7921036..cc4bd1412b1a 100644 --- a/packages/wrangler/src/experimental-assets.ts +++ b/packages/wrangler/src/experimental-assets.ts @@ -263,7 +263,7 @@ export const buildAssetManifest = async (dir: string) => { `Ensure all assets in your assets directory "${dir}" conform with the Workers maximum size requirement.` ); } - manifest[normalizeFilePath(relativeFilepath, path.sep)] = { + manifest[normalizeFilePath(relativeFilepath)] = { hash: hashFile(filepath), size: filestat.size, }; diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 4b70f4ab449d..031639717fa3 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -6,6 +6,12 @@ settings: catalogs: default: + '@vitest/runner': + specifier: ~2.1.1 + version: 2.1.1 + '@vitest/snapshot': + specifier: ~2.1.1 + version: 2.1.1 vitest: specifier: ~2.1.1 version: 2.1.1 From 86a255f28503fa9c9b11f97c8001f0487a77d8ed Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Fri, 20 Sep 2024 13:18:27 +0100 Subject: [PATCH 5/7] changeset --- .changeset/shaggy-peas-notice.md | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 .changeset/shaggy-peas-notice.md diff --git a/.changeset/shaggy-peas-notice.md b/.changeset/shaggy-peas-notice.md new file mode 100644 index 000000000000..ed15c3e20492 --- /dev/null +++ b/.changeset/shaggy-peas-notice.md @@ -0,0 +1,11 @@ +--- +"@cloudflare/workers-shared": patch +"miniflare": patch +"wrangler": patch +--- + +fix: remove filepath encoding on asset upload and handle sometimes-encoded characters + +Some characters like [ ] @ are encoded by encodeURIComponent() but are often requested at an unencoded filepath. +This change will make assets with filenames with these characters accessible at both the encoded and unencoded paths, +but use the encoded path as the canonical one. From 6860d9590e0ee757130111355e6906be98ad3fa9 Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Fri, 20 Sep 2024 15:18:32 +0100 Subject: [PATCH 6/7] fix and expand fixture tests --- .../public/about/%5Bboop%5D.html | 1 + .../public/about/[boop].html | 1 + .../public/about/[f\303\274nky].txt" | 1 + .../tests/index.test.ts | 26 +++++++++++++++++++ .../public/about/%5Bboop%5D.html | 1 + .../public/about/%5Bwomp%5D.html | 1 + .../public/about/[boop].html | 1 + .../workers-with-assets/tests/index.test.ts | 23 +++++++++++++--- .../src/workers/assets/assets-kv.worker.ts | 9 ++++++- .../asset-worker/src/handler.ts | 3 +-- .../workers-shared/asset-worker/src/index.ts | 5 ++-- packages/workers-shared/utils/helpers.ts | 5 ++-- 12 files changed, 66 insertions(+), 11 deletions(-) create mode 100644 fixtures/workers-with-assets-only/public/about/%5Bboop%5D.html create mode 100644 fixtures/workers-with-assets-only/public/about/[boop].html create mode 100644 "fixtures/workers-with-assets-only/public/about/[f\303\274nky].txt" create mode 100644 fixtures/workers-with-assets/public/about/%5Bboop%5D.html create mode 100644 fixtures/workers-with-assets/public/about/%5Bwomp%5D.html create mode 100644 fixtures/workers-with-assets/public/about/[boop].html diff --git a/fixtures/workers-with-assets-only/public/about/%5Bboop%5D.html b/fixtures/workers-with-assets-only/public/about/%5Bboop%5D.html new file mode 100644 index 000000000000..3d447362efc7 --- /dev/null +++ b/fixtures/workers-with-assets-only/public/about/%5Bboop%5D.html @@ -0,0 +1 @@ +

%255Bboop%255D.html

diff --git a/fixtures/workers-with-assets-only/public/about/[boop].html b/fixtures/workers-with-assets-only/public/about/[boop].html new file mode 100644 index 000000000000..37af005091b5 --- /dev/null +++ b/fixtures/workers-with-assets-only/public/about/[boop].html @@ -0,0 +1 @@ +

[boop].html

diff --git "a/fixtures/workers-with-assets-only/public/about/[f\303\274nky].txt" "b/fixtures/workers-with-assets-only/public/about/[f\303\274nky].txt" new file mode 100644 index 000000000000..dbb84844bd43 --- /dev/null +++ "b/fixtures/workers-with-assets-only/public/about/[f\303\274nky].txt" @@ -0,0 +1 @@ +This should work. \ No newline at end of file diff --git a/fixtures/workers-with-assets-only/tests/index.test.ts b/fixtures/workers-with-assets-only/tests/index.test.ts index 9737d11b1dba..16a3b2803b8a 100644 --- a/fixtures/workers-with-assets-only/tests/index.test.ts +++ b/fixtures/workers-with-assets-only/tests/index.test.ts @@ -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`); + }); }); diff --git a/fixtures/workers-with-assets/public/about/%5Bboop%5D.html b/fixtures/workers-with-assets/public/about/%5Bboop%5D.html new file mode 100644 index 000000000000..7a0e3696b1dc --- /dev/null +++ b/fixtures/workers-with-assets/public/about/%5Bboop%5D.html @@ -0,0 +1 @@ +

%5Bboop%5D.html

diff --git a/fixtures/workers-with-assets/public/about/%5Bwomp%5D.html b/fixtures/workers-with-assets/public/about/%5Bwomp%5D.html new file mode 100644 index 000000000000..de76167b2399 --- /dev/null +++ b/fixtures/workers-with-assets/public/about/%5Bwomp%5D.html @@ -0,0 +1 @@ +

womp

diff --git a/fixtures/workers-with-assets/public/about/[boop].html b/fixtures/workers-with-assets/public/about/[boop].html new file mode 100644 index 000000000000..37af005091b5 --- /dev/null +++ b/fixtures/workers-with-assets/public/about/[boop].html @@ -0,0 +1 @@ +

[boop].html

diff --git a/fixtures/workers-with-assets/tests/index.test.ts b/fixtures/workers-with-assets/tests/index.test.ts index 1c620282574b..a6de8dfa13e8 100644 --- a/fixtures/workers-with-assets/tests/index.test.ts +++ b/fixtures/workers-with-assets/tests/index.test.ts @@ -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 ({ diff --git a/packages/miniflare/src/workers/assets/assets-kv.worker.ts b/packages/miniflare/src/workers/assets/assets-kv.worker.ts index f90fdcd74074..9bd584cb0003 100644 --- a/packages/miniflare/src/workers/assets/assets-kv.worker.ts +++ b/packages/miniflare/src/workers/assets/assets-kv.worker.ts @@ -26,7 +26,14 @@ export default >{ 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 diff --git a/packages/workers-shared/asset-worker/src/handler.ts b/packages/workers-shared/asset-worker/src/handler.ts index fa1556c2db18..111279afa2ba 100644 --- a/packages/workers-shared/asset-worker/src/handler.ts +++ b/packages/workers-shared/asset-worker/src/handler.ts @@ -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}"`; @@ -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)) diff --git a/packages/workers-shared/asset-worker/src/index.ts b/packages/workers-shared/asset-worker/src/index.ts index b30f51b04ce0..3330fa2a702d 100644 --- a/packages/workers-shared/asset-worker/src/index.ts +++ b/packages/workers-shared/asset-worker/src/index.ts @@ -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, @@ -75,8 +75,9 @@ export default class extends WorkerEntrypoint { async unstable_canFetch(request: Request): Promise { 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", diff --git a/packages/workers-shared/utils/helpers.ts b/packages/workers-shared/utils/helpers.ts index 901726880aa7..d986fb52c9d0 100644 --- a/packages/workers-shared/utils/helpers.ts +++ b/packages/workers-shared/utils/helpers.ts @@ -3,11 +3,10 @@ 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("/"); - return "/" + encodedPath; + return "/" + relativeFilepath.split(sep).join("/"); }; export const getContentType = (absFilePath: string) => { From ec086759345b659925e31c76f8bfbee67892f507 Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Fri, 20 Sep 2024 15:21:02 +0100 Subject: [PATCH 7/7] update changeset --- .changeset/shaggy-peas-notice.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.changeset/shaggy-peas-notice.md b/.changeset/shaggy-peas-notice.md index ed15c3e20492..9c847d9b830f 100644 --- a/.changeset/shaggy-peas-notice.md +++ b/.changeset/shaggy-peas-notice.md @@ -6,6 +6,6 @@ fix: remove filepath encoding on asset upload and handle sometimes-encoded characters -Some characters like [ ] @ are encoded by encodeURIComponent() but are often requested at an unencoded filepath. +Some characters like [ ] @ are encoded by encodeURIComponent() but are often requested at an unencoded URL path. This change will make assets with filenames with these characters accessible at both the encoded and unencoded paths, -but use the encoded path as the canonical one. +but to use the encoded path as the canonical one, and to redirect requests to the canonical path if necessary.