From 6859c1f33207df791c7e74a6de1a57d63e296fdb Mon Sep 17 00:00:00 2001 From: Evert Pot Date: Wed, 6 Nov 2024 17:21:23 -0500 Subject: [PATCH] FIx CURVEBALL_TRUSTPROXY again + add a test --- changelog.md | 6 +++ package-lock.json | 22 +++++--- package.json | 4 +- src/node/request.ts | 2 +- test/application.ts | 118 +++++++++++++++++++++++++------------------ test/node/request.ts | 28 ++++++++++ 6 files changed, 122 insertions(+), 58 deletions(-) diff --git a/changelog.md b/changelog.md index 5973013..d4382c5 100644 --- a/changelog.md +++ b/changelog.md @@ -1,6 +1,12 @@ Changelog ========= +1.0.2 (????-??-??) +------------------ + +* Fixed another bug related to CURVEBALL_TRUSTPROXY + + 1.0.1 (2024-10-30) ------------------ diff --git a/package-lock.json b/package-lock.json index 9fa0f49..c2300cb 100644 --- a/package-lock.json +++ b/package-lock.json @@ -16,7 +16,7 @@ "devDependencies": { "@curveball/http-errors": "^0.5.0", "@curveball/kernel": "^1.0.0", - "@types/chai": "^4.2.15", + "@types/chai": "^5.0.1", "@types/co-body": "^6.1.0", "@types/mocha": "^10.0.1", "@types/node": "^18.19.6", @@ -24,7 +24,7 @@ "@types/sinon": "^17.0.3", "@typescript-eslint/eslint-plugin": "^6.18.1", "@typescript-eslint/parser": "^6.18.1", - "chai": "^5.0.0", + "chai": "^5.1.2", "eslint": "^8.6.0", "mocha": "^10.2.0", "node-fetch": "^3.3.0", @@ -807,11 +807,14 @@ "license": "MIT" }, "node_modules/@types/chai": { - "version": "4.3.20", - "resolved": "https://registry.npmjs.org/@types/chai/-/chai-4.3.20.tgz", - "integrity": "sha512-/pC9HAB5I/xMlc5FP77qjCnI16ChlJfW0tGa0IUcFn38VJrTV6DeZ60NU5KZBtaOZqjdpwTWohz5HU1RrhiYxQ==", + "version": "5.0.1", + "resolved": "https://registry.npmjs.org/@types/chai/-/chai-5.0.1.tgz", + "integrity": "sha512-5T8ajsg3M/FOncpLYW7sdOcD6yf4+722sze/tc4KQV0P8Z2rAr3SAuHCIkYmYpt8VbcQlnz8SxlOlPQYefe4cA==", "dev": true, - "license": "MIT" + "license": "MIT", + "dependencies": { + "@types/deep-eql": "*" + } }, "node_modules/@types/co-body": { "version": "6.1.3", @@ -824,6 +827,13 @@ "@types/qs": "*" } }, + "node_modules/@types/deep-eql": { + "version": "4.0.2", + "resolved": "https://registry.npmjs.org/@types/deep-eql/-/deep-eql-4.0.2.tgz", + "integrity": "sha512-c9h9dVVMigMPc4bwTvC5dxqtqJZwQPePsWjPlpSOnojbor6pGqdk541lfA7AqFQr5pB1BRdq0juY9db81BwyFw==", + "dev": true, + "license": "MIT" + }, "node_modules/@types/json-schema": { "version": "7.0.15", "resolved": "https://registry.npmjs.org/@types/json-schema/-/json-schema-7.0.15.tgz", diff --git a/package.json b/package.json index 9b930b3..e51a700 100644 --- a/package.json +++ b/package.json @@ -40,7 +40,7 @@ "devDependencies": { "@curveball/http-errors": "^0.5.0", "@curveball/kernel": "^1.0.0", - "@types/chai": "^4.2.15", + "@types/chai": "^5.0.1", "@types/co-body": "^6.1.0", "@types/mocha": "^10.0.1", "@types/node": "^18.19.6", @@ -48,7 +48,7 @@ "@types/sinon": "^17.0.3", "@typescript-eslint/eslint-plugin": "^6.18.1", "@typescript-eslint/parser": "^6.18.1", - "chai": "^5.0.0", + "chai": "^5.1.2", "eslint": "^8.6.0", "mocha": "^10.2.0", "node-fetch": "^3.3.0", diff --git a/src/node/request.ts b/src/node/request.ts index cecb5ff..2a53d90 100644 --- a/src/node/request.ts +++ b/src/node/request.ts @@ -83,7 +83,7 @@ export class NodeRequest extends Request { */ ip(trustProxy?: boolean): string { - if (trustProxy ?? process.env.CURVEBALL_TRUSTPROXY) { + if (trustProxy===true || process.env.CURVEBALL_TRUSTPROXY) { const forwardedForHeader = this.headers.get('X-Forwarded-For'); if (forwardedForHeader) { return forwardedForHeader.split(',')[0].trim(); diff --git a/test/application.ts b/test/application.ts index ae9f9d7..2a2eabe 100644 --- a/test/application.ts +++ b/test/application.ts @@ -1,6 +1,7 @@ -import { expect } from 'chai'; +import { strict as assert } from 'node:assert'; import { Application, middlewareCall, MemoryRequest, Context } from '../src/index.js'; import { Readable, Writable } from 'node:stream'; +import { expect } from 'chai'; let lastPort = 5555; const getPort = () => { @@ -10,12 +11,12 @@ const getPort = () => { describe('Application', () => { it('should instantiate', () => { const application = new Application(); - expect(application).to.be.an.instanceof(Application); + assert.ok(application instanceof Application); }); it('should respond to HTTP requests', async () => { const application = new Application(); - application.use((ctx, next) => { + application.use(ctx => { ctx.response.body = 'string'; }); @@ -25,16 +26,19 @@ describe('Application', () => { const response = await fetch('http://localhost:' + port); const body = await response.text(); - expect(body).to.equal('string'); - expect(response.headers.get('server')).to.match(/Curveball\//); - expect(response.status).to.equal(200); + assert.equal(body, 'string'); + assert.match( + response.headers.get('server')!, + /Curveball\// + ); + assert.equal(response.status, 200); server.close(); }); it('should work with Buffer responses', async () => { const application = new Application(); - application.use((ctx, next) => { + application.use(ctx => { ctx.response.body = Buffer.from('buffer'); }); @@ -45,16 +49,19 @@ describe('Application', () => { const response = await fetch('http://localhost:'+port); const body = await response.text(); - expect(body).to.equal('buffer'); - expect(response.headers.get('server')).to.match(/Curveball\//); - expect(response.status).to.equal(200); + assert.equal(body, 'buffer'); + assert.match( + response.headers.get('server')!, + /Curveball\// + ); + assert.equal(response.status, 200); server.close(); }); it('should work with Readable stream responses', async () => { const application = new Application(); - application.use((ctx, next) => { + application.use(ctx => { ctx.response.body = Readable.from(Buffer.from('stream')); }); @@ -64,16 +71,18 @@ describe('Application', () => { const response = await fetch('http://localhost:'+port); const body = await response.text(); - expect(body.substring(0, 6)).to.equal('stream'); - expect(response.headers.get('server')).to.match(/Curveball\//); - expect(response.status).to.equal(200); - + assert.equal(body, 'stream'); + assert.match( + response.headers.get('server')!, + /Curveball\// + ); + assert.equal(response.status, 200); server.close(); }); it('should work with a callback resonse body', async () => { const application = new Application(); - application.use((ctx, next) => { + application.use(ctx => { ctx.response.body = (stream: Writable) => { stream.write('callback'); stream.end(); @@ -85,16 +94,19 @@ describe('Application', () => { const response = await fetch('http://localhost:'+port); const body = await response.text(); - expect(body).to.equal('callback'); - expect(response.headers.get('server')).to.match(/Curveball\//); - expect(response.status).to.equal(200); + assert.equal(body, 'callback'); + assert.match( + response.headers.get('server')!, + /Curveball\// + ); + assert.equal(response.status, 200); server.close(); }); it('should automatically JSON-encode objects', async () => { const application = new Application(); - application.use((ctx, next) => { + application.use(ctx => { ctx.response.body = { foo: 'bar' }; }); const port = getPort(); @@ -103,16 +115,19 @@ describe('Application', () => { const response = await fetch('http://localhost:'+port); const body = await response.text(); - expect(body).to.equal('{\n "foo": "bar"\n}'); - expect(response.headers.get('server')).to.match(/Curveball\//); - expect(response.status).to.equal(200); + assert.equal(body, '{\n "foo": "bar"\n}'); + assert.match( + response.headers.get('server')!, + /Curveball\// + ); + assert.equal(response.status, 200); server.close(); }); it('should handle "null" bodies', async () => { const application = new Application(); - application.use((ctx, next) => { + application.use(ctx => { ctx.response.body = null; }); const port = getPort(); @@ -121,16 +136,18 @@ describe('Application', () => { const response = await fetch('http://localhost:'+port); const body = await response.text(); - expect(body).to.equal(''); - expect(response.headers.get('server')).to.match(/Curveball\//); - expect(response.status).to.equal(200); - + assert.equal(body, ''); + assert.match( + response.headers.get('server')!, + /Curveball\// + ); + assert.equal(response.status, 200); server.close(); }); it('should throw an exception for unsupported bodies', async () => { const application = new Application(); - application.use((ctx, next) => { + application.use(ctx => { ctx.response.body = 5; }); const port = getPort(); @@ -139,9 +156,12 @@ describe('Application', () => { const response = await fetch('http://localhost:'+port); const body = await response.text(); - expect(body).to.include(': 500'); - expect(response.headers.get('server')).to.match(/Curveball\//); - expect(response.status).to.equal(500); + assert.ok(body.includes(': 500')); + assert.match( + response.headers.get('server')!, + /Curveball\// + ); + assert.equal(response.status, 500); server.close(); }); @@ -152,7 +172,7 @@ describe('Application', () => { ctx.response.body = 'hi'; await next(); }); - application.use((ctx, next) => { + application.use(ctx => { ctx.response.headers.set('X-Foo', 'bar'); }); const port = getPort(); @@ -161,9 +181,9 @@ describe('Application', () => { const response = await fetch('http://localhost:'+port); const body = await response.text(); - expect(body).to.equal('hi'); - expect(response.headers.get('X-Foo')).to.equal('bar'); - expect(response.status).to.equal(200); + assert.equal(body, 'hi'); + assert.equal(response.headers.get('X-Foo'), 'bar'); + assert.equal(response.status, 200); server.close(); }); @@ -182,9 +202,9 @@ describe('Application', () => { const response = await fetch('http://localhost:'+port); const body = await response.text(); - expect(body).to.equal('hi'); - expect(response.headers.get('X-Foo')).to.equal('bar'); - expect(response.status).to.equal(200); + assert.equal(body, 'hi'); + assert.equal(response.headers.get('X-Foo'), 'bar'); + assert.equal(response.status, 200); server.close(); }); @@ -207,18 +227,18 @@ describe('Application', () => { const response = await fetch('http://localhost:'+port); const body = await response.text(); - expect(body).to.equal('hi'); - expect(response.status).to.equal(200); + assert.equal(body, 'hi'); + assert.equal(response.status, 200); server.close(); }); it('should not call sequential middlewares if next is not called', async () => { const application = new Application(); - application.use((ctx, next) => { + application.use(ctx => { ctx.response.body = 'hi'; }); - application.use((ctx, next) => { + application.use(ctx => { ctx.response.headers.set('X-Foo', 'bar'); }); const port = getPort(); @@ -227,9 +247,9 @@ describe('Application', () => { const response = await fetch('http://localhost:'+port); const body = await response.text(); - expect(body).to.equal('hi'); - expect(response.headers.get('X-Foo')).to.equal(null); - expect(response.status).to.equal(200); + assert.equal(body, 'hi'); + assert.equal(response.headers.get('X-Foo'), null); + assert.equal(response.status, 200); server.close(); }); @@ -249,8 +269,8 @@ describe('Application', () => { await fetch('http://localhost:'+port); - expect(error).to.be.an.instanceof(Error); - expect(error.message).to.equal('hi'); + assert.ok(error instanceof Error); + assert.equal(error!.message, 'hi'); server.close(); }); @@ -266,7 +286,7 @@ describe('Application', () => { const response = await fetch('http://localhost:'+port); const body = await response.text(); - expect(body).to.include(': 500'); + assert.ok(body.includes(': 500')); server.close(); }); diff --git a/test/node/request.ts b/test/node/request.ts index d843af6..f22770c 100644 --- a/test/node/request.ts +++ b/test/node/request.ts @@ -284,6 +284,34 @@ describe('NodeRequest', () => { ]); }); + it('should use X-Forwarded-For if CURVEBALL_TRUSTPROXY environment variable was set', async () => { + + const app = new Application(); + const port = getPort(); + const server = app.listen(port); + let ip; + + process.env.CURVEBALL_TRUSTPROXY = '1'; + app.use(async ctx => { + ip = ctx.ip(); + }); + + await fetch('http://localhost:'+port+'/foo/bar?a=1&b=2', { + method: 'POST', + headers: { + 'accept': 'text/html', + 'content-type': 'text/html; charset=utf-8', + 'x-forwarded-for': '127.0.0.2', + + }, + body: 'hello', + }); + + delete process.env.CURVEBALL_TRUSTPROXY; + server.close(); + expect(ip).to.eql('127.0.0.2'); + + }); it('should use the clients ip if trustProxy was true but there was no XFF header', async () => {