From b5c8c845079bd0b7ad6ed50ead6fdb4aa8439635 Mon Sep 17 00:00:00 2001 From: Matt Weber <1062734+mweberxyz@users.noreply.github.com> Date: Tue, 2 Apr 2024 12:47:03 -0400 Subject: [PATCH 1/2] http: resume kept-alive when no body allowed According to RFC9112 section 6.3.1: HEAD requests, and responses with status 204 and 304 cannot contain a message body, If a socket will be kept-alive, resume the socket during parsing so that it may be returned to the free pool. Fixes nodejs/node#47228 --- lib/_http_client.js | 18 ++++++++- test/parallel/test-http-agent-exit.js | 53 +++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 2 deletions(-) create mode 100644 test/parallel/test-http-agent-exit.js diff --git a/lib/_http_client.js b/lib/_http_client.js index adcacb752e6e..4687f927e9dd 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 000000000000..252cc7c6b3ec --- /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 = (res) => { + res.on('end', common.mustCall()); + res.on('data', common.mustNotCall()); + }; + 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(); +} From 16f9ab178c85ad779a30e8eae395872e76027f69 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Wed, 5 Aug 2026 17:50:51 +0200 Subject: [PATCH 2/2] squash! fix lint Co-authored-by: Antoine du Hamel --- test/parallel/test-http-agent-exit.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-http-agent-exit.js b/test/parallel/test-http-agent-exit.js index 252cc7c6b3ec..98be5b7eaacc 100644 --- a/test/parallel/test-http-agent-exit.js +++ b/test/parallel/test-http-agent-exit.js @@ -39,10 +39,10 @@ if (process.argv[2] === 'server') { http.get({ method: 'GET', host: common.localhostIPv4, port, agent, path: '/304' }, common.mustCall()); // Ensure handlers are called/not called as expected - const cb = (res) => { + 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);