nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
105.04k stars 28.45k forks source link

decide the fate of http2.connect(url, { socket }) #35773

Open mmomtchev opened 3 years ago

mmomtchev commented 3 years ago

What steps will reproduce the bug?

Calling http2.connect(url, { socket }) is not supported but (almost) works by passing the socket option to the tls.connect call. I don't think this was an intended behavior.

The following (broken) unit test will produce a segfault:

'use strict';

const common = require('../common');
if (!common.hasCrypto)
  common.skip('missing crypto');
const h2 = require('http2');
const net = require('net');
const fixtures = require('../common/fixtures');

const server = h2.createSecureServer({
  key: fixtures.readKey('agent1-key.pem'),
  cert: fixtures.readKey('agent1-cert.pem')
});

server.listen(0, common.mustCall(function() {
  const socket = net.connect({
    host: 'localhost',
    port: this.address().port
  }, () => {
    const client = h2.connect(`https://localhost:${this.address().port}`, {
      rejectUnauthorized: false,
      socket
    });
    const req = client.request();

    setTimeout(() => socket.destroy(), 100);
    setTimeout(() => client.close(), 200);
    setTimeout(() => server.close(), 300);

    req.on('close', common.mustCall(() => { }));
  });
}));

How often does it reproduce? Is there a required condition?

Always

What is the expected behavior?

Either

What do you see instead?

Additional information

mmomtchev commented 3 years ago

Second half of #35695