From 429113ebfb1efe28e0ab441d4ec9627f0ef09fa5 Mon Sep 17 00:00:00 2001 From: Momtchil Momtchev Date: Fri, 23 Oct 2020 13:16:15 +0200 Subject: [PATCH] http2: move events to the JSStreamSocket 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: https://github.com/nodejs/node/issues/35695 PR-URL: https://github.com/nodejs/node/pull/35772 Reviewed-By: Matteo Collina Reviewed-By: Anna Henningsen Reviewed-By: Rich Trott Reviewed-By: James M Snell Reviewed-By: Ricky Zhou <0x19951125@gmail.com> --- lib/internal/http2/core.js | 9 ++- .../test-http2-client-jsstream-destroy.js | 55 +++++++++++++++++++ 2 files changed, 61 insertions(+), 3 deletions(-) create mode 100644 test/parallel/test-http2-client-jsstream-destroy.js diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 7bd30eeaf10a10..dd7501c40b3ea3 100644 --- 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 new file mode 100644 index 00000000000000..f2f0669170c4bc --- /dev/null +++ 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(() => { })); + }); +}));