feat: prefer using x-forwarded-for as clientAddress (#11101)

linguofeng · ematipico · web-flow · commit a6916e4402bf · 2024-05-22T12:10:56.000+01:00
* feat: change node clientAddress use x-forwarded-for

when

```
adapter: node({
    mode: 'standalone',
  })
```

* feat: prefer using x-forwarded-for as clientAddress

* Update .changeset/healthy-planets-dream.md

Co-authored-by: Emanuele Stoppa &lt;my.burning@gmail.com&gt;

* Update .changeset/healthy-planets-dream.md

Co-authored-by: Emanuele Stoppa &lt;my.burning@gmail.com&gt;

* Apply suggestions from code review

---------

Co-authored-by: Emanuele Stoppa &lt;my.burning@gmail.com&gt;
diff --git a/.changeset/healthy-planets-dream.md b/.changeset/healthy-planets-dream.md
@@ -0,0 +1,7 @@
+---
+"astro": minor
+---
+
+Updates Astro's code for adapters to use the header `x-forwarded-for` to initialize the `clientAddress`.
+
+To take advantage of the new change, integration authors must upgrade the version of Astro in their adapter `peerDependencies` to `4.9.0`.
diff --git a/packages/astro/src/core/app/node.ts b/packages/astro/src/core/app/node.ts
@@ -81,7 +81,11 @@ export class NodeApp extends App {
 			Object.assign(options, makeRequestBody(req));
 		}
 		const request = new Request(url, options);
-		if (req.socket?.remoteAddress) {
+
+		const clientIp = req.headers['x-forwarded-for'];
+		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
@@ -0,0 +1,27 @@
+import * as assert from 'node:assert/strict';
+import { describe, it } from 'node:test';
+import * as cheerio from 'cheerio';
+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/',
+		});
+		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');
+	});
+});
diff --git a/packages/astro/test/fixtures/client-address-node/astro.config.mjs b/packages/astro/test/fixtures/client-address-node/astro.config.mjs
@@ -0,0 +1,8 @@
+import node from '@astrojs/node';
+import { defineConfig } from 'astro/config';
+
+// https://astro.build/config
+export default defineConfig({
+	output: 'server',
+	adapter: node({ mode: 'middleware' }),
+});
diff --git a/packages/astro/test/fixtures/client-address-node/package.json b/packages/astro/test/fixtures/client-address-node/package.json
@@ -0,0 +1,9 @@
+{
+  "name": "@test/client-address-node",
+  "version": "0.0.0",
+  "private": true,
+  "dependencies": {
+    "@astrojs/node": "workspace:*",
+    "astro": "workspace:*"
+  }
+}
diff --git a/packages/astro/test/fixtures/client-address-node/src/pages/index.astro b/packages/astro/test/fixtures/client-address-node/src/pages/index.astro
@@ -0,0 +1,13 @@
+---
+export const prerender = false;
+const address = Astro.clientAddress;
+---
+<html>
+	<head>
+		<title>Astro.clientAddress</title>
+	</head>
+	<body>
+		<h1>Astro.clientAddress</h1>
+		<div id="address">{ address }</div>
+	</body>
+</html>
diff --git a/packages/astro/test/test-utils.js b/packages/astro/test/test-utils.js
@@ -25,6 +25,8 @@ process.env.ASTRO_TELEMETRY_DISABLED = true;
  * @typedef {import('../src/core/app/index').App} App
  * @typedef {import('../src/cli/check/index').AstroChecker} AstroChecker
  * @typedef {import('../src/cli/check/index').CheckPayload} CheckPayload
+ * @typedef {import('http').IncomingMessage} NodeRequest
+ * @typedef {import('http').ServerResponse} NodeResponse
  *
  *
  * @typedef {Object} Fixture
@@ -40,6 +42,7 @@ process.env.ASTRO_TELEMETRY_DISABLED = true;
  * @property {typeof preview} preview
  * @property {() => Promise<void>} clean
  * @property {() => Promise<App>} loadTestAdapterApp
+ * @property {() => Promise<(req: NodeRequest, res: NodeResponse) => void>} loadNodeAdapterHandler
  * @property {() => Promise<void>} onNextChange
  * @property {typeof check} check
  * @property {typeof sync} sync
@@ -213,6 +216,11 @@ export async function loadFixture(inlineConfig) {
 				});
 			}
 		},
+		loadNodeAdapterHandler: async () => {
+			const url = new URL(`./server/entry.mjs?id=${fixtureId}`, config.outDir);
+			const { handler } = await import(url);
+			return handler;
+		},
 		loadTestAdapterApp: async (streaming) => {
 			const url = new URL(`./server/entry.mjs?id=${fixtureId}`, config.outDir);
 			const { createApp, manifest } = await import(url);
diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml

Original file line number	Diff line number	Diff line change
`@@ -81,7 +81,11 @@ export class NodeApp extends App {`
`81`	`81`	`Object.assign(options, makeRequestBody(req));`
`82`	`82`	`}`
`83`	`83`	`const request = new Request(url, options);`
`84`		`- if (req.socket?.remoteAddress) {`
	`84`	`+`
	`85`	`+ const clientIp = req.headers['x-forwarded-for'];`
	`86`	`+ if (clientIp) {`
	`87`	`+ Reflect.set(request, clientAddressSymbol, clientIp);`
	`88`	`+ } else if (req.socket?.remoteAddress) {`
`85`	`89`	`Reflect.set(request, clientAddressSymbol, req.socket.remoteAddress);`
`86`	`90`	`}`
`87`	`91`	`return request;`