Skip to content

Commit 34ea884

Browse files
lpincaTooTallNate
authored andcommitted
Use a net.Socket instead of a plain EventEmitter for replaying proxy 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`
1 parent 4296770 commit 34ea884

File tree

2 files changed

+27
-9
lines changed

2 files changed

+27
-9
lines changed

index.js

+8-9
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,11 @@
22
* Module dependencies.
33
*/
44

5+
var assert = require('assert');
56
var net = require('net');
67
var tls = require('tls');
78
var url = require('url');
8-
var events = require('events');
9+
var stream = require('stream');
910
var Agent = require('agent-base');
1011
var inherits = require('util').inherits;
1112
var debug = require('debug')('https-proxy-agent');
@@ -161,14 +162,16 @@ HttpsProxyAgent.prototype.callback = function connect(req, opts, fn) {
161162
// that the node core `http` can parse and handle the error status code
162163
cleanup();
163164

164-
// the original socket is closed, and a "fake socket" EventEmitter is
165+
// the original socket is closed, and a new closed socket is
165166
// returned instead, so that the proxy doesn't get the HTTP request
166167
// written to it (which may contain `Authorization` headers or other
167168
// sensitive data).
168169
//
169170
// See: https://hackerone.com/reports/541502
170171
socket.destroy();
171-
socket = new events.EventEmitter();
172+
socket = new net.Socket();
173+
socket.readable = true;
174+
172175

173176
// save a reference to the concat'd Buffer for the `onsocket` callback
174177
buffers = buffered;
@@ -182,15 +185,11 @@ HttpsProxyAgent.prototype.callback = function connect(req, opts, fn) {
182185

183186
function onsocket(socket) {
184187
debug('replaying proxy buffer for failed request');
188+
assert(socket.listenerCount('data') > 0);
185189

186190
// replay the "buffers" Buffer onto the `socket`, since at this point
187191
// the HTTP module machinery has been hooked up for the user
188-
if (socket.listenerCount('data') > 0) {
189-
socket.emit('data', buffers);
190-
} else {
191-
// never?
192-
throw new Error('should not happen...');
193-
}
192+
socket.push(buffers);
194193

195194
// nullify the cached Buffer instance
196195
buffers = null;

test/test.js

+19
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,25 @@ describe('HttpsProxyAgent', function() {
234234
done();
235235
});
236236
});
237+
it('should not error if the proxy responds with 407 and the request is aborted', function(done) {
238+
proxy.authenticate = function(req, fn) {
239+
fn(null, false);
240+
};
241+
242+
const proxyUri =
243+
process.env.HTTP_PROXY ||
244+
process.env.http_proxy ||
245+
'http://127.0.0.1:' + proxyPort;
246+
247+
const req = http.get({
248+
agent: new HttpsProxyAgent(proxyUri)
249+
}, function(res) {
250+
assert.equal(407, res.statusCode);
251+
req.abort();
252+
});
253+
254+
req.on('abort', done);
255+
});
237256
it('should emit an "error" event on the `http.ClientRequest` if the proxy does not exist', function(done) {
238257
// port 4 is a reserved, but "unassigned" port
239258
var proxyUri = 'http://127.0.0.1:4';

0 commit comments

Comments
 (0)