Skip to content

Commit 2dee82a

Browse files
committed
http: resume kept-alive when no body allowed
In accordance with https://www.rfc-editor.org/rfc/rfc9112#name-message-body-length: HEAD, 1xx, 204, and 304 responses 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 9efc84a commit 2dee82a

File tree

2 files changed

+68
-2
lines changed

2 files changed

+68
-2
lines changed

lib/_http_client.js

+16-2
Original file line numberDiff line numberDiff line change
@@ -607,6 +607,13 @@ function statusIsInformational(status) {
607607
return (status < 200 && status >= 100 && status !== 101);
608608
}
609609

610+
function responseCannotContainMessageBody(status, method) {
611+
// RFC9112 Section 6.1.1
612+
return method === 'HEAD'
613+
|| status === 204
614+
|| status === 304
615+
}
616+
610617
// client
611618
function parserOnIncomingClient(res, shouldKeepAlive) {
612619
const socket = this.socket;
@@ -654,11 +661,18 @@ function parserOnIncomingClient(res, shouldKeepAlive) {
654661
return 1; // Skip body but don't treat as Upgrade.
655662
}
656663

657-
if (req.shouldKeepAlive && !shouldKeepAlive && !req.upgradeOrConnect) {
664+
if (req.shouldKeepAlive && !req.upgradeOrConnect) {
665+
// Socket may be kept alive and response cannot contain
666+
// a message body, resume to allow socket to return to free pool
667+
if (responseCannotContainMessageBody(res.statusCode, method)){
668+
res.resume()
669+
}
658670
// Server MUST respond with Connection:keep-alive for us to enable it.
659671
// If we've been upgraded (via WebSockets) we also shouldn't try to
660672
// keep the connection open.
661-
req.shouldKeepAlive = false;
673+
if (!shouldKeepAlive) {
674+
req.shouldKeepAlive = false;
675+
}
662676
}
663677

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

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

+52
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
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+
}
13+
else if (req.url === '/204') {
14+
res.writeHead(204);
15+
res.end();
16+
}
17+
else if (req.url === '/304') {
18+
res.writeHead(304);
19+
res.end();
20+
}
21+
});
22+
23+
server.listen(0, () => {
24+
process.send(server.address().port);
25+
});
26+
} else {
27+
const serverProcess = cp.fork(__filename, ['server'], {
28+
stdio: ['ignore', 'ignore', 'ignore', 'ipc']
29+
});
30+
serverProcess.once('message', common.mustCall((port) => {
31+
serverProcess.channel.unref();
32+
serverProcess.unref();
33+
const agent = new http.Agent({ keepAlive: true });
34+
35+
// Make requests without consuming response
36+
http.get({ method: "HEAD", host: common.localhostIPv4, port, agent }, common.mustCall());
37+
http.get({ method: "GET", host: common.localhostIPv4, port, agent, path: '/204' }, common.mustCall());
38+
http.get({ method: "GET", host: common.localhostIPv4, port, agent, path: '/304' }, common.mustCall());
39+
40+
// Ensure handlers are called/not called as expected
41+
const cb = (res) => {
42+
res.on('end', common.mustCall())
43+
res.on('data', common.mustNotCall())
44+
}
45+
http.get({ method: "HEAD", host: common.localhostIPv4, port, agent }, cb);
46+
http.get({ method: "GET", host: common.localhostIPv4, port, agent, path: '/204' }, cb);
47+
http.get({ method: "GET", host: common.localhostIPv4, port, agent, path: '/304' }, cb);
48+
}));
49+
50+
// HEAD, 204, and 304 requests should not block script exit
51+
setTimeout(common.mustNotCall(), common.platformTimeout(3000)).unref();
52+
}

0 commit comments

Comments
 (0)