http2: callback valid check before closing request

Do not close the request if callback is not a function, and throw ERR_INVALID_CALLBACK TypeError PR-URL: #19061 Fixes: #18855 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Shingo Inoue <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
nodejs · Mar 4, 2018 · caaf7e3 · caaf7e3
1 parent cb94bf0
commit caaf7e3
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 2 deletions.
diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js
@@ -1769,6 +1769,8 @@ class Http2Stream extends Duplex {
       throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'code', 'number');
     if (code < 0 || code > kMaxInt)
       throw new errors.RangeError('ERR_OUT_OF_RANGE', 'code');
+    if (callback !== undefined && typeof callback !== 'function')
+      throw new errors.TypeError('ERR_INVALID_CALLBACK');
 
     // Clear timeout and remove timeout listeners
     this.setTimeout(0);
@@ -1786,8 +1788,6 @@ class Http2Stream extends Duplex {
     state.rstCode = code;
 
     if (callback !== undefined) {
-      if (typeof callback !== 'function')
-        throw new errors.TypeError('ERR_INVALID_CALLBACK');
       this.once('close', callback);
     }
 

diff --git a/test/parallel/test-http2-client-rststream-before-connect.js b/test/parallel/test-http2-client-rststream-before-connect.js
@@ -28,6 +28,18 @@ server.listen(0, common.mustCall(() => {
   );
   assert.strictEqual(req.closed, false);
 
+  [true, 1, {}, [], null, 'test'].forEach((notFunction) => {
+    common.expectsError(
+      () => req.close(closeCode, notFunction),
+      {
+        type: TypeError,
+        code: 'ERR_INVALID_CALLBACK',
+        message: 'Callback must be a function'
+      }
+    );
+    assert.strictEqual(req.closed, false);
+  });
+
   req.close(closeCode, common.mustCall());
   assert.strictEqual(req.closed, true);