Fix a regression in parsing an HTTP-Version string (#677)

* Fix a regression in parsing an HTTP-Version string, pulling the parsing code out into http_version::from_string; add extra unit tests

* Restore EOL: Unix

* Remove dependency on locale from http_version member functions. Prefer UTF-8.

Author: garethsb

Release/include/cpprest/http_msg.h

@@ -58,6 +58,17 @@ struct http_version
     inline bool operator>=(const http_version& other) const { return !(*this < other); }
     inline bool operator>(const http_version& other) const { return !(*this < other || *this == other); }
     inline bool operator<=(const http_version& other) const { return *this < other || *this == other; }
+
+    /// <summary>
+    /// Creates <c>http_version</c> from an HTTP-Version string, "HTTP" "/" 1*DIGIT "." 1*DIGIT.
+    /// </summary>
+    /// <returns>Returns a <c>http_version</c> of {0, 0} if not successful.</returns>
+    static _ASYNCRTIMP http_version __cdecl from_string(const utility::string_t& http_version_string);
+
+    /// <summary>
+    /// Returns the string representation of the <c>http_version</c>.
+    /// </summary>
+    _ASYNCRTIMP std::string to_utf8string() const;
 };
 
 /// <summary>

Release/src/http/common/http_msg.cpp

@@ -254,6 +254,35 @@ void parse_headers_string(_Inout_z_ utf16char *headersStr, http_headers &headers
 
 }
 
+http_version __cdecl http_version::from_string(const utility::string_t& http_version_string)
+{
+    utility::istringstream_t str(http_version_string);
+    str.imbue(std::locale::classic());
+
+    utility::string_t http; std::getline(str, http, _XPLATSTR('/'));
+    unsigned int major = 0; str >> major;
+    utility::char_t dot = _XPLATSTR('\0'); str >> dot;
+    unsigned int minor = 0; str >> minor;
+
+    // check no failure, fully consumed, and correct fixed text
+    if (!str.fail() && str.eof() && _XPLATSTR("HTTP") == http && _XPLATSTR('.') == dot)
+    {
+        return{ (uint8_t)major, (uint8_t)minor };
+    }
+    return{ 0, 0 };
+}
+
+std::string http_version::to_utf8string() const
+{
+    std::string ret;
+    ret.reserve(8);
+    ret.append("HTTP/");
+    ret.append(std::to_string(static_cast<unsigned int>(major)));
+    ret.append(".");
+    ret.append(std::to_string(static_cast<unsigned int>(minor)));
+    return ret;
+}
+
 static const utility::char_t * stream_was_set_explicitly = _XPLATSTR("A stream was set on the message and extraction is not possible");
 static const utility::char_t * unsupported_charset = _XPLATSTR("Charset must be iso-8859-1, utf-8, utf-16, utf-16le, or utf-16be to be extracted.");
 
@@ -1052,4 +1081,4 @@ const http_version http_versions::HTTP_1_1 = { 1, 1 };
 #undef DAT
 #endif
 }} // namespace web::http
-
+

Release/src/http/listener/http_server_asio.cpp

@@ -659,16 +659,8 @@ will_deref_and_erase_t asio_server_connection::handle_http_line(const boost::sys
         std::string http_version = http_path_and_version.substr(http_path_and_version.size() - VersionPortionSize + 1, VersionPortionSize - 2);
 
         auto m_request_impl = m_request._get_impl().get();
-        web::http::http_version parsed_version = { 0, 0 };
-        if (boost::starts_with(http_version, "HTTP/"))
-        {
-            std::istringstream version{ http_version.substr(5) };
-            version >> parsed_version.major;
-            char dot; version >> dot;
-            version >> parsed_version.minor;
-
-            m_request_impl->_set_http_version(parsed_version);
-        }
+        web::http::http_version parsed_version = web::http::http_version::from_string(utility::conversions::to_string_t(http_version));
+        m_request_impl->_set_http_version(parsed_version);
 
         // if HTTP version is 1.0 then disable pipelining
         if (parsed_version == web::http::http_versions::HTTP_1_0)

Release/tests/functional/http/listener/request_handler_tests.cpp

@@ -450,6 +450,29 @@ TEST_FIXTURE(uri_address, test_leaks)
 
 TEST_FIXTURE(uri_address, http_version)
 {
+    // formatting should succeed
+    VERIFY_IS_TRUE("HTTP/0.9" == http_versions::HTTP_0_9.to_utf8string());
+    VERIFY_IS_TRUE("HTTP/1.0" == http_versions::HTTP_1_0.to_utf8string());
+    VERIFY_IS_TRUE("HTTP/1.1" == http_versions::HTTP_1_1.to_utf8string());
+    VERIFY_IS_TRUE("HTTP/12.3" == (http_version{ 12, 3 }).to_utf8string());
+    // parsing should succeed
+    VERIFY_IS_TRUE(http_version::from_string(U("HTTP/0.9")) == http_versions::HTTP_0_9);
+    VERIFY_IS_TRUE(http_version::from_string(U("HTTP/1.0")) == http_versions::HTTP_1_0);
+    VERIFY_IS_TRUE(http_version::from_string(U("HTTP/1.1")) == http_versions::HTTP_1_1);
+    VERIFY_IS_TRUE((http_version::from_string(U("HTTP/12.3")) == http_version{ 12, 3 }));
+    // parsing should fail
+    http_version unknown = { 0, 0 };
+    VERIFY_IS_TRUE(http_version::from_string(U("http/12.3")) == unknown);
+    VERIFY_IS_TRUE(http_version::from_string(U("HTTP/12.3foo")) == unknown);
+    VERIFY_IS_TRUE(http_version::from_string(U("HTTP/12.")) == unknown);
+    VERIFY_IS_TRUE(http_version::from_string(U("HTTP/12")) == unknown);
+    VERIFY_IS_TRUE(http_version::from_string(U("HTTP/.3")) == unknown);
+    VERIFY_IS_TRUE(http_version::from_string(U("HTTP/")) == unknown);
+    VERIFY_IS_TRUE(http_version::from_string(U("HTTP")) == unknown);
+    VERIFY_IS_TRUE(http_version::from_string(U("HTTP")) == unknown);
+    VERIFY_IS_TRUE(http_version::from_string(U("foo")) == unknown);
+    VERIFY_IS_TRUE(http_version::from_string(U("")) == unknown);
+
     http_listener listener(U("http://localhost:45678/path1"));
     listener.open().wait();