We "need" a way to create an Http2Session from a socket

[grantila]

While building fetch-h2 and when discussing with node-fetch, and while participating in #14671 I realize that we need this in Node.js; a protocol-agnostic connect() to a TLS-enabled web server and let the ALPN handshake allow for both "h2" and "http/1.1". Without this, we won't be able to connect to an https-server without knowing in beforehand if that server wants to (or even can!) speak HTTP/1.1 or HTTP/2.

I'm thinking of something like tls.handoverConnect() which doesn't take application-level (http) arguments (path, headers, etc), it just connects and returns (through a callback, likely) the protocol ("http/1.1" or "h2") and a corresponding session.

A question is what the session data is. In case of HTTP/2, it should be an Http2Session, but for HTTP/1.1 it must be something where the user can perform the request, and it should probably be integrated with Agents. There's unfortunately an inheritance jungle in the design of these old functions, like https.request() where ip/tcp/application-level options are merged into one blob, but the "https/1.1 session" here (let's call it Http1SessionProxy) should be something that looks like the http module, in how it has a request() function, with the tcp/ip-level options removed.

Let's say this feature belongs in tls (which I think it does):

const connectOpts = { host: "my-server.com", port: 443, /* ... */ };

tls.handoverConnect( connectOpts, ( err, protocol ) =>
{
    if ( err ) ... ;

    // session is an Http2Session or Http1SessionProxy
    const { name, session } = protocol;

    if ( name === 'http/1.1' )
    {
        // session.request acts just like http.request
        const req = session.request(
            { path: '/foo', headers, ... },
            ( res ) => { ... }
        );
    }
    else if ( name === 'h2' )
    {
        // well, the usual http2-stuff
        const stream = session.request( headers, options );
    }
} );

In this example, perhaps an Agent could be passed in the connectOpts, so that if the session turns out to be HTTP/1.1, the connection will be added to this agent (and by default the global Agent is used, just like traditional http{s}.request()).

Any thoughts on this? I guess it's a good time to look into this now, as the http2 module is being tested in the wild, but before it's "stable".

[addaleax]

It’s not 100 % clear what you are asking for, but this feels like something one should already be able to do using existing APIs…

Just to clarify, the TLS module’s connect() method is protocol-agnostic, and you can pass it a list of supported ALPN protocols, and once connected tlsSocket.alpnProtocol will contain the negotiated protocol.

[grantila]

Right. After that callback you have two things: A socket and a protocol name. How do you then go about to turn this socket into an Http2Session (or something on which you can perform HTTP/1.1 requests)? That's what I'm after. Those things seem very internal to Node.js and not exposed by the current API's (disclaimer: I might have overlooked them).

[addaleax]

Right – I think it should be possible to create HTTP or HTTP2 sessions from arbitrary streams, or at least I think the internals structures of HTTP were made with such a use case in mind…

Edit: I’ll definitely be looking into this later today and make sure

[grantila]

Sounds great, I didn't think about it, but perhaps this already works for http1 by setting the createConnection property to a function returning an already existing socket. It sure sounds like a hack, and perhaps the option to http.request should take a socket property instead.

For http2, I'd suggest allowing http2.connect to take a socket as first "authority" argument too (not just string or URL).

It does sound like this could be a pretty simple fix with a huge impact, and given the bug #14671 I think this issue is more important to look at, since by allowing the user to first perform TLS handshake and then handover to http2, #14671 is less of a problem (and perhaps even automatically fixed as well).

[apapirovski]

The connect implementation in http2 is currently very bare-bones, basically enough to do some fetching and run our tests. Any PRs to improve it are very welcome. I don't think we ultimately expect user-code to use tls.connect if they want https & http2.

But if you were trying to do that right now, you would need tls.connect(), wait for the negotiation to happen and then create the Http2Session within that listener (3rd argument is socket).

(No one has really shown that #14671 is a real bug. As far as I can tell people are just not passing in the right options to connect to a secure server so their client is rejecting the connection. http2 literally just defers to tls so I don't really see why there would be any bugs there.)

[grantila]

@apapirovski How do I create an Http2Session from a socket?

Did you read my comment in #14671? If you still mean that I haven't shown that it's a real bug, please explain there what I do wrong.

[apapirovski]

So... we don't actually export Http2Session at the moment which means this isn't really possible but if we did, it would look something like this:

const https = require('https');
const {
  constants,
  Http2Session
} = require('http2');
const tls = require('tls');

const tlsConnection = tls.connect({
  host: 'nghttp2.org',
  port: 443,
  ALPNProtocols: ['h2', 'http/1.1']
});
tlsConnection.once('secureConnect', () => {
  if (tlsConnection.alpnProtocol === 'h2')
    withHttp2(tlsConnection);
  else if (tlsConnection.alpnProtocol === 'http/1.1')
    withHttp1(tlsConnection);
  else
    throw new Error('no valid protocol negotiated');
});

function withHttp1(socket) {
  const req = https.request({
    host: 'nghttp2.org',
    path: '/httpbin/ip',
    port: 443,
    createConnection: () => socket
  }, (res) => console.log(res.headers));
  req.end();
}

function withHttp2(socket) {
  const client = new Http2Session(constants.NGHTTP2_SESSION_CLIENT,
                                  {},
                                  socket);
  const req = client.request({ ':path': '/httpbin/ip' });
  req.on('response', (headers) => console.log(headers));
  req.end();
}

If you feel like it, you could probably extract parts of this to change connect to support the allowHTTP1 setting and open a PR. :)