Skip to content

Commit

Permalink
http2: remove waitTrailers listener after closing a stream
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
RidgeA authored and BethGriggs committed Oct 16, 2018
1 parent 57618aa commit e5175e6
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 0 deletions.
2 changes: 2 additions & 0 deletions lib/internal/http2/compat.js
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,8 @@ function onStreamCloseResponse() {
state.closed = true;

this[kProxySocket] = null;

this.removeListener('wantTrailers', onStreamTrailersReady);
this[kResponse] = undefined;

res.emit('finish');
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
'use strict';

const common = require('../common');
if (!common.hasCrypto)
common.skip('missing crypto');
const h2 = require('http2');

// This test case ensures that calling of res.end after sending
// 204, 205 and 304 HTTP statuses will not cause an error
// See issue: https://github.com/nodejs/node/issues/21740

const {
HTTP_STATUS_NO_CONTENT,
HTTP_STATUS_RESET_CONTENT,
HTTP_STATUS_NOT_MODIFIED
} = h2.constants;

const statusWithouBody = [
HTTP_STATUS_NO_CONTENT,
HTTP_STATUS_RESET_CONTENT,
HTTP_STATUS_NOT_MODIFIED,
];
const STATUS_CODES_COUNT = statusWithouBody.length;

const server = h2.createServer(common.mustCall(function(req, res) {
res.writeHead(statusWithouBody.pop());
res.end();
}, STATUS_CODES_COUNT));

server.listen(0, common.mustCall(function() {
const url = `http://localhost:${server.address().port}`;
const client = h2.connect(url, common.mustCall(() => {
let responseCount = 0;
const closeAfterResponse = () => {
if (STATUS_CODES_COUNT === ++responseCount) {
client.destroy();
server.close();
}
};

for (let i = 0; i < STATUS_CODES_COUNT; i++) {
const request = client.request();
request.on('response', common.mustCall(closeAfterResponse));
}

}));
}));

0 comments on commit e5175e6

Please sign in to comment.