From dcc64052e30f0c32100c2a98b5338b25ce5be93a Mon Sep 17 00:00:00 2001 From: Rob Wu Date: Thu, 13 Jul 2017 17:08:31 +0200 Subject: [PATCH] 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 https://github.com/node-nock/nock/commit/510e31c73e46cf44752c8b954e832d67acb8864a 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. --- lib/cors-anywhere.js | 87 ++++++++++++++++++++++++++------------------ 1 file changed, 51 insertions(+), 36 deletions(-) diff --git a/lib/cors-anywhere.js b/lib/cors-anywhere.js index 53a4696..d4ee6e2 100644 --- a/lib/cors-anywhere.js +++ b/lib/cors-anywhere.js @@ -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; };