Skip to content

Commit

Permalink
http2: fix missing 'timeout' event emit in request and response
Browse files Browse the repository at this point in the history
A timeout in a http2 stream should emit a 'timeout' event also on
request and response object. If there is no listener on stream,
request or response, stream should immediately be destroyed.

Fixes: nodejs#20079
  • Loading branch information
DaAitch committed May 23, 2018
1 parent c89669d commit 8eabb3f
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 3 deletions.
5 changes: 4 additions & 1 deletion lib/internal/http2/compat.js
Original file line number Diff line number Diff line change
Expand Up @@ -705,12 +705,15 @@ class Http2ServerResponse extends Stream {
}
}

function onServerStream(ServerRequest, ServerResponse,
function onServerStream(ServerRequest, ServerResponse, kStreamEventsComposite,
stream, headers, flags, rawHeaders) {
const server = this;
const request = new ServerRequest(stream, headers, undefined, rawHeaders);
const response = new ServerResponse(stream);

stream[kStreamEventsComposite].push(request);
stream[kStreamEventsComposite].push(response);

// Check for the CONNECT method
const method = headers[HTTP2_HEADER_METHOD];
if (method === 'CONNECT') {
Expand Down
18 changes: 16 additions & 2 deletions lib/internal/http2/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ const kState = Symbol('state');
const kType = Symbol('type');
const kUpdateTimer = Symbol('update-timer');
const kWriteGeneric = Symbol('write-generic');
const kStreamEventsComposite = Symbol('streamEventsComposite');

const kDefaultSocketTimeout = 2 * 60 * 1000;

Expand Down Expand Up @@ -1551,6 +1552,8 @@ class Http2Stream extends Duplex {
trailersReady: false
};

this[kStreamEventsComposite] = [];

this.on('pause', streamOnPause);
}

Expand Down Expand Up @@ -1640,7 +1643,17 @@ class Http2Stream extends Duplex {
}
}

this.emit('timeout');
// if there is no timeout event listener, destroy session
let hasTimeoutCallback = this.emit('timeout');
for (const component of this[kStreamEventsComposite]) {
if (component.emit('timeout')) {
hasTimeoutCallback = true;
}
}

if (!hasTimeoutCallback) {
this.session.destroy();
}
}

// true if the HEADERS frame has been sent
Expand Down Expand Up @@ -2662,7 +2675,8 @@ function setupCompat(ev) {
this.on('stream', onServerStream.bind(
this,
this[kOptions].Http2ServerRequest,
this[kOptions].Http2ServerResponse)
this[kOptions].Http2ServerResponse,
kStreamEventsComposite)
);
}
}
Expand Down
4 changes: 4 additions & 0 deletions test/parallel/test-http2-compat-serverrequest-settimeout.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ server.on('request', (req, res) => {
req.setTimeout(msecs, common.mustCall(() => {
res.end();
}));

res.on('timeout', common.mustCall());
req.on('timeout', common.mustCall());

res.on('finish', common.mustCall(() => {
req.setTimeout(msecs, common.mustNotCall());
process.nextTick(() => {
Expand Down

0 comments on commit 8eabb3f

Please sign in to comment.