Change response handling hook

When proxyReq is aborted (by us - in response to a redirect response),
it can ultimately trigger an "error" event of type ECONNRESET.
This error was unconditionally propagated to the error handler:
https://github.com/nodejitsu/node-http-proxy/blob/v1.11.1/lib/http-proxy/passes/web-incoming.js#L134-L140

Our proxy error handler responds by writing a response + error code,
which prevents the response from being overwritten.

I found this after upgrading the nock library to 5.2.1+, which contains
510e31c73e
The redirect tests that were previously passing in Node 0.10.x were
failing, with errors like "Can't set headers after they are sent.".

The new implementation makes sure that the proxied response is not
forwarded to the original response.
This commit is contained in:
Rob Wu
2017-07-13 17:08:31 +02:00
parent 81ed058784
commit dcc64052e3

View File

@@ -84,7 +84,28 @@ function proxyRequest(req, res, proxy) {
headers: {
host: location.host,
},
// HACK: Get hold of the proxyReq object, because we need it later.
// https://github.com/nodejitsu/node-http-proxy/blob/v1.11.1/lib/http-proxy/passes/web-incoming.js#L144
buffer: {
pipe: function(proxyReq) {
var proxyReqOn = proxyReq.on;
// Intercepts the handler that connects proxyRes to res.
// https://github.com/nodejitsu/node-http-proxy/blob/v1.11.1/lib/http-proxy/passes/web-incoming.js#L146-L158
proxyReq.on = function(eventName, listener) {
if (eventName !== 'response') {
return proxyReqOn.call(this, eventName, listener);
}
return proxyReqOn.call(this, 'response', function(proxyRes) {
if (onProxyResponse(proxy, proxyReq, proxyRes, req, res)) {
listener(proxyRes);
}
});
};
return req.pipe(proxyReq);
},
},
};
var proxyThroughUrl = req.corsAnywhereRequestState.getProxyForUrl(location.href);
if (proxyThroughUrl) {
proxyOptions.target = proxyThroughUrl;
@@ -99,14 +120,16 @@ function proxyRequest(req, res, proxy) {
}
/**
* "Allow observer to modify headers or abort response"
* https://github.com/nodejitsu/node-http-proxy/blob/v1.11.1/lib/http-proxy/passes/web-incoming.js#L147
*
* This method modifies the response headers of the proxied response.
* If a redirect is detected, the response is not sent to the client,
* and a new request is initiated.
*
* @param response {ClientRequest} The response of the proxied request
* client (req) -> CORS Anywhere -> (proxyReq) -> other server
* client (res) <- CORS Anywhere <- (proxyRes) <- other server
*
* @param proxy {HttpProxy}
* @param proxyReq {ClientRequest} The outgoing request to the other server.
* @param proxyRes {ServerResponse} The response from the other server.
* @param req {IncomingMessage} Incoming HTTP request, augmented with property corsAnywhereRequestState
* @param req.corsAnywhereRequestState {object}
* @param req.corsAnywhereRequestState.location {object} See parseURL
@@ -114,22 +137,21 @@ function proxyRequest(req, res, proxy) {
* @param req.corsAnywhereRequestState.proxyBaseUrl {string} Base URL of the CORS API endpoint
* @param req.corsAnywhereRequestState.maxRedirects {number} Maximum number of redirects
* @param req.corsAnywhereRequestState.redirectCount_ {number} Internally used to count redirects
* @param res {ServerResponse} Outgoing (proxied) HTTP request
* @param res {ServerResponse} Outgoing response to the client that wanted to proxy the HTTP request.
*
* @this {HttpProxy}
* @returns {boolean} true if http-proxy should continue to pipe proxyRes to res.
*/
function onProxyResponse(response, req, res) {
var proxy = this;
function onProxyResponse(proxy, proxyReq, proxyRes, req, res) {
var requestState = req.corsAnywhereRequestState;
var statusCode = response.statusCode;
var statusCode = proxyRes.statusCode;
if (!requestState.redirectCount_) {
res.setHeader('x-request-url', requestState.location.href);
}
// Handle redirects
if (statusCode === 301 || statusCode === 302 || statusCode === 303 || statusCode === 307 || statusCode === 308) {
var locationHeader = response.headers.location;
var locationHeader = proxyRes.headers.location;
if (locationHeader) {
locationHeader = url.resolve(requestState.location.href, locationHeader);
@@ -147,38 +169,32 @@ function onProxyResponse(response, req, res) {
delete req.headers['content-type'];
requestState.location = parseURL(locationHeader);
// ### Dispose the current proxied request
// Haha - hack! This should be fixed when (if?) node-http-proxy supports cancelation of requests..
// Shadow all methods that mutate the |res| object.
// See https://github.com/nodejitsu/node-http-proxy/blob/v1.11.1/lib/http-proxy/passes/web-outgoing.js#L73-L100
var setHeader = res.setHeader;
var writeHead = res.writeHead;
res.setHeader = res.writeHead = function noop() {};
response.on = function noop2() {};
response.pipe = function(res) {
res.setHeader = setHeader;
res.writeHead = writeHead;
// Trigger proxyReq.abort() (this is not of any imporance, it's just used to stop wasting resources.)
// https://github.com/nodejitsu/node-http-proxy/blob/v1.11.1/lib/http-proxy/passes/web-incoming.js#L125-L128
req.emit('aborted');
// Remove all listeners (=reset events to initial state)
req.removeAllListeners();
// Initiate a new proxy request.
proxyRequest(req, res, proxy);
};
return;
// Remove all listeners (=reset events to initial state)
req.removeAllListeners();
// Remove the error listener so that the ECONNRESET "error" that
// may occur after aborting a request does not propagate to res.
// https://github.com/nodejitsu/node-http-proxy/blob/v1.11.1/lib/http-proxy/passes/web-incoming.js#L134
proxyReq.removeAllListeners('error');
proxyReq.once('error', function catchAndIgnoreError() {});
proxyReq.abort();
// Initiate a new proxy request.
proxyRequest(req, res, proxy);
return false;
}
}
response.headers.location = requestState.proxyBaseUrl + '/' + locationHeader;
proxyRes.headers.location = requestState.proxyBaseUrl + '/' + locationHeader;
}
}
// Strip cookies
delete response.headers['set-cookie'];
delete response.headers['set-cookie2'];
delete proxyRes.headers['set-cookie'];
delete proxyRes.headers['set-cookie2'];
response.headers['x-final-url'] = requestState.location.href;
withCORS(response.headers, req);
proxyRes.headers['x-final-url'] = requestState.location.href;
withCORS(proxyRes.headers, req);
return true;
}
@@ -380,7 +396,6 @@ exports.createServer = function createServer(options) {
res.writeHead(404, {'Access-Control-Allow-Origin': '*'});
res.end('Not found because of proxy error: ' + err);
});
proxy.on('proxyRes', onProxyResponse);
return server;
};