Skip to content

Commit 1b2f41e

Browse files
committed
[501] Repairs => Can't use proxy() twice in Express middleware stack.
1 parent 7ec6740 commit 1b2f41e

7 files changed

+105
-14
lines changed

app/steps/buildProxyReq.js

+6
Original file line numberDiff line numberDiff line change
@@ -10,15 +10,21 @@ function buildProxyReq(Container) {
1010
var host = Container.proxy.host;
1111

1212
var parseBody = (!options.parseReqBody) ? Promise.resolve(null) : requestOptions.bodyContent(req, res, options);
13+
1314
var createReqOptions = requestOptions.create(req, res, options, host);
1415

1516
return Promise
1617
.all([parseBody, createReqOptions])
1718
.then(function (responseArray) {
19+
req.body = responseArray[0];
1820
Container.proxy.bodyContent = responseArray[0];
1921
Container.proxy.reqBuilder = responseArray[1];
2022
debug('proxy request options:', Container.proxy.reqBuilder);
2123
return Container;
24+
})
25+
.catch(function (err) {
26+
debug('error occurred while building proxy request:', err);
27+
return Promise.reject(err);
2228
});
2329
}
2430

app/steps/maybeSkipToNextHandler.js

-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ function maybeSkipToNextHandler(container) {
1111
.resolve(resolverFn(container.proxy.res))
1212
.then(function (shouldSkipToNext) {
1313
if (shouldSkipToNext) {
14-
container.user.res.expressHttpProxy = container.proxy;
1514
return Promise.reject();
1615
} else {
1716
return Promise.resolve(container);

index.js

+5-4
Original file line numberDiff line numberDiff line change
@@ -44,16 +44,17 @@ module.exports = function proxy(host, userOptions) {
4444
.then(decorateUserRes)
4545
.then(sendUserRes)
4646
.catch(function (err) {
47-
// I sometimes reject without an error to shortcircuit the remaining
48-
// steps and return control to the host application.
49-
5047
if (err) {
5148
var resolver = (container.options.proxyErrorHandler) ?
5249
container.options.proxyErrorHandler :
5350
handleProxyErrors;
5451
resolver(err, res, next);
5552
} else {
56-
next();
53+
// I sometimes reject without an error to shortcircuit the remaining
54+
// steps -- e.g. in maybeSkipToNextHandler -- and return control to
55+
// the host application for continuing on without raising an error.
56+
57+
return next();
5758
}
5859
});
5960
};

lib/requestOptions.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ function bodyContent(req, res, options) {
9191
if (req.body) {
9292
return Promise.resolve(req.body);
9393
} else {
94-
// Returns a promise if no callback specified and global Promise exists.
94+
// Returns a promise
9595

9696
return getRawBody(req, {
9797
length: req.headers['content-length'],

package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
"main": "index.js",
99
"scripts": {
1010
"test": "npm -s run mocha && npm run -s lint",
11-
"test:debug": "mocha debug -R spec test --recursive --exit",
11+
"test:debug": "mocha inspect --debug-brk -R spec test --recursive --exit",
1212
"mocha": "mocha -R spec test --recursive --exit",
1313
"lint": "eslint index.js app/**/*js lib/*js"
1414
},

test/defineMultipleProxyHandlers.js

+88
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
'use strict';
2+
3+
var assert = require('assert');
4+
var express = require('express');
5+
var http = require('http');
6+
var startProxyTarget = require('./support/proxyTarget');
7+
var proxy = require('../');
8+
9+
function fakeProxyServer({path, port, response}) {
10+
var proxyRouteFn = [{
11+
method: 'get',
12+
path: path,
13+
fn: function (req, res) {
14+
res.write(response);
15+
res.end();
16+
}
17+
}];
18+
19+
return startProxyTarget(port, 1000, proxyRouteFn);
20+
}
21+
22+
function simulateUserRequest() {
23+
return new Promise(function (resolve, reject) {
24+
25+
var req = http.request({ hostname: 'localhost', port: 8308, path: '/' }, function (res) {
26+
var chunks = [];
27+
res.on('data', function (chunk) { chunks.push(chunk.toString()); });
28+
res.on('end', function () { resolve(chunks); });
29+
});
30+
31+
req.on('error', function (e) {
32+
reject('problem with request:', e.message);
33+
});
34+
35+
req.end();
36+
})
37+
}
38+
39+
describe('handle multiple proxies in the same runtime', function () {
40+
this.timeout(3000);
41+
42+
var server;
43+
var targetServer, targetServer2;
44+
45+
beforeEach(function () {
46+
targetServer = fakeProxyServer({path:'/', port: '8309', response: '8309_response'});
47+
targetServer2 = fakeProxyServer({path: '/', port: '8310', response: '8310_response'});
48+
});
49+
50+
afterEach(function () {
51+
server.close();
52+
targetServer.close();
53+
targetServer2.close();
54+
});
55+
56+
57+
describe("When two distinct proxies are defined for the global route", () => {
58+
afterEach(() => server.close())
59+
60+
it('the first proxy definition should be used if it succeeds', function (done) {
61+
var app = express();
62+
app.use(proxy('http://localhost:8309', {}));
63+
app.use(proxy('http://localhost:8310', {}));
64+
server = app.listen(8308)
65+
simulateUserRequest()
66+
.then(function (res) {
67+
assert.equal(res[0], '8309_response');
68+
done();
69+
})
70+
.catch(done);
71+
});
72+
73+
it('the fall through definition should be used if the prior skipsToNext', function (done) {
74+
var app = express();
75+
app.use(proxy('http://localhost:8309', {
76+
skipToNextHandlerFilter: () => { return true } // no matter what, reject this proxy request, and call next()
77+
}));
78+
app.use(proxy('http://localhost:8310'))
79+
server = app.listen(8308)
80+
simulateUserRequest()
81+
.then(function (res) {
82+
assert.equal(res[0], '8310_response');
83+
done();
84+
})
85+
.catch(done);
86+
});
87+
})
88+
});

test/maybeSkipToNextHandler.js

+4-7
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ describe('when skipToNextHandlerFilter is defined', function () {
1717
beforeEach(function () {
1818
app = express();
1919
slowTarget = express();
20-
slowTarget.use(function (req, res) { res.sendStatus(404); });
20+
slowTarget.use(function (req, res) { res.sendStatus(407); });
2121
serverReference = slowTarget.listen(12345);
2222
});
2323

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

2828
var OUTCOMES = [
29-
{ shouldSkip: true, expectedStatus: 200 },
30-
{ shouldSkip: false, expectedStatus: 404 }
29+
{ shouldSkip: false, expectedStatus: 407 },
30+
{ shouldSkip: true, expectedStatus: 203 },
3131
];
3232

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

4343
app.use(function (req, res) {
44-
assert(res.expressHttpProxy instanceof Object);
45-
assert(res.expressHttpProxy.res instanceof http.IncomingMessage);
46-
assert(res.expressHttpProxy.req instanceof Object);
47-
res.sendStatus(200);
44+
res.sendStatus(203);
4845
});
4946

5047
request(app)

0 commit comments

Comments
 (0)