From 1a900bf8ce0b8863cb76e8580e1c2a4ae6969e10 Mon Sep 17 00:00:00 2001 From: Brennan Conroy Date: Tue, 8 Nov 2022 11:08:14 -0800 Subject: [PATCH 1/2] Validate transfer format on negotiate --- .../case_insensitive_comparison_utils.h | 56 +++++ src/signalrclient/connection_impl.cpp | 33 ++- src/signalrclient/connection_impl.h | 5 +- src/signalrclient/hub_connection_impl.cpp | 9 +- src/signalrclient/json_hub_protocol.cpp | 1 - src/signalrclient/transport.h | 4 +- src/signalrclient/websocket_transport.cpp | 7 +- src/signalrclient/websocket_transport.h | 5 +- ...ase_insensitive_comparison_utils_tests.cpp | 25 +++ test/signalrclienttests/connection_tests.cpp | 194 ++++++++++++------ .../websocket_transport_tests.cpp | 38 ++-- 11 files changed, 278 insertions(+), 99 deletions(-) diff --git a/src/signalrclient/case_insensitive_comparison_utils.h b/src/signalrclient/case_insensitive_comparison_utils.h index 5991c2a7..c0a2fb69 100644 --- a/src/signalrclient/case_insensitive_comparison_utils.h +++ b/src/signalrclient/case_insensitive_comparison_utils.h @@ -31,6 +31,62 @@ namespace signalr return true; } + + bool operator()(const char* s1, const char* s2) const + { + auto length = std::strlen(s1); + if (length != std::strlen(s2)) + { + return false; + } + + for (unsigned i = 0; i < length; ++i) + { + if (std::toupper(s1[i]) != std::toupper(s2[i])) + { + return false; + } + } + + return true; + } + + bool operator()(const std::string& s1, const char* s2) const + { + if (s1.length() != std::strlen(s2)) + { + return false; + } + + for (unsigned i = 0; i < s1.size(); ++i) + { + if (std::toupper(s1[i]) != std::toupper(s2[i])) + { + return false; + } + } + + return true; + } + + bool operator()(const char* s1, const std::string& s2) const + { + auto length = std::strlen(s1); + if (length != s2.length()) + { + return false; + } + + for (unsigned i = 0; i < length; ++i) + { + if (std::toupper(s1[i]) != std::toupper(s2[i])) + { + return false; + } + } + + return true; + } }; struct case_insensitive_hash diff --git a/src/signalrclient/connection_impl.cpp b/src/signalrclient/connection_impl.cpp index 888f969f..73040378 100644 --- a/src/signalrclient/connection_impl.cpp +++ b/src/signalrclient/connection_impl.cpp @@ -130,7 +130,7 @@ namespace signalr std::function mFunc; }; - void connection_impl::start(std::function callback) noexcept + void connection_impl::start(transfer_format transfer_format, std::function callback) noexcept { { std::lock_guard lock(m_stop_lock); @@ -147,6 +147,8 @@ namespace signalr return; } + m_transfer_format = transfer_format; + // there should not be any active transport at this point assert(!m_transport); @@ -306,6 +308,18 @@ namespace signalr start_negotiate_internal(url, 0, transport_started); } + const char* get_transfer_format(transfer_format transfer_format) + { + switch (transfer_format) + { + case transfer_format::text: + return "text"; + case transfer_format::binary: + return "binary"; + } + assert(false); + } + void connection_impl::start_negotiate_internal(const std::string& url, int redirect_count, std::function transport, std::exception_ptr)> transport_started) { if (m_disconnect_cts->is_canceled()) @@ -381,6 +395,15 @@ namespace signalr if (comparer(availableTransport.transport, "WebSockets")) { foundWebsockets = true; + + auto transfer_format = get_transfer_format(connection->m_transfer_format); + if (std::find_if(availableTransport.transfer_formats.begin(), availableTransport.transfer_formats.end(), + [comparer, transfer_format](const std::string& s) { return comparer(s, transfer_format); }) == availableTransport.transfer_formats.end()) + { + transport_started(nullptr, std::make_exception_ptr(signalr_exception(std::string("The server does not support WebSockets with the requested transfer format '").append(transfer_format).append("'.")))); + return; + } + break; } } @@ -391,8 +414,6 @@ namespace signalr return; } - // TODO: use transfer format - if (token->is_canceled()) { transport_started(nullptr, std::make_exception_ptr(canceled_exception())); @@ -499,7 +520,7 @@ namespace signalr auto query_string = "id=" + m_connection_token; auto connect_url = url_builder::build_connect(url, transport->get_transport_type(), query_string); - transport->start(connect_url, [callback, logger](std::exception_ptr exception) + transport->start(connect_url, m_transfer_format, [callback, logger](std::exception_ptr exception) mutable { try { @@ -558,7 +579,7 @@ namespace signalr } } - void connection_impl::send(const std::string& data, transfer_format transfer_format, std::function callback) noexcept + void connection_impl::send(const std::string& data, std::function callback) noexcept { // To prevent an (unlikely) condition where the transport is nulled out after we checked the connection_state // and before sending data we store the pointer in the local variable. In this case `send()` will throw but @@ -581,7 +602,7 @@ namespace signalr logger.log(trace_level::info, std::string("sending data: ").append(data)); } - transport->send(data, transfer_format, [logger, callback](std::exception_ptr exception) + transport->send(data, [logger, callback](std::exception_ptr exception) mutable { try { diff --git a/src/signalrclient/connection_impl.h b/src/signalrclient/connection_impl.h index 355670f3..915693ed 100644 --- a/src/signalrclient/connection_impl.h +++ b/src/signalrclient/connection_impl.h @@ -36,8 +36,8 @@ namespace signalr ~connection_impl(); - void start(std::function callback) noexcept; - void send(const std::string &data, transfer_format transfer_format, std::function callback) noexcept; + void start(transfer_format transfer_format, std::function callback) noexcept; + void send(const std::string &data, std::function callback) noexcept; void stop(std::function callback, std::exception_ptr exception) noexcept; connection_state get_connection_state() const noexcept; @@ -60,6 +60,7 @@ namespace signalr std::function m_message_received; std::function m_disconnected; signalr_client_config m_signalr_client_config; + transfer_format m_transfer_format; std::shared_ptr m_disconnect_cts; std::mutex m_stop_lock; diff --git a/src/signalrclient/hub_connection_impl.cpp b/src/signalrclient/hub_connection_impl.cpp index 935eba66..d62b9520 100644 --- a/src/signalrclient/hub_connection_impl.cpp +++ b/src/signalrclient/hub_connection_impl.cpp @@ -125,7 +125,8 @@ namespace signalr m_disconnect_cts = std::make_shared(); m_handshakeReceived = false; std::weak_ptr weak_connection = shared_from_this(); - m_connection->start([weak_connection, callback](std::exception_ptr start_exception) + auto transfer_format = m_protocol->transfer_format(); + m_connection->start(transfer_format, [weak_connection, callback](std::exception_ptr start_exception) { auto connection = weak_connection.lock(); if (!connection) @@ -242,7 +243,7 @@ namespace signalr return true; }); - connection->m_connection->send(handshake_request, connection->m_protocol->transfer_format(), + connection->m_connection->send(handshake_request, [handle_handshake, handshake_request_done, handshake_request_lock](std::exception_ptr exception) { { @@ -481,7 +482,7 @@ namespace signalr // weak_ptr prevents a circular dependency leading to memory leak and other problems auto weak_hub_connection = std::weak_ptr(shared_from_this()); - m_connection->send(message, m_protocol->transfer_format(), [set_completion, set_exception, weak_hub_connection, callback_id](std::exception_ptr exception) + m_connection->send(message, [set_completion, set_exception, weak_hub_connection, callback_id](std::exception_ptr exception) { if (exception) { @@ -572,7 +573,7 @@ namespace signalr std::weak_ptr weak_connection = connection; connection->m_connection->send( connection->m_cached_ping, - connection->m_protocol->transfer_format(), [weak_connection](std::exception_ptr exception) + [weak_connection](std::exception_ptr exception) { auto connection = weak_connection.lock(); if (connection) diff --git a/src/signalrclient/json_hub_protocol.cpp b/src/signalrclient/json_hub_protocol.cpp index 52280eb4..23f0fb26 100644 --- a/src/signalrclient/json_hub_protocol.cpp +++ b/src/signalrclient/json_hub_protocol.cpp @@ -219,7 +219,6 @@ namespace signalr // TODO: other message types default: // Future protocol changes can add message types, old clients can ignore them - // TODO: null break; } #pragma warning (pop) diff --git a/src/signalrclient/transport.h b/src/signalrclient/transport.h index a385e8ba..b10c427c 100644 --- a/src/signalrclient/transport.h +++ b/src/signalrclient/transport.h @@ -17,11 +17,11 @@ namespace signalr virtual ~transport(); - virtual void start(const std::string& url, std::function callback) noexcept = 0; + virtual void start(const std::string& url, transfer_format transfer_format, std::function callback) noexcept = 0; virtual void stop(std::function callback) noexcept = 0; virtual void on_close(std::function callback) = 0; - virtual void send(const std::string& payload, signalr::transfer_format transfer_format, std::function callback) noexcept = 0; + virtual void send(const std::string& payload, std::function callback) noexcept = 0; virtual void on_receive(std::function callback) = 0; diff --git a/src/signalrclient/websocket_transport.cpp b/src/signalrclient/websocket_transport.cpp index 9231671c..9bbcde91 100644 --- a/src/signalrclient/websocket_transport.cpp +++ b/src/signalrclient/websocket_transport.cpp @@ -195,7 +195,7 @@ namespace signalr } } - void websocket_transport::start(const std::string& url, std::function callback) noexcept + void websocket_transport::start(const std::string& url, transfer_format transfer_format, std::function callback) noexcept { signalr::uri uri(url); assert(uri.scheme() == "ws" || uri.scheme() == "wss"); @@ -213,6 +213,7 @@ namespace signalr std::string("[websocket transport] connecting to: ") .append(url)); + m_transfer_format = transfer_format; auto websocket_client = m_websocket_client_factory(m_signalr_client_config); { @@ -327,9 +328,9 @@ namespace signalr m_process_response_callback = callback; } - void websocket_transport::send(const std::string& payload, transfer_format transfer_format, std::function callback) noexcept + void websocket_transport::send(const std::string& payload, std::function callback) noexcept { - safe_get_websocket_client()->send(payload, transfer_format, [callback](std::exception_ptr exception) + safe_get_websocket_client()->send(payload, m_transfer_format, [callback](std::exception_ptr exception) { if (exception != nullptr) { diff --git a/src/signalrclient/websocket_transport.h b/src/signalrclient/websocket_transport.h index c1947ad0..cd3d3603 100644 --- a/src/signalrclient/websocket_transport.h +++ b/src/signalrclient/websocket_transport.h @@ -25,11 +25,11 @@ namespace signalr transport_type get_transport_type() const noexcept override; - void start(const std::string& url, std::function callback) noexcept override; + void start(const std::string& url, transfer_format transfer_format, std::function callback) noexcept override; void stop(std::function callback) noexcept override; void on_close(std::function callback) override; - void send(const std::string& payload, transfer_format transfer_format, std::function callback) noexcept override; + void send(const std::string& payload, std::function callback) noexcept override; void on_receive(std::function) override; @@ -47,6 +47,7 @@ namespace signalr bool m_disconnected; std::shared_ptr m_receive_loop_task; + transfer_format m_transfer_format; void receive_loop(); diff --git a/test/signalrclienttests/case_insensitive_comparison_utils_tests.cpp b/test/signalrclienttests/case_insensitive_comparison_utils_tests.cpp index edf5b9a4..1576c2da 100644 --- a/test/signalrclienttests/case_insensitive_comparison_utils_tests.cpp +++ b/test/signalrclienttests/case_insensitive_comparison_utils_tests.cpp @@ -11,12 +11,37 @@ TEST(case_insensitive_equals_functor, basic_comparison_tests) { case_insensitive_equals case_insensitive_compare; + // (const char *, const char *) ASSERT_TRUE(case_insensitive_compare("", "")); ASSERT_TRUE(case_insensitive_compare("abc", "ABC")); ASSERT_TRUE(case_insensitive_compare("abc123!@", "ABC123!@")); ASSERT_FALSE(case_insensitive_compare("abc", "ABCD")); ASSERT_FALSE(case_insensitive_compare("abce", "ABCD")); + + // (std::string, std::string) + ASSERT_TRUE(case_insensitive_compare(std::string(""), std::string(""))); + ASSERT_TRUE(case_insensitive_compare(std::string("abc"), std::string("ABC"))); + ASSERT_TRUE(case_insensitive_compare(std::string("abc123!@"), std::string("ABC123!@"))); + + ASSERT_FALSE(case_insensitive_compare(std::string("abc"), std::string("ABCD"))); + ASSERT_FALSE(case_insensitive_compare(std::string("abce"), std::string("ABCD"))); + + // (std::string, const char *) + ASSERT_TRUE(case_insensitive_compare(std::string(""), "")); + ASSERT_TRUE(case_insensitive_compare(std::string("abc"), "ABC")); + ASSERT_TRUE(case_insensitive_compare(std::string("abc123!@"), "ABC123!@")); + + ASSERT_FALSE(case_insensitive_compare(std::string("abc"), "ABCD")); + ASSERT_FALSE(case_insensitive_compare(std::string("abce"), "ABCD")); + + // (const char *, std::string) + ASSERT_TRUE(case_insensitive_compare("", std::string(""))); + ASSERT_TRUE(case_insensitive_compare("abc", std::string("ABC"))); + ASSERT_TRUE(case_insensitive_compare("abc123!@", std::string("ABC123!@"))); + + ASSERT_FALSE(case_insensitive_compare("abc", std::string("ABCD"))); + ASSERT_FALSE(case_insensitive_compare("abce", std::string("ABCD"))); } TEST(case_insensitive_hash_functor, basic_hash_tests) diff --git a/test/signalrclienttests/connection_tests.cpp b/test/signalrclienttests/connection_tests.cpp index 16c06d89..6a5f9969 100644 --- a/test/signalrclienttests/connection_tests.cpp +++ b/test/signalrclienttests/connection_tests.cpp @@ -38,7 +38,7 @@ TEST(connection_impl_start, cannot_start_non_disconnected_exception) auto connection = create_connection(); auto mre = manual_reset_event(); - connection->start([&mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -47,7 +47,7 @@ TEST(connection_impl_start, cannot_start_non_disconnected_exception) try { - connection->start([&mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -76,7 +76,7 @@ TEST(connection_impl_start, connection_state_is_connecting_when_connection_is_be auto connection = create_connection(websocket_client, writer, trace_level::error); auto mre = manual_reset_event(); - connection->start([&mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -99,7 +99,7 @@ TEST(connection_impl_start, connection_state_is_connected_when_connection_establ { auto connection = create_connection(); auto mre = manual_reset_event(); - connection->start([&mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -122,7 +122,7 @@ TEST(connection_impl_start, connection_state_is_disconnected_when_connection_can }, nullptr); auto mre = manual_reset_event(); - connection->start([&mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -147,7 +147,7 @@ TEST(connection_impl_start, throws_for_invalid_uri) [websocket_client](const signalr_client_config&) { return websocket_client; }); auto mre = manual_reset_event(); - connection->start([&mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -186,7 +186,7 @@ TEST(connection_impl_start, start_sets_id_query_string) }); auto mre = manual_reset_event(); - connection->start([&mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -225,7 +225,7 @@ TEST(connection_impl_start, start_appends_id_query_string) }); auto mre = manual_reset_event(); - connection->start([&mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -260,7 +260,7 @@ TEST(connection_impl_start, start_logs_exceptions) }, nullptr); auto mre = manual_reset_event(); - connection->start([&mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -295,7 +295,7 @@ TEST(connection_impl_start, start_propagates_exceptions_from_negotiate) }, [websocket_client](const signalr_client_config&) { return websocket_client; }); auto mre = manual_reset_event(); - connection->start([&mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -325,7 +325,7 @@ TEST(connection_impl_start, start_fails_if_transport_connect_throws) auto connection = create_connection(websocket_client, writer, trace_level::error); auto mre = manual_reset_event(); - connection->start([&mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -359,14 +359,14 @@ TEST(connection_impl_send, send_fails_if_transport_fails_when_receiving_messages auto connection = create_connection(websocket_client, writer, trace_level::error); auto mre = manual_reset_event(); - connection->start([&mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); mre.get(); - connection->send("message", transfer_format::text, [&mre](std::exception_ptr exception) + connection->send("message", [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -405,7 +405,7 @@ TEST(connection_impl_start, start_fails_if_negotiate_request_fails) }, [websocket_client](const signalr_client_config&) { return websocket_client; }); auto mre = manual_reset_event(); - connection->start([&mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -451,7 +451,7 @@ TEST(connection_impl_start, start_fails_if_negotiate_response_has_error) }, [websocket_client](const signalr_client_config&) { return websocket_client; }); auto mre = manual_reset_event(); - connection->start([&mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -493,7 +493,7 @@ TEST(connection_impl_start, start_fails_if_negotiate_response_does_not_have_webs }, [websocket_client](const signalr_client_config&) { return websocket_client; }); auto mre = manual_reset_event(); - connection->start([&mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -533,7 +533,7 @@ TEST(connection_impl_start, start_fails_if_negotiate_response_does_not_have_tran }, [websocket_client](const signalr_client_config&) { return websocket_client; }); auto mre = manual_reset_event(); - connection->start([&mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -573,7 +573,7 @@ TEST(connection_impl_start, start_fails_if_negotiate_response_is_invalid) }, [websocket_client](const signalr_client_config&) { return websocket_client; }); auto mre = manual_reset_event(); - connection->start([&mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -630,7 +630,7 @@ TEST(connection_impl_start, negotiate_follows_redirect) }); auto mre = manual_reset_event(); - connection->start([&mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -686,7 +686,7 @@ TEST(connection_impl_start, negotiate_redirect_uses_accessToken) }); auto mre = manual_reset_event(); - connection->start([&mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -723,7 +723,7 @@ TEST(connection_impl_start, negotiate_fails_after_too_many_redirects) }, [websocket_client](const signalr_client_config&) { return websocket_client; }); auto mre = manual_reset_event(); - connection->start([&mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -764,7 +764,7 @@ TEST(connection_impl_start, negotiate_fails_if_ProtocolVersion_in_response) }, [websocket_client](const signalr_client_config&) { return websocket_client; }); auto mre = manual_reset_event(); - connection->start([&mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -818,7 +818,7 @@ TEST(connection_impl_start, negotiate_redirect_does_not_overwrite_url) }); auto mre = manual_reset_event(); - connection->start([&mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -831,7 +831,7 @@ TEST(connection_impl_start, negotiate_redirect_does_not_overwrite_url) }, nullptr); mre.get(); - connection->start([&mre](std::exception_ptr) + connection->start(transfer_format::text, [&mre](std::exception_ptr) { mre.set(); }); @@ -882,7 +882,7 @@ TEST(connection_impl_start, negotiate_redirect_uses_own_query_string) }); auto mre = manual_reset_event(); - connection->start([&mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -937,7 +937,7 @@ TEST(connection_impl_start, negotiate_with_negotiateVersion_uses_connectionToken }); auto mre = manual_reset_event(); - connection->start([&mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -990,7 +990,7 @@ TEST(connection_impl_start, correct_connection_id_returned_with_negotiateVersion }); auto mre = manual_reset_event(); - connection->start([&mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -1033,7 +1033,7 @@ TEST(connection_impl_start, negotiate_can_be_skipped) }, true); auto mre = manual_reset_event(); - connection->start([&mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -1045,6 +1045,80 @@ TEST(connection_impl_start, negotiate_can_be_skipped) ASSERT_FALSE(negotiate_called); } +TEST(connection_impl_start, start_fails_if_negotiate_response_does_not_have_request_transfer_format) +{ + std::shared_ptr writer(std::make_shared()); + + auto http_client = std::shared_ptr(new test_http_client([](const std::string& url, http_request, cancellation_token) + { + auto response_body = + url.find("/negotiate") != std::string::npos + ? "{ \"availableTransports\": [ { \"transport\": \"WebSockets\", \"transferFormats\": [ \"Text\" ] } ] }" + : ""; + + return http_response{ 200, response_body }; + })); + + auto websocket_client = std::make_shared(); + + auto connection = + connection_impl::create(create_uri(), trace_level::info, writer, + [http_client](const signalr_client_config& config) { + http_client->set_scheduler(config.get_scheduler()); + return http_client; + }, [websocket_client](const signalr_client_config&) { return websocket_client; }); + + auto mre = manual_reset_event(); + connection->start(transfer_format::binary, [&mre](std::exception_ptr exception) + { + mre.set(exception); + }); + + try + { + mre.get(); + ASSERT_TRUE(false); // exception not thrown + } + catch (const signalr_exception& e) + { + ASSERT_STREQ("The server does not support WebSockets with the requested transfer format 'binary'.", e.what()); + } +} + +TEST(connection_impl_start, start_works_if_negotiate_response_has_requested_transfer_format) +{ + std::shared_ptr writer(std::make_shared()); + + auto http_client = std::shared_ptr(new test_http_client([](const std::string& url, http_request, cancellation_token) + { + auto response_body = + url.find("/negotiate") != std::string::npos + ? "{ \"availableTransports\": [ { \"transport\": \"WebSockets\", \"transferFormats\": [ \"Binary\" ] } ] }" + : ""; + + return http_response{ 200, response_body }; + })); + + auto websocket_client = std::make_shared(); + + auto connection = + connection_impl::create(create_uri(), trace_level::info, writer, + [http_client](const signalr_client_config& config) { + http_client->set_scheduler(config.get_scheduler()); + return http_client; + }, [websocket_client](const signalr_client_config& config) { + websocket_client->set_config(config); + return websocket_client; }); + + auto mre = manual_reset_event(); + connection->start(transfer_format::binary, [&mre](std::exception_ptr exception) + { + mre.set(exception); + }); + + mre.get(); +} + TEST(connection_impl_process_response, process_response_logs_messages) { std::shared_ptr writer(std::make_shared()); @@ -1059,7 +1133,7 @@ TEST(connection_impl_process_response, process_response_logs_messages) }); auto mre = manual_reset_event(); - connection->start([&mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -1093,14 +1167,14 @@ TEST(connection_impl_send, message_sent) const std::string message{ "Test message" }; auto mre = manual_reset_event(); - connection->start([&mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); mre.get(); - connection->send(message, transfer_format::text, [&mre](std::exception_ptr exception) + connection->send(message, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -1116,7 +1190,7 @@ TEST(connection_impl_send, send_throws_if_connection_not_connected) create_connection(create_test_websocket_client(), std::make_shared(), trace_level::none); auto mre = manual_reset_event(); - connection->send("whatever", transfer_format::text, [&mre](std::exception_ptr exception) + connection->send("whatever", [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -1144,14 +1218,14 @@ TEST(connection_impl_send, exceptions_from_send_logged_and_propagated) auto connection = create_connection(websocket_client, writer, trace_level::error); auto mre = manual_reset_event(); - connection->start([&mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); mre.get(); - connection->send("Test message", transfer_format::text, [&mre](std::exception_ptr exception) + connection->send("Test message", [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -1196,7 +1270,7 @@ TEST(connection_impl_set_message_received, callback_invoked_when_message_receive }); auto mre = manual_reset_event(); - connection->start([&mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -1233,7 +1307,7 @@ TEST(connection_impl_set_message_received, exception_from_callback_caught_and_lo }); auto mre = manual_reset_event(); - connection->start([&mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -1273,7 +1347,7 @@ TEST(connection_impl_set_message_received, non_std_exception_from_callback_caugh }); auto mre = manual_reset_event(); - connection->start([&mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -1297,7 +1371,7 @@ void can_be_set_only_in_disconnected_state(std::function auto connection = create_connection(websocket_client); auto mre = manual_reset_event(); - connection->start([&mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -1368,7 +1442,7 @@ TEST(connection_impl_stop, stopping_disconnecting_connection_returns_canceled_ta auto connection = create_connection(websocket_client, writer, trace_level::verbose); auto mre = manual_reset_event(); - connection->start([&mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -1416,7 +1490,7 @@ TEST(connection_impl_stop, can_start_and_stop_connection) auto connection = create_connection(websocket_client, writer, trace_level::verbose); auto mre = manual_reset_event(); - connection->start([&mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -1447,7 +1521,7 @@ TEST(connection_impl_stop, can_start_and_stop_connection_multiple_times) auto connection = create_connection(websocket_client, writer, trace_level::verbose); auto mre = manual_reset_event(); - connection->start([&mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -1461,7 +1535,7 @@ TEST(connection_impl_stop, can_start_and_stop_connection_multiple_times) mre.get(); - connection->start([&mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -1506,7 +1580,7 @@ TEST(connection_impl_stop, dtor_stops_the_connection) auto connection = create_connection(websocket_client, writer, trace_level::verbose); auto mre = manual_reset_event(); - connection->start([&mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -1574,7 +1648,7 @@ TEST(connection_impl_stop, stop_cancels_ongoing_start_request) }); auto mre = manual_reset_event(); - connection->start([&mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -1644,7 +1718,7 @@ TEST(connection_impl_stop, ongoing_start_request_canceled_if_connection_stopped_ }); auto mre = manual_reset_event(); - connection->start([&mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -1693,7 +1767,7 @@ TEST(connection_impl_stop, stop_invokes_disconnected_callback) connection->set_disconnected([&disconnected_invoked](std::exception_ptr) { disconnected_invoked = true; }); auto mre = manual_reset_event(); - connection->start([&mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -1720,7 +1794,7 @@ TEST(connection_impl_stop, std_exception_from_disconnected_callback_caught_and_l connection->set_disconnected([](std::exception_ptr) { throw std::runtime_error("exception from disconnected"); }); auto mre = manual_reset_event(); - connection->start([&mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -1749,7 +1823,7 @@ TEST(connection_impl_stop, exception_from_disconnected_callback_caught_and_logge connection->set_disconnected([](std::exception_ptr) { throw 42; }); auto mre = manual_reset_event(); - connection->start([&mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -1777,7 +1851,7 @@ TEST(connection_impl_stop, transport_error_invokes_disconnected_callback) connection->set_disconnected([&disconnect_mre](std::exception_ptr ex) { disconnect_mre.set(ex); }); auto mre = manual_reset_event(); - connection->start([&mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -1843,7 +1917,7 @@ TEST(connection_impl_config, custom_headers_set_in_requests) connection->set_client_config(signalr_client_config); auto mre = manual_reset_event(); - connection->start([&mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -1877,7 +1951,7 @@ TEST(connection_impl_change_state, change_state_logs) auto connection = create_connection(websocket_client, writer, trace_level::verbose); auto mre = manual_reset_event(); - connection->start([&mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -1905,7 +1979,7 @@ TEST(connection_id, connection_id_is_set_if_start_fails_but_negotiate_request_su ASSERT_EQ("", connection->get_connection_id()); auto mre = manual_reset_event(); - connection->start([&mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -1932,7 +2006,7 @@ TEST(connection_id, can_get_connection_id_when_connection_in_connected_state) std::string connection_id; auto mre = manual_reset_event(); - connection->start([&mre, &connection_id, connection](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre, &connection_id, connection](std::exception_ptr exception) { connection_id = connection->get_connection_id(); mre.set(exception); @@ -1958,7 +2032,7 @@ TEST(connection_id, can_get_connection_id_after_connection_has_stopped) auto connection = create_connection(websocket_client, writer, trace_level::verbose); auto mre = manual_reset_event(); - connection->start([&mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -2010,7 +2084,7 @@ TEST(connection_id, connection_id_reset_when_starting_connection) }); auto mre = manual_reset_event(); - connection->start([&mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -2028,7 +2102,7 @@ TEST(connection_id, connection_id_reset_when_starting_connection) fail_http_requests = true; - connection->start([&mre](std::exception_ptr) + connection->start(transfer_format::text, [&mre](std::exception_ptr) { mre.set(); }); @@ -2073,7 +2147,7 @@ TEST(connection_impl_stop, triggers_http_send_token) }); auto start_mre = manual_reset_event(); - connection->start([&start_mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&start_mre](std::exception_ptr exception) { start_mre.set(exception); }); @@ -2136,7 +2210,7 @@ TEST(connection_impl_stop, http_client_throws_from_registered_token) }); auto start_mre = manual_reset_event(); - connection->start([&start_mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&start_mre](std::exception_ptr exception) { start_mre.set(exception); }); @@ -2226,7 +2300,7 @@ TEST(connection_impl_stop, works_with_stop_callback_capturing_websocket_client_w }); auto start_mre = manual_reset_event(); - connection->start([&start_mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&start_mre](std::exception_ptr exception) { start_mre.set(exception); }); @@ -2262,7 +2336,7 @@ TEST(connection_impl_stop, works_with_stop_callback_capturing_websocket_client) }); auto start_mre = manual_reset_event(); - connection->start([&start_mre](std::exception_ptr exception) + connection->start(transfer_format::text, [&start_mre](std::exception_ptr exception) { start_mre.set(exception); }); diff --git a/test/signalrclienttests/websocket_transport_tests.cpp b/test/signalrclienttests/websocket_transport_tests.cpp index 20f40cc6..ed3ad9de 100644 --- a/test/signalrclienttests/websocket_transport_tests.cpp +++ b/test/signalrclienttests/websocket_transport_tests.cpp @@ -34,7 +34,7 @@ TEST(websocket_transport_connect, connect_connects_and_starts_receive_loop) }, signalr_client_config{}, logger(writer, trace_level::info)); auto mre = manual_reset_event(); - ws_transport->start("ws://fakeuri.org/connect?param=42", [&mre](std::exception_ptr exception) + ws_transport->start("ws://fakeuri.org/connect?param=42", transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -74,7 +74,7 @@ TEST(websocket_transport_connect, connect_propagates_exceptions) try { auto mre = manual_reset_event(); - ws_transport->start("ws://fakeuri.org", [&mre](std::exception_ptr exception) + ws_transport->start("ws://fakeuri.org", transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -103,7 +103,7 @@ TEST(websocket_transport_connect, connect_logs_exceptions) }, signalr_client_config{}, logger(writer, trace_level::error)); auto mre = manual_reset_event(); - ws_transport->start("ws://fakeuri.org", [&mre](std::exception_ptr exception) + ws_transport->start("ws://fakeuri.org", transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -134,7 +134,7 @@ TEST(websocket_transport_connect, cannot_call_connect_on_already_connected_trans }, signalr_client_config{}, logger(std::make_shared(), trace_level::none)); auto mre = manual_reset_event(); - ws_transport->start("ws://fakeuri.org", [&mre](std::exception_ptr exception) + ws_transport->start("ws://fakeuri.org", transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -142,7 +142,7 @@ TEST(websocket_transport_connect, cannot_call_connect_on_already_connected_trans try { - ws_transport->start("ws://fakeuri.org", [&mre](std::exception_ptr exception) + ws_transport->start("ws://fakeuri.org", transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -171,7 +171,7 @@ TEST(websocket_transport_connect, can_connect_after_disconnecting) }, signalr_client_config{}, logger(std::make_shared(), trace_level::none)); auto mre = manual_reset_event(); - ws_transport->start("ws://fakeuri.org", [&mre](std::exception_ptr exception) + ws_transport->start("ws://fakeuri.org", transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -183,7 +183,7 @@ TEST(websocket_transport_connect, can_connect_after_disconnecting) }); mre.get(); - ws_transport->start("ws://fakeuri.org", [&mre](std::exception_ptr exception) + ws_transport->start("ws://fakeuri.org", transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -215,13 +215,13 @@ TEST(websocket_transport_send, send_creates_and_sends_websocket_messages) }, signalr_client_config{}, logger(std::make_shared(), trace_level::none)); auto mre = manual_reset_event(); - ws_transport->start("ws://url", [&mre](std::exception_ptr exception) + ws_transport->start("ws://url", transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); mre.get(); - ws_transport->send("ABC", transfer_format::text, [&mre](std::exception_ptr exception) + ws_transport->send("ABC", [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -255,7 +255,7 @@ TEST(websocket_transport_disconnect, disconnect_closes_websocket) }, signalr_client_config{}, logger(std::make_shared(), trace_level::none)); auto mre = manual_reset_event(); - ws_transport->start("ws://url", [&mre](std::exception_ptr exception) + ws_transport->start("ws://url", transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -288,7 +288,7 @@ TEST(websocket_transport_stop, propogates_exception_from_close) }, signalr_client_config{}, logger(std::make_shared(), trace_level::none)); auto mre = manual_reset_event(); - ws_transport->start("ws://url", [&mre](std::exception_ptr exception) + ws_transport->start("ws://url", transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -325,7 +325,7 @@ TEST(websocket_transport_disconnect, disconnect_logs_exceptions) }, signalr_client_config{}, logger(writer, trace_level::error)); auto mre = manual_reset_event(); - ws_transport->start("ws://url", [&mre](std::exception_ptr exception) + ws_transport->start("ws://url", transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -374,7 +374,7 @@ TEST(websocket_transport_disconnect, receive_not_called_after_disconnect) }, signalr_client_config{}, logger(std::make_shared(), trace_level::none)); auto mre = manual_reset_event(); - ws_transport->start("ws://fakeuri.org", [&mre](std::exception_ptr) + ws_transport->start("ws://fakeuri.org", transfer_format::text, [&mre](std::exception_ptr) { mre.set(); }); @@ -388,7 +388,7 @@ TEST(websocket_transport_disconnect, receive_not_called_after_disconnect) }); mre.get(); - ws_transport->start("ws://fakeuri.org", [&mre](std::exception_ptr exception) + ws_transport->start("ws://fakeuri.org", transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -456,7 +456,7 @@ void receive_loop_logs_exception_runner(const T& e, const std::string& expected_ }, signalr_client_config{}, logger(writer, trace_level)); auto mre = manual_reset_event(); - ws_transport->start("ws://url", [&mre](std::exception_ptr exception) + ws_transport->start("ws://url", transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -511,7 +511,7 @@ TEST(websocket_transport_receive_loop, process_response_callback_called_when_mes ws_transport->on_receive(process_response); auto mre = manual_reset_event(); - ws_transport->start("ws://fakeuri.org", [&mre](std::exception_ptr exception) + ws_transport->start("ws://fakeuri.org", transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -565,7 +565,7 @@ TEST(websocket_transport_receive_loop, error_callback_called_when_exception_thro ws_transport->on_close(error_callback); auto mre = manual_reset_event(); - ws_transport->start("ws://fakeuri.org", [&mre](std::exception_ptr exception) + ws_transport->start("ws://fakeuri.org", transfer_format::text, [&mre](std::exception_ptr exception) { mre.set(exception); }); @@ -647,7 +647,7 @@ TEST(stop, error_from_receive_on_stop_properly_closes) on_close_called++; mre.set(ex); }); - ws_transport->start("ws://fakeuri.org", [](std::exception_ptr) {}); + ws_transport->start("ws://fakeuri.org", transfer_format::text, [](std::exception_ptr) {}); auto stop_mre = manual_reset_event(); ws_transport->stop([&stop_mre, &stop_called](std::exception_ptr ex) { @@ -706,7 +706,7 @@ TEST(websocket_client_custom_impl, caching_callbacks_does_not_leak) }, signalr_client_config{}, logger(std::make_shared(), trace_level::none)); - ws_transport->start("ws://fakeuri.org", [](std::exception_ptr) {}); + ws_transport->start("ws://fakeuri.org", transfer_format::text, [](std::exception_ptr) {}); auto mre = manual_reset_event(); ws_transport->stop([&mre](std::exception_ptr exception) { From 207e9ed920974a729391176f40a4d2e9ada296e8 Mon Sep 17 00:00:00 2001 From: Brennan Conroy Date: Tue, 8 Nov 2022 11:47:14 -0800 Subject: [PATCH 2/2] build --- src/signalrclient/case_insensitive_comparison_utils.h | 8 ++++---- src/signalrclient/connection_impl.cpp | 1 + 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/signalrclient/case_insensitive_comparison_utils.h b/src/signalrclient/case_insensitive_comparison_utils.h index c0a2fb69..c38d66b0 100644 --- a/src/signalrclient/case_insensitive_comparison_utils.h +++ b/src/signalrclient/case_insensitive_comparison_utils.h @@ -34,8 +34,8 @@ namespace signalr bool operator()(const char* s1, const char* s2) const { - auto length = std::strlen(s1); - if (length != std::strlen(s2)) + auto length = std::char_traits::length(s1); + if (length != std::char_traits::length(s2)) { return false; } @@ -53,7 +53,7 @@ namespace signalr bool operator()(const std::string& s1, const char* s2) const { - if (s1.length() != std::strlen(s2)) + if (s1.length() != std::char_traits::length(s2)) { return false; } @@ -71,7 +71,7 @@ namespace signalr bool operator()(const char* s1, const std::string& s2) const { - auto length = std::strlen(s1); + auto length = std::char_traits::length(s1); if (length != s2.length()) { return false; diff --git a/src/signalrclient/connection_impl.cpp b/src/signalrclient/connection_impl.cpp index 73040378..1e5f15ab 100644 --- a/src/signalrclient/connection_impl.cpp +++ b/src/signalrclient/connection_impl.cpp @@ -318,6 +318,7 @@ namespace signalr return "binary"; } assert(false); + return ""; } void connection_impl::start_negotiate_internal(const std::string& url, int redirect_count, std::function transport, std::exception_ptr)> transport_started)