Skip to content

Conversation

garethsb
Copy link
Contributor

This PR follows on from #507 by providing a mechanism to allow the HTTP version of the incoming message to be retrieved in the HTTP listener. (One motivation is to allow construction of access logs in Common Log Format.) A basic unit test is included.

It also changes http_request::get_remote_address() introduced by #507 to http_request::remote_address() for consistency with pre-existing member functions of this class.

It also includes a commit that fixes #545 which has been reported against #507.

@msftclas
Copy link

msftclas commented Oct 19, 2017

CLA assistant check
All CLA requirements met.

@ras0219-msft ras0219-msft merged commit 6397261 into microsoft:master Jan 24, 2018
@ras0219-msft
Copy link
Contributor

Thanks!

I changed http_version to be a full struct instead of just using a pair, for bonus type safety. I also restored the older get_remote_address() to avoid breaking source compatibility (with a deprecation warning).

@garethsb
Copy link
Contributor Author

Thanks for working on these, @ras0219-msft. I think there is one problem with the struct http_version commit. operator>> with uint8_t doesn't do what we want, it reads a character. If I'm right, we need to restore the temporary unsigned ints, around here: 1d35847#diff-d8600089329e519f56170a2d16d7102cR657.

@ras0219-msft
Copy link
Contributor

You are absolutely right. I'll address this tomorrow or I'd be happy to merge a PR.

@garethsb
Copy link
Contributor Author

Perhaps it's worth pulling the parsing code out into a unit testable function?
E.g. add from_string() and to_string() to http_version?
(Or op>> and op<<?)

@garethsb
Copy link
Contributor Author

Ah, the existing unit test is failing when cpprestsdk is configured with CPPREST_HTTP_LISTENER_IMPL as asio, so that's something!

...\tests\functional\http\listener\request_handler_tests.cpp(466): error : Failure in http_version:
CHECK_EQUAL(true, httpVersion == http_versions::HTTP_1_1) where true=1 and httpVersion == http_versions::HTTP_1_1=0

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.

Change to remote address introduced bug that crashes the server if client disconnects
3 participants