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 f6e2538
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 22 deletions.
24 changes: 7 additions & 17 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
11 changes: 6 additions & 5 deletions test/test.js
Expand Up @@ -234,7 +234,7 @@ describe('HttpsProxyAgent', function() {
done();
});
});
it('should not error if the request is aborted and a fake socket is assigned to it', function(done) {
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);
};
Expand All @@ -244,11 +244,12 @@ describe('HttpsProxyAgent', function() {
process.env.http_proxy ||
'http://127.0.0.1:' + proxyPort;

const req = http.get({ agent: new HttpsProxyAgent(proxyUri) });

req.on('socket', function() {
const req = http.get({
agent: new HttpsProxyAgent(proxyUri)
}, function(res) {
assert.equal(407, res.statusCode);
req.abort();
})
});

req.on('abort', done);
});
Expand Down

0 comments on commit f6e2538

Please sign in to comment.