Skip to content

Commit

Permalink
http2: move events to the JSStreamSocket
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
mmomtchev authored and danielleadams committed Nov 9, 2020
1 parent 06cc400 commit 429113e
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 3 deletions.
9 changes: 6 additions & 3 deletions lib/internal/http2/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
55 changes: 55 additions & 0 deletions test/parallel/test-http2-client-jsstream-destroy.js
Original file line number Diff line number Diff line change
@@ -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(() => { }));
});
}));

0 comments on commit 429113e

Please sign in to comment.