From d9b6eef1dd5f89e9bb9a2e9753b13c7f987dc7d8 Mon Sep 17 00:00:00 2001 From: Luigi Pinca Date: Wed, 16 Oct 2019 22:27:38 +0200 Subject: [PATCH] Use a new and closed `net.Socket` instead of a `Duplex` --- index.js | 26 +++++++------------------- test/test.js | 31 ++++++++++++++++--------------- 2 files changed, 23 insertions(+), 34 deletions(-) diff --git a/index.js b/index.js index 2be8a4d2..916e333e 100644 --- a/index.js +++ b/index.js @@ -2,6 +2,7 @@ * Module dependencies. */ +var assert = require('assert'); var net = require('net'); var tls = require('tls'); var url = require('url'); @@ -161,23 +162,16 @@ HttpsProxyAgent.prototype.callback = function connect(req, opts, fn) { // that the node core `http` can parse and handle the error status code cleanup(); - // the original socket is closed, and a "fake socket" Duplex is + // the original socket is closed, and a new closed socket is // returned instead, so that the proxy doesn't get the HTTP request // written to it (which may contain `Authorization` headers or other // sensitive data). // // See: https://hackerone.com/reports/541502 socket.destroy(); - socket = new stream.Duplex({ - read() {}, - write(chunk, encoding, callback) { - callback(); - } - }); - - if (process.versions.modules === '48') { - socket.destroy = noop; - } + socket = new net.Socket(); + socket.readable = true; + // save a reference to the concat'd Buffer for the `onsocket` callback buffers = buffered; @@ -191,15 +185,11 @@ HttpsProxyAgent.prototype.callback = function connect(req, opts, fn) { function onsocket(socket) { debug('replaying proxy buffer for failed request'); + assert(socket.listenerCount('data') > 0); // replay the "buffers" Buffer onto the `socket`, since at this point // the HTTP module machinery has been hooked up for the user - if (socket.listenerCount('data') > 0) { - socket.emit('data', buffers); - } else { - // never? - throw new Error('should not happen...'); - } + socket.push(buffers); // nullify the cached Buffer instance buffers = null; @@ -250,5 +240,3 @@ function resume(socket) { function isDefaultPort(port, secure) { return Boolean((!secure && port === 80) || (secure && port === 443)); } - -function noop() {} diff --git a/test/test.js b/test/test.js index f4ae8c1c..61a02320 100644 --- a/test/test.js +++ b/test/test.js @@ -234,24 +234,25 @@ describe('HttpsProxyAgent', function() { done(); }); }); - it('should not error if the request is aborted and a fake socket is assigned to it', function(done) { - proxy.authenticate = function(req, fn) { - fn(null, false); - }; - - const proxyUri = - process.env.HTTP_PROXY || - process.env.http_proxy || - 'http://127.0.0.1:' + proxyPort; + it('should not error if the proxy responds with 407 and the request is aborted', function(done) { + proxy.authenticate = function(req, fn) { + fn(null, false); + }; - const req = http.get({ agent: new HttpsProxyAgent(proxyUri) }); + const proxyUri = + process.env.HTTP_PROXY || + process.env.http_proxy || + 'http://127.0.0.1:' + proxyPort; - req.on('socket', function() { - req.abort(); - }) + const req = http.get({ + agent: new HttpsProxyAgent(proxyUri) + }, function(res) { + assert.equal(407, res.statusCode); + req.abort(); + }); - req.on('abort', done); - }); + req.on('abort', done); + }); it('should emit an "error" event on the `http.ClientRequest` if the proxy does not exist', function(done) { // port 4 is a reserved, but "unassigned" port var proxyUri = 'http://127.0.0.1:4';