diff --git a/.changeset/slimy-buses-agree.md b/.changeset/slimy-buses-agree.md new file mode 100644 index 000000000000..9dad2acaf676 --- /dev/null +++ b/.changeset/slimy-buses-agree.md @@ -0,0 +1,7 @@ +--- +'astro': patch +--- + +Fixes a bug in the parsing of `x-forwarded-\*` `Request` headers, where multiple values assigned to those headers were not correctly parsed. + +Now, headers like `x-forwarded-proto: https,http` are correctly parsed. diff --git a/packages/astro/src/core/app/node.ts b/packages/astro/src/core/app/node.ts index 3d0c07ded2a2..8dd312760f25 100644 --- a/packages/astro/src/core/app/node.ts +++ b/packages/astro/src/core/app/node.ts @@ -61,16 +61,33 @@ export class NodeApp extends App { * ``` */ static createRequest(req: NodeRequest, { skipBody = false } = {}): Request { - const protocol = - req.headers['x-forwarded-proto'] ?? - ('encrypted' in req.socket && req.socket.encrypted ? 'https' : 'http'); - const hostname = - req.headers['x-forwarded-host'] ?? req.headers.host ?? req.headers[':authority']; - const port = req.headers['x-forwarded-port']; + const isEncrypted = 'encrypted' in req.socket && req.socket.encrypted; + + // Parses multiple header and returns first value if available. + const getFirstForwardedValue = (multiValueHeader?: string | string[]) => { + return multiValueHeader + ?.toString() + ?.split(',') + .map((e) => e.trim())?.[0]; + }; + + // Get the used protocol between the end client and first proxy. + // NOTE: Some proxies append values with spaces and some do not. + // We need to handle it here and parse the header correctly. + // @example "https, http,http" => "http" + const forwardedProtocol = getFirstForwardedValue(req.headers['x-forwarded-proto']); + const protocol = forwardedProtocol ?? (isEncrypted ? 'https' : 'http'); + + // @example "example.com,www2.example.com" => "example.com" + const forwardedHostname = getFirstForwardedValue(req.headers['x-forwarded-host']); + const hostname = forwardedHostname ?? req.headers.host ?? req.headers[':authority']; - const portInHostname = - typeof hostname === 'string' && typeof port === 'string' && hostname.endsWith(port); - const hostnamePort = portInHostname ? hostname : hostname + (port ? `:${port}` : ''); + // @example "443,8080,80" => "443" + const forwardedPort = getFirstForwardedValue(req.headers['x-forwarded-port']); + const port = forwardedPort ?? req.socket?.remotePort?.toString() ?? (isEncrypted ? '443' : '80'); + + const portInHostname = typeof hostname === 'string' && /:\d+$/.test(hostname); + const hostnamePort = portInHostname ? hostname : `${hostname}:${port}` const url = `${protocol}://${hostnamePort}${req.url}`; const options: RequestInit = { @@ -81,14 +98,17 @@ export class NodeApp extends App { if (bodyAllowed) { Object.assign(options, makeRequestBody(req)); } + const request = new Request(url, options); - const clientIp = req.headers['x-forwarded-for']; - if (clientIp) { + // Get the IP of end client behind the proxy. + // @example "1.1.1.1,8.8.8.8" => "1.1.1.1" + const forwardedClientIp = getFirstForwardedValue(req.headers['x-forwarded-for']); + const clientIp = forwardedClientIp || req.socket?.remoteAddress; + if (clientIp) { Reflect.set(request, clientAddressSymbol, clientIp); - } else if (req.socket?.remoteAddress) { - Reflect.set(request, clientAddressSymbol, req.socket.remoteAddress); } + return request; } diff --git a/packages/astro/test/client-address-node.test.js b/packages/astro/test/client-address-node.test.js index 8114ea42d210..2cc582ac1e3a 100644 --- a/packages/astro/test/client-address-node.test.js +++ b/packages/astro/test/client-address-node.test.js @@ -5,23 +5,47 @@ import { loadFixture } from './test-utils.js'; import { createRequestAndResponse } from './units/test-utils.js'; describe('NodeClientAddress', () => { - it('clientAddress is 1.1.1.1', async () => { - const fixture = await loadFixture({ - root: './fixtures/client-address-node/', + describe('single value', () => { + it('clientAddress is 1.1.1.1', async () => { + const fixture = await loadFixture({ + root: './fixtures/client-address-node/', + }); + await fixture.build(); + const handle = await fixture.loadNodeAdapterHandler(); + const { req, res, text } = createRequestAndResponse({ + method: 'GET', + url: '/', + headers: { + 'x-forwarded-for': '1.1.1.1', + }, + }); + handle(req, res); + const html = await text(); + const $ = cheerio.load(html); + assert.equal(res.statusCode, 200); + assert.equal($('#address').text(), '1.1.1.1'); }); - await fixture.build(); - const handle = await fixture.loadNodeAdapterHandler(); - const { req, res, text } = createRequestAndResponse({ - method: 'GET', - url: '/', - headers: { - 'x-forwarded-for': '1.1.1.1', - }, + }); + + describe('multiple values', () => { + it('clientAddress is 1.1.1.1', async () => { + const fixture = await loadFixture({ + root: './fixtures/client-address-node/', + }); + await fixture.build(); + const handle = await fixture.loadNodeAdapterHandler(); + const { req, res, text } = createRequestAndResponse({ + method: 'GET', + url: '/', + headers: { + 'x-forwarded-for': '1.1.1.1,8.8.8.8, 8.8.8.2', + }, + }); + handle(req, res); + const html = await text(); + const $ = cheerio.load(html); + assert.equal(res.statusCode, 200); + assert.equal($('#address').text(), '1.1.1.1'); }); - handle(req, res); - const html = await text(); - const $ = cheerio.load(html); - assert.equal(res.statusCode, 200); - assert.equal($('#address').text(), '1.1.1.1'); }); }); diff --git a/packages/astro/test/units/app/node.test.js b/packages/astro/test/units/app/node.test.js new file mode 100644 index 000000000000..06065406edb5 --- /dev/null +++ b/packages/astro/test/units/app/node.test.js @@ -0,0 +1,175 @@ +import * as assert from 'node:assert/strict'; +import { describe, it } from 'node:test'; +import { NodeApp } from '../../../dist/core/app/node.js'; + +const mockNodeRequest = { + url: '/', + method: 'GET', + headers: { + host: 'example.com', + }, + socket: { + encrypted: true, + remoteAddress: '2.2.2.2', + }, +}; + +describe('NodeApp', () => { + describe('createRequest', () => { + describe('x-forwarded-for', () => { + it('parses client IP from single-value x-forwarded-for header', () => { + const result = NodeApp.createRequest({ + ...mockNodeRequest, + headers: { + 'x-forwarded-for': '1.1.1.1', + }, + }); + assert.equal(result[Symbol.for('astro.clientAddress')], '1.1.1.1'); + }); + + it('parses client IP from multi-value x-forwarded-for header', () => { + const result = NodeApp.createRequest({ + ...mockNodeRequest, + headers: { + 'x-forwarded-for': '1.1.1.1,8.8.8.8', + }, + }); + assert.equal(result[Symbol.for('astro.clientAddress')], '1.1.1.1'); + }); + + it('parses client IP from multi-value x-forwarded-for header with spaces', () => { + const result = NodeApp.createRequest({ + ...mockNodeRequest, + headers: { + 'x-forwarded-for': ' 1.1.1.1, 8.8.8.8, 8.8.8.2', + }, + }); + assert.equal(result[Symbol.for('astro.clientAddress')], '1.1.1.1'); + }); + + it('fallbacks to remoteAddress when no x-forwarded-for header is present', () => { + const result = NodeApp.createRequest({ + ...mockNodeRequest, + headers: {}, + }); + assert.equal(result[Symbol.for('astro.clientAddress')], '2.2.2.2'); + }); + }); + + describe('x-forwarded-host', () => { + it('parses host from single-value x-forwarded-host header', () => { + const result = NodeApp.createRequest({ + ...mockNodeRequest, + headers: { + 'x-forwarded-host': 'www2.example.com', + }, + }); + assert.equal(result.url, 'https://www2.example.com/'); + }); + + it('parses host from multi-value x-forwarded-host header', () => { + const result = NodeApp.createRequest({ + ...mockNodeRequest, + headers: { + 'x-forwarded-host': 'www2.example.com,www3.example.com', + }, + }); + assert.equal(result.url, 'https://www2.example.com/'); + }); + + it('fallbacks to host header when no x-forwarded-host header is present', () => { + const result = NodeApp.createRequest({ + ...mockNodeRequest, + headers: { + host: 'example.com', + }, + }); + assert.equal(result.url, 'https://example.com/'); + }); + }); + + describe('x-forwarded-proto', () => { + it('parses protocol from single-value x-forwarded-proto header', () => { + const result = NodeApp.createRequest({ + ...mockNodeRequest, + headers: { + host: 'example.com', + 'x-forwarded-proto': 'http', + 'x-forwarded-port': '80', + }, + }); + assert.equal(result.url, 'http://example.com/'); + }); + + it('parses protocol from multi-value x-forwarded-proto header', () => { + const result = NodeApp.createRequest({ + ...mockNodeRequest, + headers: { + host: 'example.com', + 'x-forwarded-proto': 'http,https', + 'x-forwarded-port': '80,443', + }, + }); + assert.equal(result.url, 'http://example.com/'); + }); + + it('fallbacks to encrypted property when no x-forwarded-proto header is present', () => { + const result = NodeApp.createRequest({ + ...mockNodeRequest, + headers: { + host: 'example.com', + }, + }); + assert.equal(result.url, 'https://example.com/'); + }); + }); + + describe('x-forwarded-port', () => { + it('parses port from single-value x-forwarded-port header', () => { + const result = NodeApp.createRequest({ + ...mockNodeRequest, + headers: { + host: 'example.com', + 'x-forwarded-port': '8443', + }, + }); + assert.equal(result.url, 'https://example.com:8443/'); + }); + + it('parses port from multi-value x-forwarded-port header', () => { + const result = NodeApp.createRequest({ + ...mockNodeRequest, + headers: { + host: 'example.com', + 'x-forwarded-port': '8443,3000', + }, + }); + assert.equal(result.url, 'https://example.com:8443/'); + }); + + it('prefers port from host', () => { + const result = NodeApp.createRequest({ + ...mockNodeRequest, + headers: { + host: 'example.com:3000', + 'x-forwarded-port': '443', + }, + }); + assert.equal(result.url, 'https://example.com:3000/'); + }); + + + it('prefers port from x-forwarded-host', () => { + const result = NodeApp.createRequest({ + ...mockNodeRequest, + headers: { + host: 'example.com:443', + 'x-forwarded-host': 'example.com:3000', + 'x-forwarded-port': '443', + }, + }); + assert.equal(result.url, 'https://example.com:3000/'); + }); + }); + }); +});