feat (pages): Proxying (200) in _redirects (#2708)

* feat (pages): Proxying (200) in _redirects * Add tests * Reuse the validateUrl method for relative check * parseRedirects tests
cloudflare · Feb 16, 2023 · b3346cf · b3346cf
1 parent 0f29093
commit b3346cf
Show file tree

Hide file tree

Showing 11 changed files with 125 additions and 14 deletions.
diff --git a/.changeset/moody-bats-repair.md b/.changeset/moody-bats-repair.md
@@ -0,0 +1,12 @@
+---
+"@cloudflare/pages-shared": minor
+"wrangler": minor
+---
+
+Feat: Pages now supports Proxying (200 status) redirects in it's \_redirects file
+
+This will look something like the following, where a request to /users/123 will appear as that in the browser, but will internally go to /users/[id].html.
+
+```
+/users/:id /users/[id] 200
+```
diff --git a/fixtures/pages-functions-app/public/_redirects b/fixtures/pages-functions-app/public/_redirects
@@ -1 +1,2 @@
-/redirect /me
+/redirect /me
+/users/:id /users/[id] 200
diff --git a/fixtures/pages-functions-app/public/users/[id].html b/fixtures/pages-functions-app/public/users/[id].html
@@ -0,0 +1,6 @@
+<!DOCTYPE html>
+<html>
+	<body>
+		<h1>Hello, /users/[id]!</h1>
+	</body>
+</html>
diff --git a/fixtures/pages-functions-app/tests/index.test.ts b/fixtures/pages-functions-app/tests/index.test.ts
@@ -209,6 +209,15 @@ describe.concurrent("Pages Functions", () => {
 			expect(response.status).toEqual(302);
 			expect(response.headers.get("Location")).toEqual("/me");
 		});
+
+		it("should support proxying (200) redirects", async ({ expect }) => {
+			const response = await fetch(`http://${ip}:${port}/users/123`, {
+				redirect: "manual",
+			});
+			const text = await response.text();
+			expect(response.status).toEqual(200);
+			expect(text).toContain("Hello, /users/[id]!");
+		});
 	});
 
 	describe.concurrent("headers", () => {

diff --git a/packages/pages-shared/__tests__/asset-server/handler.test.ts b/packages/pages-shared/__tests__/asset-server/handler.test.ts
@@ -17,11 +17,18 @@ describe("asset-server handler", () => {
 					from: `/${status}`,
 					to: "/home",
 				}))
-				.concat({
-					status: 302,
-					from: "/500",
-					to: "/home",
-				})
+				.concat(
+					{
+						status: 302,
+						from: "/500",
+						to: "/home",
+					},
+					{
+						status: 200,
+						from: "/200",
+						to: "/proxied-file",
+					}
+				)
 		);
 
 		const tests = statuses.map(async (status) => {
@@ -43,6 +50,20 @@ describe("asset-server handler", () => {
 
 		expect(response.status).toBe(302);
 		expect(response.headers.get("Location")).toBe("/home");
+
+		const { response: proxyResponse } = await getTestResponse({
+			request: "https://foo.com/200",
+			metadata,
+			findAssetEntryForPath: async (path: string) => {
+				if (path === "/proxied-file.html") {
+					return "proxied file";
+				}
+				return null;
+			},
+		});
+
+		expect(proxyResponse.status).toBe(200);
+		expect(proxyResponse.headers.get("Location")).toBeNull();
 	});
 
 	test("Match exact pathnames, before any HTML redirection", async () => {

diff --git a/packages/pages-shared/__tests__/metadata-generator/parseRedirects.invalid.test.ts b/packages/pages-shared/__tests__/metadata-generator/parseRedirects.invalid.test.ts
@@ -34,18 +34,18 @@ test("parseRedirects should reject invalid status codes", () => {
 	const input = `
     # Valid token sails through
     /a /b 301
-    # 200 NOT OK
-    /c /d 200
+    # 418 NOT OK
+    /c /d 418
   `;
 	const result = parseRedirects(input);
 	expect(result).toEqual({
 		rules: [{ from: "/a", status: 301, to: "/b", lineNumber: 3 }],
 		invalid: [
 			{
-				line: `/c /d 200`,
+				line: `/c /d 418`,
 				lineNumber: 5,
 				message:
-					"Valid status codes are 301, 302 (default), 303, 307, or 308. Got 200.",
+					"Valid status codes are 200, 301, 302 (default), 303, 307, or 308. Got 418.",
 			},
 		],
 	});
@@ -255,3 +255,21 @@ test("parseRedirects should reject malformed URLs", () => {
 		],
 	});
 });
+
+test("parseRedirects should reject non-relative URLs for proxying (200) redirects", () => {
+	const input = `
+	/a https://example.com/b 200
+`;
+	const result = parseRedirects(input);
+	expect(result).toEqual({
+		rules: [],
+		invalid: [
+			{
+				line: `/a https://example.com/b 200`,
+				lineNumber: 2,
+				message:
+					"Proxy (200) redirects can only point to relative paths. Got https://example.com/b",
+			},
+		],
+	});
+});
diff --git a/packages/pages-shared/__tests__/metadata-generator/parseRedirects.valid.test.ts b/packages/pages-shared/__tests__/metadata-generator/parseRedirects.valid.test.ts
@@ -120,3 +120,21 @@ test("parseRedirects should preserve fragments which contain a hash sign and are
 		invalid: [],
 	});
 });
+
+test("parseRedirects should accept 200 (proxying) redirects", () => {
+	const input = `
+	/a /b 200
+`;
+	const result = parseRedirects(input);
+	expect(result).toEqual({
+		rules: [
+			{
+				from: "/a",
+				status: 200,
+				to: "/b",
+				lineNumber: 2,
+			},
+		],
+		invalid: [],
+	});
+});
diff --git a/packages/pages-shared/asset-server/handler.ts b/packages/pages-shared/asset-server/handler.ts
@@ -204,7 +204,16 @@ export async function generateHandler<
 			staticRedirectsMatcher() || generateRedirectsMatcher()({ request })[0];
 
 		if (match) {
-			return getResponseFromMatch(match, url);
+			if (match.status === 200) {
+				// A 200 redirect means that we are proxying to a different asset, for example,
+				// a request with url /users/12345 could be pointed to /users/id.html. In order to
+				// do this, we overwrite the pathname, and instead match for assets with that url,
+				// and importantly, do not use the regular redirect handler - as the url visible to
+				// the user does not change
+				pathname = new URL(match.to, request.url).pathname;
+			} else {
+				return getResponseFromMatch(match, url);
+			}
 		}
 
 		if (!request.method.match(/^(get|head)$/i)) {

diff --git a/packages/pages-shared/metadata-generator/constants.ts b/packages/pages-shared/metadata-generator/constants.ts
@@ -3,7 +3,7 @@ export const HEADERS_VERSION = 2;
 export const ANALYTICS_VERSION = 1;
 export const ROUTES_JSON_VERSION = 1;
 
-export const PERMITTED_STATUS_CODES = new Set([301, 302, 303, 307, 308]);
+export const PERMITTED_STATUS_CODES = new Set([200, 301, 302, 303, 307, 308]);
 export const HEADER_SEPARATOR = ":";
 export const MAX_LINE_LENGTH = 2000;
 export const MAX_HEADER_RULES = 100;

diff --git a/packages/pages-shared/metadata-generator/parseRedirects.ts b/packages/pages-shared/metadata-generator/parseRedirects.ts
@@ -104,7 +104,7 @@ export function parseRedirects(input: string): ParsedRedirects {
 			invalid.push({
 				line,
 				lineNumber: i + 1,
-				message: `Valid status codes are 301, 302 (default), 303, 307, or 308. Got ${str_status}.`,
+				message: `Valid status codes are 200, 301, 302 (default), 303, 307, or 308. Got ${str_status}.`,
 			});
 			continue;
 		}
@@ -119,6 +119,22 @@ export function parseRedirects(input: string): ParsedRedirects {
 		}
 		seen_paths.add(from);
 
+		if (status === 200) {
+			// Error can only be that it's not relative - given validateUrl is called above without onlyRelative:true,
+			// so if it's present, we can error to the user that proxying only expects relative urls
+			const [proxyTo, error] = validateUrl(to, true, true, true);
+			if (error) {
+				invalid.push({
+					line,
+					lineNumber: i + 1,
+					message: `Proxy (200) redirects can only point to relative paths. Got ${to}`,
+				});
+				continue;
+			}
+			rules.push({ from, to: proxyTo as string, status, lineNumber: i + 1 });
+			continue;
+		}
+
 		rules.push({ from, to, status, lineNumber: i + 1 });
 	}
 

diff --git a/packages/pages-shared/package.json b/packages/pages-shared/package.json
@@ -14,7 +14,8 @@
 	],
 	"scripts": {
 		"check:type": "tsc",
-		"test": "jest"
+		"test": "jest",
+		"test:ci": "jest"
 	},
 	"jest": {
 		"coverageReporters": [