Skip to content

Commit

Permalink
http: remove default 'timeout' listener on upgrade
Browse files Browse the repository at this point in the history
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 <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
  • Loading branch information
lpinca committed Jun 8, 2019
1 parent a18e27d commit d5577f0
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 1 deletion.
4 changes: 4 additions & 0 deletions lib/_http_client.js
Expand Up @@ -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);

Expand Down
1 change: 1 addition & 0 deletions lib/_http_server.js
Expand Up @@ -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);
Expand Down
5 changes: 4 additions & 1 deletion test/parallel/test-http-connect.js
Expand Up @@ -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');

Expand All @@ -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) => {
Expand All @@ -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) => {
Expand Down

0 comments on commit d5577f0

Please sign in to comment.