workers assets shared routing cleanup

.changeset/thin-pianos-create.md

@@ -0,0 +1,7 @@
+---
+"@cloudflare/workers-shared": minor
+---
+
+Track alternate URL paths in Workers Assets
+
+Add internal telemetry to identify when encoded or repeated-slash paths would produce different routing decisions. Customer request handling is unchanged.

packages/workers-shared/asset-worker/src/analytics.ts

@@ -64,6 +64,10 @@ type Data = {
 	compatibilityFlags?: string[]; // converted into a bitmask
 	// double7 - Entrypoint discriminator (see EntrypointType enum)
 	entrypoint?: EntrypointType;
+	// double8 - Canonicalization performed while in shadow mode
+	pathNormalization?: number;
+	// double9 - Shadow decisions that differ from current behavior
+	pathNormalizationDifference?: number;
 
 	// -- Blobs --
 	// blob1 - Hostname of the request
@@ -142,6 +146,8 @@ export class Analytics {
 				this.data.status ?? -1, // double5
 				compatibilityFlagsBitmask, // double6
 				this.data.entrypoint ?? -1, // double7
+				this.data.pathNormalization ?? 0, // double8
+				this.data.pathNormalizationDifference ?? 0, // double9
 			],
 			blobs: [
 				this.data.hostname?.substring(0, 256), // blob1 - trim to 256 bytes

packages/workers-shared/asset-worker/src/handler.ts

@@ -16,13 +16,19 @@ import {
 	flagIsEnabled,
 	SEC_FETCH_MODE_NAVIGATE_HEADER_PREFERS_ASSET_SERVING,
 } from "./compatibility-flags";
-import { attachCustomHeaders, getAssetHeaders } from "./utils/headers";
+import { canonicalizePath } from "./utils/canonical-path";
+import {
+	attachCustomHeaders,
+	getAssetHeaders,
+	getCustomHeaderMatches,
+} from "./utils/headers";
 import {
 	generateRedirectsMatcher,
 	staticRedirectsMatcher,
 } from "./utils/rules-engine";
 import type { AssetConfig } from "../../utils/types";
 import type { Analytics, ServedBy } from "./analytics";
+import type { CanonicalRoutingPath } from "./utils/canonical-path";
 import type EntrypointType from "./worker";
 import type { Env } from "./worker";
 
@@ -36,6 +42,12 @@ type AssetIntent = {
 
 export type AssetIntentWithResolver = AssetIntent & { resolver: Resolver };
 
+// Bitmask of rule decisions that differ between raw and canonical paths.
+const enum CanonicalPathRuleDifference {
+	Redirect = 1 << 0,
+	Headers = 1 << 1,
+}
+
 const getResponseOrAssetIntent = async (
 	request: Request,
 	env: Env,
@@ -45,6 +57,8 @@ const getResponseOrAssetIntent = async (
 ): Promise<Response | AssetIntentWithResolver> => {
 	const url = new URL(request.url);
 	const { search } = url;
+	// Shadow-only: raw-path matching remains authoritative.
+	recordCanonicalPathRuleDifferences(request, configuration, analytics);
 
 	const redirectResult = handleRedirects(
 		env,
@@ -1039,9 +1053,7 @@ const handleRedirects = (
 ): { proxied: boolean; pathname: string } | Response => {
 	const jaeger = env.JAEGER ?? mockJaegerBinding();
 	return jaeger.enterSpan("handle_redirects", (span) => {
-		const redirectMatch =
-			staticRedirectsMatcher(configuration, host, pathname) ||
-			generateRedirectsMatcher(configuration)({ request })[0];
+		const redirectMatch = getRedirectMatch(request, configuration, host);
 
 		let proxied = false;
 		if (redirectMatch) {
@@ -1101,3 +1113,73 @@ const handleRedirects = (
 		return { proxied, pathname };
 	});
 };
+
+// Returns the first matching redirect rule, optionally using a canonical path.
+export function getRedirectMatch(
+	request: Request,
+	configuration: Required<AssetConfig>,
+	host: string,
+	canonicalPath?: CanonicalRoutingPath
+) {
+	const pathname = canonicalPath ?? new URL(request.url).pathname;
+	return (
+		staticRedirectsMatcher(configuration, host, pathname) ||
+		generateRedirectsMatcher(configuration)({ request, canonicalPath })[0]
+	);
+}
+
+/**
+ * Records rule differences caused by canonicalization without changing the
+ * raw-path response.
+ */
+function recordCanonicalPathRuleDifferences(
+	request: Request,
+	configuration: Required<AssetConfig>,
+	analytics?: Analytics
+) {
+	if (!analytics) {
+		return;
+	}
+
+	const url = new URL(request.url);
+	const canonicalPath = canonicalizePath(url.pathname);
+	if (canonicalPath.routingPath === url.pathname) {
+		return;
+	}
+
+	let difference = 0;
+	// Compare selected _redirects rules, not the response they would produce.
+	const currentRedirect = getRedirectMatch(request, configuration, url.host);
+	const canonicalRedirect = getRedirectMatch(
+		request,
+		configuration,
+		url.host,
+		canonicalPath.routingPath
+	);
+	if (ruleMatchesDiffer(currentRedirect, canonicalRedirect)) {
+		difference |= CanonicalPathRuleDifference.Redirect;
+	}
+
+	// Compare selected _headers rules after placeholder substitution.
+	const currentHeaders = getCustomHeaderMatches(request, configuration);
+	const canonicalHeaders = getCustomHeaderMatches(
+		request,
+		configuration,
+		canonicalPath.routingPath
+	);
+	if (ruleMatchesDiffer(currentHeaders, canonicalHeaders)) {
+		difference |= CanonicalPathRuleDifference.Headers;
+	}
+
+	if (difference !== 0) {
+		analytics.setData({
+			pathNormalization: canonicalPath.normalization,
+			pathNormalizationDifference: difference,
+		});
+	}
+}
+
+// Matcher outputs are JSON-safe and generated in deterministic rule order.
+function ruleMatchesDiffer(left: unknown, right: unknown): boolean {
+	return JSON.stringify(left) !== JSON.stringify(right);
+}

packages/workers-shared/asset-worker/src/utils/canonical-path.ts

@@ -0,0 +1,71 @@
+declare const routingPathBrand: unique symbol;
+declare const assetPathBrand: unique symbol;
+
+// Replaces raw URL pathname handling for routing and rule matching after rollout.
+export type CanonicalRoutingPath = string & {
+	readonly [routingPathBrand]: "CanonicalRoutingPath";
+};
+
+// Reserved for asset lookup after decoding exactly once.
+export type DecodedAssetPath = string & {
+	readonly [assetPathBrand]: "DecodedAssetPath";
+};
+
+// Bitmask of transformations observed while canonicalizing a request path.
+export const enum PathNormalization {
+	None = 0,
+	Decoded = 1 << 0,
+	CollapsedSlashes = 1 << 1,
+	MalformedEncoding = 1 << 2,
+	Reencoded = 1 << 3,
+}
+
+export type CanonicalPath = {
+	routingPath: CanonicalRoutingPath;
+	assetPath: DecodedAssetPath;
+	normalization: PathNormalization;
+};
+
+// Returns both path representations so routing cannot accidentally reuse lookup input.
+export function canonicalizePath(pathname: string): CanonicalPath {
+	let decodedPathname = pathname;
+	let normalization = PathNormalization.None;
+
+	try {
+		decodedPathname = decodeURIComponent(pathname);
+		if (decodedPathname !== pathname) {
+			normalization |= PathNormalization.Decoded;
+		}
+	} catch {
+		normalization |= PathNormalization.MalformedEncoding;
+	}
+
+	const collapsedPathname = decodedPathname.replace(/\/{2,}/g, "/");
+	if (collapsedPathname !== decodedPathname) {
+		normalization |= PathNormalization.CollapsedSlashes;
+	}
+
+	const routingPath = encodePath(collapsedPathname);
+	if (routingPath !== collapsedPathname) {
+		normalization |= PathNormalization.Reencoded;
+	}
+
+	return {
+		routingPath: routingPath as CanonicalRoutingPath,
+		assetPath: collapsedPathname as DecodedAssetPath,
+		normalization,
+	};
+}
+
+function encodePath(pathname: string): string {
+	return pathname
+		.split("/")
+		.map((segment) => {
+			try {
+				return encodeURIComponent(segment);
+			} catch {
+				return segment;
+			}
+		})
+		.join("/");
+}

packages/workers-shared/asset-worker/src/utils/headers.ts

@@ -9,6 +9,7 @@ import { generateRulesMatcher, replacer } from "./rules-engine";
 import type { AssetConfig, JaegerTracing } from "../../../utils/types";
 import type { AssetIntentWithResolver } from "../handler";
 import type { Env } from "../worker";
+import type { CanonicalRoutingPath } from "./canonical-path";
 
 /**
  * Returns a Headers object that contains additional headers (to those
@@ -68,23 +69,7 @@ export function attachCustomHeaders(
 ) {
 	const jaeger: JaegerTracing = env.JAEGER ?? mockJaegerBinding();
 	return jaeger.enterSpan("add_headers", (span) => {
-		// Iterate through rules and find rules that match the path
-		const headersMatcher = generateRulesMatcher(
-			configuration.headers?.version === HEADERS_VERSION
-				? configuration.headers.rules
-				: {},
-			({ set = {}, unset = [] }, replacements) => {
-				const replacedSet: Record<string, string> = {};
-				Object.entries(set).forEach(([key, value]) => {
-					replacedSet[key] = replacer(value, replacements);
-				});
-				return {
-					set: replacedSet,
-					unset,
-				};
-			}
-		);
-		const matches = headersMatcher({ request });
+		const matches = getCustomHeaderMatches(request, configuration);
 
 		// This keeps track of every header that we've set from _headers
 		// because we want to combine user declared headers but overwrite
@@ -111,3 +96,28 @@ export function attachCustomHeaders(
 		return response;
 	});
 }
+
+// Returns matching _headers rules for raw or canonical paths after substitutions.
+export function getCustomHeaderMatches(
+	request: Request,
+	configuration: Required<AssetConfig>,
+	canonicalPath?: CanonicalRoutingPath
+) {
+	const headersMatcher = generateRulesMatcher(
+		configuration.headers?.version === HEADERS_VERSION
+			? configuration.headers.rules
+			: {},
+		({ set = {}, unset = [] }, replacements) => {
+			const replacedSet: Record<string, string> = {};
+			Object.entries(set).forEach(([key, value]) => {
+				replacedSet[key] = replacer(value, replacements);
+			});
+			return {
+				set: replacedSet,
+				unset,
+			};
+		}
+	);
+
+	return headersMatcher({ request, canonicalPath });
+}

packages/workers-shared/asset-worker/src/utils/rules-engine.ts

@@ -4,6 +4,7 @@
 
 import { REDIRECTS_VERSION } from "../handler";
 import type { AssetConfig } from "../../../utils/types";
+import type { CanonicalRoutingPath } from "./canonical-path";
 
 // As the answer says, there's no downside to escaping these extra characters, so better safe than sorry
 const ESCAPE_REGEX_CHARACTERS = /[-/\\^$*+?.()|[\]{}]/g;
@@ -87,8 +88,15 @@ export const generateRulesMatcher = <T>(
 		T,
 	][];
 
-	return ({ request }: { request: Request }) => {
-		const { pathname, hostname } = new URL(request.url);
+	return ({
+		request,
+		canonicalPath,
+	}: {
+		request: Request;
+		canonicalPath?: CanonicalRoutingPath;
+	}) => {
+		const { pathname: requestPathname, hostname } = new URL(request.url);
+		const pathname = canonicalPath ?? requestPathname;
 
 		return compiledRules
 			.map(([{ crossHost, regExp }, match]) => {
@@ -161,8 +169,14 @@ export const generateRedirectsMatcher = (
 
 export const generateStaticRoutingRuleMatcher =
 	(rules: string[]) =>
-	({ request }: { request: Request }) => {
-		const { pathname } = new URL(request.url);
+	({
+		request,
+		canonicalPath,
+	}: {
+		request: Request;
+		canonicalPath?: CanonicalRoutingPath;
+	}) => {
+		const pathname = canonicalPath ?? new URL(request.url).pathname;
 		for (const rule of rules) {
 			try {
 				const regExp = generateGlobOnlyRuleRegExp(rule);

packages/workers-shared/asset-worker/tests/canonical-path.test.ts

@@ -0,0 +1,51 @@
+import { describe, it } from "vitest";
+import {
+	canonicalizePath,
+	PathNormalization,
+} from "../src/utils/canonical-path";
+
+describe("canonicalizePath", () => {
+	it("decodes once and collapses repeated slashes", ({ expect }) => {
+		const path = canonicalizePath("//%65xample%2Freport.json");
+
+		expect(path.routingPath).toBe("/example/report.json");
+		expect(path.assetPath).toBe("/example/report.json");
+		expect(path.normalization).toBe(
+			PathNormalization.Decoded | PathNormalization.CollapsedSlashes
+		);
+	});
+
+	it("does not double-decode asset names", ({ expect }) => {
+		const path = canonicalizePath("/%252Freport.json");
+
+		expect(path.routingPath).toBe("/%252Freport.json");
+		expect(path.assetPath).toBe("/%2Freport.json");
+		expect(path.normalization).toBe(
+			PathNormalization.Decoded | PathNormalization.Reencoded
+		);
+	});
+
+	it("re-encodes literal characters that have an encoded routing spelling", ({
+		expect,
+	}) => {
+		const path = canonicalizePath("/docs+draft");
+
+		expect(path.routingPath).toBe("/docs%2Bdraft");
+		expect(path.assetPath).toBe("/docs+draft");
+		expect(path.normalization).toBe(PathNormalization.Reencoded);
+	});
+
+	it("records malformed percent-encoding", ({ expect }) => {
+		const path = canonicalizePath("/%");
+
+		expect(path.routingPath).toBe("/%25");
+		expect(path.assetPath).toBe("/%");
+		expect(path.normalization).toBe(
+			PathNormalization.MalformedEncoding | PathNormalization.Reencoded
+		);
+	});
+
+	it("does not throw for an invalid Unicode character", ({ expect }) => {
+		expect(() => canonicalizePath("/\uD800")).not.toThrow();
+	});
+});