Skip to content

Commit

Permalink
Use a net.Socket instead of a plain EventEmitter for replaying pr…
Browse files Browse the repository at this point in the history
…oxy errors (#83)

* Run CI on pull requests

* Use a `Duplex` instead of a plain `EventEmitter`

Fixes: #81

* Use a new and closed `net.Socket` instead of a `Duplex`
  • Loading branch information
lpinca authored and TooTallNate committed Oct 17, 2019
1 parent 9d30891 commit 4c0d37e
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 10 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
@@ -1,6 +1,6 @@
name: Node CI

on: [push]
on: [push, pull_request]

jobs:
build:
Expand Down
17 changes: 8 additions & 9 deletions index.js
Expand Up @@ -2,10 +2,11 @@
* Module dependencies.
*/

var assert = require('assert');
var net = require('net');
var tls = require('tls');
var url = require('url');
var events = require('events');
var stream = require('stream');
var Agent = require('agent-base');
var inherits = require('util').inherits;
var debug = require('debug')('https-proxy-agent');
Expand Down Expand Up @@ -161,14 +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" EventEmitter 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 events.EventEmitter();
socket = new net.Socket();
socket.readable = true;


// save a reference to the concat'd Buffer for the `onsocket` callback
buffers = buffered;
Expand All @@ -182,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
19 changes: 19 additions & 0 deletions test/test.js
Expand Up @@ -234,6 +234,25 @@ describe('HttpsProxyAgent', 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);
};

const proxyUri =
process.env.HTTP_PROXY ||
process.env.http_proxy ||
'http://127.0.0.1:' + proxyPort;

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

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 4c0d37e

Please sign in to comment.