From 5f5a5699b2d542321740b79374884e3b55fb0a6c Mon Sep 17 00:00:00 2001 From: Dom Harrington Date: Tue, 6 Dec 2016 17:21:48 -0800 Subject: [PATCH 1/8] Add support for reverse proxies by checking req.hostname or req.host. Fixes #20 This is so that the module works when express has `trust proxy` enabled and the `Host` header is expected to come from `X-Forwarded-Host` --- index.js | 4 +++- package.json | 1 + test/test.js | 41 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 45 insertions(+), 1 deletion(-) diff --git a/index.js b/index.js index 8bd89bc..179eeb2 100644 --- a/index.js +++ b/index.js @@ -74,7 +74,9 @@ function vhost (hostname, handle) { */ function hostnameof (req) { - var host = req.headers.host + var host = req.hostname || // express v4 + req.host || // express v3 + req.headers.host // http if (!host) { return diff --git a/package.json b/package.json index eaf9ddc..24089f0 100644 --- a/package.json +++ b/package.json @@ -9,6 +9,7 @@ "license": "MIT", "repository": "expressjs/vhost", "devDependencies": { + "connect": "^3.5.0", "eslint": "2.11.1", "eslint-config-standard": "5.3.1", "eslint-plugin-promise": "1.3.1", diff --git a/test/test.js b/test/test.js index d5f80b8..f284c1c 100644 --- a/test/test.js +++ b/test/test.js @@ -2,6 +2,7 @@ var assert = require('assert') var http = require('http') var request = require('supertest') +var connect = require('connect') var vhost = require('..') describe('vhost(hostname, server)', function () { @@ -22,6 +23,46 @@ describe('vhost(hostname, server)', function () { .expect(200, 'tobi', done) }) + it('should route by `req.hostname` (express v4)', function (done) { + var app = connect() + + app.use(function (req, res, next) { + req.hostname = 'anotherhost.com' + return next() + }) + + app.use(vhost('anotherhost.com', anotherhost)) + app.use(vhost('loki.com', loki)) + + function anotherhost (req, res) { res.end('anotherhost') } + function loki (req, res) { res.end('loki') } + + request(app) + .get('/') + .set('Host', 'tobi.com') + .expect(200, 'anotherhost', done) + }) + + it('should route by `req.host` (express v3)', function (done) { + var app = connect() + + app.use(function (req, res, next) { + req.host = 'anotherhost.com' + return next() + }) + + app.use(vhost('anotherhost.com', anotherhost)) + app.use(vhost('loki.com', loki)) + + function anotherhost (req, res) { res.end('anotherhost') } + function loki (req, res) { res.end('loki') } + + request(app) + .get('/') + .set('Host', 'tobi.com') + .expect(200, 'anotherhost', done) + }) + it('should ignore port in Host', function (done) { var app = createServer('tobi.com', function (req, res) { res.end('tobi') From be2a50334dbf9db10c5a4b4bf162711a3adb3050 Mon Sep 17 00:00:00 2001 From: Dom Harrington Date: Wed, 7 Dec 2016 10:58:49 -0800 Subject: [PATCH 2/8] Remove connect dependency from tests --- package.json | 1 - test/test.js | 32 +++++++++++++++++--------------- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/package.json b/package.json index 24089f0..eaf9ddc 100644 --- a/package.json +++ b/package.json @@ -9,7 +9,6 @@ "license": "MIT", "repository": "expressjs/vhost", "devDependencies": { - "connect": "^3.5.0", "eslint": "2.11.1", "eslint-config-standard": "5.3.1", "eslint-plugin-promise": "1.3.1", diff --git a/test/test.js b/test/test.js index f284c1c..c7aa83f 100644 --- a/test/test.js +++ b/test/test.js @@ -1,8 +1,6 @@ - var assert = require('assert') var http = require('http') var request = require('supertest') -var connect = require('connect') var vhost = require('..') describe('vhost(hostname, server)', function () { @@ -24,16 +22,17 @@ describe('vhost(hostname, server)', function () { }) it('should route by `req.hostname` (express v4)', function (done) { - var app = connect() + var vhosts = [] + + vhosts.push(vhost('anotherhost.com', anotherhost)) + vhosts.push(vhost('loki.com', loki)) - app.use(function (req, res, next) { + var app = createServer(vhosts) + + app.on('request', function (req) { req.hostname = 'anotherhost.com' - return next() }) - app.use(vhost('anotherhost.com', anotherhost)) - app.use(vhost('loki.com', loki)) - function anotherhost (req, res) { res.end('anotherhost') } function loki (req, res) { res.end('loki') } @@ -44,16 +43,17 @@ describe('vhost(hostname, server)', function () { }) it('should route by `req.host` (express v3)', function (done) { - var app = connect() + var vhosts = [] + + vhosts.push(vhost('anotherhost.com', anotherhost)) + vhosts.push(vhost('loki.com', loki)) - app.use(function (req, res, next) { + var app = createServer(vhosts) + + app.on('request', function (req) { req.host = 'anotherhost.com' - return next() }) - app.use(vhost('anotherhost.com', anotherhost)) - app.use(vhost('loki.com', loki)) - function anotherhost (req, res) { res.end('anotherhost') } function loki (req, res) { res.end('loki') } @@ -272,6 +272,8 @@ function createServer (hostname, server) { vhost(req, res, next) } - next() + // Running tests on next tick so we can modify the request object via + // the `request` event on http.Server + process.nextTick(next) }) } From 1c6655a22d1e5cc84e34cfc12fa0ca1c948ee5bd Mon Sep 17 00:00:00 2001 From: Dom Harrington Date: Wed, 7 Dec 2016 11:00:39 -0800 Subject: [PATCH 3/8] Add documentation for where the hostname comes from --- README.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/README.md b/README.md index 377e361..619046c 100644 --- a/README.md +++ b/README.md @@ -34,6 +34,11 @@ property will be populated with an object. This object will have numeric propert corresponding to each wildcard (or capture group if RegExp object provided) and the `hostname` that was matched. +Where the `hostname` of the request comes from depends on the type of server you're running. +If you're running a raw Node.js/connect server, this comes from [`req.headers.host`](https://nodejs.org/dist/latest/docs/api/http.html#http_message_headers). +If you're running an express v3 server, this comes from [`req.host`](http://expressjs.com/en/3x/api.html#req.host). +If you're running an express v4 server, this comes from [`req.hostname`](http://expressjs.com/en/4x/api.html#req.hostname). + ```js // for match of "foo.bar.example.com:8080" against "*.*.example.com": req.vhost.host === 'foo.bar.example.com:8080' From 85dfed927b7e033aa027a155c9479110d052b49e Mon Sep 17 00:00:00 2001 From: Dom Harrington Date: Wed, 7 Dec 2016 13:28:52 -0800 Subject: [PATCH 4/8] Add failing test cases to show IPv6 problems with double parsing --- test/test.js | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/test/test.js b/test/test.js index c7aa83f..6aa9603 100644 --- a/test/test.js +++ b/test/test.js @@ -85,6 +85,59 @@ describe('vhost(hostname, server)', function () { .expect(200, 'loopback', done) }) + it('should support IPv6 literal in Host with no port', function (done) { + var app = createServer('[::1]', function (req, res) { + res.end('loopback') + }) + + request(app) + .get('/') + .set('Host', '::1') + .expect(200, 'loopback', done) + }) + + it('should support IPv6 literal in `req.host` with port (express v5)', function (done) { + var app = createServer('[::1]', function (req, res) { + res.end('loopback') + }) + + app.on('request', function (req) { + req.host = '[::1]:8080' + }) + + request(app) + .get('/') + .expect(200, 'loopback', done) + }) + + it('should support IPv6 literal in `req.hostname` (express v4)', function (done) { + var app = createServer('[::1]', function (req, res) { + res.end('loopback') + }) + + app.on('request', function (req) { + req.hostname = '::1' + }) + + request(app) + .get('/') + .expect(200, 'loopback', done) + }) + + it('should support IPv6 literal in `req.host` without port (express v3)', function (done) { + var app = createServer('[::1]', function (req, res) { + res.end('loopback') + }) + + app.on('request', function (req) { + req.host = '::1' + }) + + request(app) + .get('/') + .expect(200, 'loopback', done) + }) + it('should 404 unless matched', function (done) { var vhosts = [] From 50f299627e1f337fa54eafcbae29649f62af69a3 Mon Sep 17 00:00:00 2001 From: Dom Harrington Date: Wed, 7 Dec 2016 14:05:48 -0800 Subject: [PATCH 5/8] Remove unneeded test and fix invalid Host headers --- test/test.js | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/test/test.js b/test/test.js index 6aa9603..d9145d0 100644 --- a/test/test.js +++ b/test/test.js @@ -85,17 +85,6 @@ describe('vhost(hostname, server)', function () { .expect(200, 'loopback', done) }) - it('should support IPv6 literal in Host with no port', function (done) { - var app = createServer('[::1]', function (req, res) { - res.end('loopback') - }) - - request(app) - .get('/') - .set('Host', '::1') - .expect(200, 'loopback', done) - }) - it('should support IPv6 literal in `req.host` with port (express v5)', function (done) { var app = createServer('[::1]', function (req, res) { res.end('loopback') @@ -116,7 +105,7 @@ describe('vhost(hostname, server)', function () { }) app.on('request', function (req) { - req.hostname = '::1' + req.hostname = '[::1]' }) request(app) @@ -130,7 +119,7 @@ describe('vhost(hostname, server)', function () { }) app.on('request', function (req) { - req.host = '::1' + req.host = '[::1]' }) request(app) From 496f19be396ab2531149862bfacfdd72e8ae1751 Mon Sep 17 00:00:00 2001 From: Dom Harrington Date: Wed, 7 Dec 2016 14:22:18 -0800 Subject: [PATCH 6/8] Remove `process.nextTick` from the tests --- test/test.js | 32 ++++++++++++-------------------- 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/test/test.js b/test/test.js index d9145d0..34e5898 100644 --- a/test/test.js +++ b/test/test.js @@ -27,9 +27,7 @@ describe('vhost(hostname, server)', function () { vhosts.push(vhost('anotherhost.com', anotherhost)) vhosts.push(vhost('loki.com', loki)) - var app = createServer(vhosts) - - app.on('request', function (req) { + var app = createServer(vhosts, null, function (req) { req.hostname = 'anotherhost.com' }) @@ -48,9 +46,7 @@ describe('vhost(hostname, server)', function () { vhosts.push(vhost('anotherhost.com', anotherhost)) vhosts.push(vhost('loki.com', loki)) - var app = createServer(vhosts) - - app.on('request', function (req) { + var app = createServer(vhosts, null, function (req) { req.host = 'anotherhost.com' }) @@ -88,9 +84,7 @@ describe('vhost(hostname, server)', function () { it('should support IPv6 literal in `req.host` with port (express v5)', function (done) { var app = createServer('[::1]', function (req, res) { res.end('loopback') - }) - - app.on('request', function (req) { + }, function (req) { req.host = '[::1]:8080' }) @@ -102,9 +96,7 @@ describe('vhost(hostname, server)', function () { it('should support IPv6 literal in `req.hostname` (express v4)', function (done) { var app = createServer('[::1]', function (req, res) { res.end('loopback') - }) - - app.on('request', function (req) { + }, function (req) { req.hostname = '[::1]' }) @@ -116,9 +108,7 @@ describe('vhost(hostname, server)', function () { it('should support IPv6 literal in `req.host` without port (express v3)', function (done) { var app = createServer('[::1]', function (req, res) { res.end('loopback') - }) - - app.on('request', function (req) { + }, function (req) { req.host = '[::1]' }) @@ -294,12 +284,16 @@ describe('vhost(hostname, server)', function () { }) }) -function createServer (hostname, server) { +function createServer (hostname, server, pretest) { var vhosts = !Array.isArray(hostname) ? [vhost(hostname, server)] : hostname - return http.createServer(function onRequest (req, res) { + // This allows you to perform changes to the request/response + // objects before our assertions + pretest = pretest || function () {} + + return http.createServer().on('request', pretest).on('request', function onRequest (req, res) { var index = 0 function next (err) { @@ -314,8 +308,6 @@ function createServer (hostname, server) { vhost(req, res, next) } - // Running tests on next tick so we can modify the request object via - // the `request` event on http.Server - process.nextTick(next) + next() }) } From 9429ea10cd3c5a37d672598427f40459787e499d Mon Sep 17 00:00:00 2001 From: Dom Harrington Date: Wed, 7 Dec 2016 17:53:37 -0800 Subject: [PATCH 7/8] Stop reassigning pretest function. Remove multiple listeners --- test/test.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/test/test.js b/test/test.js index 34e5898..d11b9ed 100644 --- a/test/test.js +++ b/test/test.js @@ -289,11 +289,10 @@ function createServer (hostname, server, pretest) { ? [vhost(hostname, server)] : hostname - // This allows you to perform changes to the request/response - // objects before our assertions - pretest = pretest || function () {} - - return http.createServer().on('request', pretest).on('request', function onRequest (req, res) { + return http.createServer(function onRequest (req, res) { + // This allows you to perform changes to the request/response + // objects before our assertions + if (pretest) pretest(req, res) var index = 0 function next (err) { From 0f7cd48f6c1edb38e3e7f821ff83214854861454 Mon Sep 17 00:00:00 2001 From: Dom Harrington Date: Wed, 24 May 2023 13:09:12 +0100 Subject: [PATCH 8/8] ci: attempt to fix gh action I think this was a duplicate line from the one above, so this node-version wasn't getting applied when trying to install from 14.x --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ace0b35..8f59862 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -96,7 +96,7 @@ jobs: - name: Node.js 13.x node-version: "13.14" - - name: Node.js 13.x + - name: Node.js 14.x node-version: "14.21" - name: Node.js 15.x