Any plan to add URL parser?

[tatsuhiro-t]

Do you have any plan to add URL parser to llhttp? http-parser kindly includes URL parser which is very convenient. Currently llhttp lacks this functionality. I can use URL parser from http-parser, but some macro names conflict, so I have to provide a header file to declare URL parser struct and functions only.

[indutny]

That should be most certainly possible. The best way to do it would probably be through separate compilation target. I'll look into it ASAP.

[indutny]

What is the API that your use case requires?

[tatsuhiro-t]

I use the following API functions to parse URLs:

/* Initialize all http_parser_url members to 0 */
void http_parser_url_init(struct http_parser_url *u);

/* Parse a URL; return nonzero on failure */
int http_parser_parse_url(const char *buf, size_t buflen,
                          int is_connect,
                          struct http_parser_url *u);

[indutny]

Sorry for delay. Do you need full compatibility with http_parser with regards to the fields of http_parser_url, or are there some fields that you don't need at all?

[tatsuhiro-t]

I need all fields which http_parser_url has because original URL cannot be reconstructed from http_parser_url if there are any missing fields.

[derekargueta]

👋 Just wanted to chip in that this would be super helpful to get Envoy migrated to llhttp from http_parser (https://github.com/envoyproxy/envoy/issues/5155). We have some utility URL code that depends on it.

[huming2207]

Any updates for this issue?

This is the only problem for me to migrate http_parser my project to llhttp .

[indutny]

@huming2207 unfortunately there is no update on this. Despite originally being shipped together with http_parser, there is no specific need for URL parser to live in llhttp itself.

It might be interesting to try taking llparse and the necessary boilerplate from llhttp to produce something like llurl for parsing URLS.

[andrew-aladev]

Guys, despite your experience I don't think that this http parser will be widely used. It depends on "llparse" bicycle (that has already been abandoned) and llvm. It is not tested even on 1% of original http parser.

[knopp]

@andrew-aladev, IIRC llhttp is default parser in nodejs since node 12, and it does not depend on llvm.

[bsergean]

@indutny we might try to help here. One question thought as I see 2 paths forward.

1. Should we try to add support directly in llhttp ?
2. Should we make a standalone llurl ?

I found https://github.com/nodejs/llhttp/blob/master/src/llhttp/url.ts / should we use this as a start ? (I noticed it does not seem to support parsing port numbers, but everything else (path, query, scheme, etc...) looks there).

[briangreenery]

I maintain a project that has this same issue. I want to upgrade from http_parser to llhttp to move to maintained code, but I also depend on http_parser_parse_url. Depending on both projects at the same time wouldn't work (without some effort) because the http_method / http_errno and llhttp_method / lhttp_errno enums define duplicate names.

Would it make sense to add url parsing to llhttp? As the previous commenter pointed out, url.ts seems to already do most of this work, so perhaps keeping it within this library instead of duplicating it in an llurl library might be better to avoid duplication? I'm not sure though.

[bsergean]

In our case we ended up depending on both library but it was a bit painful build wise, because of the duplicate symbols in both library.

I think a great option would be to support url parsing in llhttp, but someone has to do it :)

On Oct 13, 2021, at 2:28 PM, Brian Green @.***> wrote:

I maintain a project that has this same issue. I want to upgrade from http_parser to llhttp to move to maintained code, but I also depend on http_parser_parse_url. Depending on both projects at the same time wouldn't work (without some effort) because the http_method / http_errno and llhttp_method / lhttp_errno enums define duplicate names.

Would it make sense to add url parsing to llhttp? As the previous commenter pointed out, url.ts seems to already do most of this work, so perhaps keeping it within this library instead of duplicating it in an llurl library might be better to avoid duplication? I'm not sure though.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/nodejs/llhttp/issues/7#issuecomment-942735193, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC2O6UJYIIVU6C55L47DEYDUGX2ZTANCNFSM4GGCMGVQ. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

[ShogunPanda]

Since no URL support is planned at the moment I'll close this.

[bsergean]

ps: if anyone is blocked here, you can link with libcurl it has url parsing capabilities now -> https://curl.se/libcurl/c/libcurl-url.html

We managed to move to to llhttp from http_parser, and we just used libcurl for url parsing.