feat: HTTP/2

[metcoder95]

Inherited from #720

This relates to...
HTTP/2-3 #399

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):

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
  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]

Good luck :)

[codecov-commenter]

Codecov Report

Merging #866 (5330889) into main (d31de8c) will decrease coverage by 0.04%.
The diff coverage is 75.00%.

@@             Coverage Diff             @@
##              main     #866      +/-   ##
===========================================
- Coverage   100.00%   99.95%   -0.05%     
===========================================
  Files           26       26              
  Lines         2105     2108       +3     
===========================================
+ Hits          2105     2107       +2     
- Misses           0        1       +1     

Impacted Files	Coverage Δ
lib/client.js	99.87% <75.00%> (-0.13%)	⬇️

---

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 d31de8c...5330889. Read the comment docs.

[metcoder95]

Might sound naive but does anybody know an easy way to test an upgrade from http to http2. As the last one is executed to run under the security layer is not so easy to trigger for h2c upgrades. Otherwise, I'll need to create a small server for it (just looking for some already made solution before making a handmade one)

cc: @ronag @mcollina

[mcollina]

I don't have a clue... maybe @jasnell do.

[szmarczak]

@metcoder95 What security layer are you talking about?

[metcoder95]

@szmarczak I was referring that trying to visit an HTTP2 website forces you to go under TLS, but I'm trying to test the upgrade from HTTP/1.1 to HTTP/2 with h2c upgrade headers

[szmarczak]

If you're running h2c then you don't need TLS

[metcoder95]

Yes sure. I'm aware of that, I was just asking if there's any open server or tool where I can do upgrades by h2c as HTTP/2 by default this runs under the secure version. If not, I'll need to create a small server by myself 😅

[metcoder95]

How the http2client should look like? Exactly equals to Client? Or we can re-use the client implementation but just changing the way we parsed the exchange of data? I think I'm missing the big picture of that

[metcoder95]

Just out of curiosity, after the upgrade the way how the HTTP/2 data exchange is done is quite different from an HTTP/1.1, from this point how do we proceed with the parser, is lhttp supporting it? I'm asking because the upgrade is basically done for h2c, the next step is to continue with the parsing of the different responses/requests (and of course h2).

Sorry for taking some time, I've been having rush days, I'll try to start being more concurrent next week :)

cc: @mcollina @ronag

[ronag]

from this point how do we proceed with the parser, is lhttp supporting it?

You should bypass all of that. llhttp does not support http2.

[metcoder95]

Got it, just to understand once for all, reading at the issue from where this PR comes out, I do agree with all the points stated there (mostly how HTTP/2 is quite more like a transition), then my question is, what and how do we achieve HTTP/2 support?

Besides upgrade which seems straightforward, we should write a parser to accept the full HTTP/2 spec or at least the most basic part?
I'm asking because of how HTTP/2 works, the different frames for headers and body, the support for duplex, headers, etc.

Or maybe use http2 from Node internals?

The full topic is quite interesting but I do agree with the points explained in the issue 😄

[ronag]

You should use http2 from node internals.

[szmarczak]

we should write a parser to accept the full HTTP/2 spec

I don't think we need to write another parser. It's already provided by Node.js, which is a good start. The thing is that when using h2c the server MUST reply with the response - which is kinda hard to convince the native module to accept the HTTP/2 response with the HTTP/1 request.

If I'm missing something, then it may be easier to write another parser. /cc @ronag

HTTP/2 is way more complicated than HTTP/1 so it will take definitely more time than implementing HTTP/1.

[metcoder95]

I don't think we need to write another parser. It's already provided by Node.js, which is a good start. The thing is that when using h2c the server MUST reply with the response - which is kinda hard to convince the native module to accept the HTTP/2 response with the HTTP/1 request.

Actually, for those scenarios, I was asking for. This also includes, e.g. checks for prior requests if an HTTP 101 is response before the upgrade of protocols.

which is kinda hard to convince the native module to accept the HTTP/2 response with the HTTP/1 request.

Hmm, correct me if I'm wrong please but I think an HTTP/1.1 101 will be the response and the next exchange of data should be in the HTTP/2 protocol. Then we can "pipeline" the connection to the native module, or I'm way far?

HTTP/2 is way more complicated than HTTP/1 so it will take definitely more time than implementing HTTP/1.

Agree

[szmarczak]

Then we can "pipeline" the connection to the native module, or I'm way far?

Yeah but you need to accept the response in HTTP/2 somehow without the request in HTTP/2. But the actual request is via HTTP/1. So we need to create a stream that has been already sent and is awaiting response.

[mcollina]

I would recommend we focus on supporting http/2 -> http/2 before working on upgrade paths.

@jasnell have you done any research on implementing the upgrade in nodejs?

[Nytelife26]

have you done any research on implementing the upgrade in nodejs?

All information required to implement the upgrade has been summarized in the original PR, before I handed off the project.
Particularly, this comment should be of interest.

You should use http2 from node internals

Didn't we already discuss that being an idea that is somewhat antithetical to undici's purpose?
To add onto this part of the conversation, it was revealed in an earlier issue that node's native http2 would be a bottleneck given its inferior performance. If we could get a more recent version of that benchmark, that may be prevalent.

[Nytelife26]

It seems like this PR was opened solely to get around not having write permissions for the branch of the other PR. Instead of making a duplicate, if you'd like @metcoder95 I can add you to the branch?

I'm noticing a lot of lost information and repeated discussion due to the conversations from the old PR being neglected, so that's my main concern here.

[szmarczak]

Didn't we already discuss that being an idea that is somewhat antithetical to undici's purpose?

I don't see anyone agreed to write a HTTP/2 parser from scratch.

node's native http2 would be a bottleneck given its inferior performance

I'd rather take that 20% h2 drop and prefer http/1.1 over h2 on ALPN negotiation rather to write a brand new parser for exactly this reason. I don't consider it worth the time.

[Nytelife26]

I don't see anyone agreed to write a HTTP/2 parser from scratch.

That's what was started here. It's necessary to parse HTTP/2 transport frames in order to successfully carry out the upgrade, so we'd have to parse the frames for the upgrade anyway.

I'd rather take that 20% h2 drop and prefer http/1.1 over h2 on ALPN negotiation rather to write a brand new parser for exactly this reason. I don't consider it worth the time.

Once again, we have to do most of it anyway. It wouldn't be too much extra effort to complete it for the performance boost. After all, undici's goal is to provide a fast alternative to native.

[metcoder95]

I would recommend we focus on supporting http/2 -> http/2 before working on upgrade paths.

By using the Node.js internal module? Then a wrapper?

All information required to implement the upgrade has been summarized in the original PR, before I handed off the project.
Particularly, this comment should be of interest.

Thanks! Actually, from the original PR, I'm basing most of the assumptions I'm doing on how the API should look, but the issue comes the way how we need to parse the frames from HTTP/2, which is where the topic gets complicated.

It seems like this PR was opened solely to get around not having write permissions for the branch of the other PR. Instead of making a duplicate, if you'd like @metcoder95 I can add you to the branch?

Yeah, sure, no issue. Is better to merge all the conversations into a single place 👍

---

If I can add my two cents here with my limited knowledge, I think that we should re-defined a little bit the scope of what we want to achieve. It is just to add support for the upgrade with a wrapper around the Node http2 module, I do agree that is the right step to do. The complexity is quite lower (having in mind the complexity of handling two streams at the same time somehow), and we can have the feature in place in a shorter time.
On the other hand, if we want to add full support for HTTP/2, well we should consider what's the state of the protocol towards HTTP/3 as stated in the issue linked to the PR. But if we still want to go in that direction, yeah maybe a parser from scratch will be better as otherwise we'll be just making a wrapper around it http2 (which doesn't have any issue at all but worth it to think if is really needed).
Are just my two cents, hope I'm right in something 😅

[Nytelife26]

I invited you to the repository, so you'll be able to make changes @metcoder95.