Skip to content

Commit 238ef58

Browse files
RidgeAtargos
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 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 1019c2d commit 238ef58

2 files changed

Lines changed: 49 additions & 0 deletions

File tree

‎lib/internal/http2/compat.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -391,6 +391,8 @@ function onStreamCloseResponse() {
391391
state.closed=true;
392392

393393
this[kProxySocket]=null;
394+
395+
this.removeListener('wantTrailers',onStreamTrailersReady);
394396
this[kResponse]=undefined;
395397

396398
res.emit('finish');
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
'use strict';
2+
3+
constcommon=require('../common');
4+
if(!common.hasCrypto)
5+
common.skip('missing crypto');
6+
consth2=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+
conststatusWithouBody=[
19+
HTTP_STATUS_NO_CONTENT,
20+
HTTP_STATUS_RESET_CONTENT,
21+
HTTP_STATUS_NOT_MODIFIED,
22+
];
23+
constSTATUS_CODES_COUNT=statusWithouBody.length;
24+
25+
constserver=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+
consturl=`http://localhost:${server.address().port}`;
32+
constclient=h2.connect(url,common.mustCall(()=>{
33+
letresponseCount=0;
34+
constcloseAfterResponse=()=>{
35+
if(STATUS_CODES_COUNT===++responseCount){
36+
client.destroy();
37+
server.close();
38+
}
39+
};
40+
41+
for(leti=0;i<STATUS_CODES_COUNT;i++){
42+
constrequest=client.request();
43+
request.on('response',common.mustCall(closeAfterResponse));
44+
}
45+
46+
}));
47+
}));

0 commit comments

Comments
 (0)