[pull] main from cloudflare:main

.changeset/fix-infinite-loop-detection.md

@@ -0,0 +1,11 @@
+---
+"wrangler": patch
+---
+
+Fix false positive infinite loop detection for exact path redirects
+
+Fixed an issue where the redirect validation incorrectly flagged exact path redirects like `/ /index.html 200` as infinite loops. This was particularly problematic when `html_handling` is set to "none", where such redirects are valid.
+
+The fix makes the validation more specific to only block wildcard patterns (like `/* /index.html`) that would actually cause infinite loops, while allowing exact path matches that are valid in certain configurations.
+
+Fixes: https://github.com/cloudflare/workers-sdk/issues/11824

packages/miniflare/src/plugins/assets/index.ts

@@ -112,7 +112,9 @@ export const ASSETS_PLUGIN: Plugin<typeof AssetsOptionsSchema> = {
 
 		let parsedRedirects: AssetConfig["redirects"] | undefined;
 		if (redirectsContents !== undefined) {
-			const redirects = parseRedirects(redirectsContents);
+			const redirects = parseRedirects(redirectsContents, {
+				htmlHandling: options.assets.assetConfig?.html_handling,
+			});
 			parsedRedirects = RedirectsSchema.parse(
 				constructRedirects({
 					redirects,

packages/quick-edit-extension/README.md

@@ -0,0 +1,4 @@
+# Quick Edit VS Code Extension
+
+The code in this package is used by the `quick-edit` package, which in turn is used in the `workers-playground` package.
+To debug this package locally check out the [Quick Edit package README](../quick-edit/README.md).

packages/quick-edit/README.md

@@ -2,18 +2,39 @@
 
 This package contains Cloudflare's fork of VSCode for Web, to support web editing of Workers. This package contains code and setup for building VSCode for Web.
 
+The code in this package is used by the `workers-playground` package.
+
 ## Developing
 
-Currently it's not possible to run VSCode's dev server to develop patches. This is because of a limitation with the `wrangler dev` file server for Workers Assets and the number of assets that would be watched locally.
+Note: Currently it's not possible to run VSCode's dev server to develop patches. This is because of a limitation with the `wrangler dev` file server for Workers Assets and the number of assets that would be watched locally.
 
 ## Building
 
-1. You must switch your NodeJS version to NodeJS 22 (using a tool like nvm). VSCode's build process requires this. For instance, if you use `nvm`, running `nvm use` would be enough to switch to the correct NodeJS version.
-2. Run `pnpm install`
-3. Run `pnpm run setup`, which will install dependencies, clone VSCode (currently v1.102.1), apply the patches specified in `./patches`, and symlink the top level packages within `workers-sdk`.
-4. Run `pnpm run custom:build`. It's `custom:build` rather than `build` because it's _really slow_, and shouldn't be regularly run by people building other packages in the repo.
-
-You should then be able to test out the local VSCode for Web instance by running `pnpm wrangler dev` at http://localhost:8787
+- Use Node.js 22
+  - VSCode's build process requires this
+- Run `pnpm install`
+- Run `pnpm -F "@cloudflare/quick-edit" run setup`
+  - installs dependencies
+  - clones VSCode (currently v1.102.1)
+  - applies the patches specified in `./patches`
+  - symlinks the top level packages within `workers-sdk`
+- Run `pnpm -F "@cloudflare/quick-edit" run custom:build`
+  - This takes up to 15 mins to run
+  - It's not `build` because it would then run when people run the top level `pnpm run build` to build other packages in the repository.
+
+## Debugging
+
+- Change the `quickEditHost` constant in [workers-playground/src/QuickEditor/VSCodeEditor.tsx](../workers-playground/src/QuickEditor/VSCodeEditor.tsx) to http://localhost:8787
+- Run the following in parallel:
+  - `pnpm -F workers-playground dev` -> `http://localhost:5173`
+  - `pnpm -F "@cloudflare/quick-edit" exec wrangler dev` -> `http://localhost:8787`
+- Get a user token to connect to a preview Worker
+  - Navigate to https://playground-testing.devprod.cloudflare.dev/
+  - This will respond with a `error code: 522` response, which is expected
+  - Grab the `user` cookie from the 522 response
+- Open http://localhost:5173/playground
+  - Open the devtools and add the `user` cookie from the previous step to this site
+  - Refresh the page and play around with the editor
 
 ## Deployment
 

packages/vite-plugin-cloudflare/src/asset-config.ts

@@ -108,7 +108,9 @@ export function getAssetsConfig(
 		redirectsContents &&
 		RedirectsSchema.parse(
 			constructRedirects({
-				redirects: parseRedirects(redirectsContents),
+				redirects: parseRedirects(redirectsContents, {
+					htmlHandling: assetsConfig?.html_handling,
+				}),
 				redirectsFile,
 				logger,
 			}).redirects

packages/workers-shared/utils/configuration/parseRedirects.ts

@@ -7,6 +7,7 @@ import {
 	SPLAT_REGEX,
 } from "./constants";
 import { urlHasHost, validateUrl } from "./validateURL";
+import type { AssetConfig } from "../types";
 import type {
 	InvalidRedirectRule,
 	ParsedRedirects,
@@ -17,10 +18,12 @@ import type {
 export function parseRedirects(
 	input: string,
 	{
+		htmlHandling = undefined,
 		maxStaticRules = MAX_STATIC_REDIRECT_RULES,
 		maxDynamicRules = MAX_DYNAMIC_REDIRECT_RULES,
 		maxLineLength = MAX_LINE_LENGTH,
 	}: {
+		htmlHandling?: AssetConfig["html_handling"];
 		maxStaticRules?: number;
 		maxDynamicRules?: number;
 		maxLineLength?: number;
@@ -122,10 +125,21 @@ export function parseRedirects(
 			continue;
 		}
 
-		// We want to always block the `/* /index.html` redirect - this will cause TOO_MANY_REDIRECTS errors as
-		// the asset worker will redirect it back to `/`, removing the `/index.html`. This is the case for regular
-		// redirects, as well as proxied (200) rewrites. We only want to run this on relative urls
-		if (/\/\*?$/.test(from) && /\/index(.html)?$/.test(to) && !urlHasHost(to)) {
+		// Here we reject two patterns:
+		// 1. `/* /index.html` Is always rejected.
+		// 2. `/ /index` Is rejected when HTML handling is enabled.
+		// Allowing the redirect in other cases will cause TOO_MANY_REDIRECTS errors as the asset Worker will
+		// redirect it back to `/` by removing the `/index.html`.
+		// We only want to run this on relative URLs.
+		const hasRelativePath = !urlHasHost(to);
+		const hasWildcardToIndex =
+			/\/\*$/.test(from) && /\/index(.html)?$/.test(to);
+		const hasRootToIndex = /\/$/.test(from) && /\/index(.html)?$/.test(to);
+		const hasHTMLHandling = htmlHandling !== "none"; // HTML handling is enabled by default.
+		if (
+			hasRelativePath &&
+			(hasWildcardToIndex || (hasRootToIndex && hasHTMLHandling))
+		) {
 			invalid.push({
 				line,
 				lineNumber: i + 1,

packages/workers-shared/utils/tests/parseRedirects.invalid.test.ts

@@ -275,42 +275,18 @@ test("parseRedirects should reject non-relative URLs for proxying (200) redirect
 	});
 });
 
-test("parseRedirects should reject '/* /index.html'", () => {
+test("parseRedirects should reject wildcard patterns to index", () => {
 	const input = `
 /* /index.html 200
 /* /index 200
 / /index.html
 / /index
-/* /foo/index.html
-
-/* /foo
-/foo/* /bar 200
-/ /foo
 `;
 	const invalidRedirectError =
 		"Infinite loop detected in this rule and has been ignored. This will cause a redirect to strip `.html` or `/index` and end up triggering this rule again. Please fix or remove this rule to silence this warning.";
 	const result = parseRedirects(input);
 	expect(result).toEqual({
-		rules: [
-			{
-				from: "/*",
-				status: 302,
-				to: "/foo",
-				lineNumber: 8,
-			},
-			{
-				from: "/foo/*",
-				status: 200,
-				to: "/bar",
-				lineNumber: 9,
-			},
-			{
-				from: "/",
-				status: 302,
-				to: "/foo",
-				lineNumber: 10,
-			},
-		],
+		rules: [],
 		invalid: [
 			{
 				line: "/* /index.html 200",
@@ -332,11 +308,49 @@ test("parseRedirects should reject '/* /index.html'", () => {
 				lineNumber: 5,
 				message: invalidRedirectError,
 			},
+		],
+	});
+});
+
+test("parseRedirects should allow root patterns to index when HTML handling disabled", () => {
+	// This test documents the fix for https://github.com/cloudflare/workers-sdk/issues/11824
+	// Exact path matches like "/ /index.html" are valid when html_handling is "none"
+	// because there's no automatic index.html serving, so no infinite loop occurs.
+	// However, wildcard patterns like "/* /index.html" should still be blocked.
+	const input = `
+/* /index.html 200
+/* /index 200
+/ /index.html
+/ /index
+`;
+	const invalidRedirectError =
+		"Infinite loop detected in this rule and has been ignored. This will cause a redirect to strip `.html` or `/index` and end up triggering this rule again. Please fix or remove this rule to silence this warning.";
+	const result = parseRedirects(input, { htmlHandling: "none" });
+	expect(result).toEqual({
+		rules: [
+			{
+				from: "/",
+				status: 302,
+				to: "/index.html",
+				lineNumber: 4,
+			},
+		],
+		invalid: [
+			{
+				line: "/* /index.html 200",
+				lineNumber: 2,
+				message: invalidRedirectError,
+			},
 			{
-				line: "/* /foo/index.html",
-				lineNumber: 6,
+				line: "/* /index 200",
+				lineNumber: 3,
 				message: invalidRedirectError,
 			},
+			{
+				line: "/ /index",
+				lineNumber: 5,
+				message: "Ignoring duplicate rule for path /.",
+			},
 		],
 	});
 });

packages/wrangler/src/miniflare-cli/assets.ts

@@ -155,7 +155,9 @@ async function generateAssetsFetch(
 	let redirects: ParsedRedirects | undefined;
 	if (existsSync(redirectsFile)) {
 		const contents = readFileSync(redirectsFile, "utf-8");
-		redirects = parseRedirects(contents);
+		redirects = parseRedirects(contents, {
+			htmlHandling: undefined, // Pages dev server doesn't expose html_handling configuration in this context.
+		});
 	}
 
 	let headers: ParsedHeaders | undefined;
@@ -185,7 +187,9 @@ async function generateAssetsFetch(
 				case redirectsFile: {
 					log.log("_redirects modified. Re-evaluating...");
 					const contents = readFileSync(redirectsFile).toString();
-					redirects = parseRedirects(contents);
+					redirects = parseRedirects(contents, {
+						htmlHandling: undefined, // Pages dev server doesn't expose html_handling configuration in this context.
+					});
 					break;
 				}
 			}