http: remove default 'timeout' listener on upgrade

Remove the default listener of the `'timeout'` event from the socket before emitting the `'connect'` or `'upgrade'` event. PR-URL: #26030 Fixes: #23857 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Rich Trott <[email protected]>
nodejs · Jun 8, 2019 · d5577f0 · d5577f0
1 parent a18e27d
commit d5577f0
Show file tree

Hide file tree

Showing 3 changed files with 9 additions and 1 deletion.
diff --git a/lib/_http_client.js b/lib/_http_client.js
@@ -465,6 +465,10 @@ function socketOnData(d) {
     socket.removeListener('data', socketOnData);
     socket.removeListener('end', socketOnEnd);
     socket.removeListener('drain', ondrain);
+
+    if (req.timeoutCb)
+      socket.removeListener('timeout', req.timeoutCb);
+
     parser.finish();
     freeParser(parser, req, socket);
 

diff --git a/lib/_http_server.js b/lib/_http_server.js
@@ -568,6 +568,7 @@ function onParserExecuteCommon(server, socket, parser, state, ret, d) {
     socket.removeListener('drain', state.onDrain);
     socket.removeListener('drain', ondrain);
     socket.removeListener('error', socketOnError);
+    socket.removeListener('timeout', socketOnTimeout);
     unconsume(parser, socket);
     parser.finish();
     freeParser(parser, req, socket);

diff --git a/test/parallel/test-http-connect.js b/test/parallel/test-http-connect.js
@@ -36,6 +36,7 @@ server.on('connect', common.mustCall((req, socket, firstBodyChunk) => {
   assert.strictEqual(socket.listenerCount('data'), 0);
   assert.strictEqual(socket.listenerCount('end'), 1);
   assert.strictEqual(socket.listenerCount('error'), 0);
+  assert.strictEqual(socket.listenerCount('timeout'), 0);
 
   socket.write('HTTP/1.1 200 Connection established\r\n\r\n');
 
@@ -53,7 +54,8 @@ server.listen(0, common.mustCall(() => {
   const req = http.request({
     port: server.address().port,
     method: 'CONNECT',
-    path: 'google.com:443'
+    path: 'google.com:443',
+    timeout: 20000
   }, common.mustNotCall());
 
   req.on('socket', common.mustCall((socket) => {
@@ -80,6 +82,7 @@ server.listen(0, common.mustCall(() => {
     assert.strictEqual(socket.listenerCount('close'), 0);
     assert.strictEqual(socket.listenerCount('error'), 0);
     assert.strictEqual(socket.listenerCount('agentRemove'), 0);
+    assert.strictEqual(socket.listenerCount('timeout'), 0);
 
     let data = firstBodyChunk.toString();
     socket.on('data', (buf) => {