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

HTTP2/3 Support #399

Closed
wesleytodd opened this issue Sep 4, 2020 · 17 comments · Fixed by #2061
Closed

HTTP2/3 Support #399

wesleytodd opened this issue Sep 4, 2020 · 17 comments · Fixed by #2061
Labels
enhancement New feature or request

Comments

@wesleytodd
Copy link
Member

Is there any interest in adding additional api's for http2/3 support? While I understand this is a new domain for this package to cover, if we want to get folks moving over to what will someday be our unified http server apis, it would be nice to have a corresponding client which can nicely handle all three.

I feel like the current api's are pretty close to being ready for the underlying paradigm change, and it would be an especially nice UX if the client handled the ALPN swap over via the alt-svc headers. I could see this fitting pretty easily in the .dispatch api with just a small addition to support push streams, and then we would have to consider how to map headers which changed or are no longer allowed. Both of those seem pretty simple to reason about.

What other things am I missing here? There must be something. Anyway, just wanted to have a place to have this discussion since I couldn't find an existing issue.

@ronag
Copy link
Member

ronag commented Sep 4, 2020

Is there any interest in adding additional api's for http2/3 support

Maybe. Depends on the scope of changes and work required. I'm not familiar with http2/3 at all and would probably need help/involvement from e.g. @jasnell to sort that out.

Other than the API's I think the backend would have to be totally re-written for http2/3. We would need some logic between the API and what is today the backend so that we can switch between the protocol implementations.

I'm definitely interested in trying http1 over QUIC.

@wesleytodd
Copy link
Member Author

I think http1 over QUIC is the best route for the long term if it works well, it is still up in the air if http2 over QUIC will work. That said, as we move forward with the work on the server side I think it will be difficult for us to drive adoption if we don't also have a well supported client. I haven't read much of the internals for undici, so I was only thinking of the top level api with my comment above. I will try to carve out some time to dig into the implementation details here.

@ronag
Copy link
Member

ronag commented Sep 5, 2020

@wesleytodd: I'm a little unsure if it's even worth spending any time on http2? There is very little legacy out there (unlike http1) and no advantages over http3.

Right now it just feels like something we do to make it "complete" and we as programmers don't like to see an empty slot between 1 and 3 😄 .

What I'm asking is whether it's maybe better to focus our energy on http1 and http3 and not "waste" it on http2. Or am I too radical in this thought?

@wesleytodd
Copy link
Member Author

wesleytodd commented Sep 5, 2020

I completely agree! My thinking is that http2 is similar enough to 3 that we might get it at minimal effort, but I recognize that might be a naive idea. I don't think it should be a focus.

As I spend some time digging into the server side over the next few weeks I will make sure to also carve out some time to understand what needs a client would have and read the source here to see how much work the addition will be. Also, happy to have anyone else take a look as well and collaborate on what a solution would look like.

@szmarczak
Copy link
Member

I agree with @ronag. IMO rewriting HTTP/2 support would be a waste of time, the native implementation is rather new.

@ronag ronag mentioned this issue Jul 1, 2021
@metcoder95 metcoder95 mentioned this issue Jul 6, 2021
7 tasks
@jasonwilliams
Copy link

Not sure if related, I asked the node devs about HTTP/3 support an it sounds like its just getting nodejs/node#38233 reviewed. Im assuming a bulk of the implementation would e here and indici uses some of it (although im unsure).

Would be great to see h3 happen though.

@AlttiRi
Copy link

AlttiRi commented Feb 16, 2022

I think HTTP/2 support should be added by the next Node.js LTS release, in 2 months.

The current Node.js 17 already uses OpenSSL 3.0 and some requests can't bypass Cloudflare DDoS Protection.
I get 403 for such URLs, while when I manually do the same request with http2, I get 200.


http2 example:

const url = "";
const headers = {};
const authority = new URL(url).host; // Simplified case // [ userinfo "@" ] host [ ":" port ]
const path = new URL(url).pathname + new URL(url).search;
const scheme = new URL(url).protocol.slice(0, -1);

import http2 from "http2";
const client = http2.connect(url);
const req = client.request({
    ":authority": authority,
    ":method": "GET",
    ":path": path,
    ":scheme": scheme,
    ...headers
});
req.setEncoding("utf8");

let data = "";
req.on("response", (headers, flags) => {
    for (const name in headers) {
        console.log(`${name}: ${headers[name]}`);
    }
});
req.on("data", chunk => {
    data += chunk;
});
req.on("end", () => {
    // console.log(data); // may be encoded
    client.close();
});
req.end();

By the way, there is the same problem with the latest Python 3.10, which updated OpenSSL to 3.0 too. (In comparison with Python 3.9)

@mcollina
Copy link
Member

Would you like to work on it?

@mohit61
Copy link

mohit61 commented Mar 11, 2022

When can we expect http2 support in undici?

@mcollina
Copy link
Member

When can we expect http2 support in undici?

Would you like to work on it?

@dheeraj1997
Copy link

dheeraj1997 commented Apr 5, 2022

@mcollina I would like to contribute. But can you suggest some good first issue first to start with.

@mcollina
Copy link
Member

mcollina commented Apr 5, 2022

@jasonwilliams
Copy link

jasonwilliams commented Dec 24, 2022

Would you like to work on it?

Hey @mcollina

I appreciate you may want someone to contribute to this but it could be useful to add some context if anyone wants to pick this up. I believe there’s some moving parts which need to happen first. Maybe once people here have a better idea of the issue they can help.

First, is my understanding correct that undici implements the HTTP stack on top of the underlying connections (net and tls). In which case, the h2 implementation on the agent cannot be used here?

For H3, QUIC is being implemented in Node, but does need reviews, I assume undici will build on top of QUIC once it’s in, similar to net and tls?

If anybody wants to see H3 happen I guess it may be best to help @jasnell with his checklist here nodejs/node#44325 (comment) as QUIC would need to be stable first (assuming the above is correct).

@mcollina
Copy link
Member

You can use the existing http2 implementation of Node.js for this. What needs to be done is a Pool/Client combo.

@mattrobenolt
Copy link

Not to detract from support in undici, since I also would like to see it here, in the meantime, there's https://github.com/grantila/fetch-h2 which implements an http2 transport using the builtin http2 libraries.

I strongly suspect to get to http3 is going to require at least landing QUIC support into node core. While possible to just implement the full QUIC + HTTP/3 stack on top of UDP sockets as a standalone package, it seems counter product when there's also work into getting it into core.

@metcoder95
Copy link
Member

Hi @mcollina, @ronag, just to double-check if the plan is the same to use the http2 from the core, or if something has changed since then.

I'll give it another shoot soon, but just want to double-check the plan remains equals 🙂

@mcollina
Copy link
Member

mcollina commented Mar 8, 2023

The plan did not change ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants