http: use 'connect' event only if socket is connecting
Fixes a bug that prevented `ClientRequest.prototype.setTimeout()` from working properly when the socket was reused for multiple requests. Fixes: https://github.com/nodejs/node/issues/16716 Refs: https://github.com/nodejs/node/pull/8895 PR-URL: https://github.com/nodejs/node/pull/16725 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
This commit is contained in:
parent
fb31e07450
commit
f60c692499
@ -728,9 +728,13 @@ ClientRequest.prototype.setTimeout = function setTimeout(msecs, callback) {
|
||||
}
|
||||
|
||||
this.once('socket', function(sock) {
|
||||
sock.once('connect', function() {
|
||||
if (sock.connecting) {
|
||||
sock.once('connect', function() {
|
||||
sock.setTimeout(msecs, emitTimeout);
|
||||
});
|
||||
} else {
|
||||
sock.setTimeout(msecs, emitTimeout);
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
return this;
|
||||
|
42
test/parallel/test-http-client-timeout-connect-listener.js
Normal file
42
test/parallel/test-http-client-timeout-connect-listener.js
Normal file
@ -0,0 +1,42 @@
|
||||
'use strict';
|
||||
const common = require('../common');
|
||||
|
||||
// This test ensures that `ClientRequest.prototype.setTimeout()` does
|
||||
// not add a listener for the `'connect'` event to the socket if the
|
||||
// socket is already connected.
|
||||
|
||||
const assert = require('assert');
|
||||
const http = require('http');
|
||||
|
||||
// Maximum allowed value for timeouts.
|
||||
const timeout = 2 ** 31 - 1;
|
||||
|
||||
const server = http.createServer((req, res) => {
|
||||
res.end();
|
||||
});
|
||||
|
||||
server.listen(0, common.mustCall(() => {
|
||||
const agent = new http.Agent({ keepAlive: true, maxSockets: 1 });
|
||||
const options = { port: server.address().port, agent: agent };
|
||||
|
||||
doRequest(options, common.mustCall(() => {
|
||||
const req = doRequest(options, common.mustCall(() => {
|
||||
agent.destroy();
|
||||
server.close();
|
||||
}));
|
||||
|
||||
req.on('socket', common.mustCall((socket) => {
|
||||
assert.strictEqual(socket.listenerCount('connect'), 0);
|
||||
}));
|
||||
}));
|
||||
}));
|
||||
|
||||
function doRequest(options, callback) {
|
||||
const req = http.get(options, (res) => {
|
||||
res.on('end', callback);
|
||||
res.resume();
|
||||
});
|
||||
|
||||
req.setTimeout(timeout);
|
||||
return req;
|
||||
}
|
Loading…
x
Reference in New Issue
Block a user