Skip to content

Commit

Permalink
Use a new and closed net.Socket instead of a Duplex
Browse files Browse the repository at this point in the history
  • Loading branch information
lpinca committed Oct 16, 2019
1 parent f90cd90 commit 093b8de
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 34 deletions.
26 changes: 7 additions & 19 deletions index.js
Expand Up @@ -2,6 +2,7 @@
* Module dependencies.
*/

var assert = require('assert');
var net = require('net');
var tls = require('tls');
var url = require('url');
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -250,5 +240,3 @@ function resume(socket) {
function isDefaultPort(port, secure) {
return Boolean((!secure && port === 80) || (secure && port === 443));
}

function noop() {}
31 changes: 16 additions & 15 deletions test/test.js
Expand Up @@ -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';
Expand Down

0 comments on commit 093b8de

Please sign in to comment.