http2: do not allow socket manipulation

Because of the specific serialization and processing requirements
of HTTP/2, sockets should not be directly manipulated. This
forbids any interactions with destroy, emit, end, pause, read,
resume and write methods of the socket. It also redirects
setTimeout to session instead of socket.

PR-URL: https://github.com/nodejs/node/pull/16330
Fixes: https://github.com/nodejs/node/issues/16252
Refs: https://github.com/nodejs/node/pull/16211
Reviewed-By: James M Snell <jasnell@gmail.com>

4 files changed, 61 insertions, 40 deletions

diff --git a/lib/internal/errors.js b/lib/internal/errors.js
index 1ebc1b3ca2d..ca08652e636 100644
--- a/lib/internal/errors.js
+++ b/lib/internal/errors.js
@@ -199,8 +199,7 @@ E('ERR_HTTP2_INVALID_STREAM', 'The stream has been destroyed');
 E('ERR_HTTP2_MAX_PENDING_SETTINGS_ACK',
   (max) => `Maximum number of pending settings acknowledgements (${max})`);
 E('ERR_HTTP2_NO_SOCKET_MANIPULATION',
-  'HTTP/2 sockets should not be directly read from, written to, ' +
-  'paused and/or resumed.');
+  'HTTP/2 sockets should not be directly manipulated (e.g. read and written)');
 E('ERR_HTTP2_OUT_OF_STREAMS',
   'No stream ID is available because maximum stream ID has been reached');
 E('ERR_HTTP2_PAYLOAD_FORBIDDEN',
diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js
index 84f15b2ed8a..c96fc931969 100644
--- a/lib/internal/http2/compat.js
+++ b/lib/internal/http2/compat.js
@@ -5,6 +5,7 @@ const Readable = Stream.Readable;
 const binding = process.binding('http2');
 const constants = binding.constants;
 const errors = require('internal/errors');
+const { kSocket } = require('internal/http2/util');
 
 const kFinish = Symbol('finish');
 const kBeginSend = Symbol('begin-send');
@@ -176,15 +177,15 @@ const proxySocketHandler = {
         throw new errors.Error('ERR_HTTP2_NO_SOCKET_MANIPULATION');
       default:
         const ref = stream.session !== undefined ?
-          stream.session.socket : stream;
+          stream.session[kSocket] : stream;
         const value = ref[prop];
         return typeof value === 'function' ? value.bind(ref) : value;
     }
   },
   getPrototypeOf(stream) {
     if (stream.session !== undefined)
-      return stream.session.socket.constructor.prototype;
-    return stream.prototype;
+      return Reflect.getPrototypeOf(stream.session[kSocket]);
+    return Reflect.getPrototypeOf(stream);
   },
   set(stream, prop, value) {
     switch (prop) {
@@ -201,9 +202,9 @@ const proxySocketHandler = {
       case 'setTimeout':
         const session = stream.session;
         if (session !== undefined)
-          session[prop] = value;
+          session.setTimeout = value;
         else
-          stream[prop] = value;
+          stream.setTimeout = value;
         return true;
       case 'write':
       case 'read':
@@ -212,7 +213,7 @@ const proxySocketHandler = {
         throw new errors.Error('ERR_HTTP2_NO_SOCKET_MANIPULATION');
       default:
         const ref = stream.session !== undefined ?
-          stream.session.socket : stream;
+          stream.session[kSocket] : stream;
         ref[prop] = value;
         return true;
     }
diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js
index 77f7ae40c91..71489ba4ec9 100644
--- a/lib/internal/http2/core.js
+++ b/lib/internal/http2/core.js
@@ -38,6 +38,7 @@ const {
   getSettings,
   getStreamState,
   isPayloadMeaningless,
+  kSocket,
   mapToHeaders,
   NghttpError,
   sessionName,
@@ -70,10 +71,10 @@ const kOptions = Symbol('options');
 const kOwner = Symbol('owner');
 const kProceed = Symbol('proceed');
 const kProtocol = Symbol('protocol');
+const kProxySocket = Symbol('proxy-socket');
 const kRemoteSettings = Symbol('remote-settings');
 const kServer = Symbol('server');
 const kSession = Symbol('session');
-const kSocket = Symbol('socket');
 const kState = Symbol('state');
 const kType = Symbol('type');
 
@@ -672,6 +673,48 @@ function finishSessionDestroy(self, socket) {
   debug(`[${sessionName(self[kType])}] nghttp2session destroyed`);
 }
 
+const proxySocketHandler = {
+  get(session, prop) {
+    switch (prop) {
+      case 'setTimeout':
+        return session.setTimeout.bind(session);
+      case 'destroy':
+      case 'emit':
+      case 'end':
+      case 'pause':
+      case 'read':
+      case 'resume':
+      case 'write':
+        throw new errors.Error('ERR_HTTP2_NO_SOCKET_MANIPULATION');
+      default:
+        const socket = session[kSocket];
+        const value = socket[prop];
+        return typeof value === 'function' ? value.bind(socket) : value;
+    }
+  },
+  getPrototypeOf(session) {
+    return Reflect.getPrototypeOf(session[kSocket]);
+  },
+  set(session, prop, value) {
+    switch (prop) {
+      case 'setTimeout':
+        session.setTimeout = value;
+        return true;
+      case 'destroy':
+      case 'emit':
+      case 'end':
+      case 'pause':
+      case 'read':
+      case 'resume':
+      case 'write':
+        throw new errors.Error('ERR_HTTP2_NO_SOCKET_MANIPULATION');
+      default:
+        session[kSocket][prop] = value;
+        return true;
+    }
+  }
+};
+
 // 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 {
@@ -707,6 +750,7 @@ class Http2Session extends EventEmitter {
     };
 
     this[kType] = type;
+    this[kProxySocket] = null;
     this[kSocket] = socket;
 
     // Do not use nagle's algorithm
@@ -756,7 +800,10 @@ class Http2Session extends EventEmitter {
 
   // The socket owned by this session
   get socket() {
-    return this[kSocket];
+    const proxySocket = this[kProxySocket];
+    if (proxySocket === null)
+      return this[kProxySocket] = new Proxy(this, proxySocketHandler);
+    return proxySocket;
   }
 
   // The session type
@@ -957,6 +1004,7 @@ class Http2Session extends EventEmitter {
     // Disassociate from the socket and server
     const socket = this[kSocket];
     // socket.pause();
+    delete this[kProxySocket];
     delete this[kSocket];
     delete this[kServer];
 
@@ -2155,30 +2203,6 @@ function socketDestroy(error) {
   this.destroy(error);
 }
 
-function socketOnResume() {
-  if (this._paused)
-    return this.pause();
-  if (this._handle && !this._handle.reading) {
-    this._handle.reading = true;
-    this._handle.readStart();
-  }
-}
-
-function socketOnPause() {
-  if (this._handle && this._handle.reading) {
-    this._handle.reading = false;
-    this._handle.readStop();
-  }
-}
-
-function socketOnDrain() {
-  const needPause = 0 > this._writableState.highWaterMark;
-  if (this._paused && !needPause) {
-    this._paused = false;
-    this.resume();
-  }
-}
-
 // When an Http2Session emits an error, first try to forward it to the
 // server as a sessionError; failing that, forward it to the socket as
 // a sessionError; failing that, destroy, remove the error listener, and
@@ -2267,9 +2291,6 @@ function connectionListener(socket) {
   }
 
   socket.on('error', socketOnError);
-  socket.on('resume', socketOnResume);
-  socket.on('pause', socketOnPause);
-  socket.on('drain', socketOnDrain);
   socket.on('close', socketOnClose);
 
   // Set up the Session
@@ -2426,9 +2447,6 @@ function connect(authority, options, listener) {
   }
 
   socket.on('error', socketOnError);
-  socket.on('resume', socketOnResume);
-  socket.on('pause', socketOnPause);
-  socket.on('drain', socketOnDrain);
   socket.on('close', socketOnClose);
 
   const session = new ClientHttp2Session(options, socket);
diff --git a/lib/internal/http2/util.js b/lib/internal/http2/util.js
index 4610be7e4d3..2426a409cfc 100644
--- a/lib/internal/http2/util.js
+++ b/lib/internal/http2/util.js
@@ -3,6 +3,8 @@
 const binding = process.binding('http2');
 const errors = require('internal/errors');
 
+const kSocket = Symbol('socket');
+
 const {
   NGHTTP2_SESSION_CLIENT,
   NGHTTP2_SESSION_SERVER,
@@ -551,6 +553,7 @@ module.exports = {
   getSettings,
   getStreamState,
   isPayloadMeaningless,
+  kSocket,
   mapToHeaders,
   NghttpError,
   sessionName,