diff --git a/lib/_http_client.js b/lib/_http_client.js index adcacb752e6..4687f927e9d 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -880,6 +880,13 @@ function statusIsInformational(status) { return (status < 200 && status >= 100 && status !== 101); } +function responseCannotContainMessageBody(status, method) { + // RFC9112 Section 6.1.1 + return method === 'HEAD' || + status === 204 || + status === 304; +} + // client function parserOnIncomingClient(res, shouldKeepAlive) { const socket = this.socket; @@ -934,11 +941,18 @@ function parserOnIncomingClient(res, shouldKeepAlive) { return 1; // Skip body but don't treat as Upgrade. } - if (req.shouldKeepAlive && !shouldKeepAlive && !req.upgradeOrConnect) { + if (req.shouldKeepAlive && !req.upgradeOrConnect) { + // Socket may be kept alive and response cannot contain + // a message body, resume to allow socket to return to free pool + if (responseCannotContainMessageBody(res.statusCode, method)) { + res.resume(); + } // Server MUST respond with Connection:keep-alive for us to enable it. // If we've been upgraded (via WebSockets) we also shouldn't try to // keep the connection open. - req.shouldKeepAlive = false; + if (!shouldKeepAlive) { + req.shouldKeepAlive = false; + } } if (req[kClientRequestStatistics] && hasObserver('http')) { diff --git a/test/parallel/test-http-agent-exit.js b/test/parallel/test-http-agent-exit.js new file mode 100644 index 00000000000..98be5b7eaac --- /dev/null +++ b/test/parallel/test-http-agent-exit.js @@ -0,0 +1,53 @@ +'use strict'; + +const common = require('../common'); +const cp = require('child_process'); +const http = require('http'); + +if (process.argv[2] === 'server') { + const server = http.createServer((req, res) => { + if (req.method === 'HEAD') { + res.writeHead(200); + res.end(); + } else if (req.url === '/204') { + res.writeHead(204); + res.end(); + } else if (req.url === '/304') { + res.writeHead(304); + res.end(); + } + }); + + server.listen(0, () => { + process.send(server.address().port); + + // Unref server prior to final mustNotCall, server close will be prevented if sockets are still open + setTimeout(() => server.unref(), common.platformTimeout(1000)); + }); +} else { + const serverProcess = cp.fork(__filename, ['server'], { + stdio: ['ignore', 'ignore', 'ignore', 'ipc'] + }); + serverProcess.once('message', common.mustCall((port) => { + serverProcess.channel.unref(); + serverProcess.unref(); + const agent = new http.Agent({ keepAlive: true }); + + // Make requests without consuming response + http.get({ method: 'HEAD', host: common.localhostIPv4, port, agent }, common.mustCall()); + http.get({ method: 'GET', host: common.localhostIPv4, port, agent, path: '/204' }, common.mustCall()); + http.get({ method: 'GET', host: common.localhostIPv4, port, agent, path: '/304' }, common.mustCall()); + + // Ensure handlers are called/not called as expected + const cb = common.mustCall((res) => { + res.on('end', common.mustCall()); + res.on('data', common.mustNotCall()); + }, 3); + http.get({ method: 'HEAD', host: common.localhostIPv4, port, agent }, cb); + http.get({ method: 'GET', host: common.localhostIPv4, port, agent, path: '/204' }, cb); + http.get({ method: 'GET', host: common.localhostIPv4, port, agent, path: '/304' }, cb); + })); + + // HEAD, 204, and 304 requests should not block script exit + setTimeout(common.mustNotCall(), common.platformTimeout(3000)).unref(); +}