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

Header parsing assumes whitespace after colon #161

Closed
wants to merge 2 commits into from

Conversation

stewartbright
Copy link
Contributor

RFC7230 (HTTP 1.1) 3.2 "Header fields" specifies that whitespace between colon and value is optional, but http_client_asio::read_headers started reading the value at one after the colon, assuming there would always be exactly one space.

For an HTTP server that doesn't add whitespace between colon and value, cpprestsdk reads an incorrect value and can read a truncated response, or try to read one that is 2^64 in the case where Content-Length has a single digit value.

Examples:

Content-Length:6

  • value is read as an empty string and interpreted as content of 2^64 in length.

Content-Length:26

  • value is read as 6 and truncated response is read.

RFC7230 (HTTP 1.1) 3.2 "Header fields" specifies that whitespace between
colon and value is optional, but http_client_asio::read_headers started
reading the value at one after the colon, assuming there would always be
exactly one space.
Not sure how, but fix line was deleted rather than committed.
@msftclas
Copy link

Hi @stewartbright, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!

This seems like a small (but important) contribution, so no Contribution License Agreement is required at this point. Real humans will now evaluate your PR.

TTYL, MSBOT;

@ras0219-msft
Copy link
Contributor

Thank you for this patch. It has been merged into the development branch.

@stewartbright stewartbright deleted the header-fix branch May 13, 2021 01:04
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.

3 participants