Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

this.socket.destroy is not a function #81

Closed
gajus opened this issue Oct 9, 2019 · 3 comments · Fixed by #83
Closed

this.socket.destroy is not a function #81

gajus opened this issue Oct 9, 2019 · 3 comments · Fixed by #83

Comments

@gajus
Copy link

gajus commented Oct 9, 2019

After upgrading to v3, I started to get errors:

TypeError: this.socket.destroy is not a function
    at ClientRequest.abort (_http_client.js:328:17)
    at Immediate.timeoutHandler (/srv/node_modules/got/source/utils/timed-out.js:66:11)
    at processImmediate (internal/timers.js:441:21)
    at process.topLevelDomainCallback (domain.js:131:23)

So far I wasn't able to isolate what is causing them.

@lpinca
Copy link
Contributor

lpinca commented Oct 12, 2019

I think it is caused by this https://github.com/TooTallNate/node-https-proxy-agent/blob/3.0.0/index.js#L171. To fix https://hackerone.com/reports/541502, a fake socket is now returned to the client if the proxy server does not respond with 200.

I wonder if using a Duplex instead of a plain EventEmitter fixes this issue. Can you try this patch?

diff --git a/index.js b/index.js
index aeb624d..ea0a9f8 100644
--- a/index.js
+++ b/index.js
@@ -5,7 +5,7 @@
 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');
@@ -168,7 +168,10 @@ HttpsProxyAgent.prototype.callback = function connect(req, opts, fn) {
 			//
 			// See: https://hackerone.com/reports/541502
 			socket.destroy();
-			socket = new events.EventEmitter();
+			socket = new stream.Duplex({
+				read() {},
+				write() {}
+			});
 
 			// save a reference to the concat'd Buffer for the `onsocket` callback
 			buffers = buffered;

@gajus
Copy link
Author

gajus commented Oct 12, 2019

I have not been able to replicate this issue in isolation. Cannot deploy a patch to production. I have added it to local development, but I have not really seen this happen locally.

@gajus
Copy link
Author

gajus commented Oct 16, 2019

Any plans on releasing this patch?

lpinca added a commit to lpinca/node-https-proxy-agent that referenced this issue Oct 16, 2019
lpinca added a commit to lpinca/node-https-proxy-agent that referenced this issue Oct 16, 2019
lpinca added a commit to lpinca/node-https-proxy-agent that referenced this issue Oct 16, 2019
lpinca added a commit to lpinca/node-https-proxy-agent that referenced this issue Oct 16, 2019
TooTallNate pushed a commit that referenced this issue Oct 17, 2019
…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`
TooTallNate pushed a commit that referenced this issue Oct 17, 2019
TooTallNate pushed a commit that referenced this issue Oct 25, 2019
…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`
adrianchu0120 added a commit to adrianchu0120/proxy-agents that referenced this issue Aug 25, 2024
…oxy errors (#83)

* Run CI on pull requests

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

Fixes: TooTallNate/proxy-agents#81

* Use a new and closed `net.Socket` instead of a `Duplex`
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
2 participants