Skip to content

Commit

Permalink
[501] Repairs => Can't use proxy() twice in Express middleware stack.
Browse files Browse the repository at this point in the history
  • Loading branch information
nik-blue-lava authored and monkpow committed Sep 11, 2023
1 parent 7ec6740 commit 21479d2
Show file tree
Hide file tree
Showing 7 changed files with 105 additions and 14 deletions.
6 changes: 6 additions & 0 deletions app/steps/buildProxyReq.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,21 @@ function buildProxyReq(Container) {
var host = Container.proxy.host;

var parseBody = (!options.parseReqBody) ? Promise.resolve(null) : requestOptions.bodyContent(req, res, options);

var createReqOptions = requestOptions.create(req, res, options, host);

return Promise
.all([parseBody, createReqOptions])
.then(function (responseArray) {
req.body = responseArray[0];
Container.proxy.bodyContent = responseArray[0];
Container.proxy.reqBuilder = responseArray[1];
debug('proxy request options:', Container.proxy.reqBuilder);
return Container;
})
.catch(function (err) {
debug('error occurred while building proxy request:', err);
return Promise.reject(err);
});
}

Expand Down
1 change: 0 additions & 1 deletion app/steps/maybeSkipToNextHandler.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ function maybeSkipToNextHandler(container) {
.resolve(resolverFn(container.proxy.res))
.then(function (shouldSkipToNext) {
if (shouldSkipToNext) {
container.user.res.expressHttpProxy = container.proxy;
return Promise.reject();
} else {
return Promise.resolve(container);
Expand Down
9 changes: 5 additions & 4 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,16 +44,17 @@ module.exports = function proxy(host, userOptions) {
.then(decorateUserRes)
.then(sendUserRes)
.catch(function (err) {
// I sometimes reject without an error to shortcircuit the remaining
// steps and return control to the host application.

if (err) {
var resolver = (container.options.proxyErrorHandler) ?
container.options.proxyErrorHandler :
handleProxyErrors;
resolver(err, res, next);
} else {
next();
// I sometimes reject without an error to shortcircuit the remaining
// steps -- e.g. in maybeSkipToNextHandler -- and return control to
// the host application for continuing on without raising an error.

return next();
}
});
};
Expand Down
2 changes: 1 addition & 1 deletion lib/requestOptions.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ function bodyContent(req, res, options) {
if (req.body) {
return Promise.resolve(req.body);
} else {
// Returns a promise if no callback specified and global Promise exists.
// Returns a promise

return getRawBody(req, {
length: req.headers['content-length'],
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
"main": "index.js",
"scripts": {
"test": "npm -s run mocha && npm run -s lint",
"test:debug": "mocha debug -R spec test --recursive --exit",
"test:debug": "mocha inspect --debug-brk -R spec test --recursive --exit",
"mocha": "mocha -R spec test --recursive --exit",
"lint": "eslint index.js app/**/*js lib/*js"
},
Expand Down
88 changes: 88 additions & 0 deletions test/defineMultipleProxyHandlers.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
'use strict';

var assert = require('assert');
var express = require('express');
var http = require('http');
var startProxyTarget = require('./support/proxyTarget');
var proxy = require('../');

function fakeProxyServer({path, port, response}) {
var proxyRouteFn = [{
method: 'get',
path: path,
fn: function (req, res) {
res.write(response);
res.end();
}
}];

return startProxyTarget(port, 1000, proxyRouteFn);
}

function simulateUserRequest() {
return new Promise(function (resolve, reject) {

var req = http.request({ hostname: 'localhost', port: 8308, path: '/' }, function (res) {
var chunks = [];
res.on('data', function (chunk) { chunks.push(chunk.toString()); });
res.on('end', function () { resolve(chunks); });
});

req.on('error', function (e) {
reject('problem with request:', e.message);
});

req.end();
})
}

describe('handle multiple proxies in the same runtime', function () {
this.timeout(3000);

var server;
var targetServer, targetServer2;

beforeEach(function () {
targetServer = fakeProxyServer({path:'/', port: '8309', response: '8309_response'});
targetServer2 = fakeProxyServer({path: '/', port: '8310', response: '8310_response'});
});

afterEach(function () {
server.close();
targetServer.close();
targetServer2.close();
});


describe("When two distinct proxies are defined for the global route", () => {
afterEach(() => server.close())

it('the first proxy definition should be used if it succeeds', function (done) {
var app = express();
app.use(proxy('http://localhost:8309', {}));
app.use(proxy('http://localhost:8310', {}));
server = app.listen(8308)
simulateUserRequest()
.then(function (res) {
assert.equal(res[0], '8309_response');
done();
})
.catch(done);
});

it('the fall through definition should be used if the prior skipsToNext', function (done) {
var app = express();
app.use(proxy('http://localhost:8309', {
skipToNextHandlerFilter: () => { return true } // no matter what, reject this proxy request, and call next()
}));
app.use(proxy('http://localhost:8310'))
server = app.listen(8308)
simulateUserRequest()
.then(function (res) {
assert.equal(res[0], '8310_response');
done();
})
.catch(done);
});
})
});
11 changes: 4 additions & 7 deletions test/maybeSkipToNextHandler.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ describe('when skipToNextHandlerFilter is defined', function () {
beforeEach(function () {
app = express();
slowTarget = express();
slowTarget.use(function (req, res) { res.sendStatus(404); });
slowTarget.use(function (req, res) { res.sendStatus(407); });
serverReference = slowTarget.listen(12345);
});

Expand All @@ -26,8 +26,8 @@ describe('when skipToNextHandlerFilter is defined', function () {
});

var OUTCOMES = [
{ shouldSkip: true, expectedStatus: 200 },
{ shouldSkip: false, expectedStatus: 404 }
{ shouldSkip: false, expectedStatus: 407 },
{ shouldSkip: true, expectedStatus: 203 },
];

OUTCOMES.forEach(function (outcome) {
Expand All @@ -41,10 +41,7 @@ describe('when skipToNextHandlerFilter is defined', function () {
}));

app.use(function (req, res) {
assert(res.expressHttpProxy instanceof Object);
assert(res.expressHttpProxy.res instanceof http.IncomingMessage);
assert(res.expressHttpProxy.req instanceof Object);
res.sendStatus(200);
res.sendStatus(203);
});

request(app)
Expand Down

0 comments on commit 21479d2

Please sign in to comment.