Skip to content

Commit e21ef89

Browse files
committed
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 #47228
1 parent c752615 commit e21ef89

File tree

2 files changed

+69
-2
lines changed

2 files changed

+69
-2
lines changed

lib/_http_client.js

+16-2
Original file line numberDiff line numberDiff line change
@@ -625,6 +625,13 @@ function statusIsInformational(status) {
625625
return (status < 200 && status >= 100 && status !== 101);
626626
}
627627

628+
function responseCannotContainMessageBody(status, method) {
629+
// RFC9112 Section 6.1.1
630+
return method === 'HEAD' ||
631+
status === 204 ||
632+
status === 304;
633+
}
634+
628635
// client
629636
function parserOnIncomingClient(res, shouldKeepAlive) {
630637
const socket = this.socket;
@@ -672,11 +679,18 @@ function parserOnIncomingClient(res, shouldKeepAlive) {
672679
return 1; // Skip body but don't treat as Upgrade.
673680
}
674681

675-
if (req.shouldKeepAlive && !shouldKeepAlive && !req.upgradeOrConnect) {
682+
if (req.shouldKeepAlive && !req.upgradeOrConnect) {
683+
// Socket may be kept alive and response cannot contain
684+
// a message body, resume to allow socket to return to free pool
685+
if (responseCannotContainMessageBody(res.statusCode, method)) {
686+
res.resume();
687+
}
676688
// Server MUST respond with Connection:keep-alive for us to enable it.
677689
// If we've been upgraded (via WebSockets) we also shouldn't try to
678690
// keep the connection open.
679-
req.shouldKeepAlive = false;
691+
if (!shouldKeepAlive) {
692+
req.shouldKeepAlive = false;
693+
}
680694
}
681695

682696
if (req[kClientRequestStatistics] && hasObserver('http')) {

test/parallel/test-http-agent-exit.js

+53
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const cp = require('child_process');
5+
const http = require('http');
6+
7+
if (process.argv[2] === 'server') {
8+
const server = http.createServer((req, res) => {
9+
if (req.method === 'HEAD') {
10+
res.writeHead(200);
11+
res.end();
12+
} else if (req.url === '/204') {
13+
res.writeHead(204);
14+
res.end();
15+
} else if (req.url === '/304') {
16+
res.writeHead(304);
17+
res.end();
18+
}
19+
});
20+
21+
server.listen(0, () => {
22+
process.send(server.address().port);
23+
24+
// Unref server prior to final mustNotCall, server close will be prevented if sockets are still open
25+
setTimeout(() => server.unref(), common.platformTimeout(1000));
26+
});
27+
} else {
28+
const serverProcess = cp.fork(__filename, ['server'], {
29+
stdio: ['ignore', 'ignore', 'ignore', 'ipc']
30+
});
31+
serverProcess.once('message', common.mustCall((port) => {
32+
serverProcess.channel.unref();
33+
serverProcess.unref();
34+
const agent = new http.Agent({ keepAlive: true });
35+
36+
// Make requests without consuming response
37+
http.get({ method: 'HEAD', host: common.localhostIPv4, port, agent }, common.mustCall());
38+
http.get({ method: 'GET', host: common.localhostIPv4, port, agent, path: '/204' }, common.mustCall());
39+
http.get({ method: 'GET', host: common.localhostIPv4, port, agent, path: '/304' }, common.mustCall());
40+
41+
// Ensure handlers are called/not called as expected
42+
const cb = (res) => {
43+
res.on('end', common.mustCall());
44+
res.on('data', common.mustNotCall());
45+
};
46+
http.get({ method: 'HEAD', host: common.localhostIPv4, port, agent }, cb);
47+
http.get({ method: 'GET', host: common.localhostIPv4, port, agent, path: '/204' }, cb);
48+
http.get({ method: 'GET', host: common.localhostIPv4, port, agent, path: '/304' }, cb);
49+
}));
50+
51+
// HEAD, 204, and 304 requests should not block script exit
52+
setTimeout(common.mustNotCall(), common.platformTimeout(3000)).unref();
53+
}

0 commit comments

Comments
 (0)