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

We "need" a way to create an Http2Session from a socket - Server-side #46152

Closed
ItalyPaleAle opened this issue Jan 10, 2023 · 18 comments
Closed
Labels
feature request Issues that request new features to be added to Node.js. stale

Comments

@ItalyPaleAle
Copy link

What is the problem this feature will solve?

This issue is named after a 6-years old issue: #16256

The original issue asked for a way to be able to create a http2.Http2Session from a net.Socket, providing an example of code like this (which currently doesn't work):

// socket is a `net.Socket`
const client = new Http2Session(constants.NGHTTP2_SESSION_CLIENT, {}, socket);

The original issue was specifically about problems on the HTTP2 client, and it was fixed by introducing a createConnection handler in http2.connect.

However, we still cannot create a Http2Session object from an existing net.Socket, and this is a problem for our use case in the server.

In our case, we have an app A that needs to invoke app B. App B is behind a firewall so cannot create a listener. How we've solved this problem (in Go) is to create an ephemeral server on app A, so that app A can create a TCP socket to that, and then start a HTTP/2 server (in this case, for gRPC) on top of the existing TCP socket.

What is the feature you are proposing to solve the problem?

We are looking for a way to implement the same solution in Node.js, and a possible solution seems to be in the ability to create Http2Session objects to then serve streams on.

Alternatively, it would be nice to have a way to "start" a HTTP/2 server without using .listen(), but rather on a specific TCP socket.

What alternatives have you considered?

It doesn't look like there's an alternative in Node.js today, sans re-implementing our own HTTP/2 stack.

@ItalyPaleAle ItalyPaleAle added the feature request Issues that request new features to be added to Node.js. label Jan 10, 2023
@marco-ippolito
Copy link
Member

As stated by documentation about Http2Session Instances of this class are not intended to be constructed directly by user code..

At the moment the socket is created by the underlying net.Server and it is converted to by Http2Session to JSStreamSocket in the connectionListener callback from net.Server.
I think this feature requires a whole new api like Http2Server.from I'm not sure tho @mcollina

@mcollina
Copy link
Member

This would require a new API which would be quite a bit of work to test and develop. I recommend that we invest some effort in implementing HTTP/3. Having said this, I'll be ok to review a PR with this new API implemented.

@jasnell wdyt?

@ItalyPaleAle
Copy link
Author

Grazie per la risposta @mcollina & @marco-ippolito :)

From an API design point of view, how would you envision this? Knowing that my ultimate goal is to be able to create a HTTP/2 listener on an existing TCP socket, essentially applying the .on(‘stream’) handler on a session that is created:

  1. Allow instantiating custom Http2Session objects from a given net.Socket. @marco-ippolito ’s message contains a proposed Http2Server.from method
  2. Apply a method on a Http2Server object, just like .listen() but to serve on an existing connection (maybe .serve which accepts a net.Socket)?

@mcollina
Copy link
Member

I would probably do a http2server.createSession(duplex) if it was possible.

@bnoordhuis
Copy link
Member

I wonder how feasible that is. There are likely security considerations that need to be taken into account. HTTP/2 is not nearly as transport-agnostic as HTTP/1 is.

For example, HTTP/2 prohibits TLS renegotiation but you can probably circumvent that (intentionally or unwittingly) when you create a session from an arbitrary TLS socket.

@jasnell
Copy link
Member

jasnell commented Jan 12, 2023

The Http2Session would have to completely take over ownership of the socket after it is created such that nothing else could use it. That said, I'm not convinced that creating an Http2Session from an existing socket is what you want...

Alternatively, it would be nice to have a way to "start" a HTTP/2 server without using .listen(), but rather on a specific TCP socket.

To me, this sounds more like you want to have Http2Server take over ownership of a socket, which is a very different thing.

@ItalyPaleAle
Copy link
Author

I am completely open to any API design that would allow us to achieve our business objective. Which at the end of the day has to do with firewall traversals for establishing a gRPC (hence, HTTP2/) connection between two apps and do that by re-using an established TCP connection.

The Http2Session would have to completely take over ownership of the socket after it is created such that nothing else could use it.

Indeed. Although not sure if this needs to be enforced in some way, or should just be a warning in the documentation. A part of me would think that if someone were to meddle with low-level things like these, they should be responsible for any "collateral damages" they cause.

@bnoordhuis
Copy link
Member

bnoordhuis commented Jan 14, 2023

firewall traversals for establishing a gRPC (hence, HTTP2/) connection between two apps and do that by re-using an established TCP connection

This is client->server traffic? Are you using e.g. HTTP/1.1 CONNECT to punch through your firewall and then want to initiate HTTP/2 over that connection? http2.connect() already supports that, it's the createConnection option.

@ItalyPaleAle
Copy link
Author

No, we establish a HTTP/2 connection directly because it's for gRPC.

App A calls app B. But app B is behind a firewall. So, first app A sets up a TCP listener, then app B establishes a TCP connection to that port (this is outbound), then we start a HTTP/2 server in app B using the established connection.

@github-actions
Copy link
Contributor

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

@github-actions github-actions bot added the stale label Jul 14, 2023
@ItalyPaleAle
Copy link
Author

still relevant here

@bnoordhuis
Copy link
Member

There doesn't seem to be consensus though, neither on the how nor the if. Might as well close it if it's not going to happen.

@mcollina
Copy link
Member

@bnoordhuis @ItalyPaleAle, this issue seems to lack an implementation volunteer. I think it's possible to make this functionality happen but it's a somewhat extensive amount of work with very limited interested by collaborators.

Let's close and revisit if somebody volunteers to implement this.

If this is a feature that Microsoft needs, they might be willing to sponsor its development.

@bnoordhuis
Copy link
Member

Sounds reasonable. I'll go ahead and close it then.

@ItalyPaleAle fyi, I take on contracting work. Email is in my profile.

@bnoordhuis bnoordhuis closed this as not planned Won't fix, can't repro, duplicate, stale Jul 14, 2023
@ItalyPaleAle
Copy link
Author

Thanks, I'll see what I can do.

Just a note that this is needed by Dapr, a CNCF project, not Microsoft!

@mcollina
Copy link
Member

Is there a referencing issue we can track?

@ItalyPaleAle
Copy link
Author

@mcollina Yes: dapr/dapr#5392

The POC was done but it was stalled due to lack of support in some (most?) gRPC SDKs. The Node.js one is blocked by this issue.

@superhero
Copy link

superhero commented Dec 2, 2024

FYI - I was also in need of the ability to create a session from a socket, in my case a client tls socket - post preface check, with read buffer unshifted back to the socket.

I managed to solve my problem using http2.performServerHandshake(socket) maybe it could help someone else looking for something like this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. stale
Projects
None yet
Development

No branches or pull requests

6 participants