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

Fix request parsing (fixes #60) #62

Merged
merged 1 commit into from
Feb 4, 2019

Conversation

saks
Copy link
Contributor

@saks saks commented Feb 4, 2019

While investigating issue #60, I found that at times not the entire request body is read after one stream.read call. I noticed that from time to time code was reading only 123 bytes out of 125. Remaining 2 bytes were making request body JSON impossible to parse which has resulted into the error that I've reported in comments for #60.

I also noticed that current implementation does not allow reading requests that are longer than 1024 bytes. I've tried to address it.

Another issue that I found is that even parser status is httparse::Status::Complete, it does not mean that entire body was read already. It should be fixed in proposed PR as well.

It is tricky to automate testing of scenario when not entire request is read in one take, since it's random. Instead I've added a test that ensures that we can parse request that is longer than 1024 bytes.

@saks saks force-pushed the fix_request_parsing branch from 0a191be to 723b6ff Compare February 4, 2019 04:21
@lipanski lipanski merged commit d750852 into lipanski:master Feb 4, 2019
@lipanski
Copy link
Owner

lipanski commented Feb 4, 2019

@saks this is great, thanks!

Released as 0.15.1

@saks
Copy link
Contributor Author

saks commented Feb 4, 2019

thank you @lipanski!

@saks saks deleted the fix_request_parsing branch February 4, 2019 21:25
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.

2 participants