fix: ensure upstream_protocol is passed to the Worker (#4656)

* fix: ensure upstream_protocol is passed to the Worker

In `wrangler dev` it is possible to set the `upstream_protocol`,
which is the protocol under which the User Worker believes it has been
requested, as recorded in the `request.url` that can be used for
forwarding on requests to the origin.

This setting defaults to `https`. Previously, it was not being passed
to `wrangler dev` in local mode. Instead it was always set to `http`.

Note that setting `upstream_protocol` to `http` is not supported in
`wrangler dev` remote mode, which is the case since Wrangler v2.0.

Fixes #4539

.changeset/giant-kings-heal.md

@@ -0,0 +1,21 @@
+---
+"wrangler": patch
+---
+
+fix: ensure upstream_protocol is passed to the Worker
+
+In `wrangler dev` it is possible to set the `upstream_protocol`,
+which is the protocol under which the User Worker believes it has been
+requested, as recorded in the `request.url` that can be used for
+forwarding on requests to the origin.
+
+Previously, it was not being passed to `wrangler dev` in local mode.
+Instead it was always set to `http`.
+
+Note that setting `upstream_protocol` to `http` is not supported in
+`wrangler dev` remote mode, which is the case since Wrangler v2.0.
+
+This setting now defaults to `https` in remote mode (since that is the only option),
+and to the same as `local_protocol` in local mode.
+
+Fixes #4539

fixtures/worker-app/src/index.js

@@ -22,12 +22,10 @@ export default {
 			return new Response(request.url);
 		}
 
-		console.log(
-			request.method,
-			request.url,
-			new Map([...request.headers]),
-			request.cf
-		);
+		console.log("METHOD =", request.method);
+		console.log("URL = ", request.url);
+		console.log("HEADERS =", new Map([...request.headers]));
+		console.log("CF =", request.cf);
 
 		logErrors();
 

fixtures/worker-app/tests/index.test.ts

@@ -13,7 +13,7 @@ describe("'wrangler dev' correctly renders pages", () => {
 	beforeAll(async () => {
 		({ ip, port, stop, getOutput } = await runWranglerDev(
 			resolve(__dirname, ".."),
-			["--port=0", "--inspector-port=0"]
+			["--port=0", "--inspector-port=0", "--upstream-protocol=https"]
 		));
 	});
 
@@ -22,10 +22,12 @@ describe("'wrangler dev' correctly renders pages", () => {
 	});
 
 	it("renders ", async ({ expect }) => {
+		// Note that the local protocol defaults to http
 		const response = await fetch(`http://${ip}:${port}/`);
 		const text = await response.text();
 		expect(response.status).toBe(200);
-		expect(text).toContain(`http://${ip}:${port}/`);
+		// Note that the upstream protocol defaults to https
+		expect(text).toContain(`https://${ip}:${port}/`);
 
 		// Wait up to 5s for all request logs to be flushed
 		for (let i = 0; i < 10; i++) {
@@ -77,12 +79,13 @@ describe("'wrangler dev' correctly renders pages", () => {
 		expect(text).toMatch(/[0-9a-f]{16}/); // 8 hex bytes
 	});
 
-	it("passes through URL unchanged", async ({ expect }) => {
+	it("passes through URL host and path unchanged", async ({ expect }) => {
 		const url = `http://${ip}:${port}//thing?a=1`;
-		const response = await fetch(url, {
+		const response = await fetch(`http://${ip}:${port}//thing?a=1`, {
 			headers: { "X-Test-URL": "true" },
 		});
 		const text = await response.text();
-		expect(text).toBe(url);
+		// Note that the protocol changes due to the upstream_protocol configuration of Wrangler in the `beforeAll()`.
+		expect(text).toBe(`https://${ip}:${port}//thing?a=1`);
 	});
 });

packages/wrangler/src/__tests__/configuration.test.ts

@@ -33,7 +33,7 @@ describe("normalizeAndValidateConfig()", () => {
 				ip: process.platform === "win32" ? "127.0.0.1" : "localhost",
 				local_protocol: "http",
 				port: undefined, // the default of 8787 is set at runtime
-				upstream_protocol: "https",
+				upstream_protocol: "http",
 				host: undefined,
 			},
 			cloudchamber: {},

packages/wrangler/src/__tests__/dev.test.tsx

@@ -748,12 +748,12 @@ describe("wrangler dev", () => {
 	});
 
 	describe("upstream-protocol", () => {
-		it("should default upstream-protocol to `https`", async () => {
+		it("should default upstream-protocol to `https` if remote mode", async () => {
 			writeWranglerToml({
 				main: "index.js",
 			});
 			fs.writeFileSync("index.js", `export default {};`);
-			await runWrangler("dev");
+			await runWrangler("dev --remote");
 			expect((Dev as jest.Mock).mock.calls[0][0].upstreamProtocol).toEqual(
 				"https"
 			);
@@ -762,24 +762,52 @@ describe("wrangler dev", () => {
 			expect(std.err).toMatchInlineSnapshot(`""`);
 		});
 
-		it("should warn if `--upstream-protocol=http` is used", async () => {
+		it("should warn if `--upstream-protocol=http` is used in remote mode", async () => {
 			writeWranglerToml({
 				main: "index.js",
 			});
 			fs.writeFileSync("index.js", `export default {};`);
-			await runWrangler("dev --upstream-protocol=http");
+			await runWrangler("dev --upstream-protocol=http --remote");
 			expect((Dev as jest.Mock).mock.calls[0][0].upstreamProtocol).toEqual(
 				"http"
 			);
 			expect(std.out).toMatchInlineSnapshot(`""`);
 			expect(std.warn).toMatchInlineSnapshot(`
-			        "[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1mSetting upstream-protocol to http is not currently implemented.[0m
+			"[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1mSetting upstream-protocol to http is not currently supported for remote mode.[0m
 
-			          If this is required in your project, please add your use case to the following issue:
-			          [4mhttps://github.com/cloudflare/workers-sdk/issues/583[0m.
+			  If this is required in your project, please add your use case to the following issue:
+			  [4mhttps://github.com/cloudflare/workers-sdk/issues/583[0m.
 
-			        "
-		      `);
+			"
+		`);
+			expect(std.err).toMatchInlineSnapshot(`""`);
+		});
+
+		it("should default upstream-protocol to local-protocol if local mode", async () => {
+			writeWranglerToml({
+				main: "index.js",
+			});
+			fs.writeFileSync("index.js", `export default {};`);
+			await runWrangler("dev --local-protocol=https");
+			expect((Dev as jest.Mock).mock.calls[0][0].upstreamProtocol).toEqual(
+				"https"
+			);
+			expect(std.out).toMatchInlineSnapshot(`""`);
+			expect(std.warn).toMatchInlineSnapshot(`""`);
+			expect(std.err).toMatchInlineSnapshot(`""`);
+		});
+
+		it("should default upstream-protocol to http if no local-protocol in local mode", async () => {
+			writeWranglerToml({
+				main: "index.js",
+			});
+			fs.writeFileSync("index.js", `export default {};`);
+			await runWrangler("dev");
+			expect((Dev as jest.Mock).mock.calls[0][0].upstreamProtocol).toEqual(
+				"http"
+			);
+			expect(std.out).toMatchInlineSnapshot(`""`);
+			expect(std.warn).toMatchInlineSnapshot(`""`);
 			expect(std.err).toMatchInlineSnapshot(`""`);
 		});
 	});

packages/wrangler/src/config/config.ts

@@ -215,11 +215,10 @@ export interface DevConfig {
 	/**
 	 * Protocol that wrangler dev forwards requests on
 	 *
-	 * Setting this to `http` is not currently implemented.
+	 * Setting this to `http` is not currently implemented for remote mode.
 	 * See https://github.com/cloudflare/workers-sdk/issues/583
 	 *
-	 * @default `https`
-	 * @todo this needs to be implemented https://github.com/cloudflare/workers-sdk/issues/583
+	 * @default `https` in remote mode; same as local_protocol in local mode.
 	 */
 	upstream_protocol: "https" | "http";
 

packages/wrangler/src/config/validation.ts

@@ -1,3 +1,4 @@
+import assert from "node:assert";
 import path from "node:path";
 import TOML from "@iarna/toml";
 import { getConstellationWarningFromEnv } from "../constellation/utils";
@@ -52,7 +53,7 @@ const ENGLISH = new Intl.ListFormat("en");
 export function normalizeAndValidateConfig(
 	rawConfig: RawConfig,
 	configPath: string | undefined,
-	args: unknown
+	args: Record<string, unknown>
 ): {
 	config: Config;
 	diagnostics: Diagnostics;
@@ -116,9 +117,9 @@ export function normalizeAndValidateConfig(
 
 	// TODO: set the default to false to turn on service environments as the default
 	const isLegacyEnv =
-		(args as { "legacy-env": boolean | undefined })["legacy-env"] ??
-		rawConfig.legacy_env ??
-		true;
+		typeof args["legacy-env"] === "boolean"
+			? args["legacy-env"]
+			: rawConfig.legacy_env ?? true;
 
 	// TODO: remove this once service environments goes GA.
 	if (!isLegacyEnv) {
@@ -134,7 +135,8 @@ export function normalizeAndValidateConfig(
 	);
 
 	//TODO: find a better way to define the type of Args that can be passed to the normalizeAndValidateConfig()
-	const envName = (args as { env: string | undefined }).env;
+	const envName = args.env;
+	assert(envName === undefined || typeof envName === "string");
 
 	let activeEnv = topLevelEnv;
 	if (envName !== undefined) {
@@ -196,7 +198,7 @@ export function normalizeAndValidateConfig(
 		send_metrics: rawConfig.send_metrics,
 		keep_vars: rawConfig.keep_vars,
 		...activeEnv,
-		dev: normalizeAndValidateDev(diagnostics, rawConfig.dev ?? {}),
+		dev: normalizeAndValidateDev(diagnostics, rawConfig.dev ?? {}, args),
 		migrations: normalizeAndValidateMigrations(
 			diagnostics,
 			rawConfig.migrations ?? [],
@@ -387,8 +389,26 @@ function normalizeAndValidateBaseDirField(
  */
 function normalizeAndValidateDev(
 	diagnostics: Diagnostics,
-	rawDev: RawDevConfig
+	rawDev: RawDevConfig,
+	args: Record<string, unknown>
 ): DevConfig {
+	assert(typeof args === "object" && args !== null && !Array.isArray(args));
+	const {
+		localProtocol: localProtocolArg,
+		upstreamProtocol: upstreamProtocolArg,
+		remote: remoteArg,
+	} = args;
+	assert(
+		localProtocolArg === undefined ||
+			localProtocolArg === "http" ||
+			localProtocolArg === "https"
+	);
+	assert(
+		upstreamProtocolArg === undefined ||
+			upstreamProtocolArg === "http" ||
+			upstreamProtocolArg === "https"
+	);
+	assert(remoteArg === undefined || typeof remoteArg === "boolean");
 	const {
 		// On Windows, when specifying `localhost` as the socket hostname, `workerd`
 		// will only listen on the IPv4 loopback `127.0.0.1`, not the IPv6 `::1`:
@@ -400,8 +420,11 @@ function normalizeAndValidateDev(
 		ip = process.platform === "win32" ? "127.0.0.1" : "localhost",
 		port,
 		inspector_port,
-		local_protocol = "http",
-		upstream_protocol = "https",
+		local_protocol = localProtocolArg ?? "http",
+		// In remote mode upstream_protocol must be https, otherwise it defaults to local_protocol.
+		upstream_protocol = upstreamProtocolArg ?? remoteArg
+			? "https"
+			: local_protocol,
 		host,
 		...rest
 	} = rawDev;

packages/wrangler/src/dev.tsx

@@ -756,9 +756,9 @@ async function validateDevServerSettings(
 	}
 	const upstreamProtocol =
 		args.upstreamProtocol ?? config.dev.upstream_protocol;
-	if (upstreamProtocol === "http") {
+	if (upstreamProtocol === "http" && args.remote) {
 		logger.warn(
-			"Setting upstream-protocol to http is not currently implemented.\n" +
+			"Setting upstream-protocol to http is not currently supported for remote mode.\n" +
 				"If this is required in your project, please add your use case to the following issue:\n" +
 				"https://github.com/cloudflare/workers-sdk/issues/583."
 		);

packages/wrangler/src/dev/dev.tsx

@@ -441,6 +441,7 @@ function DevSession(props: DevSessionProps) {
 			queueConsumers={props.queueConsumers}
 			localProtocol={"http"} // hard-code for userworker, DevEnv-ProxyWorker now uses this prop value
 			localUpstream={props.localUpstream}
+			upstreamProtocol={props.upstreamProtocol}
 			inspect={props.inspect}
 			onReady={announceAndOnReady}
 			enablePagesAssetsServiceBinding={props.enablePagesAssetsServiceBinding}

packages/wrangler/src/dev/local.tsx

@@ -38,6 +38,7 @@ export interface LocalProps {
 	crons: Config["triggers"]["crons"];
 	queueConsumers: Config["queues"]["consumers"];
 	localProtocol: "http" | "https";
+	upstreamProtocol: "http" | "https";
 	localUpstream: string | undefined;
 	inspect: boolean;
 	onReady:
@@ -93,6 +94,7 @@ export async function localPropsToConfigBundle(
 		queueConsumers: props.queueConsumers,
 		localProtocol: props.localProtocol,
 		localUpstream: props.localUpstream,
+		upstreamProtocol: props.upstreamProtocol,
 		inspect: props.inspect,
 		serviceBindings,
 	};
@@ -177,7 +179,7 @@ function useLocalWorker(props: LocalProps) {
 						pathname: `/core:user:${props.name ?? DEFAULT_WORKER_NAME}`,
 					},
 					userWorkerInnerUrlOverrides: {
-						protocol: props.localProtocol,
+						protocol: props.upstreamProtocol,
 						hostname: props.localUpstream,
 						port: props.localUpstream ? "" : undefined, // `localUpstream` was essentially `host`, not `hostname`, so if it was set delete the `port`
 					},

packages/wrangler/src/dev/miniflare.ts

@@ -112,6 +112,7 @@ export interface ConfigBundle {
 	queueConsumers: Config["queues"]["consumers"];
 	localProtocol: "http" | "https";
 	localUpstream: string | undefined;
+	upstreamProtocol: "http" | "https";
 	inspect: boolean;
 	serviceBindings: Record<string, (_request: Request) => Promise<Response>>;
 }
@@ -564,7 +565,7 @@ async function buildMiniflareOptions(
 
 	const upstream =
 		typeof config.localUpstream === "string"
-			? `${config.localProtocol}://${config.localUpstream}`
+			? `${config.upstreamProtocol}://${config.localUpstream}`
 			: undefined;
 
 	const sourceOptions = await buildSourceOptions(config);

packages/wrangler/src/dev/start-server.ts

@@ -95,7 +95,7 @@ export async function startDevServer(
 				port: props.inspectorPort,
 			},
 			urlOverrides: {
-				secure: props.localProtocol === "https",
+				secure: props.upstreamProtocol === "https",
 				hostname: props.localUpstream,
 			},
 			liveReload: props.liveReload,
@@ -168,6 +168,7 @@ export async function startDevServer(
 			queueConsumers: props.queueConsumers,
 			localProtocol: props.localProtocol,
 			localUpstream: props.localUpstream,
+			upstreamProtocol: props.upstreamProtocol,
 			inspect: true,
 			onReady: async (ip, port, proxyData) => {
 				// at this point (in the layers of onReady callbacks), we have devEnv in scope
@@ -412,7 +413,7 @@ export async function startLocalServer(
 					pathname: `/core:user:${props.name ?? DEFAULT_WORKER_NAME}`,
 				},
 				userWorkerInnerUrlOverrides: {
-					protocol: props.localProtocol,
+					protocol: props.upstreamProtocol,
 					hostname: props.localUpstream,
 					port: props.localUpstream ? "" : undefined, // `localUpstream` was essentially `host`, not `hostname`, so if it was set delete the `port`
 				},