fix: preserve HTTPS/HTTP2 host port in @cloudflare/vite-plugin requests - #14933
fix: preserve HTTPS/HTTP2 host port in @cloudflare/vite-plugin requests#14933intrdx wants to merge 5 commits into
Conversation
🦋 Changeset detectedLatest commit: 94e47f4 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 |
|
Codeowners approval required for this PR:
Show detailed file reviewers |
f42939d to
7db27e5
Compare
@cloudflare/autoconfig
@cloudflare/build-output-utils
@cloudflare/codemods
@cloudflare/config
create-cloudflare
@cloudflare/deploy-helpers
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-functions
@cloudflare/pages-shared
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-plugin
@cloudflare/workers-auth
@cloudflare/workers-editor-shared
@cloudflare/workers-utils
wrangler
commit: |
workers-devprod
left a comment
There was a problem hiding this comment.
Codeowners reviews satisfied
|
Codeowners approval required for this PR:
Show detailed file reviewers |
58d0eed to
3c2ea78
Compare
|
Codeowners approval required for this PR:
Show detailed file reviewers |
|
We recently landed #14994, a large change to Miniflare's configuration internals that touched ~177 files across the repo. Leaving this PR on its old base was likely to cause conflicts, so we've rebased it onto the latest Your local copy of this branch is now out of date. Before you push again, please reset to the new version: git fetch origin
git checkout fix/vite-plugin-https-http2-host-port
git reset --hard origin/fix/vite-plugin-https-http2-host-portBecause the base moved a long way, it's also worth reinstalling before you carry on — the lockfile changed: pnpm installSorry for the interruption. If the rebase looks wrong, or CI now fails in a way that seems related to the Miniflare config change rather than your own work, comment here and we'll help get it sorted. |
3c2ea78 to
a83337c
Compare
|
Codeowners approval required for this PR:
Show detailed file reviewers |
ada9a3a to
fe28cff
Compare
Co-authored-by: Cursor <cursoragent@cursor.com>
fe28cff to
347c7c6
Compare
|
@NuroDev, ready to merge now. |
|
@NuroDev Circling back, CI failures after your approval are fixed, and this is still the only PR with a maintainer approve for issue #14931. There are now three other PRs for the same issue (#15237, #15519, #15533). #15533 from @vahidshaik1901 looks great. Can adopt the cleaner createRequestForIncomingMessage + real h2c test approach from #15533 on this branch in case you'd rather merge here than restart review on a newer PR. Goal is just to ship the port bug fix as soon as possible. |
|
@intrdx Please do. #14933 has carried the approval since August and is the right one to land; my only interest is the fix shipping, and a fourth PR in the queue does not help with that. Take A |
Adopt createRequestForIncomingMessage host resolution and toMiniflareRequest URL-host fallback from cloudflare#15533, replacing the outer Host force-set. Cover the :authority path with cleartext HTTP/2 tests. Co-authored-by: SHAIK VAHID <38548782+vahidshaik1901@users.noreply.github.com>
|
Codeowners approval required for this PR:
Show detailed file reviewers |
There was a problem hiding this comment.
Devin Review found 1 potential issue.
1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
|
Done @vahidshaik1901, let me know if you want anything tweaked. |
Fixes #14931.
Preserve host and non-default port from
Host/:authoritywhen constructing Vite HTTPS/HTTP2 requests in@cloudflare/vite-plugin.Under HTTPS, Vite uses HTTP/2. Pseudo-headers like
:authorityare skipped when building Fetch headers, soHostcan be missing and host resolution falls back tolocalhostwithout:5173. Auth libraries like Clerk then build handshakeredirect_url/X-Forwarded-Hostfrom that broken origin and loop.Changes
getRequestHost()to resolve authority fromHost, then:authoritycreateRequestForIncomingMessagesorequest.urlkeeps host + non-default porttoMiniflareRequest, fall back tonew URL(request.url).hostwhenHostis absent soX-Forwarded-Hoststill gets e.g.localhost:5173h2c) tests that drive a realnode:http2server throughcreateRequestHandlerValidated locally with an equivalent pnpm patch against
@cloudflare/vite-plugin(Clerk handshake no longer drops:5173under HTTPS).Related: #8684 / #8706, #10884, #13801 / #13920
server.https(mkcert) +--hosthttps://localhost:5173with Clerk middlewareredirect_urlkeeps:5173(no redirect loop tohttps://localhost/)http://localhost:5173still works