undici
undici copied to clipboard
feat: add http2
This relates to...
- #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):
- 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
- [x] I have read and agreed to the Developer's Certificate of Origin
- [ ] Tested
- [ ] Benchmarked (optional)
- [ ] Documented
- [ ] Review ready
- [ ] In review
- [ ] Merge ready
@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 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.
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.
I would just focus on creating a Http2Client
right now.
The hardest part is figuring out is the upgrade to HTTP/2
@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);
...
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.
I think http2 should always be https? I don’t think we need to support the non TLS case for http2.
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.
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.
We shouldn't just ignore usages that are part of the specification itself for our own convenience
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.
It's needed and used within microservices networks.
Thank you.
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.
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.
FYI: Google Cloud Run supports h2c https://cloud.google.com/run/docs/configuring/http2.
My take is that h2c support might be really handy.
h2c is super useful for debugging tricky protocol issues using Wireshark, etc.
Summary
"What we need is either modify existing
Client
to support both or create a newHttp2Client
, renameClient
toHttpClient
and then create anotherClient
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 ofh2
for HTTPS,h2c
for HTTP) andHTTP2-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 afterh2c
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.
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.
Just use that and we'll polyfill later.
Anyway, @panva do you know what's the best polyfill for that?
@mcollina you can take inspiration from https://github.com/panva/jose/blob/main/src/runtime/node/base64url.ts
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.
The api methods are assigned to the prototypes in index.js. See, https://github.com/nodejs/undici/blob/main/index.js#L15.
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.
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.
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.
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.
No idea. As far as I know there is no body after upgrade so I'm not sure what you mean here...
As far as I know there is no body after upgrade
According to the HTTP/2 specification, the server may bundle the response frames to any previous requests along with its upgrade acknowledgement. For that reason, I need to be able to access the response body in order to check for those.
Answers, anyone?
Answers, anyone?
I don’t have a better answer. As far as I know there is no body with an upgrade so your question doesn’t make sense to me. Sorry. Maybe I’m missing something...
Right after sending the 101 status code, the server can begin speaking the new protocol, performing any additional protocol-specific handshakes as necessary. Effectively, the connection becomes a two-way pipe as soon as the upgraded response is complete, and the request that initiated the upgrade can be completed over the new protocol.
Answers, anyone?
I didn't thought it was a question, more of a statement. I'd recommend to try and see if it's possible.
I can not continue to work on this, unfortunately. I have pushed my most recent work and hopefully that in combination with this discussion will serve useful to anyone willing to pick up. I apologise and wish you all well, but I no longer have the time, as I have packages to maintain and college is a heavy weight.