http2: move events to the JSStreamSocket

mmomtchev · danielleadams · commit 429113ebfb1e · 2020-11-09T13:35:03.000-05:00
When using a JSStreamSocket, the HTTP2Session constructor will replace the socket object http2 events should be attached to the JSStreamSocket object because the http2 session handle lives there Fixes: #35695 PR-URL: #35772 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ricky Zhou <0x19951125@gmail.com>
diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js
@@ -3132,11 +3132,14 @@ function connect(authority, options, listener) {
     }
   }
 
-  socket.on('error', socketOnError);
-  socket.on('close', socketOnClose);
-
   const session = new ClientHttp2Session(options, socket);
 
+  // ClientHttp2Session may create a new socket object
+  // when socket is a JSSocket (socket != kSocket)
+  // https://github.com/nodejs/node/issues/35695
+  session[kSocket].on('error', socketOnError);
+  session[kSocket].on('close', socketOnClose);
+
   session[kAuthority] = `${options.servername || host}:${port}`;
   session[kProtocol] = protocol;
 
diff --git a/test/parallel/test-http2-client-jsstream-destroy.js b/test/parallel/test-http2-client-jsstream-destroy.js
@@ -0,0 +1,55 @@
+'use strict';
+
+const common = require('../common');
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+const h2 = require('http2');
+const tls = require('tls');
+const fixtures = require('../common/fixtures');
+const { Duplex } = require('stream');
+
+const server = h2.createSecureServer({
+  key: fixtures.readKey('agent1-key.pem'),
+  cert: fixtures.readKey('agent1-cert.pem')
+});
+
+class JSSocket extends Duplex {
+  constructor(socket) {
+    super({ emitClose: true });
+    socket.on('close', () => this.destroy());
+    socket.on('data', (data) => this.push(data));
+    this.socket = socket;
+  }
+
+  _write(data, encoding, callback) {
+    this.socket.write(data, encoding, callback);
+  }
+
+  _read(size) {
+  }
+
+  _final(cb) {
+    cb();
+  }
+}
+
+server.listen(0, common.mustCall(function() {
+  const socket = tls.connect({
+    rejectUnauthorized: false,
+    host: 'localhost',
+    port: this.address().port,
+    ALPNProtocols: ['h2']
+  }, () => {
+    const proxy = new JSSocket(socket);
+    const client = h2.connect(`https://localhost:${this.address().port}`, {
+      createConnection: () => proxy
+    });
+    const req = client.request();
+
+    setTimeout(() => socket.destroy(), 200);
+    setTimeout(() => client.close(), 400);
+    setTimeout(() => server.close(), 600);
+
+    req.on('close', common.mustCall(() => { }));
+  });
+}));
