Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit e5175e6

Browse filesBrowse files
RidgeABethGriggs
authored andcommitted
http2: remove waitTrailers listener after closing a stream
When `writeHeader` of `Http2ServerResponse` instance are called with 204, 205 and 304 status codes an underlying stream closes. If call `end` method after sending any of these status codes it will cause an error `TypeError: Cannot read property 'Symbol(trailers)' of undefined` because a reference to `Http2ServerResponse` instance associated with Http2Stream already was deleted. The closing of stream causes emitting `waitTrailers` event and, when this event handles inside `onStreamTrailerReady` handler, there is no reference to Http2ServerResponse instance. Fixes: #21740 Backport-PR-URL: #22850 PR-URL: #21764 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
1 parent 57618aa commit e5175e6
Copy full SHA for e5175e6

File tree

Expand file treeCollapse file tree

2 files changed

+49
-0
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

2 files changed

+49
-0
lines changed
Open diff view settings
Collapse file

‎lib/internal/http2/compat.js‎

Copy file name to clipboardExpand all lines: lib/internal/http2/compat.js
+2Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -377,6 +377,8 @@ function onStreamCloseResponse() {
377377
state.closed = true;
378378

379379
this[kProxySocket] = null;
380+
381+
this.removeListener('wantTrailers', onStreamTrailersReady);
380382
this[kResponse] = undefined;
381383

382384
res.emit('finish');
Collapse file
+47Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
if (!common.hasCrypto)
5+
common.skip('missing crypto');
6+
const h2 = require('http2');
7+
8+
// This test case ensures that calling of res.end after sending
9+
// 204, 205 and 304 HTTP statuses will not cause an error
10+
// See issue: https://github.com/nodejs/node/issues/21740
11+
12+
const {
13+
HTTP_STATUS_NO_CONTENT,
14+
HTTP_STATUS_RESET_CONTENT,
15+
HTTP_STATUS_NOT_MODIFIED
16+
} = h2.constants;
17+
18+
const statusWithouBody = [
19+
HTTP_STATUS_NO_CONTENT,
20+
HTTP_STATUS_RESET_CONTENT,
21+
HTTP_STATUS_NOT_MODIFIED,
22+
];
23+
const STATUS_CODES_COUNT = statusWithouBody.length;
24+
25+
const server = h2.createServer(common.mustCall(function(req, res) {
26+
res.writeHead(statusWithouBody.pop());
27+
res.end();
28+
}, STATUS_CODES_COUNT));
29+
30+
server.listen(0, common.mustCall(function() {
31+
const url = `http://localhost:${server.address().port}`;
32+
const client = h2.connect(url, common.mustCall(() => {
33+
let responseCount = 0;
34+
const closeAfterResponse = () => {
35+
if (STATUS_CODES_COUNT === ++responseCount) {
36+
client.destroy();
37+
server.close();
38+
}
39+
};
40+
41+
for (let i = 0; i < STATUS_CODES_COUNT; i++) {
42+
const request = client.request();
43+
request.on('response', common.mustCall(closeAfterResponse));
44+
}
45+
46+
}));
47+
}));

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.