Consider switching http-parser to llhttp

[micwoj92]

http-parser is not actively maintained and maintainers suggest switching to llhttp
nodejs/http-parser#522

[neithernut]

The readme says

Port of http_parser to llparse.

So I suppose that the interface is the same, modulo some differences in naming. In this case, switching should be rather easy.

However, allowing the usage of llhttp rather than http_parser as a compile-time choice would be preferable imo, for the sake of both compatibility for existing users and bundling libs. If I'm not mistaken, llparse requires nodejs for parser generation. The libgit2-maintainers (of which I'm not one btw.) may have reservations regarding additional build-time dependency for bundled libs, and checking in generated sources would probably be a no-go as well. Hence, it may make sense to still support using at least a bundled version of http_parser.

[ethomson]

I'm happy with llhttp, I think that the issue is porting nodejs/http-parser#483 over, which I don't mind doing but also not a high priority for me.

[neithernut]

Ugh. Completely forgot about that one.

Maybe we're lucky and they don't have that bug, but looking at their tests it doesn't seem that they covered that specific case, yet. Apparently the url-parser doesn't even distinguish between host and port but just returns the entire host:port-part as "host" and I've not found any code breaking it down, yet. In fact, I've not even found the C API for url parsing (maybe they generate some headers? I somehow doubt it.).

That being said, I merely had a quick look. Maybe I missed something.

[ozbenh]

I'm looking at shipping libgit2 in Amazon Linux 2023 and this dependency on the dead http-parser is a problem for us as well FYI

[ozbenh]

Note: I also noticed that the bundled http-parser differs from the latest (2.9.4) .. would you take a PR to update it or it's not worthwhile at this point ?

[ethomson]

Sure. Please cherry-pick nodejs/http-parser#483 if you do?

I'd also be happy with a move to llhttp? But I don't know how much that helps or how hard it actually is.

[data-man]

PicoHTTPParser is "a tiny, primitive, fast HTTP request/response parser".

[ozbenh]

If I was libgit2 maintainer, I would rather use something actively maintained, which doesn't seem to be the case of PicoHTTPParser (unless it's just very stable). From a distro person perspective, any new dependency on something "out there" that isn't already packaged or actively maintained is another supply chain risk, extra CVE exposure, etc...

[eslerm]

To add a distro security perspective, the last http-parser commit was on Oct 1, 2020 and the repo was archived on Nov 6, 2022. Any vulnerability found since then will not be addressed. These vulnerabilities will affect libgit2 and any software which depends on libgit2.

This is particularly important to Rust security, as libgit2 (and http-parser) is an optional dependency of cargo.

From https://github.com/nodejs/llhttp

Let's face it, http_parser is practically unmaintainable. Even introduction of a single new method results in a significant code churn.

Glad to hear PR discussion 🎉