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

Add httptools failing test for long url #925

Closed
wants to merge 3 commits into from

Conversation

euri10
Copy link
Member

@euri10 euri10 commented Dec 29, 2020

Does nothing for now, just shows the issue
May fix #924

@euri10 euri10 mentioned this pull request Dec 29, 2020
@Kludex
Copy link
Sponsor Member

Kludex commented Jun 1, 2021

@euri10 Can you rebase and see if we still have this issue, please? This should have been solved by httptools==0.2.0.

@euri10
Copy link
Member Author

euri10 commented Jun 1, 2021

it's still failing in parsed_url = httptools.parse_url(url) which returns None everywhere for that length

@euri10
Copy link
Member Author

euri10 commented Jun 1, 2021

kind of normal given the parser accepts url up to https://github.com/nodejs/http-parser/blob/ec8b5ee63f0e51191ea43bb0c6eac7bfbff3141d/http_parser.c#L31 so not sure we should handle that or ask it upstream this is the old parser...

re-edit in fact httptools still does the url parsing with http_parser so using httptools 0.2.0 wont improve things for us, since it doesnt change anything on that front

@euri10
Copy link
Member Author

euri10 commented Jun 7, 2021

closing this
see MagicStack/httptools#68

@euri10 euri10 closed this Jun 7, 2021
@euri10 euri10 deleted the 344_long_url branch June 8, 2021 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Httptools trims long urls
2 participants