Improved error reporting for websocketpp based implementation. Added tests for invalid server certificates with websocket client.

stgates · stgates · commit 06e97009e68e · 2014-10-27T17:15:39.000-07:00
diff --git a/Release/include/cpprest/ws_client.h b/Release/include/cpprest/ws_client.h
@@ -277,9 +277,14 @@ class websocket_exception : public std::exception
     }
 
     /// <summary>
-    /// Destroys the <c>websocket_exception</c> object.
+    /// Creates a <c>websocket_exception</c> from a error code and string message to use as the what() argument.
+    /// <param name="code">Error code.</param>
+    /// <param name="whatArg">Message to use in what() string.</param>
     /// </summary>
-    ~websocket_exception() _noexcept {}
+    websocket_exception(std::error_code code, std::string whatArg) :
+        m_errorCode(std::move(code)),
+        m_msg(std::move(whatArg))
+    {}
 
     /// <summary>
     /// Gets a string identifying the cause of the exception.
diff --git a/Release/src/websockets/client/ws_client.cpp b/Release/src/websockets/client/ws_client.cpp
@@ -49,6 +49,7 @@
 #undef ntohll
 #undef htonll
 #endif
+#define _WEBSOCKETPP_CPP11_SYSTEM_ERROR_
 #include <websocketpp/config/asio_client.hpp>
 #include <websocketpp/config/asio_no_tls_client.hpp>
 #include <websocketpp/client.hpp>
@@ -90,6 +91,17 @@ namespace client
 namespace details
 {
 
+// Utility function to build up error string based on error code and location.
+static std::string build_error_msg(const std::error_code &ec, const std::string &location)
+{
+    std::string msg(location);
+    msg.append(": ");
+    msg.append(std::to_string(ec.value()));
+    msg.append(": ");
+    msg.append(ec.message());
+    return msg;
+}
+
 static utility::string_t g_subProtocolHeader(_XPLATSTR("Sec-WebSocket-Protocol"));
 
 class wspp_client : public _websocket_client_impl, public std::enable_shared_from_this<wspp_client>
@@ -151,6 +163,7 @@ class wspp_client : public _websocket_client_impl, public std::enable_shared_fro
             catch (...) {}
             break;
         }
+
         // We have released the lock on all paths here
         m_service.stop();
         if (m_thread.joinable())
@@ -225,13 +238,17 @@ class wspp_client : public _websocket_client_impl, public std::enable_shared_fro
             m_connect_tce.set();
         });
 
-        client.set_fail_handler([this](websocketpp::connection_hdl)
+        client.set_fail_handler([this](websocketpp::connection_hdl con_hdl)
         {
             _ASSERTE(m_state == CONNECTING);
+            auto &client = m_client->client<WebsocketConfigType>();
+            const auto &ec = client.get_con_from_hdl(con_hdl)->get_ec();
+            websocket_exception exc(ec, build_error_msg(ec, "set_fail_handler"));
+
             std::lock_guard<std::mutex> lock(m_receive_queue_lock);
-            close_pending_tasks_with_error();
+            close_pending_tasks_with_error(exc);
             m_state = CLOSED;
-            m_connect_tce.set_exception(websocket_exception("Connection attempt failed."));
+            m_connect_tce.set_exception(exc);
         });
 
         client.set_message_handler([this](websocketpp::connection_hdl, const websocketpp::config::asio_client::message_type::ptr &msg)
@@ -278,11 +295,15 @@ class wspp_client : public _websocket_client_impl, public std::enable_shared_fro
             }
         });
 
-        client.set_close_handler([this](websocketpp::connection_hdl)
+        client.set_close_handler([this](websocketpp::connection_hdl con_hdl)
         {
-            std::unique_lock<std::mutex> lock(m_receive_queue_lock);
             _ASSERTE(m_state != CLOSED);
-            close_pending_tasks_with_error();
+            auto &client = m_client->client<WebsocketConfigType>();
+            const auto &ec = client.get_con_from_hdl(con_hdl)->get_ec();
+            websocket_exception exc(ec, build_error_msg(ec, "set_close_handler"));
+
+            std::lock_guard<std::mutex> lock(m_receive_queue_lock);
+            close_pending_tasks_with_error(exc);
             m_close_tce.set();
             m_state = CLOSED;
         });
@@ -294,7 +315,7 @@ class wspp_client : public _websocket_client_impl, public std::enable_shared_fro
         m_con = con;
         if (ec.value() != 0)
         {
-            return pplx::task_from_exception<void>(websocket_exception(ec.message()));
+            return pplx::task_from_exception<void>(websocket_exception(ec, build_error_msg(ec, "get_connection")));
         }
 
         // Add any request headers specified by the user.
@@ -316,7 +337,7 @@ class wspp_client : public _websocket_client_impl, public std::enable_shared_fro
                 con->add_subprotocol(utility::conversions::to_utf8string(value), ec);
                 if (ec.value())
                 {
-                    return pplx::task_from_exception<void>(websocket_exception(ec.message()));
+                    return pplx::task_from_exception<void>(websocket_exception(ec, build_error_msg(ec, "add_subprotocol")));
                 }
             }
         }
@@ -328,12 +349,6 @@ class wspp_client : public _websocket_client_impl, public std::enable_shared_fro
         {
             m_service.run();
             _ASSERTE(m_state == CLOSED);
-            {
-                std::unique_lock<std::mutex> lock(m_receive_queue_lock);
-                close_pending_tasks_with_error();
-            }
-            _ASSERTE(m_state == CLOSED);
-            return 0;
         });
 
         return pplx::create_task(m_connect_tce);
@@ -534,10 +549,10 @@ class wspp_client : public _websocket_client_impl, public std::enable_shared_fro
             try
             {
                 // Catch exceptions from previous tasks, if any and convert it to websocket exception.
-                auto ec = previousTask.get();
+                const auto &ec = previousTask.get();
                 if (ec.value() != 0)
                 {
-                    eptr = std::make_exception_ptr(websocket_exception(ec.message()));
+                    eptr = std::make_exception_ptr(websocket_exception(ec, build_error_msg(ec, "sending message")));
                 }
             }
             catch (...)
@@ -612,14 +627,14 @@ class wspp_client : public _websocket_client_impl, public std::enable_shared_fro
     }
 
     // Note: must be called while m_receive_queue_lock is locked.
-    void close_pending_tasks_with_error()
+    void close_pending_tasks_with_error(const websocket_exception &exc)
     {
         while (!m_receive_task_queue.empty())
         {
             // There are tasks waiting to receive a message, signal them
             auto tce = m_receive_task_queue.front();
             m_receive_task_queue.pop();
-            tce.set_exception(std::make_exception_ptr(websocket_exception("Websocket connection has been closed.")));
+            tce.set_exception(std::make_exception_ptr(exc));
         }
     }
 
diff --git a/Release/tests/functional/websockets/client/authentication_tests.cpp b/Release/tests/functional/websockets/client/authentication_tests.cpp
@@ -86,7 +86,7 @@ TEST_FIXTURE(uri_address, auth_with_credentials, "Ignore", "245")
 }
 #endif
 
-TEST_FIXTURE(uri_address, ssl_test)
+TEST(ssl_test)
 {
     websocket_client client;
     client.connect(U("wss://echo.websocket.org/")).wait();
@@ -109,6 +109,40 @@ TEST_FIXTURE(uri_address, ssl_test)
     client.close().wait();
 }
 
+// These tests are specific to our websocketpp based implementation.
+#if !defined(__cplusplus_winrt)
+
+void handshake_error_test_impl(const ::utility::string_t &host)
+{
+    websocket_client client;
+    try
+    {
+        client.connect(host).wait();
+        VERIFY_IS_TRUE(false);
+    }
+    catch (const websocket_exception &e)
+    {
+        VERIFY_ARE_EQUAL("TLS handshake failed", e.error_code().message());
+    }
+}
+
+TEST(self_signed_cert)
+{
+    handshake_error_test_impl(U("wss://www.pcwebshop.co.uk/"));
+}
+
+TEST(hostname_mismatch)
+{
+    handshake_error_test_impl(U("wss://swordsoftruth.com/"));
+}
+
+TEST(cert_expired)
+{
+    handshake_error_test_impl(U("wss://tv.eurosport.com/"));
+}
+
+#endif
+
 } // SUITE(authentication_tests)
 
 }}}}
