Cannot CONNECT http2-over-http2 in Node 20.12.0

[pimterry]

Version

v20.12.0

Platform

Linux 6.5.0-21-generic #21-Ubuntu SMP PREEMPT_DYNAMIC Wed Feb 7 14:17:40 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

http2

What steps will reproduce the bug?

Test-suite ready demo:

'use strict';

const common = require('../common');
const assert = require('assert');
const h2 = require('http2');

const server = h2.createServer();

server.listen(0, common.mustCall(function() {
  const proxyClient = h2.connect(`http://localhost:${server.address().port}`);

  const request = proxyClient.request({
    ':method': 'CONNECT',
    ':authority': 'example.com:80'
  });

  request.on('response', common.mustCall((connectResponse) => {
    assert.strictEqual(connectResponse[':status'], 200);

    const proxiedClient = h2.connect('http://example.com', {
      createConnection: () => request // Tunnel via first request stream
    });

    const proxiedRequest = proxiedClient.request();
    proxiedRequest.on('response', common.mustCall((proxiedResponse) => {
      assert.strictEqual(proxiedResponse[':status'], 204);

      proxiedClient.close();
      proxyClient.close();
      server.close();
    }));
  }));
}));

server.once('connect', common.mustCall((req, res) => {
  assert.strictEqual(req.headers[':method'], 'CONNECT');
  res.writeHead(200); // Accept the CONNECT tunnel

  // Handle this stream as a new 'proxied' connection (pretend to forward
  // but actually just unwrap the tunnel ourselves):
  server.emit('connection', res.stream);
}));

// Handle the 'proxied' request itself:
server.once('request', common.mustCall((req, res) => {
  res.writeHead(204);
  res.end();
}));

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

Fails every time in Node v20.12.

Works every time in Node v20.11 and previous releases (for a few years, at least)

What is the expected behavior? Why is that the expected behavior?

It should be possible to make an HTTP/2 CONNECT request (opening a tunnel to a target server through an HTTP/2 proxy) and then make another HTTP/2 request to the target server.

What do you see instead?

Instead, creating an HTTP/2 connection on top of an HTTP/2 tunnel fails with:

Error [ERR_HTTP2_SOCKET_BOUND]: The socket is already bound to an Http2Session
    at new Http2Session (node:internal/http2/core:1238:13)
    at new ServerHttp2Session (node:internal/http2/core:1640:5)
    at Http2Server.connectionListener (node:internal/http2/core:3102:19)
    at Http2Server.emit (node:events:518:28)
    at Http2Server.<anonymous> (/home/tim/Dropbox/programming/javascript/node/test/parallel/test-http2-client-proxy-over-http2.js:41:10)
    at Http2Server.<anonymous> (/home/tim/Dropbox/programming/javascript/node/test/common/index.js:473:15)
    at Object.onceWrapper (node:events:633:26)
    at Http2Server.emit (node:events:518:28)
    at Http2Server.onServerStream (node:internal/http2/compat:956:17)
    at Http2Server.emit (node:events:518:28) {
  code: 'ERR_HTTP2_SOCKET_BOUND'
}

Additional information

I'm fairly sure I know the issue here: kSession is used in the HTTP/2 code as a single symbol with two meanings:

• On sockets, marking them as being owned by an HTTP/2 session (i.e. there is a session currently running within this socket)
• On HTTP/2 streams, providing access to the HTTP/2 session that contains them (i.e. this stream is running within that session)

In this case, these two meanings mix: the CONNECT stream should be within an HTTP/2 session (the initial connection) and should act as a socket and contain an HTTP/2 session. That conflict is why this blows up.

This didn't happen in the past because the inner stream uses a JSStreamSocket to handle this, and kSession wasn't passed through that wrapper, so the JSS and the inner H2 stream had independent kSession values. That changed in c50524a#diff-8dd7127678f00bf090a030256bad7ae645834f7100e54154b4fc81c8cbe9c3fc, which (reasonably I think) exposed the 'real' kSession through that wrapper.

I think the solution here is two split these two use cases, and track the 'socket-in-use-by' and 'stream-powered-by' state under separate keys, so a single H2 stream can have both. I'm going to keep digging and hopefully open a PR for this within a day or two, but if anybody has any context on that in the meantime and whether this is going to work or whether there's a good reason the two are the same, that would be very interesting.