Skip to content

Commit 191a3b2

Browse files
authored
http2: emit close for aborted HEAD compat responses
The compat response defers 'finish' and 'close' for a HEAD request until response.end(), because the stream of a headers-only response closes as soon as the headers are sent. The same deferral also applied to a HEAD stream that closed before any response was sent, for example when the client cancelled it or the session was destroyed. Nothing was left to call end(), so the response never emitted 'close' and the abort could not be observed on it. Defer only once the headers were sent, and otherwise close the response as for any other method. The writable side of a HEAD stream is finished from the start, so 'finish' is emitted only after the headers were sent, and an aborted HEAD response does not report success. Assisted-by: Opus 5.5 Signed-off-by: Robert Nagy <ronagy@icloud.com> PR-URL: #66310 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
1 parent cff7b12 commit 191a3b2

2 files changed

Lines changed: 88 additions & 4 deletions

File tree

‎lib/internal/http2/compat.js‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -459,8 +459,14 @@ function onStreamCloseResponse() {
459459

460460
const state = res[kState];
461461

462-
if (this.headRequest !== state.headRequest)
463-
return;
462+
if (this.headRequest !== state.headRequest) {
463+
// A headers-only HEAD response closes its stream as soon as the headers
464+
// are sent; defer to response.end() as HTTP/1 does. A stream that closed
465+
// before the response was sent was aborted and closes the response now.
466+
if (this.headersSent)
467+
return;
468+
state.headRequest = this.headRequest;
469+
}
464470

465471
state.closed = true;
466472

@@ -469,8 +475,10 @@ function onStreamCloseResponse() {
469475
this.removeListener('wantTrailers', onStreamTrailersReady);
470476
this[kResponse] = undefined;
471477

472-
// Only emit 'finish' when the underlying writable actually finished
473-
if (this.writableFinished)
478+
// Only emit 'finish' when the underlying writable actually finished. The
479+
// writable side of a HEAD stream is finished from the start, so it counts
480+
// only once the headers were sent.
481+
if (this.writableFinished && this.headersSent)
474482
res.emit('finish');
475483
res.emit('close');
476484
}
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
if (!common.hasCrypto)
5+
common.skip('missing crypto');
6+
const assert = require('assert');
7+
const h2 = require('http2');
8+
9+
// A HEAD response must receive a close event when its stream closes before
10+
// the response was sent, like a response to any other method. A headers-only
11+
// HEAD response still waits for response.end() before finish and close.
12+
13+
{
14+
// The client cancels the stream before the server responds.
15+
let request;
16+
const server = h2.createServer(common.mustCall((req, res) => {
17+
res.on('finish', common.mustNotCall());
18+
res.on('close', common.mustCall(() => {
19+
// Ending an already closed response still calls back.
20+
res.end(common.mustCall());
21+
server.close();
22+
}));
23+
request.close(h2.constants.NGHTTP2_CANCEL);
24+
}));
25+
26+
server.listen(0, common.mustCall(() => {
27+
const client = h2.connect(`http://localhost:${server.address().port}`);
28+
request = client.request({ ':method': 'HEAD' });
29+
request.on('close', common.mustCall(() => client.close()));
30+
}));
31+
}
32+
33+
{
34+
// The connection is lost before the server responds.
35+
let client;
36+
const server = h2.createServer(common.mustCall((req, res) => {
37+
res.on('finish', common.mustNotCall());
38+
res.on('close', common.mustCall(() => server.close()));
39+
client.destroy();
40+
}));
41+
42+
server.listen(0, common.mustCall(() => {
43+
client = h2.connect(`http://localhost:${server.address().port}`);
44+
client.on('error', () => {});
45+
client.request({ ':method': 'HEAD' }).on('error', () => {});
46+
}));
47+
}
48+
49+
{
50+
// The stream of a headers-only response closes before response.end().
51+
const server = h2.createServer(common.mustCall((req, res) => {
52+
let ended = false;
53+
res.on('finish', common.mustCall(() => assert(ended)));
54+
res.on('close', common.mustCall(() => {
55+
assert(ended);
56+
server.close();
57+
}));
58+
req.stream.on('close', common.mustCall(() => {
59+
setImmediate(() => {
60+
ended = true;
61+
res.end();
62+
});
63+
}));
64+
res.writeHead(200);
65+
}));
66+
67+
server.listen(0, common.mustCall(() => {
68+
const client = h2.connect(`http://localhost:${server.address().port}`);
69+
const request = client.request({ ':method': 'HEAD' });
70+
request.on('response', common.mustCall((headers) => {
71+
assert.strictEqual(headers[':status'], 200);
72+
}));
73+
request.resume();
74+
request.on('close', common.mustCall(() => client.close()));
75+
}));
76+
}

0 commit comments

Comments
 (0)