Skip to content

Commit

Permalink
fix: add warning to fetch() calls that will change the requested port
Browse files Browse the repository at this point in the history
In Workers published to the Edge (rather than previews) there is a bug where a custom port on a downstream fetch request is ignored, defaulting to the standard port.
For example, `https://my.example.com:668` will actually send the request to `https://my.example.com:443`.

This does not happen when using `wrangler dev` (both in remote and local mode), but to ensure that developers are aware of it this change displays a runtime warning in the console when the bug is hit.

Closes #1320
  • Loading branch information
petebacondarwin committed Jun 28, 2022
1 parent 2d93604 commit 65ad2fc
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 0 deletions.
12 changes: 12 additions & 0 deletions .changeset/big-tables-judge.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
"wrangler": patch
---

fix: add warning to `fetch()` calls that will change the requested port

In Workers published to the Edge (rather than previews) there is a bug where a custom port on a downstream fetch request is ignored, defaulting to the standard port.
For example, `https://my.example.com:668` will actually send the request to `https://my.example.com:443`.

This does not happen when using `wrangler dev` (both in remote and local mode), but to ensure that developers are aware of it this change displays a runtime warning in the console when the bug is hit.

Closes #1320
5 changes: 5 additions & 0 deletions packages/wrangler/src/bundle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ export async function bundleWorker(
minify: boolean | undefined;
nodeCompat: boolean | undefined;
define: Config["define"];
checkFetch: boolean;
}
): Promise<BundleResult> {
const {
Expand All @@ -71,6 +72,7 @@ export async function bundleWorker(
tsconfig,
minify,
nodeCompat,
checkFetch,
} = options;
const entryDirectory = path.dirname(entry.file);
const moduleCollector = createModuleCollector({
Expand All @@ -95,6 +97,9 @@ export async function bundleWorker(
bundle: true,
absWorkingDir: entry.directory,
outdir: destination,
inject: checkFetch
? [path.resolve(__dirname, "../templates/checked-fetch.js")]
: [],
external: ["__STATIC_CONTENT_MANIFEST"],
format: entry.format === "modules" ? "esm" : "iife",
target: "es2020",
Expand Down
1 change: 1 addition & 0 deletions packages/wrangler/src/dev/use-esbuild.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ export function useEsbuild({
minify,
nodeCompat,
define,
checkFetch: true,
});

// Capture the `stop()` method to use as the `useEffect()` destructor.
Expand Down
1 change: 1 addition & 0 deletions packages/wrangler/src/publish.ts
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,7 @@ See https://developers.cloudflare.com/workers/platform/compatibility-dates for m
minify,
nodeCompat,
define: config.define,
checkFetch: false,
}
);

Expand Down
22 changes: 22 additions & 0 deletions packages/wrangler/templates/checked-fetch.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
globalThis.fetch = (() => {
const originalFetch = globalThis.fetch;
return (request, init) => {
const url = new URL(
(typeof request === "string" ? new Request(request, init) : request).url
);
if (
url.port !== undefined &&
url.protocol === "https" &&
url.port != "443"
) {
const actualUrl = new URL(url.toString());
actualUrl.port = "443";
console.warn(
`WARNING: known issue with \`fetch()\` requests to custom HTTPS ports in published Workers:\n` +
`The custom port of the URL being fetched "${url.toString()}" will be ignored.\n` +
`Instead it will use the "default port" (${actualUrl}) when the Worker is published to Cloudflare.`
);
}
return originalFetch(request, init);
};
})();

0 comments on commit 65ad2fc

Please sign in to comment.