Skip to content

Commit

Permalink
http2: propagate session destroy code to streams
Browse files Browse the repository at this point in the history
Currently, when an HTTP2 session is destroyed with a code, that
code is not propagated to the destroy() call of the session's
streams. This commit forwards any code used to destroy a session
to its corresponding streams.

PR-URL: #28435
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
  • Loading branch information
cjihrig authored and targos committed Jul 2, 2019
1 parent dce4947 commit d8d4f9b
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 2 deletions.
9 changes: 7 additions & 2 deletions lib/internal/http2/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -946,6 +946,7 @@ class Http2Session extends EventEmitter {
socket[kSession] = this;

this[kState] = {
destroyCode: NGHTTP2_NO_ERROR,
flags: SESSION_FLAGS_PENDING,
goawayCode: null,
goawayLastStreamID: null,
Expand Down Expand Up @@ -1210,6 +1211,7 @@ class Http2Session extends EventEmitter {

const state = this[kState];
state.flags |= SESSION_FLAGS_DESTROYED;
state.destroyCode = code;

// Clear timeout and remove timeout listeners
this.setTimeout(0);
Expand Down Expand Up @@ -1941,10 +1943,13 @@ class Http2Stream extends Duplex {

debug(`Http2Stream ${this[kID] || '<pending>'} [Http2Session ` +
`${sessionName(session[kType])}]: destroying stream`);

const state = this[kState];
const sessionCode = session[kState].goawayCode ||
session[kState].destroyCode;
const code = err != null ?
NGHTTP2_INTERNAL_ERROR : (state.rstCode || NGHTTP2_NO_ERROR);

sessionCode || NGHTTP2_INTERNAL_ERROR :
state.rstCode || sessionCode;
const hasHandle = handle !== undefined;

if (!this.closed)
Expand Down
54 changes: 54 additions & 0 deletions test/parallel/test-http2-propagate-session-destroy-code.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
'use strict';
const common = require('../common');

if (!common.hasCrypto)
common.skip('missing crypto');

const assert = require('assert');
const http2 = require('http2');
const server = http2.createServer();
const errRegEx = /Session closed with error code 7/;
const destroyCode = http2.constants.NGHTTP2_REFUSED_STREAM;

server.on('error', common.mustNotCall());

server.on('session', (session) => {
session.on('close', common.mustCall());
session.on('error', common.mustCall((err) => {
assert(errRegEx.test(err));
assert.strictEqual(session.closed, false);
assert.strictEqual(session.destroyed, true);
}));

session.on('stream', common.mustCall((stream) => {
stream.on('error', common.mustCall((err) => {
assert.strictEqual(session.closed, false);
assert.strictEqual(session.destroyed, true);
assert(errRegEx.test(err));
assert.strictEqual(stream.rstCode, destroyCode);
}));

session.destroy(destroyCode);
}));
});

server.listen(0, common.mustCall(() => {
const session = http2.connect(`http://localhost:${server.address().port}`);

session.on('error', common.mustCall((err) => {
assert(errRegEx.test(err));
assert.strictEqual(session.closed, false);
assert.strictEqual(session.destroyed, true);
}));

const stream = session.request({ [http2.constants.HTTP2_HEADER_PATH]: '/' });

stream.on('error', common.mustCall((err) => {
assert(errRegEx.test(err));
assert.strictEqual(stream.rstCode, destroyCode);
}));

stream.on('close', common.mustCall(() => {
server.close();
}));
}));

0 comments on commit d8d4f9b

Please sign in to comment.