fix: correctly set x-forwarded-* in Middleware#57815
Conversation
Tests Passed |
Stats from current PRDefault Build (Increase detected
|
| vercel/next.js canary | vercel/next.js fix/x-forwarded-headers-middleware | Change | |
|---|---|---|---|
| buildDuration | 10.5s | 10.6s | |
| buildDurationCached | 6s | 6.8s | |
| nodeModulesSize | 175 MB | 175 MB | N/A |
| nextStartRea..uration (ms) | 401ms | 400ms | N/A |
Client Bundles (main, webpack)
| vercel/next.js canary | vercel/next.js fix/x-forwarded-headers-middleware | Change | |
|---|---|---|---|
| 199-HASH.js gzip | 30 kB | 30 kB | N/A |
| 3f784ff6-HASH.js gzip | 53.2 kB | 53.2 kB | ✓ |
| 494.HASH.js gzip | 182 B | 182 B | ✓ |
| framework-HASH.js gzip | 45.5 kB | 45.5 kB | ✓ |
| main-app-HASH.js gzip | 253 B | 252 B | N/A |
| main-HASH.js gzip | 33.1 kB | 33.1 kB | N/A |
| webpack-HASH.js gzip | 1.75 kB | 1.75 kB | N/A |
| Overall change | 98.9 kB | 98.9 kB | ✓ |
Legacy Client Bundles (polyfills)
| vercel/next.js canary | vercel/next.js fix/x-forwarded-headers-middleware | Change | |
|---|---|---|---|
| polyfills-HASH.js gzip | 31 kB | 31 kB | ✓ |
| Overall change | 31 kB | 31 kB | ✓ |
Client Pages
| vercel/next.js canary | vercel/next.js fix/x-forwarded-headers-middleware | Change | |
|---|---|---|---|
| _app-HASH.js gzip | 205 B | 205 B | ✓ |
| _error-HASH.js gzip | 182 B | 181 B | N/A |
| amp-HASH.js gzip | 505 B | 507 B | N/A |
| css-HASH.js gzip | 322 B | 323 B | N/A |
| dynamic-HASH.js gzip | 2.59 kB | 2.59 kB | N/A |
| edge-ssr-HASH.js gzip | 258 B | 259 B | N/A |
| head-HASH.js gzip | 348 B | 347 B | N/A |
| hooks-HASH.js gzip | 369 B | 368 B | N/A |
| image-HASH.js gzip | 4.38 kB | 4.38 kB | N/A |
| index-HASH.js gzip | 256 B | 256 B | ✓ |
| link-HASH.js gzip | 2.67 kB | 2.67 kB | N/A |
| routerDirect..HASH.js gzip | 318 B | 318 B | ✓ |
| script-HASH.js gzip | 384 B | 383 B | N/A |
| withRouter-HASH.js gzip | 319 B | 320 B | N/A |
| 1afbb74e6ecf..834.css gzip | 106 B | 106 B | ✓ |
| Overall change | 885 B | 885 B | ✓ |
Client Build Manifests
| vercel/next.js canary | vercel/next.js fix/x-forwarded-headers-middleware | Change | |
|---|---|---|---|
| _buildManifest.js gzip | 484 B | 484 B | ✓ |
| Overall change | 484 B | 484 B | ✓ |
Rendered Page Sizes
| vercel/next.js canary | vercel/next.js fix/x-forwarded-headers-middleware | Change | |
|---|---|---|---|
| index.html gzip | 528 B | 527 B | N/A |
| link.html gzip | 541 B | 542 B | N/A |
| withRouter.html gzip | 524 B | 521 B | N/A |
| Overall change | 0 B | 0 B | ✓ |
Edge SSR bundle Size Overall increase ⚠️
| vercel/next.js canary | vercel/next.js fix/x-forwarded-headers-middleware | Change | |
|---|---|---|---|
| edge-ssr.js gzip | 96.1 kB | 96.2 kB | |
| page.js gzip | 140 kB | 140 kB | |
| Overall change | 236 kB | 236 kB |
Middleware size Overall increase ⚠️
| vercel/next.js canary | vercel/next.js fix/x-forwarded-headers-middleware | Change | |
|---|---|---|---|
| middleware-b..fest.js gzip | 625 B | 627 B | N/A |
| middleware-r..fest.js gzip | 148 B | 151 B | N/A |
| middleware.js gzip | 23 kB | 24.8 kB | |
| edge-runtime..pack.js gzip | 1.92 kB | 1.92 kB | ✓ |
| Overall change | 25 kB | 26.8 kB |
Diff details
Diff for page.js
Diff too large to display
Diff for middleware.js
Diff too large to display
Diff for edge-ssr.js
Diff too large to display
Diff for main-HASH.js
Diff too large to display
…om/vercel/next.js into fix/x-forwarded-headers-middleware
| target, | ||
| changeOrigin: true, | ||
| ignorePath: true, | ||
| xfwd: true, |
There was a problem hiding this comment.
With xfwd: true, http-proxy attempts to set x-forwarded-* headers. Since we are setting these headers higher up in base-server.ts, this should not be necessary to set here anymore, to avoid double setting them, as reported in #52266
| // we don't time out WebSocket requests to allow proxying | ||
| proxyTimeout: proxyTimeout === null ? undefined : proxyTimeout || 30_000, | ||
| headers: { | ||
| 'x-forwarded-host': req.headers.host || '', |
There was a problem hiding this comment.
Still set this header to respect #17557, but without concating without adding a comma ,. See https://github.com/vercel/next.js/pull/57815/files#r1378313718
|
I believe this is missing an edge case that breaks my application’s behavior—in the old implementation (prior to #52492) the |
Co-authored-by: @brkalow <bryce@clerk.dev> ### What? A number of our customers have been experiencing issues stemming from an `x-forwarded-host` header that doesn't match the `host` header. ### Why? [This PR](#57815) removes functionality which sets `x-forwarded-host` to `req.headers['host']` and relies solely on the server's hostname and port. This can be seen locally when visiting the app via a localhost subdomain. The `x-forwarded-host` header will remain as `localhost:${port}` while the actual requested host will contain the subdomain. ### Related - #57815 (comment) --------- Co-authored-by: BRKalow <bryce@clerk.dev> Co-authored-by: Zack Tanner <zacktanner@gmail.com>
What?
Follow-up of #56797
While working on this, I noticed that some logic around stripping internal headers was duplicated, so I did some cleanup too.
Why?
In #56797 we set these headers, but it only affected Route Handlers, Middleware is still missing them, which is a regression introduced in #52492
(Related: vercel/next-learn#252)
How?
Move to set these headers up to
base-server.tsso they are present in Middleware too.Closes NEXT-
Fixes #52266