From 9058cd0a8adea3ed974f5673772702232abcb810 Mon Sep 17 00:00:00 2001 From: Shani Singh Date: Thu, 6 Aug 2026 07:49:36 +0530 Subject: [PATCH] http: fix keylog listener setup on existing agent sockets `maybeEnableKeylog()` runs as the agent's `'newListener'` handler and attaches the agent's keylog handler to the sockets the agent already owns. `agent.sockets` maps a name to an array of sockets, but the loop treated those arrays as sockets and called `.on()` on them. Adding a `'keylog'` listener to an agent that already owned a socket therefore threw `TypeError: sockets[i].on is not a function` out of `agent.on('keylog', ...)`. Since the throw happened inside the `'newListener'` handler it propagated before the listener was stored, so the caller got an exception and no listener. Sockets parked in `agent.freeSockets` were never visited at all. Walk both maps the way `Agent.prototype.destroy()` does. Signed-off-by: Shani Singh --- lib/_http_agent.js | 15 ++-- ...test-http-agent-keylog-existing-sockets.js | 71 +++++++++++++++++++ 2 files changed, 82 insertions(+), 4 deletions(-) create mode 100644 test/parallel/test-http-agent-keylog-existing-sockets.js diff --git a/lib/_http_agent.js b/lib/_http_agent.js index edf988a046ae..fe5fbd0abe25 100644 --- a/lib/_http_agent.js +++ b/lib/_http_agent.js @@ -280,10 +280,17 @@ function maybeEnableKeylog(eventName) { this[kOnKeylog] = function onkeylog(keylog) { agent.emit('keylog', keylog, this); }; - // Existing sockets will start listening on keylog now. - const sockets = ObjectValues(this.sockets); - for (let i = 0; i < sockets.length; i++) { - sockets[i].on('keylog', this[kOnKeylog]); + // Existing sockets will start listening on keylog now. Both maps hold + // arrays of sockets keyed by name, so each bucket has to be walked. + const sets = [this.freeSockets, this.sockets]; + for (let s = 0; s < sets.length; s++) { + const buckets = ObjectValues(sets[s]); + for (let b = 0; b < buckets.length; b++) { + const sockets = buckets[b]; + for (let n = 0; n < sockets.length; n++) { + sockets[n].on('keylog', this[kOnKeylog]); + } + } } } } diff --git a/test/parallel/test-http-agent-keylog-existing-sockets.js b/test/parallel/test-http-agent-keylog-existing-sockets.js new file mode 100644 index 000000000000..1a391c09b8be --- /dev/null +++ b/test/parallel/test-http-agent-keylog-existing-sockets.js @@ -0,0 +1,71 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const http = require('http'); + +// Adding a 'keylog' listener to an agent is wired up by maybeEnableKeylog(), +// which attaches the agent's keylog handler to the sockets the agent already +// owns. `agent.sockets` and `agent.freeSockets` map a name to an *array* of +// sockets, so each bucket has to be walked. Treating the buckets themselves as +// sockets threw a TypeError out of `agent.on('keylog', ...)`, which also meant +// the listener was never registered. + +// Two servers so the two sockets get different names, which keeps one parked +// in freeSockets instead of being reused by the second request. +const idleServer = http.createServer((req, res) => res.end('idle')); +const busyServer = http.createServer((req, res) => { + setTimeout(() => res.end('busy'), common.platformTimeout(200)); +}); + +function countSockets(agent) { + let free = 0; + let active = 0; + for (const bucket of Object.values(agent.freeSockets)) free += bucket.length; + for (const bucket of Object.values(agent.sockets)) active += bucket.length; + return { free, active }; +} + +idleServer.listen(0, common.mustCall(() => { + busyServer.listen(0, common.mustCall(() => { + const agent = new http.Agent({ keepAlive: true, maxSockets: 4 }); + + // First request finishes, so its socket is released into freeSockets. + http.get({ port: idleServer.address().port, agent }, common.mustCall((res) => { + res.resume(); + res.on('end', common.mustCall(() => { + // Second request is still in flight, so its socket is in sockets. + const req = http.get({ port: busyServer.address().port, agent }, + common.mustCall((res2) => { + res2.resume(); + res2.on('end', common.mustCall(() => { + agent.destroy(); + idleServer.close(); + busyServer.close(); + })); + })); + + req.on('socket', common.mustCall(() => { + setImmediate(common.mustCall(() => { + const { free, active } = countSockets(agent); + assert.strictEqual(free, 1); + assert.strictEqual(active, 1); + + // Used to throw `TypeError: sockets[i].on is not a function`. + agent.on('keylog', common.mustNotCall()); + assert.strictEqual(agent.listenerCount('keylog'), 1); + + // Every existing socket, idle or in use, is now listening. + for (const set of [agent.freeSockets, agent.sockets]) { + for (const bucket of Object.values(set)) { + for (const socket of bucket) { + assert.strictEqual(socket.listenerCount('keylog'), 1); + } + } + } + })); + })); + })); + })); + })); +}));