http2: do not allow socket manipulation

doc/api/errors.md

@@ -810,8 +810,8 @@ reached.
 <a id="ERR_HTTP2_NO_SOCKET_MANIPULATION"></a>
 ### ERR_HTTP2_NO_SOCKET_MANIPULATION
 
-Used when attempting to read, write, pause, and/or resume a socket attached to
-an `Http2Session`.
+Used when attempting to directly manipulate (e.g read, write, pause, resume,
+etc.) a socket attached to an `Http2Session`.
 
 <a id="ERR_HTTP2_OUT_OF_STREAMS"></a>
 ### ERR_HTTP2_OUT_OF_STREAMS

doc/api/http2.md

@@ -463,12 +463,16 @@ added: v8.4.0
 
 * Value: {net.Socket|tls.TLSSocket}
 
-A reference to the [`net.Socket`][] or [`tls.TLSSocket`][] to which this
-`Http2Session` instance is bound.
+Returns a Proxy object that acts as a `net.Socket` (or `tls.TLSSocket`) but
+limits available methods to ones safe to use with HTTP/2.
 
-*Note*: It is not recommended for user code to interact directly with a
-`Socket` bound to an `Http2Session`. See [Http2Session and Sockets][] for
-details.
+`destroy`, `emit`, `end`, `pause`, `read`, `resume`, and `write` will throw
+an error with code `ERR_HTTP2_NO_SOCKET_MANIPULATION`. See
+[Http2Session and Sockets][] for more information.
+
+`setTimeout` method will be called on this `Http2Session`.
+
+All other interactions will be routed directly to the socket.
 
 #### http2session.state
 <!-- YAML
@@ -2138,10 +2142,10 @@ Returns `request`.
 added: v8.4.0
 -->
 
-* {net.Socket}
+* {net.Socket|tls.TLSSocket}
 
-Returns a Proxy object that acts as a `net.Socket` but applies getters,
-setters and methods based on HTTP/2 logic.
+Returns a Proxy object that acts as a `net.Socket` (or `tls.TLSSocket`) but
+applies getters, setters and methods based on HTTP/2 logic.
 
 `destroyed`, `readable`, and `writable` properties will be retrieved from and
 set on `request.stream`.
@@ -2293,7 +2297,7 @@ will result in a [`TypeError`][] being thrown.
 added: v8.4.0
 -->
 
-* {net.Socket}
+* {net.Socket|tls.TLSSocket}
 
 See [`response.socket`][].
 
@@ -2510,10 +2514,10 @@ Returns `response`.
 added: v8.4.0
 -->
 
-* {net.Socket}
+* {net.Socket|tls.TLSSocket}
 
-Returns a Proxy object that acts as a `net.Socket` but applies getters,
-setters and methods based on HTTP/2 logic.
+Returns a Proxy object that acts as a `net.Socket` (or `tls.TLSSocket`) but
+applies getters, setters and methods based on HTTP/2 logic.
 
 `destroyed`, `readable`, and `writable` properties will be retrieved from and
 set on `response.stream`.

lib/internal/errors.js

@@ -208,8 +208,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',

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;
     }

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);

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,

test/parallel/test-http2-client-destroy.js

@@ -1,10 +1,13 @@
+// Flags: --expose-internals
+
 'use strict';
 
 const common = require('../common');
 if (!common.hasCrypto)
   common.skip('missing crypto');
 const assert = require('assert');
 const h2 = require('http2');
+const { kSocket } = require('internal/http2/util');
 
 {
   const server = h2.createServer();
@@ -13,7 +16,7 @@ const h2 = require('http2');
     common.mustCall(() => {
       const destroyCallbacks = [
         (client) => client.destroy(),
-        (client) => client.socket.destroy()
+        (client) => client[kSocket].destroy()
       ];
 
       let remaining = destroyCallbacks.length;
@@ -23,9 +26,9 @@ const h2 = require('http2');
         client.on(
           'connect',
           common.mustCall(() => {
-            const socket = client.socket;
+            const socket = client[kSocket];
 
-            assert(client.socket, 'client session has associated socket');
+            assert(socket, 'client session has associated socket');
             assert(
               !client.destroyed,
               'client has not been destroyed before destroy is called'
@@ -41,7 +44,7 @@ const h2 = require('http2');
             destroyCallback(client);
 
             assert(
-              !client.socket,
+              !client[kSocket],
               'client.socket undefined after destroy is called'
             );
 

test/parallel/test-http2-client-socket-destroy.js

@@ -1,9 +1,13 @@
+// Flags: --expose-internals
+
 'use strict';
 
 const common = require('../common');
 if (!common.hasCrypto)
   common.skip('missing crypto');
 const h2 = require('http2');
+const { kSocket } = require('internal/http2/util');
+
 const body =
   '<html><head></head><body><h1>this is some data</h2></body></html>';
 
@@ -32,7 +36,7 @@ server.on('listening', common.mustCall(function() {
 
   req.on('response', common.mustCall(() => {
     // send a premature socket close
-    client.socket.destroy();
+    client[kSocket].destroy();
   }));
   req.on('data', common.mustNotCall());
 

test/parallel/test-http2-compat-socket-set.js

@@ -12,8 +12,8 @@ const h2 = require('http2');
 const errMsg = {
   code: 'ERR_HTTP2_NO_SOCKET_MANIPULATION',
   type: Error,
-  message: 'HTTP/2 sockets should not be directly read from, written to, ' +
-           'paused and/or resumed.'
+  message: 'HTTP/2 sockets should not be directly manipulated ' +
+           '(e.g. read and written)'
 };
 
 const server = h2.createServer();

test/parallel/test-http2-compat-socket.js

@@ -14,8 +14,8 @@ const net = require('net');
 const errMsg = {
   code: 'ERR_HTTP2_NO_SOCKET_MANIPULATION',
   type: Error,
-  message: 'HTTP/2 sockets should not be directly read from, written to, ' +
-           'paused and/or resumed.'
+  message: 'HTTP/2 sockets should not be directly manipulated ' +
+           '(e.g. read and written)'
 };
 
 const server = h2.createServer();