From 2c85d7c3921253c32f80d821c85d0f24338aaf37 Mon Sep 17 00:00:00 2001 From: Trivikram Kamat Date: Thu, 28 Sep 2017 23:22:50 -0500 Subject: [PATCH] http2: setting shuttingDown=true after validation In shutdown(), shuttingDown was set to true before validating options. If invalid options are passed, error was thrown and server remained in shuttingDown state. This code change fixes it. PR-URL: https://github.com/nodejs/node/pull/15676 Fixes: https://github.com/nodejs/node/issues/15666 Refs: https://github.com/nodejs/node/issues/14985 Reviewed-By: James M Snell Reviewed-By: Ruben Bridgewater --- lib/internal/http2/core.js | 6 +- ...st-http2-server-shutdown-options-errors.js | 62 +++++++++++++++++++ 2 files changed, 65 insertions(+), 3 deletions(-) create mode 100644 test/parallel/test-http2-server-shutdown-options-errors.js diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 78a4896ab7d834..34753b53b2411e 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -982,9 +982,6 @@ class Http2Session extends EventEmitter { if (this[kState].shutdown || this[kState].shuttingDown) return; - debug(`[${sessionName(this[kType])}] initiating shutdown`); - this[kState].shuttingDown = true; - const type = this[kType]; if (typeof options === 'function') { @@ -1022,6 +1019,9 @@ class Http2Session extends EventEmitter { options.lastStreamID); } + debug(`[${sessionName(this[kType])}] initiating shutdown`); + this[kState].shuttingDown = true; + if (callback) { this.on('shutdown', callback); } diff --git a/test/parallel/test-http2-server-shutdown-options-errors.js b/test/parallel/test-http2-server-shutdown-options-errors.js new file mode 100644 index 00000000000000..673723e961c87d --- /dev/null +++ b/test/parallel/test-http2-server-shutdown-options-errors.js @@ -0,0 +1,62 @@ +// Flags: --expose-http2 +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); +const http2 = require('http2'); + +const server = http2.createServer(); + +const optionsToTest = { + opaqueData: 'Uint8Array', + graceful: 'boolean', + errorCode: 'number', + lastStreamID: 'number' +}; + +const types = { + boolean: true, + number: 1, + object: {}, + array: [], + null: null, + Uint8Array: Buffer.from([0x1, 0x2, 0x3, 0x4, 0x5]) +}; + +server.on( + 'stream', + common.mustCall((stream) => { + Object.keys(optionsToTest).forEach((option) => { + Object.keys(types).forEach((type) => { + if (type === optionsToTest[option]) { + return; + } + common.expectsError( + () => + stream.session.shutdown( + { [option]: types[type] }, + common.mustNotCall() + ), + { + type: TypeError, + code: 'ERR_INVALID_OPT_VALUE', + message: `The value "${String(types[type])}" is invalid ` + + `for option "${option}"` + } + ); + }); + }); + stream.session.destroy(); + }) +); + +server.listen( + 0, + common.mustCall(() => { + const client = http2.connect(`http://localhost:${server.address().port}`); + const req = client.request(); + req.resume(); + req.on('end', common.mustCall(() => server.close())); + }) +);