Fix a regression in parsing an HTTP-Version string

[garethsb]

Also pulling code out into http_version::from_string; adds some extra unit tests too.

As discussed in commentary on #565.

[ras0219-msft]

Thanks for the PR!

I adjusted the functions to reduce dependency on iostreams and locale. I also changed the to_string() function to be to_utf8string() -- this doesn't prevent a future to_string_t() but helps walk towards UTF-8 everywhere.

[garethsb]

I'm sorry, I don't understand the rationale for the adjustments.

Most of the utility, web::json and other SDK APIs are in terms of utility::char_t, string_t, etc. Why make the change to UTF8 std::string just for this?

I think we'd be better to be consistent and transition all the APIs together if that's where you're going?

Or if you want to stick with to_utf8string, please make from_string match?

[ras0219-msft]

the rationale for the adjustments.

Primarily, this was avoiding adding any more functions where the word "string" refers to anything besides either "std::string" or the overload set of (potentially) all string types. It is a longer term goal to support UTF-8 in all APIs on all platforms (not necessarily to the exclusion of wstring), so any functions with signatures like utility::string_t func_gives_string() need to at least be renamed to func_gives_string_t to minimize confusion and churn in the future (Note that this argument doesn't apply to from_string(std::string) because we can add overloads to handle both string types).

Since I'm not really sure what the concrete use cases are for these APIs (I just don't have any example programs to put them into context), I wanted to default to only adding the UTF-8 overload. However, the rest of the library does often use string to refer to string_t unfortunately, so that would be ambiguous by default.

All the above said, I agree with making from_string match and have pushed that to master.

[garethsb]

Hi @ras0219-msft,

Thanks very much for taking the time to explain.

Primarily, this was avoiding adding any more functions where the word "string" refers to anything besides either "std::string" or the overload set of (potentially) all string types.

Personally, if that's the goal, I would try to get there by making utility::string_t be std::string on all platforms. Those Windows users currently using utility::string_t everywhere would only see problems "at the edges", where their cpprestsdk-facing code interacts with other code, and even then only if they are mistakenly relying onstd::wstring directly rather than e.g. using utility::to_utf16string which would continue to work whether utility::string_t were wstring or string.

All the above said, I agree with making from_string match and have pushed that to master.

Please would you fix https://github.com/Microsoft/cpprestsdk/blob/master/Release/src/http/listener/http_server_asio.cpp#L662 as well?

Thanks.