feat: add http2

[Nytelife26]

This relates to...

• http2 #547

Rationale

HTTP/2 support is one of the few things currently missing from Undici
that sets it behind native Node http.

Changes

This pull request is a work in progress. There are no changes just
yet. The plan, however, as discussed in the original issue, is to
implement HTTP/2 support around a Dispatcher instance for Pool,
Client and Agent.

Implementation can work in one of the following ways (see RFC7540 and
RFC8740):

1. Prior knowledge

• Servers using HTTP/2 can advertise the capability via methods like ALT-SVC
• Under RFC7540 Section 3.5, the HTTP/2 connection preface must be sent
• All future frames may immediately use the HTTP/2

2. HTTP/1.1 Initialization (servers where HTTP/2 support status is unknown)

• Without prior knowledge, the client should start an HTTP/1.1 connection
  using the Upgrade header (h2c for http URI schema, h2 for TLS) and
  HTTP2-Settings header
• If the response is a HTTP 101 Switching Protocols request, the request
  will be accompanied by a response to prior requests and the connection can
  upgrade to HTTP/2. If the response is normal, exempli gratia, a HTTP 200 OK,
  the client must not switch to HTTP/2.

As for HTTPS, TLS negotiation must occur prior to any upgrade to HTTP/2.

Features

HTTP/2 variants of Pool, Client and Agent. Alternatively, an option in the
aforementioned classes that causes them to attempt a HTTP/2 upgrade.

Bug Fixes

N/A.

Breaking Changes and Deprecations

N/A.

Status

KEY: S = Skipped, x = complete

• I have read and agreed to the Developer's Certificate of Origin
• Tested
• Benchmarked (optional)
• Documented
• Review ready
• In review
• Merge ready

[Nytelife26]

@mcollina as noted in the "Features" section of this pull request, we have two options.

"HTTP/2 variants of Pool, Client and Agent. Alternatively, an option in the
aforementioned classes that causes them to attempt a HTTP/2 upgrade."

We have only discussed the prior, so I'd like to take this opportunity to ask what the desired approach is before I begin.

[codecov-io]

Codecov Report

Merging #720 (f5427d6) into main (dac0517) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##              main      #720   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           24        24           
  Lines         1854      1854           
=========================================
  Hits          1854      1854           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dac0517...f5427d6. Read the comment docs.

[ronag]

I don't think you need to develop a new Agent or Pool. What we need is either modify existing Client to support both or create a new Http2Client, rename Client to HttpClient and then create another Client that somehow switches between the two as needed.

[mcollina]

I would just focus on creating a Http2Client right now.

The hardest part is figuring out is the upgrade to HTTP/2

[szmarczak]

@mcollina This is pretty easy. All we need to do is just create a TLS connection and check the ALPN protocol. Something like:

if (socket.alpnProtocol === 'h2') {
    const session = http2.connect(origin, {
        createConnection: () => socket
    });
    ...
}

const client = new HttpClient(socket);
...

[Nytelife26]

All we need to do is just create a TLS connection and check the ALPN protocol.

That depends - the protocol is different depending on whether the desired connection is HTTPS or HTTP. As I mentioned, if TLS is present, you must use ALPN first. If not, you have to follow the standard HTTP Upgrade procedure.

[ronag]

I think http2 should always be https? I don’t think we need to support the non TLS case for http2.

[Nytelife26]

I think http2 should always be https? I don’t think we need to support the non TLS case for http2.

That doesn't sit right with me. We shouldn't just ignore usages that are part of the specification itself for our own convenience, especially when it'll likely result in someone filing an issue later on.

[mcollina]

I think http2 should always be https? I don’t think we need to support the non TLS case for http2.

It's needed and used within microservices networks.

[szmarczak]

We shouldn't just ignore usages that are part of the specification itself for our own convenience

undici already does.

I think http2 should always be https? I don’t think we need to support the non TLS case for http2.

I thought the same - browsers don't [support h2c].

It's needed and used within microservices networks.

I believe there are better solutions in these cases. Anyway they can just use the native http2.connect instead.

[Nytelife26]

It's needed and used within microservices networks.

Thank you.

undici already does.

That's not quite the same as dropping an entire protocol. Although, I do trust and hope there are technical reasons for not following those parts too, beyond merely convenience of implementation.

I thought the same - browsers don't [support h2c].

Not yet. Most of the point of HTTP/2 was for browsers so It most likely will.

I believe there are better solutions in these cases. Anyway they can just use the native http2.connect instead.

You could also argue that you could use the native http instead of undici altogether. But the point of undici is that it is faster. So yes, they can use an inferior solution, but I see no reason to not do our best just because other solutions already exist.

[szmarczak]

Most of the point of HTTP/2 was for browsers so It most likely will.

I don't think h2c is coming anytime soon. Even CloudFlare doesn't support it.

you could use the native http instead of undici altogether

That's not quite the same as undici is a complete rewrite, it doesn't use the http module internally. The point is that internally you have to use http2.connect(...) anyway. The API is very friendly so people should use it directly whenever the can.

I'd prefer if undici targeted HTTP/2 with TLS first, but if anybody wants to go after h2c as well - I don't mind.

[mcollina]

FYI: Google Cloud Run supports h2c https://cloud.google.com/run/docs/configuring/http2.

My take is that h2c support might be really handy.

[kanongil]

h2c is super useful for debugging tricky protocol issues using Wireshark, etc.

[Nytelife26]

Summary

"What we need is either modify existing Client to support both or create a new Http2Client, rename Client to HttpClient and then create another Client that somehow switches between the two as needed."

Decision: To start with, I will be implementing the methods on a new Http2Client. It is likely that I will then merge those under an option into Client, given that a client with switching may be hard to maintain and create unnecessary overhead, depending on how complex the Http2Client will be.

The hardest part is figuring out is the upgrade to HTTP/2

Decision:

• For TLS connections, the TLS side must be handled first. Then, we can follow standard procedure.
• The client will either initialize or use the existing HTTP/1.1 connection and send a request with the HTTP Upgrade (value of h2 for HTTPS, h2c for HTTP) and HTTP2-Settings headers.
  • Implementation on capability advertisement via things like ALT-SVC is currently unclear.
• If the response is a HTTP 101, check for responses to prior requests and begin using HTTP/2. Else, do not switch to HTTP/2.

I'd prefer if undici targeted HTTP/2 with TLS first, but if anybody wants to go after h2c as well - I don't mind.

Decision: As aforementioned, TLS support is part of the natural protocol of any HTTP/2 upgrade. It is an extension, not a separate method, and therefore both can be achieved simultaneously.

[Nytelife26]

Does anyone here know about polyfill support for base64url and whether it would be possible to implement it here? The HTTP/2 specification requires that the HTTP2-Settings header be encoded in base64url (without padding), however, it seems to be a recent feature and I am uncertain of how it would affect compatibility if we were to use it here.

[mcollina]

Just use that and we'll polyfill later.

Anyway, @panva do you know what's the best polyfill for that?

[panva]

@mcollina you can take inspiration from https://github.com/panva/jose/blob/main/src/runtime/node/base64url.ts

[Nytelife26]

What's the situation regarding client.upgrade? It's somewhat vital for this to work, however it doesn't seem to be implemented on Client or Dispatcher, despite what the documentation says.

[ronag]

The api methods are assigned to the prototypes in index.js. See, https://github.com/nodejs/undici/blob/main/index.js#L15.

[Nytelife26]

Interesting, thank you, that didn't display in GitHub search weirdly. Peculiar approach, but it makes life easier actually since I can avoid a circular dependency between the client and its http2 variant when they merge.

[Nytelife26]

Can we safely implement a wider Client.upgrade return context? It only provides the headers and socket, but I need the full response body too.

[Nytelife26]

Can we safely implement a wider Client.upgrade return context? It only provides the headers and socket, but I need the full response body too.

cc: @ronag @mcollina

See my above comment when you have the time, please and thank you in advance.

[metcoder95]

Just to double-check and give a life signal 😅
The way to move forward will be using the native http2 module from node?
And if so, any suggestion on how the abstraction/client should look like? :)

[mcollina]

Just to double-check and give a life signal 😅

The way to move forward will be using the native http2 module from node?

Yes.

And if so, any suggestion on how the abstraction/client should look like? :)

I think this was extensively covered in this PR. To be honest, having something that worked would be a good starting point. My suggestion is to start by creating a HTTP2Client.

[metcoder95]

I think @szmarczak is already working on a new PR adding HTTP/2. Happy to take a look at if so, really interested in how does it works 🙂

[Nytelife26]

I think @szmarczak is already working on a new PR adding HTTP/2. Happy to take a look at if so, really interested in how does it works 🙂

As far as I've seen, using node core `http2`, despite our former discussions. Either way, hopefully it'll turn out well.

[simoneb]

Is there any work being done on this?

[Nytelife26]

Is there any work being done on this?

Not to my knowledge, feel free to take over.

[metcoder95]

I'll try to revisit it as soon as I have time, but I think @szmarczak has some progress already

[metcoder95]

Superseded by #2061