http2: general cleanups

PR-URL: #17328
Fixes: #15303
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Sebastiaan Deckers <[email protected]>

Author: jasnell

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 {
@@ -330,9 +332,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
@@ -580,14 +582,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) {
@@ -842,19 +845,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',
@@ -869,11 +872,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
@@ -959,13 +963,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() {
@@ -1366,7 +1371,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);
@@ -2360,19 +2365,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',
@@ -2423,22 +2428,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) {