Skip to content

Commit

Permalink
fix: set x-forwarded-host based on request (#58500)
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
3 people authored Nov 16, 2023
1 parent 24b2ff1 commit c26c771
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 1 deletion.
2 changes: 1 addition & 1 deletion packages/next/src/server/base-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -910,7 +910,7 @@ export default abstract class Server<ServerOptions extends Options = Options> {
)
}

req.headers['x-forwarded-host'] ??= `${this.hostname}:${this.port}`
req.headers['x-forwarded-host'] ??= req.headers['host'] ?? this.hostname
req.headers['x-forwarded-port'] ??= this.port?.toString()
const { originalRequest } = req as NodeNextRequest
req.headers['x-forwarded-proto'] ??= (originalRequest.socket as TLSSocket)
Expand Down
50 changes: 50 additions & 0 deletions test/e2e/app-dir/x-forwarded-headers/x-forwarded-headers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ createNextDescribe('x-forwarded-headers', { files: __dirname }, ({ next }) => {
const res = await next.fetch('/')
const headers = await res.json()
const url = new URL(next.url)

expect(headers['x-forwarded-host']).toBe(url.host)
expect(headers['x-forwarded-port']).toBe(url.port)
expect(headers['x-forwarded-proto']).toBe(url.protocol.replace(':', ''))
Expand All @@ -14,4 +15,53 @@ createNextDescribe('x-forwarded-headers', { files: __dirname }, ({ next }) => {
url.protocol.replace(':', '')
)
})

describe('host header exists', () => {
it('should include x-forwarded-* headers relative to host', async () => {
const url = new URL(next.url)
const reqHeaders = {
host: `subdomain.localhost:${url.port}`,
}
const res = await next.fetch('/', {
headers: reqHeaders,
})
const headers = await res.json()

expect(headers['x-forwarded-host']).toBe(reqHeaders.host)
expect(headers['x-forwarded-port']).toBe(url.port)
expect(headers['x-forwarded-proto']).toBe(url.protocol.replace(':', ''))
expect(headers['middleware-x-forwarded-host']).toBe(reqHeaders.host)
expect(headers['middleware-x-forwarded-port']).toBe(url.port)
expect(headers['middleware-x-forwarded-proto']).toBe(
url.protocol.replace(':', '')
)
})
})

describe('already assigned', () => {
it('should not override existing x-forwarded-* headers', async () => {
const url = new URL(next.url)
const reqHeaders = {
host: `subdomain.localhost:${url.port}`,
port: '1234',
proto: 'https',
}
const res = await next.fetch('/', {
headers: {
host: 'override.localhost',
'x-forwarded-host': reqHeaders.host,
'x-forwarded-port': reqHeaders.port,
'x-forwarded-proto': reqHeaders.proto,
},
})
const headers = await res.json()

expect(headers['x-forwarded-host']).toBe(reqHeaders.host)
expect(headers['x-forwarded-port']).toBe(reqHeaders.port)
expect(headers['x-forwarded-proto']).toBe(reqHeaders.proto)
expect(headers['middleware-x-forwarded-host']).toBe(reqHeaders.host)
expect(headers['middleware-x-forwarded-port']).toBe(reqHeaders.port)
expect(headers['middleware-x-forwarded-proto']).toBe(reqHeaders.proto)
})
})
})

0 comments on commit c26c771

Please sign in to comment.