Use correct host in exchange #2651
Conversation
🦋 Changeset detectedLatest commit: 1ebf435 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
A wrangler prerelease is available for testing. You can install this latest build in your project with: npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/runs/4205879066/npm-package-wrangler-2651You can reference the automatically updated head of this PR with: npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/prs/2651/npm-package-wrangler-2651Or you can use npx https://prerelease-registry.devprod.cloudflare.dev/runs/4205879066/npm-package-wrangler-2651 dev path/to/script.jsAdditional artifacts:npm install https://prerelease-registry.devprod.cloudflare.dev/runs/4205879066/npm-package-cloudflare-pages-shared-2651 |
Codecov Report
@@ Coverage Diff @@
## main #2651 +/- ##
==========================================
- Coverage 68.16% 68.12% -0.05%
==========================================
Files 166 166
Lines 10155 10161 +6
Branches 2703 2705 +2
==========================================
Hits 6922 6922
- Misses 3233 3239 +6
|
| host, | ||
| inspectorUrl: new URL(`${inspector_websocket}?${query}`), | ||
| prewarmUrl: new URL(prewarm), | ||
| host: ctx.host ?? inspector.host, |
There was a problem hiding this comment.
Can the host be "" (I don't remember if the config validator prevents that) if so this could have unintended behavior.
| // the worker route host, which is more likely to be orange-clouded | ||
| function switchHost(originalUrl: string, host?: string): URL { | ||
| const url = new URL(originalUrl); | ||
| url.hostname = host ?? url.hostname; |
There was a problem hiding this comment.
If host can be falsey "" this could cause unintended behaviors
There was a problem hiding this comment.
Is there any reason not to always just use the probably-orange-clouded host?
There was a problem hiding this comment.
Sometimes we might not be able to parse an orange-clouded host (i.e. when the only route is */*)
There was a problem hiding this comment.
So is there a circumstance when I'm using a wildcard route e.g. */* but my apex domain is grey-clouded, where this won't work?
There was a problem hiding this comment.
Yes, but there's kinda nothing we can do there if you've provided no other information. I think there's a dev.host config parameter you can add to explicitly set this, but I'll double check
penalosa
force-pushed
the
penalosa/use-host-in-exchange
branch
from
9be7694
to
c735ceb
Compare
penalosa
force-pushed
the
penalosa/use-host-in-exchange
branch
from
121a008
to
1ebf435
Compare
What this PR solves / how to test:
URLs from the Cloudflare API are usually relative to the zone. Sometimes the base zone will be grey-clouded, and so the host must be swapped out for the worker route host, which is more likely to be orange-clouded.
Associated docs issues/PR:
Author has included the following, where applicable:
Reviewer has performed the following, where applicable:
Fixes #2018, #2602, #2581