From 516881e22803142690bf2f6c5eff5aebdda669f1 Mon Sep 17 00:00:00 2001
From: James M Snell <jasnell@gmail.com>
Date: Fri, 24 Nov 2017 12:43:05 -0800
Subject: [PATCH 1/7] http2: use correct connect event for TLS Socket

---
 lib/internal/http2/core.js | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js
index 39f67c482382b2..d00ed6044183a5 100644
--- a/lib/internal/http2/core.js
+++ b/lib/internal/http2/core.js
@@ -707,7 +707,9 @@ class Http2Session extends EventEmitter {
     const setupFn = setupHandle(this, socket, type, options);
     if (socket.connecting) {
       this[kState].connecting = true;
-      socket.once('connect', setupFn);
+      const connectEvent =
+        socket instanceof tls.TLSSocket ? 'secureConnect' : 'connect';
+      socket.once(connectEvent, setupFn);
     } else {
       setupFn();
     }

From a21f2c9009914777a4b11dd2f88141a5dca91d91 Mon Sep 17 00:00:00 2001
From: James M Snell <jasnell@gmail.com>
Date: Sat, 25 Nov 2017 08:26:46 -0800
Subject: [PATCH 2/7] http2: general cleanups

---
 lib/internal/http2/core.js | 49 +++++++++++++++++++++-----------------
 1 file changed, 27 insertions(+), 22 deletions(-)

diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js
index d00ed6044183a5..5a28ee7492a0df 100644
--- 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) {

From 48514ce1c4a8dcf53b354e6ec171f6c680ae3548 Mon Sep 17 00:00:00 2001
From: James M Snell <jasnell@gmail.com>
Date: Sat, 25 Nov 2017 12:44:07 -0800
Subject: [PATCH 3/7] http2: reduce code duplication in settings

---
 lib/internal/http2/core.js                    | 106 ++++++------------
 test/parallel/test-http2-getpackedsettings.js |  10 +-
 2 files changed, 35 insertions(+), 81 deletions(-)

diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js
index 5a28ee7492a0df..4225c9c2439c84 100644
--- a/lib/internal/http2/core.js
+++ b/lib/internal/http2/core.js
@@ -661,6 +661,33 @@ function pingCallback(cb) {
   };
 }
 
+function validateSettings(settings) {
+  settings = Object.assign({}, settings);
+  assertWithinRange('headerTableSize',
+                    settings.headerTableSize,
+                    0, kMaxInt);
+  assertWithinRange('initialWindowSize',
+                    settings.initialWindowSize,
+                    0, kMaxInt);
+  assertWithinRange('maxFrameSize',
+                    settings.maxFrameSize,
+                    16384, kMaxFrameSize);
+  assertWithinRange('maxConcurrentStreams',
+                    settings.maxConcurrentStreams,
+                    0, kMaxStreams);
+  assertWithinRange('maxHeaderListSize',
+                    settings.maxHeaderListSize,
+                    0, kMaxInt);
+  if (settings.enablePush !== undefined &&
+      typeof settings.enablePush !== 'boolean') {
+    const err = new errors.TypeError('ERR_HTTP2_INVALID_SETTING_VALUE',
+                                     'enablePush', settings.enablePush);
+    err.actual = settings.enablePush;
+    throw err;
+  }
+  return settings;
+}
+
 // Upon creation, the Http2Session takes ownership of the socket. The session
 // may not be ready to use immediately if the socket is not yet fully connected.
 class Http2Session extends EventEmitter {
@@ -844,29 +871,7 @@ class Http2Session extends EventEmitter {
 
     // Validate the input first
     assertIsObject(settings, 'settings');
-    settings = Object.assign(Object.create(null), settings);
-    assertWithinRange('headerTableSize',
-                      settings.headerTableSize,
-                      0, kMaxInt);
-    assertWithinRange('initialWindowSize',
-                      settings.initialWindowSize,
-                      0, kMaxInt);
-    assertWithinRange('maxFrameSize',
-                      settings.maxFrameSize,
-                      16384, kMaxFrameSize);
-    assertWithinRange('maxConcurrentStreams',
-                      settings.maxConcurrentStreams,
-                      0, kMaxStreams);
-    assertWithinRange('maxHeaderListSize',
-                      settings.maxHeaderListSize,
-                      0, kMaxInt);
-    if (settings.enablePush !== undefined &&
-        typeof settings.enablePush !== 'boolean') {
-      const err = new errors.TypeError('ERR_HTTP2_INVALID_SETTING_VALUE',
-                                       'enablePush', settings.enablePush);
-      err.actual = settings.enablePush;
-      throw err;
-    }
+    settings = validateSettings(settings);
     if (state.pendingAck === state.maxPendingAck) {
       throw new errors.Error('ERR_HTTP2_MAX_PENDING_SETTINGS_ACK',
                              this[kState].pendingAck);
@@ -2364,30 +2369,7 @@ function createServer(options, handler) {
 // HTTP2-Settings header frame.
 function getPackedSettings(settings) {
   assertIsObject(settings, 'settings');
-  settings = settings || Object.create(null);
-  assertWithinRange('headerTableSize',
-                    settings.headerTableSize,
-                    0, kMaxInt);
-  assertWithinRange('initialWindowSize',
-                    settings.initialWindowSize,
-                    0, kMaxInt);
-  assertWithinRange('maxFrameSize',
-                    settings.maxFrameSize,
-                    16384, kMaxFrameSize);
-  assertWithinRange('maxConcurrentStreams',
-                    settings.maxConcurrentStreams,
-                    0, kMaxStreams);
-  assertWithinRange('maxHeaderListSize',
-                    settings.maxHeaderListSize,
-                    0, kMaxInt);
-  if (settings.enablePush !== undefined &&
-      typeof settings.enablePush !== 'boolean') {
-    const err = new errors.TypeError('ERR_HTTP2_INVALID_SETTING_VALUE',
-                                     'enablePush', settings.enablePush);
-    err.actual = settings.enablePush;
-    throw err;
-  }
-  updateSettingsBuffer(settings);
+  updateSettingsBuffer(validateSettings(settings));
   return binding.packSettings();
 }
 
@@ -2398,7 +2380,7 @@ function getUnpackedSettings(buf, options = {}) {
   }
   if (buf.length % 6 !== 0)
     throw new errors.RangeError('ERR_HTTP2_INVALID_PACKED_SETTINGS_LENGTH');
-  const settings = Object.create(null);
+  const settings = {};
   let offset = 0;
   while (offset < buf.length) {
     const id = buf.readUInt16BE(offset);
@@ -2409,7 +2391,7 @@ function getUnpackedSettings(buf, options = {}) {
         settings.headerTableSize = value;
         break;
       case NGHTTP2_SETTINGS_ENABLE_PUSH:
-        settings.enablePush = value;
+        settings.enablePush = value !== 0;
         break;
       case NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS:
         settings.maxConcurrentStreams = value;
@@ -2427,30 +2409,8 @@ function getUnpackedSettings(buf, options = {}) {
     offset += 4;
   }
 
-  if (options != null && options.validate) {
-    assertWithinRange('headerTableSize',
-                      settings.headerTableSize,
-                      0, kMaxInt);
-    assertWithinRange('enablePush',
-                      settings.enablePush,
-                      0, 1);
-    assertWithinRange('initialWindowSize',
-                      settings.initialWindowSize,
-                      0, kMaxInt);
-    assertWithinRange('maxFrameSize',
-                      settings.maxFrameSize,
-                      16384, kMaxFrameSize);
-    assertWithinRange('maxConcurrentStreams',
-                      settings.maxConcurrentStreams,
-                      0, kMaxStreams);
-    assertWithinRange('maxHeaderListSize',
-                      settings.maxHeaderListSize,
-                      0, kMaxInt);
-  }
-
-  if (settings.enablePush !== undefined) {
-    settings.enablePush = !!settings.enablePush;
-  }
+  if (options != null && options.validate)
+    validateSettings(settings);
 
   return settings;
 }
diff --git a/test/parallel/test-http2-getpackedsettings.js b/test/parallel/test-http2-getpackedsettings.js
index 7461176c5fcde7..16c84913893d0c 100644
--- a/test/parallel/test-http2-getpackedsettings.js
+++ b/test/parallel/test-http2-getpackedsettings.js
@@ -128,7 +128,6 @@ assert.doesNotThrow(() => http2.getPackedSettings({ enablePush: false }));
   assert.strictEqual(settings.enablePush, true);
 }
 
-//should throw if enablePush is not 0 or 1
 {
   const packed = Buffer.from([
     0x00, 0x02, 0x00, 0x00, 0x00, 0x00]);
@@ -140,13 +139,8 @@ assert.doesNotThrow(() => http2.getPackedSettings({ enablePush: false }));
   const packed = Buffer.from([
     0x00, 0x02, 0x00, 0x00, 0x00, 0x64]);
 
-  assert.throws(() => {
-    http2.getUnpackedSettings(packed, { validate: true });
-  }, common.expectsError({
-    code: 'ERR_HTTP2_INVALID_SETTING_VALUE',
-    type: RangeError,
-    message: 'Invalid value for setting "enablePush": 100'
-  }));
+  const settings = http2.getUnpackedSettings(packed, { validate: true });
+  assert.strictEqual(settings.enablePush, true);
 }
 
 //check for what happens if passing {validate: true} and no errors happen

From e7ad83de34c67da9a18aa95df123facc71b9263d Mon Sep 17 00:00:00 2001
From: James M Snell <jasnell@gmail.com>
Date: Sat, 25 Nov 2017 13:02:16 -0800
Subject: [PATCH 4/7] http2: use 'close' event instead of 'streamClosed'

---
 doc/api/http2.md                              | 26 +++++++++----------
 lib/internal/http2/compat.js                  |  4 +--
 lib/internal/http2/core.js                    |  8 +++---
 .../test-http2-client-http1-server.js         |  2 +-
 ...t-http2-client-rststream-before-connect.js |  2 +-
 ...p2-client-stream-destroy-before-connect.js |  2 +-
 .../test-http2-client-unescaped-path.js       |  2 +-
 .../test-http2-compat-serverresponse-end.js   |  4 +--
 test/parallel/test-http2-compat-socket.js     |  4 +--
 test/parallel/test-http2-multiheaders-raw.js  |  2 +-
 test/parallel/test-http2-multiheaders.js      |  2 +-
 ...test-http2-options-max-reserved-streams.js |  6 ++---
 .../test-http2-respond-file-errors.js         |  2 +-
 .../test-http2-respond-file-fd-errors.js      |  2 +-
 .../test-http2-respond-file-fd-range.js       |  4 +--
 test/parallel/test-http2-respond-no-data.js   |  2 +-
 .../parallel/test-http2-response-splitting.js |  2 +-
 .../test-http2-server-rst-before-respond.js   |  2 +-
 test/parallel/test-http2-server-rst-stream.js |  2 +-
 .../parallel/test-http2-server-socketerror.js |  2 +-
 test/parallel/test-http2-stream-client.js     |  2 +-
 .../test-http2-stream-destroy-event-order.js  |  2 +-
 test/parallel/test-http2-too-large-headers.js |  2 +-
 test/parallel/test-http2-too-many-headers.js  |  2 +-
 test/parallel/test-http2-write-callbacks.js   |  2 +-
 25 files changed, 45 insertions(+), 47 deletions(-)

diff --git a/doc/api/http2.md b/doc/api/http2.md
index 3500c563a2782a..51d2bec2a913f0 100644
--- a/doc/api/http2.md
+++ b/doc/api/http2.md
@@ -633,7 +633,7 @@ All [`Http2Stream`][] instances are destroyed either when:
 When an `Http2Stream` instance is destroyed, an attempt will be made to send an
 `RST_STREAM` frame will be sent to the connected peer.
 
-Once the `Http2Stream` instance is destroyed, the `'streamClosed'` event will
+When the `Http2Stream` instance is destroyed, the `'close'` event will
 be emitted. Because `Http2Stream` is an instance of `stream.Duplex`, the
 `'end'` event will also be emitted if the stream data is currently flowing.
 The `'error'` event may also be emitted if `http2stream.destroy()` was called
@@ -655,6 +655,18 @@ abnormally aborted in mid-communication.
 *Note*: The `'aborted'` event will only be emitted if the `Http2Stream`
 writable side has not been ended.
 
+#### Event: 'close'
+<!-- YAML
+added: v8.4.0
+-->
+
+The `'close'` event is emitted when the `Http2Stream` is destroyed. Once
+this event is emitted, the `Http2Stream` instance is no longer usable.
+
+The listener callback is passed a single argument specifying the HTTP/2 error
+code specified when closing the stream. If the code is any value other than
+`NGHTTP2_NO_ERROR` (`0`), an `'error'` event will also be emitted.
+
 #### Event: 'error'
 <!-- YAML
 added: v8.4.0
@@ -674,18 +686,6 @@ argument identifying the frame type, and an integer argument identifying the
 error code. The `Http2Stream` instance will be destroyed immediately after the
 `'frameError'` event is emitted.
 
-#### Event: 'streamClosed'
-<!-- YAML
-added: v8.4.0
--->
-
-The `'streamClosed'` event is emitted when the `Http2Stream` is destroyed. Once
-this event is emitted, the `Http2Stream` instance is no longer usable.
-
-The listener callback is passed a single argument specifying the HTTP/2 error
-code specified when closing the stream. If the code is any value other than
-`NGHTTP2_NO_ERROR` (`0`), an `'error'` event will also be emitted.
-
 #### Event: 'timeout'
 <!-- YAML
 added: v8.4.0
diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js
index 89a22270086418..5ddde39d387785 100644
--- a/lib/internal/http2/compat.js
+++ b/lib/internal/http2/compat.js
@@ -250,7 +250,7 @@ class Http2ServerRequest extends Readable {
     stream.on('close', onStreamClosedRequest);
     stream.on('aborted', onStreamAbortedRequest);
     const onfinish = this[kFinish].bind(this);
-    stream.on('streamClosed', onfinish);
+    stream.on('close', onfinish);
     stream.on('finish', onfinish);
     this.on('pause', onRequestPause);
     this.on('resume', onRequestResume);
@@ -383,7 +383,7 @@ class Http2ServerResponse extends Stream {
     stream.on('close', onStreamClosedResponse);
     stream.on('aborted', onStreamAbortedResponse);
     const onfinish = this[kFinish].bind(this);
-    stream.on('streamClosed', onfinish);
+    stream.on('close', onfinish);
     stream.on('finish', onfinish);
   }
 
diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js
index 4225c9c2439c84..025b0e363356ea 100644
--- a/lib/internal/http2/core.js
+++ b/lib/internal/http2/core.js
@@ -226,10 +226,8 @@ function onStreamTrailers() {
   return headersList;
 }
 
-// Called when the stream is closed. The streamClosed event is emitted on the
-// Http2Stream instance. Note that this event is distinctly different than the
-// require('stream') interface 'close' event which deals with the state of the
-// Readable and Writable sides of the Duplex.
+// Called when the stream is closed. The close event is emitted on the
+// Http2Stream instance
 function onStreamClose(code) {
   const stream = this[kOwner];
   stream[kUpdateTimer]();
@@ -1485,7 +1483,7 @@ function continueStreamDestroy(err, callback) {
   abort(this);
   this.push(null);  // Close the readable side
   this.end();       // Close the writable side
-  process.nextTick(emit, this, 'streamClosed', code);
+  process.nextTick(emit, this, 'close', code);
 }
 
 function finishStreamDestroy() {
diff --git a/test/parallel/test-http2-client-http1-server.js b/test/parallel/test-http2-client-http1-server.js
index 44d8d392f4ebff..53f7bf42c465e1 100644
--- a/test/parallel/test-http2-client-http1-server.js
+++ b/test/parallel/test-http2-client-http1-server.js
@@ -13,7 +13,7 @@ server.listen(0, common.mustCall(() => {
   const client = http2.connect(`http://localhost:${server.address().port}`);
 
   const req = client.request();
-  req.on('streamClosed', common.mustCall());
+  req.on('close', common.mustCall());
 
   client.on('error', common.expectsError({
     code: 'ERR_HTTP2_ERROR',
diff --git a/test/parallel/test-http2-client-rststream-before-connect.js b/test/parallel/test-http2-client-rststream-before-connect.js
index e4aff87be98b5f..eb3a0087d7893c 100644
--- a/test/parallel/test-http2-client-rststream-before-connect.js
+++ b/test/parallel/test-http2-client-rststream-before-connect.js
@@ -27,7 +27,7 @@ server.on('listening', common.mustCall(() => {
   // second call doesn't do anything
   assert.doesNotThrow(() => req.rstStream(8));
 
-  req.on('streamClosed', common.mustCall((code) => {
+  req.on('close', common.mustCall((code) => {
     assert.strictEqual(req.destroyed, true);
     assert.strictEqual(code, 0);
     server.close();
diff --git a/test/parallel/test-http2-client-stream-destroy-before-connect.js b/test/parallel/test-http2-client-stream-destroy-before-connect.js
index 4eefc4b9f322b3..06afbf3ce8ceb2 100644
--- a/test/parallel/test-http2-client-stream-destroy-before-connect.js
+++ b/test/parallel/test-http2-client-stream-destroy-before-connect.js
@@ -41,7 +41,7 @@ server.on('listening', common.mustCall(() => {
     })(err);
   }));
 
-  req.on('streamClosed', common.mustCall((code) => {
+  req.on('close', common.mustCall((code) => {
     assert.strictEqual(req.rstCode, NGHTTP2_INTERNAL_ERROR);
     assert.strictEqual(code, NGHTTP2_INTERNAL_ERROR);
     server.close();
diff --git a/test/parallel/test-http2-client-unescaped-path.js b/test/parallel/test-http2-client-unescaped-path.js
index 3a7a607bc3539a..adfbd61fe762b4 100644
--- a/test/parallel/test-http2-client-unescaped-path.js
+++ b/test/parallel/test-http2-client-unescaped-path.js
@@ -30,7 +30,7 @@ server.listen(0, common.mustCall(() => {
       type: Error,
       message: 'Stream closed with error code 1'
     }));
-    req.on('streamClosed', common.mustCall(maybeClose));
+    req.on('close', common.mustCall(maybeClose));
   }
 
   for (let i = 0; i <= count; i += 1)
diff --git a/test/parallel/test-http2-compat-serverresponse-end.js b/test/parallel/test-http2-compat-serverresponse-end.js
index fafb3ea76dd204..366d52321554fe 100644
--- a/test/parallel/test-http2-compat-serverresponse-end.js
+++ b/test/parallel/test-http2-compat-serverresponse-end.js
@@ -183,10 +183,10 @@ const {
 
 
 {
-  // Should be able to call .end with cb from stream 'streamClosed'
+  // Should be able to call .end with cb from stream 'close'
   const server = createServer(mustCall((request, response) => {
     response.writeHead(HTTP_STATUS_OK, { foo: 'bar' });
-    response.stream.on('streamClosed', mustCall(() => {
+    response.stream.on('close', mustCall(() => {
       response.end(mustCall());
     }));
   }));
diff --git a/test/parallel/test-http2-compat-socket.js b/test/parallel/test-http2-compat-socket.js
index aed74149f7248e..5d97872dfb6a14 100644
--- a/test/parallel/test-http2-compat-socket.js
+++ b/test/parallel/test-http2-compat-socket.js
@@ -63,8 +63,8 @@ server.on('request', common.mustCall(function(request, response) {
   assert.strictEqual(request.socket.connecting, false);
 
   // socket events are bound and emitted on Http2Stream
-  request.socket.on('streamClosed', common.mustCall());
-  request.socket.once('streamClosed', common.mustCall());
+  request.socket.on('close', common.mustCall());
+  request.socket.once('close', common.mustCall());
   request.socket.on('testEvent', common.mustCall());
   request.socket.emit('testEvent');
 }));
diff --git a/test/parallel/test-http2-multiheaders-raw.js b/test/parallel/test-http2-multiheaders-raw.js
index 5db253da3c3fa1..c06bf23bff3071 100644
--- a/test/parallel/test-http2-multiheaders-raw.js
+++ b/test/parallel/test-http2-multiheaders-raw.js
@@ -42,7 +42,7 @@ server.on('stream', common.mustCall((stream, headers, flags, rawHeaders) => {
 server.listen(0, common.mustCall(() => {
   const client = http2.connect(`http://localhost:${server.address().port}`);
   const req = client.request(src);
-  req.on('streamClosed', common.mustCall(() => {
+  req.on('close', common.mustCall(() => {
     server.close();
     client.destroy();
   }));
diff --git a/test/parallel/test-http2-multiheaders.js b/test/parallel/test-http2-multiheaders.js
index 7a118214e8f68b..5e477104091cb1 100644
--- a/test/parallel/test-http2-multiheaders.js
+++ b/test/parallel/test-http2-multiheaders.js
@@ -54,7 +54,7 @@ server.listen(0, common.mustCall(() => {
   const client = http2.connect(`http://localhost:${server.address().port}`);
   const req = client.request(src);
   req.on('response', common.mustCall(checkHeaders));
-  req.on('streamClosed', common.mustCall(() => {
+  req.on('close', common.mustCall(() => {
     server.close();
     client.destroy();
   }));
diff --git a/test/parallel/test-http2-options-max-reserved-streams.js b/test/parallel/test-http2-options-max-reserved-streams.js
index 17009a4c11e3e3..d54ca6a7886b3c 100644
--- a/test/parallel/test-http2-options-max-reserved-streams.js
+++ b/test/parallel/test-http2-options-max-reserved-streams.js
@@ -34,7 +34,7 @@ server.on('stream', common.mustCall((stream) => {
     pushedStream.respond({ ':status': 200 });
     pushedStream.on('aborted', common.mustCall());
     pushedStream.on('error', common.mustNotCall());
-    pushedStream.on('streamClosed',
+    pushedStream.on('close',
                     common.mustCall((code) => assert.strictEqual(code, 8)));
   }));
 
@@ -67,12 +67,12 @@ server.on('listening', common.mustCall(() => {
   client.on('stream', common.mustCall((stream) => {
     stream.resume();
     stream.on('end', common.mustCall());
-    stream.on('streamClosed', common.mustCall(maybeClose));
+    stream.on('close', common.mustCall(maybeClose));
   }));
 
   req.on('response', common.mustCall());
 
   req.resume();
   req.on('end', common.mustCall());
-  req.on('streamClosed', common.mustCall(maybeClose));
+  req.on('close', common.mustCall(maybeClose));
 }));
diff --git a/test/parallel/test-http2-respond-file-errors.js b/test/parallel/test-http2-respond-file-errors.js
index b57d9b1046cca0..c2c749873c82ac 100644
--- a/test/parallel/test-http2-respond-file-errors.js
+++ b/test/parallel/test-http2-respond-file-errors.js
@@ -117,7 +117,7 @@ server.listen(0, common.mustCall(() => {
   const client = http2.connect(`http://localhost:${server.address().port}`);
   const req = client.request();
 
-  req.on('streamClosed', common.mustCall(() => {
+  req.on('close', common.mustCall(() => {
     client.destroy();
     server.close();
   }));
diff --git a/test/parallel/test-http2-respond-file-fd-errors.js b/test/parallel/test-http2-respond-file-fd-errors.js
index e5cc280613f72e..9458b2f49af087 100644
--- a/test/parallel/test-http2-respond-file-fd-errors.js
+++ b/test/parallel/test-http2-respond-file-fd-errors.js
@@ -144,7 +144,7 @@ server.listen(0, common.mustCall(() => {
   const client = http2.connect(`http://localhost:${server.address().port}`);
   const req = client.request();
 
-  req.on('streamClosed', common.mustCall(() => {
+  req.on('close', common.mustCall(() => {
     client.destroy();
     server.close();
   }));
diff --git a/test/parallel/test-http2-respond-file-fd-range.js b/test/parallel/test-http2-respond-file-fd-range.js
index fdf9b5e2cd3b4b..8479dca518558e 100644
--- a/test/parallel/test-http2-respond-file-fd-range.js
+++ b/test/parallel/test-http2-respond-file-fd-range.js
@@ -71,7 +71,7 @@ server.listen(0, () => {
     req.on('end', common.mustCall(() => {
       assert.strictEqual(check, data.toString('utf8', 8, 11));
     }));
-    req.on('streamClosed', common.mustCall(maybeClose));
+    req.on('close', common.mustCall(maybeClose));
     req.end();
   }
 
@@ -88,7 +88,7 @@ server.listen(0, () => {
     req.on('end', common.mustCall(() => {
       assert.strictEqual(check, data.toString('utf8', 8, 28));
     }));
-    req.on('streamClosed', common.mustCall(maybeClose));
+    req.on('close', common.mustCall(maybeClose));
     req.end();
   }
 
diff --git a/test/parallel/test-http2-respond-no-data.js b/test/parallel/test-http2-respond-no-data.js
index 74f85a9cd0cccd..d891fe4e8ddd2b 100644
--- a/test/parallel/test-http2-respond-no-data.js
+++ b/test/parallel/test-http2-respond-no-data.js
@@ -13,7 +13,7 @@ const server = http2.createServer();
 const status = [204, 205, 304];
 
 server.on('stream', common.mustCall((stream) => {
-  stream.on('streamClosed', common.mustCall(() => {
+  stream.on('close', common.mustCall(() => {
     assert.strictEqual(stream.destroyed, true);
   }));
   stream.respond({ ':status': status.shift() });
diff --git a/test/parallel/test-http2-response-splitting.js b/test/parallel/test-http2-response-splitting.js
index 8aae2156e43fd9..1d9b616105f450 100644
--- a/test/parallel/test-http2-response-splitting.js
+++ b/test/parallel/test-http2-response-splitting.js
@@ -67,7 +67,7 @@ server.listen(0, common.mustCall(() => {
     }));
     req.resume();
     req.on('end', common.mustCall());
-    req.on('streamClosed', common.mustCall(maybeClose));
+    req.on('close', common.mustCall(maybeClose));
   }
 
   doTest(str, 'location', str);
diff --git a/test/parallel/test-http2-server-rst-before-respond.js b/test/parallel/test-http2-server-rst-before-respond.js
index 25dfa7e010610d..47ba68bd29ed81 100644
--- a/test/parallel/test-http2-server-rst-before-respond.js
+++ b/test/parallel/test-http2-server-rst-before-respond.js
@@ -35,7 +35,7 @@ server.on('listening', common.mustCall(() => {
 
   req.on('headers', common.mustNotCall());
 
-  req.on('streamClosed', common.mustCall((code) => {
+  req.on('close', common.mustCall((code) => {
     assert.strictEqual(h2.constants.NGHTTP2_NO_ERROR, code);
     server.close();
     client.destroy();
diff --git a/test/parallel/test-http2-server-rst-stream.js b/test/parallel/test-http2-server-rst-stream.js
index dd38efb42f9d47..4b04f29c8ec7c0 100644
--- a/test/parallel/test-http2-server-rst-stream.js
+++ b/test/parallel/test-http2-server-rst-stream.js
@@ -43,7 +43,7 @@ server.listen(0, common.mustCall(() => {
       ':method': 'POST',
       rstmethod: test[0]
     });
-    req.on('streamClosed', common.mustCall((code) => {
+    req.on('close', common.mustCall((code) => {
       assert.strictEqual(code, test[1]);
       countdown.dec();
     }));
diff --git a/test/parallel/test-http2-server-socketerror.js b/test/parallel/test-http2-server-socketerror.js
index 24945e531af42e..9f52b9280d2779 100644
--- a/test/parallel/test-http2-server-socketerror.js
+++ b/test/parallel/test-http2-server-socketerror.js
@@ -49,7 +49,7 @@ server.listen(0, common.mustCall(() => {
   const req = client.request();
   req.resume();
   req.on('end', common.mustCall());
-  req.on('streamClosed', common.mustCall(() => {
+  req.on('close', common.mustCall(() => {
     client.destroy();
     server.close();
   }));
diff --git a/test/parallel/test-http2-stream-client.js b/test/parallel/test-http2-stream-client.js
index 3b802f6e6d5a2a..aa722c5ff2b6d9 100644
--- a/test/parallel/test-http2-stream-client.js
+++ b/test/parallel/test-http2-stream-client.js
@@ -21,7 +21,7 @@ server.listen(0, common.mustCall(() => {
   const client = http2.connect(`http://localhost:${server.address().port}`);
   const req = client.request();
   req.resume();
-  req.on('streamClosed', common.mustCall(() => {
+  req.on('close', common.mustCall(() => {
     client.destroy();
     server.close();
   }));
diff --git a/test/parallel/test-http2-stream-destroy-event-order.js b/test/parallel/test-http2-stream-destroy-event-order.js
index 16bb5e6a622033..c375bd80b38e22 100644
--- a/test/parallel/test-http2-stream-destroy-event-order.js
+++ b/test/parallel/test-http2-stream-destroy-event-order.js
@@ -11,7 +11,7 @@ let req;
 const server = http2.createServer();
 server.on('stream', common.mustCall((stream) => {
   stream.on('error', common.mustCall(() => {
-    stream.on('streamClosed', common.mustCall((code) => {
+    stream.on('close', common.mustCall((code) => {
       assert.strictEqual(code, 2);
       client.destroy();
       server.close();
diff --git a/test/parallel/test-http2-too-large-headers.js b/test/parallel/test-http2-too-large-headers.js
index fa15b6ea114007..f7ac25170b846f 100644
--- a/test/parallel/test-http2-too-large-headers.js
+++ b/test/parallel/test-http2-too-large-headers.js
@@ -22,7 +22,7 @@ server.listen(0, common.mustCall(() => {
       type: Error,
       message: 'Stream closed with error code 11'
     }));
-    req.on('streamClosed', common.mustCall((code) => {
+    req.on('close', common.mustCall((code) => {
       assert.strictEqual(code, NGHTTP2_ENHANCE_YOUR_CALM);
       server.close();
       client.destroy();
diff --git a/test/parallel/test-http2-too-many-headers.js b/test/parallel/test-http2-too-many-headers.js
index 3d63727ed505ca..eff0fa9c351c32 100644
--- a/test/parallel/test-http2-too-many-headers.js
+++ b/test/parallel/test-http2-too-many-headers.js
@@ -25,7 +25,7 @@ server.listen(0, common.mustCall(() => {
     type: Error,
     message: 'Stream closed with error code 11'
   }));
-  req.on('streamClosed', common.mustCall((code) => {
+  req.on('close', common.mustCall((code) => {
     assert.strictEqual(code, NGHTTP2_ENHANCE_YOUR_CALM);
     server.close();
     client.destroy();
diff --git a/test/parallel/test-http2-write-callbacks.js b/test/parallel/test-http2-write-callbacks.js
index e196ca8d3c5fa1..44e33573a680b6 100644
--- a/test/parallel/test-http2-write-callbacks.js
+++ b/test/parallel/test-http2-write-callbacks.js
@@ -30,7 +30,7 @@ server.listen(0, common.mustCall(() => {
   req.setEncoding('utf8');
   req.on('data', (chunk) => actual += chunk);
   req.on('end', common.mustCall(() => assert.strictEqual(actual, 'abcxyz')));
-  req.on('streamClosed', common.mustCall(() => {
+  req.on('close', common.mustCall(() => {
     client.destroy();
     server.close();
   }));

From 72c5f1f9ae550c147ba736e509d09b144f791ab5 Mon Sep 17 00:00:00 2001
From: James M Snell <jasnell@gmail.com>
Date: Sun, 26 Nov 2017 11:40:50 -0800
Subject: [PATCH 5/7] test: fixup test-http2-create-client-secure-session

General improvements to test and verify that a secureConnect
handler is present
---
 ...test-http2-create-client-secure-session.js | 54 +++++++++----------
 1 file changed, 24 insertions(+), 30 deletions(-)

diff --git a/test/parallel/test-http2-create-client-secure-session.js b/test/parallel/test-http2-create-client-secure-session.js
index 62a79148dcac47..811ef772d5903a 100644
--- a/test/parallel/test-http2-create-client-secure-session.js
+++ b/test/parallel/test-http2-create-client-secure-session.js
@@ -20,10 +20,7 @@ function loadKey(keyname) {
 function onStream(stream, headers) {
   const socket = stream.session[kSocket];
   assert(headers[':authority'].startsWith(socket.servername));
-  stream.respond({
-    'content-type': 'text/html',
-    ':status': 200
-  });
+  stream.respond({ 'content-type': 'application/json' });
   stream.end(JSON.stringify({
     servername: socket.servername,
     alpnProtocol: socket.alpnProtocol
@@ -33,35 +30,32 @@ function onStream(stream, headers) {
 function verifySecureSession(key, cert, ca, opts) {
   const server = h2.createSecureServer({ cert, key });
   server.on('stream', common.mustCall(onStream));
-  server.listen(0);
-  server.on('listening', common.mustCall(function() {
-    const headers = { ':path': '/' };
-    if (!opts) {
-      opts = {};
-    }
+  server.listen(0, common.mustCall(() => {
+    opts = opts || { };
     opts.secureContext = tls.createSecureContext({ ca });
-    const client = h2.connect(`https://localhost:${this.address().port}`, opts, function() {
-      const req = client.request(headers);
+    const client = h2.connect(`https://localhost:${server.address().port}`,
+                              opts);
+    // Verify that a 'secureConnect' listener is attached
+    assert.strictEqual(client.socket.listenerCount('secureConnect'), 1);
+    const req = client.request();
 
-      req.on('response', common.mustCall(function(headers) {
-        assert.strictEqual(headers[':status'], 200, 'status code is set');
-        assert.strictEqual(headers['content-type'], 'text/html',
-                           'content type is set');
-        assert(headers['date'], 'there is a date');
-      }));
+    req.on('response', common.mustCall((headers) => {
+      assert.strictEqual(headers[':status'], 200);
+      assert.strictEqual(headers['content-type'], 'application/json');
+      assert(headers['date']);
+    }));
 
-      let data = '';
-      req.setEncoding('utf8');
-      req.on('data', (d) => data += d);
-      req.on('end', common.mustCall(() => {
-        const jsonData = JSON.parse(data);
-        assert.strictEqual(jsonData.servername, opts.servername || 'localhost');
-        assert.strictEqual(jsonData.alpnProtocol, 'h2');
-        server.close();
-        client[kSocket].destroy();
-      }));
-      req.end();
-    });
+    let data = '';
+    req.setEncoding('utf8');
+    req.on('data', (d) => data += d);
+    req.on('end', common.mustCall(() => {
+      const jsonData = JSON.parse(data);
+      assert.strictEqual(jsonData.servername,
+                         opts.servername || 'localhost');
+      assert.strictEqual(jsonData.alpnProtocol, 'h2');
+      server.close();
+      client[kSocket].destroy();
+    }));
   }));
 }
 

From 379f9999c79c7b64463f749fcdd882eeefc74624 Mon Sep 17 00:00:00 2001
From: James M Snell <jasnell@gmail.com>
Date: Sun, 26 Nov 2017 11:50:59 -0800
Subject: [PATCH 6/7] http2: remove unnecessary event handlers

---
 lib/internal/http2/compat.js | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js
index 5ddde39d387785..ec1f0ba64eff0a 100644
--- a/lib/internal/http2/compat.js
+++ b/lib/internal/http2/compat.js
@@ -122,20 +122,6 @@ function onStreamDrain() {
     response.emit('drain');
 }
 
-// TODO Http2Stream does not emit 'close'
-function onStreamClosedRequest() {
-  const request = this[kRequest];
-  if (request !== undefined)
-    request.push(null);
-}
-
-// TODO Http2Stream does not emit 'close'
-function onStreamClosedResponse() {
-  const response = this[kResponse];
-  if (response !== undefined)
-    response.emit('finish');
-}
-
 function onStreamAbortedRequest() {
   const request = this[kRequest];
   if (request !== undefined && request[kState].closed === false) {
@@ -247,7 +233,6 @@ class Http2ServerRequest extends Readable {
     stream.on('trailers', onStreamTrailers);
     stream.on('end', onStreamEnd);
     stream.on('error', onStreamError);
-    stream.on('close', onStreamClosedRequest);
     stream.on('aborted', onStreamAbortedRequest);
     const onfinish = this[kFinish].bind(this);
     stream.on('close', onfinish);
@@ -380,7 +365,6 @@ class Http2ServerResponse extends Stream {
     stream[kResponse] = this;
     this.writable = true;
     stream.on('drain', onStreamDrain);
-    stream.on('close', onStreamClosedResponse);
     stream.on('aborted', onStreamAbortedResponse);
     const onfinish = this[kFinish].bind(this);
     stream.on('close', onfinish);

From 4b964e5df51f4248ba19b40b690ca47fc96d1fb0 Mon Sep 17 00:00:00 2001
From: James M Snell <jasnell@gmail.com>
Date: Sun, 26 Nov 2017 12:02:14 -0800
Subject: [PATCH 7/7] http2: use more descriptive names

---
 lib/internal/http2/core.js | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js
index 025b0e363356ea..78446d2fb6e112 100644
--- a/lib/internal/http2/core.js
+++ b/lib/internal/http2/core.js
@@ -582,15 +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);
+  const shutdownFn = 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(fn);
+    setImmediate(shutdownFn);
     return;
   }
-  fn();
+  shutdownFn();
 }
 
 function finishSessionDestroy(socket) {
@@ -877,12 +877,12 @@ class Http2Session extends EventEmitter {
     debug(`Http2Session ${sessionName(this[kType])}: sending settings`);
 
     state.pendingAck++;
-    const fn = submitSettings.bind(this, settings);
+    const settingsFn = submitSettings.bind(this, settings);
     if (state.connecting) {
-      this.once('connect', fn);
+      this.once('connect', settingsFn);
       return;
     }
-    fn();
+    settingsFn();
   }
 
   // Destroy the Http2Session
@@ -968,14 +968,14 @@ class Http2Session extends EventEmitter {
       this.on('shutdown', callback);
     }
 
-    const fn = submitShutdown.bind(this, options);
+    const shutdownFn = submitShutdown.bind(this, options);
     if (state.connecting) {
-      this.once('connect', fn);
+      this.once('connect', shutdownFn);
       return;
     }
 
     debug(`Http2Session ${sessionName(type)}: sending shutdown`);
-    fn();
+    shutdownFn();
   }
 
   _onTimeout() {
@@ -1379,12 +1379,12 @@ class Http2Stream extends Duplex {
     if (code < 0 || code > kMaxInt)
       throw new errors.RangeError('ERR_OUT_OF_RANGE', 'code');
 
-    const fn = submitRstStream.bind(this, code);
+    const rstStreamFn = submitRstStream.bind(this, code);
     if (this[kID] === undefined) {
-      this.once('ready', fn);
+      this.once('ready', rstStreamFn);
       return;
     }
-    fn();
+    rstStreamFn();
   }
 
   rstWithNoError() {
@@ -1415,12 +1415,12 @@ class Http2Stream extends Duplex {
     options = Object.assign({}, options);
     validatePriorityOptions(options);
 
-    const fn = submitPriority.bind(this, options);
+    const priorityFn = submitPriority.bind(this, options);
     if (this[kID] === undefined) {
-      this.once('ready', fn);
+      this.once('ready', priorityFn);
       return;
     }
-    fn();
+    priorityFn();
   }
 
   // Called by this.destroy().