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

Httptools trims long urls #924

Closed
euri10 opened this issue Dec 29, 2020 · 5 comments
Closed

Httptools trims long urls #924

euri10 opened this issue Dec 29, 2020 · 5 comments

Comments

@euri10
Copy link
Member

euri10 commented Dec 29, 2020

Prompted as part of the discussion in #344 , it seems httptools doenst handle well long url, see #925 for a failing test

I can say that for my case the issue is not URLs longer than 65,535 characters, but rather that on_url is being called multiple times for whatever reason: https://github.com/nodejs/http-parser#parsing-urls.

Originally posted by @malthejorgensen in #344 (comment)

@victoraugustolls
Copy link

@euri10 There is a pr to move httptools from http-parser tô llhttp. If the bug is in http-parser, this might help!

@euri10
Copy link
Member Author

euri10 commented Dec 29, 2020

Cool, I subscribed to it thanks for the heads up @victoraugustolls !

@Kludex
Copy link
Sponsor Member

Kludex commented May 24, 2021

jfyk, @victoraugustolls PR was merged, and we have a PR using the new httptools version: #1024

@euri10
Copy link
Member Author

euri10 commented Jun 2, 2021

url parsing in httptools is still handled by http-parser and llhttp has a long standing enhancement request to provide a way to do it, so using the new httptools version in fact does not change anything to this issue

@euri10
Copy link
Member Author

euri10 commented Jun 7, 2021

closing this a wontfix since it's an issue upstream > upstream (httptools using http-parse for url parsing)

@euri10 euri10 closed this as completed Jun 7, 2021
@euri10 euri10 added the wontfix label Jun 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants