Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[501] Repairs => Can't use proxy() twice in Express middleware stack. #529

Merged
merged 1 commit into from
Sep 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading