http2: general cleanups

PR-URL: nodejs#17328 Fixes: nodejs#15303 Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Sebastiaan Deckers <[email protected]>
marcushill · Nov 28, 2017 · d4111d0 · d4111d0
1 parent f3686f2
commit d4111d0
Showing 1 changed file with 27 additions and 22 deletions.
diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js
@@ -28,6 +28,8 @@ const { _connectionListener: httpConnectionListener } = require('http');
 const { createPromise, promiseResolve } = process.binding('util');
 const debug = util.debuglog('http2');
 
+const kMaxFrameSize = (2 ** 24) - 1;
+const kMaxInt = (2 ** 32) - 1;
 const kMaxStreams = (2 ** 31) - 1;
 
 const {
@@ -332,9 +334,9 @@ function emitGoaway(self, code, lastStreamID, buf) {
  return;
  if (!state.shuttingDown && !state.shutdown) {
  self.shutdown({}, self.destroy.bind(self));
- } else {
- self.destroy();
+ return;
  }
+ self.destroy();
 }
 
 // Called by the native layer when a goaway frame has been received
@@ -582,14 +584,15 @@ function doShutdown(options) {
 function submitShutdown(options) {
  const type = this[kType];
  debug(`Http2Session ${sessionName(type)}: submitting shutdown request`);
+ const fn = doShutdown.bind(this, options);
  if (type === NGHTTP2_SESSION_SERVER && options.graceful === true) {
  // first send a shutdown notice
  this[kHandle].shutdownNotice();
  // then, on flip of the event loop, do the actual shutdown
- setImmediate(doShutdown.bind(this), options);
- } else {
- doShutdown.call(this, options);
+ setImmediate(fn);
+ return;
  }
+ fn();
 }
 
 function finishSessionDestroy(socket) {
@@ -844,19 +847,19 @@ class Http2Session extends EventEmitter {
  settings = Object.assign(Object.create(null), settings);
  assertWithinRange('headerTableSize',
  settings.headerTableSize,
- 0, 2 ** 32 - 1);
+ 0, kMaxInt);
  assertWithinRange('initialWindowSize',
  settings.initialWindowSize,
- 0, 2 ** 32 - 1);
+ 0, kMaxInt);
  assertWithinRange('maxFrameSize',
  settings.maxFrameSize,
- 16384, 2 ** 24 - 1);
+ 16384, kMaxFrameSize);
  assertWithinRange('maxConcurrentStreams',
  settings.maxConcurrentStreams,
  0, kMaxStreams);
  assertWithinRange('maxHeaderListSize',
  settings.maxHeaderListSize,
- 0, 2 ** 32 - 1);
+ 0, kMaxInt);
  if (settings.enablePush !== undefined &&
  typeof settings.enablePush !== 'boolean') {
  const err = new errors.TypeError('ERR_HTTP2_INVALID_SETTING_VALUE',
@@ -871,11 +874,12 @@ class Http2Session extends EventEmitter {
  debug(`Http2Session ${sessionName(this[kType])}: sending settings`);
 
  state.pendingAck++;
+ const fn = submitSettings.bind(this, settings);
  if (state.connecting) {
- this.once('connect', submitSettings.bind(this, settings));
+ this.once('connect', fn);
  return;
  }
- submitSettings.call(this, settings);
+ fn();
  }
 
  // Destroy the Http2Session
@@ -961,13 +965,14 @@ class Http2Session extends EventEmitter {
  this.on('shutdown', callback);
  }
 
+ const fn = submitShutdown.bind(this, options);
  if (state.connecting) {
- this.once('connect', submitShutdown.bind(this, options));
+ this.once('connect', fn);
  return;
  }
 
  debug(`Http2Session ${sessionName(type)}: sending shutdown`);
- submitShutdown.call(this, options);
+ fn();
  }
 
  _onTimeout() {
@@ -1368,7 +1373,7 @@ class Http2Stream extends Duplex {
  rstStream(code = NGHTTP2_NO_ERROR) {
  if (typeof code !== 'number')
  throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'code', 'number');
- if (code < 0 || code > 2 ** 32 - 1)
+ if (code < 0 || code > kMaxInt)
  throw new errors.RangeError('ERR_OUT_OF_RANGE', 'code');
 
  const fn = submitRstStream.bind(this, code);
@@ -2362,19 +2367,19 @@ function getPackedSettings(settings) {
  settings = settings || Object.create(null);
  assertWithinRange('headerTableSize',
  settings.headerTableSize,
- 0, 2 ** 32 - 1);
+ 0, kMaxInt);
  assertWithinRange('initialWindowSize',
  settings.initialWindowSize,
- 0, 2 ** 32 - 1);
+ 0, kMaxInt);
  assertWithinRange('maxFrameSize',
  settings.maxFrameSize,
- 16384, 2 ** 24 - 1);
+ 16384, kMaxFrameSize);
  assertWithinRange('maxConcurrentStreams',
  settings.maxConcurrentStreams,
  0, kMaxStreams);
  assertWithinRange('maxHeaderListSize',
  settings.maxHeaderListSize,
- 0, 2 ** 32 - 1);
+ 0, kMaxInt);
  if (settings.enablePush !== undefined &&
  typeof settings.enablePush !== 'boolean') {
  const err = new errors.TypeError('ERR_HTTP2_INVALID_SETTING_VALUE',
@@ -2425,22 +2430,22 @@ function getUnpackedSettings(buf, options = {}) {
  if (options != null && options.validate) {
  assertWithinRange('headerTableSize',
  settings.headerTableSize,
- 0, 2 ** 32 - 1);
+ 0, kMaxInt);
  assertWithinRange('enablePush',
  settings.enablePush,
  0, 1);
  assertWithinRange('initialWindowSize',
  settings.initialWindowSize,
- 0, 2 ** 32 - 1);
+ 0, kMaxInt);
  assertWithinRange('maxFrameSize',
  settings.maxFrameSize,
- 16384, 2 ** 24 - 1);
+ 16384, kMaxFrameSize);
  assertWithinRange('maxConcurrentStreams',
  settings.maxConcurrentStreams,
  0, kMaxStreams);
  assertWithinRange('maxHeaderListSize',
  settings.maxHeaderListSize,
- 0, 2 ** 32 - 1);
+ 0, kMaxInt);
  }
 
  if (settings.enablePush !== undefined) {