Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
mmomtchev opened this issue Oct 23, 2020 · 1 comment
Open

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

mmomtchev opened this issue Oct 23, 2020 · 1 comment
Labels
http2 Issues or PRs related to the http2 subsystem.

Comments

@mmomtchev
Copy link
Contributor

  • Version: 12, 14 and master
  • Platform: All
  • Subsystem: http2

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

  • socket option is ignored which will further brake existing (broken) user code
  • socket option is made to work
  • intimidate developers into fixing their code with a nasty warning when this is detected
  • something else

What do you see instead?

Additional information

@mmomtchev
Copy link
Contributor Author

Second half of #35695

@PoojaDurgad PoojaDurgad added the http2 Issues or PRs related to the http2 subsystem. label Oct 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
http2 Issues or PRs related to the http2 subsystem.
Projects
None yet
Development

No branches or pull requests

2 participants