diff --git a/CMakeLists.txt b/CMakeLists.txt index cebbfe859..b5a407ac7 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -11,11 +11,6 @@ project( LANGUAGES CXX C ) -if (${CMAKE_VERSION} VERSION_GREATER_EQUAL "3.24") - # Affects robustness of timestamp checking on FetchContent dependencies. - cmake_policy(SET CMP0135 NEW) -endif () - # All projects in this repo should share the same version of 3rd party depends. # It's the only way to remain sane. set(CMAKE_FILES "${CMAKE_CURRENT_SOURCE_DIR}/cmake") @@ -36,6 +31,10 @@ if (BUILD_TESTING) endif () endif () include(FetchContent) + if (${CMAKE_VERSION} VERSION_GREATER_EQUAL "3.24") + # Affects robustness of timestamp checking on FetchContent dependencies. + cmake_policy(SET CMP0135 NEW) + endif () FetchContent_Declare( googletest URL https://github.com/google/googletest/archive/03597a01ee50ed33e9dfd640b249b4be3799d395.zip diff --git a/apps/hello-cpp/main.cpp b/apps/hello-cpp/main.cpp index b72f52eb0..f6e7d053b 100644 --- a/apps/hello-cpp/main.cpp +++ b/apps/hello-cpp/main.cpp @@ -60,7 +60,7 @@ int main() { client.WaitForReadySync(std::chrono::seconds(30)); - auto value = client.BoolVariationDetail("my-bool-flag", false); + auto value = client.BoolVariationDetail("my-boolean-flag", false); LD_LOG(logger, LogLevel::kInfo) << "Value was: " << *value; LD_LOG(logger, LogLevel::kInfo) << "Reason was: " << value.Reason(); diff --git a/apps/sse-contract-tests/src/entity_manager.cpp b/apps/sse-contract-tests/src/entity_manager.cpp index 574b23b0d..8dbf1f345 100644 --- a/apps/sse-contract-tests/src/entity_manager.cpp +++ b/apps/sse-contract-tests/src/entity_manager.cpp @@ -65,6 +65,7 @@ bool EntityManager::destroy(std::string const& id) { return false; } + it->second.first->close(); it->second.second->stop(); entities_.erase(it); diff --git a/cmake/certify.cmake b/cmake/certify.cmake deleted file mode 100644 index e6a87e47a..000000000 --- a/cmake/certify.cmake +++ /dev/null @@ -1,10 +0,0 @@ -cmake_minimum_required(VERSION 3.11) - -include(FetchContent) - -FetchContent_Declare(certify - GIT_REPOSITORY https://github.com/djarek/certify.git - GIT_TAG 97f5eebfd99a5d6e99d07e4820240994e4e59787 -) - -FetchContent_MakeAvailable(certify) diff --git a/cmake/expected.cmake b/cmake/expected.cmake index 7e3b1d620..18864faa6 100644 --- a/cmake/expected.cmake +++ b/cmake/expected.cmake @@ -2,6 +2,11 @@ cmake_minimum_required(VERSION 3.11) include(FetchContent) +if (${CMAKE_VERSION} VERSION_GREATER_EQUAL "3.24") + # Affects robustness of timestamp checking on FetchContent dependencies. + cmake_policy(SET CMP0135 NEW) +endif () + FetchContent_Declare(tl-expected GIT_REPOSITORY https://github.com/TartanLlama/expected.git GIT_TAG 292eff8bd8ee230a7df1d6a1c00c4ea0eb2f0362 diff --git a/cmake/foxy.cmake b/cmake/foxy.cmake new file mode 100644 index 000000000..f0451d263 --- /dev/null +++ b/cmake/foxy.cmake @@ -0,0 +1,19 @@ +cmake_minimum_required(VERSION 3.11) + +include(FetchContent) + +if (${CMAKE_VERSION} VERSION_GREATER_EQUAL "3.24") + # Affects robustness of timestamp checking on FetchContent dependencies. + cmake_policy(SET CMP0135 NEW) +endif () + + +FetchContent_Declare(foxy + GIT_REPOSITORY https://github.com/launchdarkly/foxy.git + GIT_TAG 7f4ac0495ad2ed9cd0eca5994743d677ac1d2636 + ) + + +set(BUILD_TESTING OFF) +FetchContent_MakeAvailable(foxy) +set(BUILD_TESTING ON) diff --git a/cmake/json.cmake b/cmake/json.cmake index ee1c9d42d..b1a82e5ad 100644 --- a/cmake/json.cmake +++ b/cmake/json.cmake @@ -4,8 +4,13 @@ include(FetchContent) set(JSON_ImplicitConversions OFF) +if (${CMAKE_VERSION} VERSION_GREATER_EQUAL "3.24") + # Affects robustness of timestamp checking on FetchContent dependencies. + cmake_policy(SET CMP0135 NEW) +endif () + FetchContent_Declare(json - URL https://github.com/nlohmann/json/releases/download/v3.11.2/json.tar.xz -) + URL https://github.com/nlohmann/json/releases/download/v3.11.2/json.tar.xz + ) FetchContent_MakeAvailable(json) diff --git a/libs/client-sdk/src/data_sources/streaming_data_source.cpp b/libs/client-sdk/src/data_sources/streaming_data_source.cpp index 99168af19..c36272a12 100644 --- a/libs/client-sdk/src/data_sources/streaming_data_source.cpp +++ b/libs/client-sdk/src/data_sources/streaming_data_source.cpp @@ -90,6 +90,15 @@ StreamingDataSource::StreamingDataSource( auto& http_properties = config.HttpProperties(); + // TODO: can the read timeout be shared with *all* http requests? Or should + // it have a default in defaults.hpp? This must be greater than the + // heartbeat interval of the streaming service. + client_builder.read_timeout(std::chrono::minutes(5)); + + client_builder.write_timeout(http_properties.WriteTimeout()); + + client_builder.connect_timeout(http_properties.ConnectTimeout()); + client_builder.header("authorization", config.SdkKey()); for (auto const& header : http_properties.BaseHeaders()) { client_builder.header(header.first, header.second); diff --git a/libs/common/CMakeLists.txt b/libs/common/CMakeLists.txt index 1fb44c4ad..772e31dec 100644 --- a/libs/common/CMakeLists.txt +++ b/libs/common/CMakeLists.txt @@ -37,7 +37,7 @@ message(STATUS "LaunchDarkly: using Boost v${Boost_VERSION}") include(${CMAKE_FILES}/expected.cmake) -include(${CMAKE_FILES}/certify.cmake) +include(${CMAKE_FILES}/foxy.cmake) # Add main SDK sources. add_subdirectory(src) diff --git a/libs/common/include/config/detail/builders/http_properties_builder.hpp b/libs/common/include/config/detail/builders/http_properties_builder.hpp index 69cd2a00c..259435767 100644 --- a/libs/common/include/config/detail/builders/http_properties_builder.hpp +++ b/libs/common/include/config/detail/builders/http_properties_builder.hpp @@ -58,6 +58,16 @@ class HttpPropertiesBuilder { */ HttpPropertiesBuilder& ReadTimeout(std::chrono::milliseconds read_timeout); + /** + * Set a write timeout. This is how long it takes to perform a write + * operation. + * + * @param read_timeout The write timeout. + * @return A reference to this builder. + */ + HttpPropertiesBuilder& WriteTimeout( + std::chrono::milliseconds write_timeout); + /** * The time for the first byte to be received during a read. If a byte * is not received within this time, then the request will be cancelled. @@ -115,6 +125,7 @@ class HttpPropertiesBuilder { private: std::chrono::milliseconds connect_timeout_; std::chrono::milliseconds read_timeout_; + std::chrono::milliseconds write_timeout_; std::chrono::milliseconds response_timeout_; std::string wrapper_name_; std::string wrapper_version_; diff --git a/libs/common/include/config/detail/built/http_properties.hpp b/libs/common/include/config/detail/built/http_properties.hpp index d55a8a342..5f51c07ef 100644 --- a/libs/common/include/config/detail/built/http_properties.hpp +++ b/libs/common/include/config/detail/built/http_properties.hpp @@ -11,12 +11,15 @@ class HttpProperties final { public: HttpProperties(std::chrono::milliseconds connect_timeout, std::chrono::milliseconds read_timeout, + std::chrono::milliseconds write_timeout, std::chrono::milliseconds response_timeout, std::string user_agent, std::map base_headers); [[nodiscard]] std::chrono::milliseconds ConnectTimeout() const; [[nodiscard]] std::chrono::milliseconds ReadTimeout() const; + [[nodiscard]] std::chrono::milliseconds WriteTimeout() const; + [[nodiscard]] std::chrono::milliseconds ResponseTimeout() const; [[nodiscard]] std::string const& UserAgent() const; [[nodiscard]] std::map const& BaseHeaders() const; @@ -24,6 +27,7 @@ class HttpProperties final { private: std::chrono::milliseconds connect_timeout_; std::chrono::milliseconds read_timeout_; + std::chrono::milliseconds write_timeout_; std::chrono::milliseconds response_timeout_; std::string user_agent_; std::map base_headers_; diff --git a/libs/common/include/config/detail/defaults.hpp b/libs/common/include/config/detail/defaults.hpp index ab1a219f7..0e3169ee1 100644 --- a/libs/common/include/config/detail/defaults.hpp +++ b/libs/common/include/config/detail/defaults.hpp @@ -46,14 +46,16 @@ struct Defaults { } static auto HttpProperties() -> built::HttpProperties { - return { - std::chrono::milliseconds{10000}, std::chrono::milliseconds{10000}, - std::chrono::milliseconds{10000}, SdkName() + "/" + SdkVersion(), - std::map()}; + return {std::chrono::seconds{10}, + std::chrono::seconds{10}, + std::chrono::seconds{10}, + std::chrono::seconds{10}, + SdkName() + "/" + SdkVersion(), + std::map()}; } static auto StreamingConfig() -> built::StreamingConfig { - return {std::chrono::milliseconds{1000}, "/meval"}; + return {std::chrono::seconds{1}, "/meval"}; } static auto DataSourceConfig() -> built::DataSourceConfig { @@ -91,14 +93,16 @@ struct Defaults { } static auto HttpProperties() -> built::HttpProperties { - return { - std::chrono::milliseconds{2000}, std::chrono::milliseconds{10000}, - std::chrono::milliseconds{10000}, SdkName() + "/" + SdkVersion(), - std::map()}; + return {std::chrono::seconds{2}, + std::chrono::seconds{10}, + std::chrono::seconds{10}, + std::chrono::seconds{10}, + SdkName() + "/" + SdkVersion(), + std::map()}; } static auto StreamingConfig() -> built::StreamingConfig { - return {std::chrono::milliseconds{1000}}; + return {std::chrono::seconds{1}}; } static auto DataSourceConfig() -> built::DataSourceConfig { diff --git a/libs/common/include/network/detail/asio_requester.hpp b/libs/common/include/network/detail/asio_requester.hpp index 7ca90b76c..d3e0182eb 100644 --- a/libs/common/include/network/detail/asio_requester.hpp +++ b/libs/common/include/network/detail/asio_requester.hpp @@ -9,8 +9,7 @@ #include #include -#include -#include +#include #include #include @@ -74,6 +73,7 @@ static http::request MakeBeastRequest( } else { beast_request.target(request.Path()); } + beast_request.prepare_payload(); beast_request.set(http::field::host, request.Host()); @@ -108,136 +108,86 @@ static std::optional MakeRedirectRequest(HttpRequest const& req, return std::nullopt; } -template -class - Session { // NOLINT(cppcoreguidelines-non-private-member-variables-in-classes) +static boost::optional ToOptRef( + net::ssl::context* maybe_val) { + if (maybe_val) { + return *maybe_val; + } + return boost::none; +} + +class FoxyClient + : public std::enable_shared_from_this< + FoxyClient> { // NOLINT(cppcoreguidelines-non-private-member-variables-in-classes) public: using ResponseHandler = std::function; private: - Derived& GetDerived() { return static_cast(*this); } + std::shared_ptr ssl_context_; + std::string host_; + std::string port_; http::request req_; std::chrono::milliseconds connect_timeout_; std::chrono::milliseconds response_timeout_; - std::chrono::milliseconds read_timeout_; - - protected: - beast::flat_buffer buffer_; - std::string host_; - std::string port_; - tcp::resolver resolver_; - http::response_parser parser_; ResponseHandler handler_; + http::response resp_; + foxy::client_session session_; public: - Session(net::any_io_executor const& exec, - std::string host, - std::string port, - http::request req, - std::chrono::milliseconds connect_timeout, - std::chrono::milliseconds response_timeout, - std::chrono::milliseconds read_timeout, - ResponseHandler handler) - : req_(std::move(req)), - resolver_(exec), + FoxyClient(net::any_io_executor const& exec, + std::shared_ptr ssl_context, + std::string host, + std::string port, + http::request req, + std::chrono::milliseconds connect_timeout, + std::chrono::milliseconds response_timeout, + ResponseHandler handler) + : ssl_context_(std::move(ssl_context)), host_(std::move(host)), port_(std::move(port)), + req_(std::move(req)), connect_timeout_(connect_timeout), response_timeout_(response_timeout), - read_timeout_(read_timeout), - handler_(std::move(handler)) {} - - void Fail(beast::error_code ec, char const* what) { - // TODO: Is it safe to cancel this if it has already failed? - DoClose(); - std::optional error_string = - std::string(what) + ": " + ec.message(); - handler_(HttpResult(error_string)); - } - - void DoResolve() { - resolver_.async_resolve( - host_, port_, - beast::bind_front_handler(&Session::OnResolve, - GetDerived().shared_from_this())); + handler_(std::move(handler)), + session_(exec, + foxy::session_opts{.ssl_ctx = ToOptRef(ssl_context_.get()), + .timeout = connect_timeout_}), + resp_() {} + + void Run() { + session_.async_connect(host_, port_, + beast::bind_front_handler(&FoxyClient::OnConnect, + shared_from_this())); } - void OnResolve(beast::error_code ec, tcp::resolver::results_type results) { - if (ec) { - return Fail(ec, "resolve"); - } - - beast::get_lowest_layer(GetDerived().Stream()) - .expires_after(connect_timeout_); - - beast::get_lowest_layer(GetDerived().Stream()) - .async_connect(results, beast::bind_front_handler( - &Session::OnConnect, - GetDerived().shared_from_this())); - } - - void OnConnect(beast::error_code ec, - tcp::resolver::results_type::endpoint_type eps) { - boost::ignore_unused(eps); + void OnConnect(boost::system::error_code ec) { if (ec) { return Fail(ec, "connect"); } - - GetDerived().DoHandshake(); + session_.opts.timeout = response_timeout_; + session_.async_request( + req_, resp_, + beast::bind_front_handler(&FoxyClient::OnResponse, + shared_from_this())); } - void OnHandshake(beast::error_code ec) { + void OnResponse(boost::system::error_code ec) { if (ec) { - return Fail(ec, "handshake"); + return Fail(ec, "request"); } - - DoWrite(); - } - - void DoWrite() { - beast::get_lowest_layer(GetDerived().Stream()) - .expires_after(response_timeout_); - - http::async_write( - GetDerived().Stream(), req_, - beast::bind_front_handler(&Session::OnWrite, - GetDerived().shared_from_this())); + handler_(MakeResult()); + session_.async_shutdown(beast::bind_front_handler( + &FoxyClient::OnShutdown, shared_from_this())); } - void OnWrite(beast::error_code ec, std::size_t unused) { - boost::ignore_unused(unused); - if (ec) { - return Fail(ec, "write"); - } - - http::async_read_some( - GetDerived().Stream(), buffer_, parser_, - beast::bind_front_handler(&Session::OnRead, - GetDerived().shared_from_this())); + void Fail(beast::error_code ec, char const* what) { + std::string error_string = std::string(what) + ": " + ec.message(); + handler_(HttpResult(error_string)); + session_.async_shutdown(beast::bind_front_handler( + &FoxyClient::OnShutdown, shared_from_this())); } - void OnRead(beast::error_code ec, std::size_t bytes_transferred) { - boost::ignore_unused(bytes_transferred); - if (ec) { - return Fail(ec, "read"); - } - if (parser_.is_done()) { - DoClose(); - - handler_(MakeResult()); - return; - } - - // TODO: Does this refresh the timeout? We want it to be a total - // timeout. - beast::get_lowest_layer(GetDerived().Stream()) - .expires_after(read_timeout_); - - http::async_read_some( - GetDerived().Stream(), buffer_, parser_, - beast::bind_front_handler(&Session::OnRead, - GetDerived().shared_from_this())); - } + void OnShutdown(boost::system::error_code ec) { boost::ignore_unused(ec); } /** * Produce an HttpResult from the parser_. Should be called if the parser @@ -246,110 +196,14 @@ class */ [[nodiscard]] HttpResult MakeResult() const { auto headers = HttpResult::HeadersType(); - for (auto const& field : parser_.get().base()) { + for (auto const& field : resp_.base()) { headers.insert_or_assign(field.name_string(), field.value()); } - auto result = HttpResult(parser_.get().result_int(), - std::make_optional(parser_.get().body()), - std::move(headers)); + auto result = + HttpResult(resp_.result_int(), std::make_optional(resp_.body()), + std::move(headers)); return result; } - - void DoClose() { beast::get_lowest_layer(GetDerived().Stream()).cancel(); } -}; - -class PlaintextClient : public Session, - public std::enable_shared_from_this { - public: - virtual ~PlaintextClient() = default; - using ResponseHandler = Session::ResponseHandler; - - PlaintextClient(net::any_io_executor ex, - http::request req, - std::string host, - std::string port, - std::chrono::milliseconds connect_timeout, - std::chrono::milliseconds response_timeout, - std::chrono::milliseconds read_timeout, - ResponseHandler handler) - : Session(ex, - std::move(host), - std::move(port), - std::move(req), - connect_timeout, - response_timeout, - read_timeout, - std::move(handler)), - stream_{ex} {} - - void DoHandshake() { - // No handshake for plaintext; immediately send the request instead. - DoWrite(); - } - - void Run() { DoResolve(); } - - beast::tcp_stream& Stream() { return stream_; } - - private: - beast::tcp_stream stream_; -}; - -class EncryptedClient : public Session, - public std::enable_shared_from_this { - public: - virtual ~EncryptedClient() = default; - using ResponseHandler = Session::ResponseHandler; - - EncryptedClient(net::any_io_executor ex, - std::shared_ptr ssl_ctx, - http::request req, - std::string host, - std::string port, - std::chrono::milliseconds connect_timeout, - std::chrono::milliseconds response_timeout, - std::chrono::milliseconds read_timeout, - ResponseHandler handler) - : Session(ex, - std::move(host), - std::move(port), - std::move(req), - connect_timeout, - response_timeout, - read_timeout, - std::move(handler)), - ssl_ctx_(ssl_ctx), - stream_{ex, *ssl_ctx} {} - - virtual void Run() { - // Set SNI Hostname (many hosts need this to handshake successfully) - if (!SSL_set_tlsext_host_name(stream_.native_handle(), host_.c_str())) { - beast::error_code ec{static_cast(::ERR_get_error()), - net::error::get_ssl_category()}; - - DoClose(); - // TODO: Should this be treated as a terminal error for the request. - std::optional error_string = - "failed to set TLS host name extension: " + ec.message(); - handler_(HttpResult(error_string)); - return; - } - - DoResolve(); - } - - void DoHandshake() { - stream_.async_handshake( - ssl::stream_base::client, - beast::bind_front_handler(&EncryptedClient::OnHandshake, - shared_from_this())); - } - - beast::ssl_stream& Stream() { return stream_; } - - private: - std::shared_ptr ssl_ctx_; - beast::ssl_stream stream_; }; /** @@ -398,13 +252,9 @@ class AsioRequester { public: AsioRequester(net::any_io_executor ctx) : ctx_(std::move(ctx)), - ssl_ctx_( - std::make_shared(ssl::context::tlsv12_client)) { - ssl_ctx_->set_verify_mode(ssl::verify_peer | - ssl::verify_fail_if_no_peer_cert); - + ssl_ctx_(std::make_shared( + foxy::make_ssl_ctx(ssl::context::tlsv12_client))) { ssl_ctx_->set_default_verify_paths(); - boost::certify::enable_native_https_server_verification(*ssl_ctx_); } template @@ -422,7 +272,7 @@ class AsioRequester { Handler handler(std::forward(token)); Result result(handler); - InnerRequest(request, std::move(handler), 0); + InnerRequest(net::make_strand(ctx_), request, std::move(handler), 0); return result.get(); } @@ -436,13 +286,12 @@ class AsioRequester { */ std::shared_ptr ssl_ctx_; - void InnerRequest(std::optional request, + void InnerRequest(boost::asio::any_io_executor exec, + std::optional request, std::function callback, unsigned char redirect_count) { - auto strand = net::make_strand(ctx_); - if (redirect_count > kRedirectLimit) { - boost::asio::post(strand, [callback, request]() mutable { + boost::asio::post(exec, [callback, request]() mutable { callback( HttpResult("Redirects exceeded 20, cancelling request.")); }); @@ -452,44 +301,37 @@ class AsioRequester { // The request is invalid and cannot be made, so produce an error // result. if (!request || !request->Valid()) { - boost::asio::post(strand, [callback, request]() mutable { + boost::asio::post(exec, [callback, request]() mutable { callback(HttpResult( "The request was malformed and could not be made.")); }); return; } - boost::asio::post(strand, [strand, callback, request, this, - redirect_count]() mutable { + boost::asio::post(exec, [exec, callback, request, this, + redirect_count]() mutable { auto beast_request = MakeBeastRequest(*request); const auto& properties = request->Properties(); - if (request->Https()) { - std::make_shared( - strand, ssl_ctx_, beast_request, request->Host(), - request->Port(), properties.ConnectTimeout(), - properties.ResponseTimeout(), properties.ReadTimeout(), - [callback, request, this, redirect_count](auto res) { - NeedsRedirect(res) - ? InnerRequest(MakeRedirectRequest(*request, res), - callback, redirect_count + 1) - : callback(res); - }) - ->Run(); - } else { - std::make_shared( - strand, beast_request, request->Host(), request->Port(), - properties.ConnectTimeout(), properties.ResponseTimeout(), - properties.ReadTimeout(), - [callback, request, this, redirect_count](auto res) { - NeedsRedirect(res) - ? InnerRequest(MakeRedirectRequest(*request, res), - callback, redirect_count + 1) - : callback(res); - }) - ->Run(); + std::string service = + request->Port().value_or(request->Https() ? "https" : "http"); + + std::shared_ptr ssl; + if (service == "https") { + ssl = this->ssl_ctx_; } + + std::make_shared( + exec, std::move(ssl), request->Host(), service, beast_request, + properties.ConnectTimeout(), properties.ResponseTimeout(), + [exec, callback, request, this, redirect_count](auto res) { + NeedsRedirect(res) + ? InnerRequest(exec, MakeRedirectRequest(*request, res), + callback, redirect_count + 1) + : callback(res); + }) + ->Run(); }); } }; diff --git a/libs/common/include/network/detail/http_requester.hpp b/libs/common/include/network/detail/http_requester.hpp index 8da763ad3..ed92cdbf9 100644 --- a/libs/common/include/network/detail/http_requester.hpp +++ b/libs/common/include/network/detail/http_requester.hpp @@ -89,7 +89,7 @@ class HttpRequest { [[nodiscard]] config::detail::built::HttpProperties const& Properties() const; [[nodiscard]] std::string const& Host() const; - [[nodiscard]] std::string const& Port() const; + [[nodiscard]] std::optional const& Port() const; [[nodiscard]] std::string const& Path() const; [[nodiscard]] std::string const& Url() const; @@ -124,7 +124,7 @@ class HttpRequest { std::optional body_; config::detail::built::HttpProperties properties_; std::string host_; - std::string port_; + std::optional port_; std::string path_; std::map params_; bool is_https_; diff --git a/libs/common/src/CMakeLists.txt b/libs/common/src/CMakeLists.txt index a30d091e9..c926b67ce 100644 --- a/libs/common/src/CMakeLists.txt +++ b/libs/common/src/CMakeLists.txt @@ -59,7 +59,7 @@ add_library(${LIBNAME} add_library(launchdarkly::common ALIAS ${LIBNAME}) target_link_libraries(${LIBNAME} - PUBLIC Boost::headers tl::expected OpenSSL::SSL certify::core + PUBLIC Boost::headers tl::expected OpenSSL::SSL foxy PRIVATE Boost::url Boost::json) # Need the public headers to build. diff --git a/libs/common/src/config/http_properties.cpp b/libs/common/src/config/http_properties.cpp index dfdecd2b4..ef30a534f 100644 --- a/libs/common/src/config/http_properties.cpp +++ b/libs/common/src/config/http_properties.cpp @@ -6,11 +6,13 @@ namespace launchdarkly::config::detail::built { HttpProperties::HttpProperties(std::chrono::milliseconds connect_timeout, std::chrono::milliseconds read_timeout, + std::chrono::milliseconds write_timeout, std::chrono::milliseconds response_timeout, std::string user_agent, std::map base_headers) : connect_timeout_(connect_timeout), read_timeout_(read_timeout), + write_timeout_(write_timeout), response_timeout_(response_timeout), user_agent_(std::move(user_agent)), base_headers_(std::move(base_headers)) {} @@ -23,6 +25,10 @@ std::chrono::milliseconds HttpProperties::ReadTimeout() const { return read_timeout_; } +std::chrono::milliseconds HttpProperties::WriteTimeout() const { + return write_timeout_; +} + std::chrono::milliseconds HttpProperties::ResponseTimeout() const { return response_timeout_; } @@ -37,6 +43,7 @@ std::map const& HttpProperties::BaseHeaders() const { bool operator==(HttpProperties const& lhs, HttpProperties const& rhs) { return lhs.ReadTimeout() == rhs.ReadTimeout() && + lhs.WriteTimeout() == rhs.WriteTimeout() && lhs.ConnectTimeout() == rhs.ConnectTimeout() && lhs.BaseHeaders() == rhs.BaseHeaders() && lhs.UserAgent() == rhs.UserAgent(); diff --git a/libs/common/src/config/http_properties_builder.cpp b/libs/common/src/config/http_properties_builder.cpp index 74e905484..beedd7e8f 100644 --- a/libs/common/src/config/http_properties_builder.cpp +++ b/libs/common/src/config/http_properties_builder.cpp @@ -15,6 +15,7 @@ HttpPropertiesBuilder::HttpPropertiesBuilder( built::HttpProperties const& properties) { connect_timeout_ = properties.ConnectTimeout(); read_timeout_ = properties.ReadTimeout(); + write_timeout_ = properties.WriteTimeout(); response_timeout_ = properties.ResponseTimeout(); base_headers_ = properties.BaseHeaders(); user_agent_ = properties.UserAgent(); @@ -76,11 +77,11 @@ built::HttpProperties HttpPropertiesBuilder::Build() const { std::map headers_with_wrapper(base_headers_); headers_with_wrapper["X-LaunchDarkly-Wrapper"] = wrapper_name_ + "/" + wrapper_version_; - return {connect_timeout_, read_timeout_, response_timeout_, user_agent_, - headers_with_wrapper}; + return {connect_timeout_, read_timeout_, write_timeout_, + response_timeout_, user_agent_, headers_with_wrapper}; } - return {connect_timeout_, read_timeout_, response_timeout_, user_agent_, - base_headers_}; + return {connect_timeout_, read_timeout_, write_timeout_, + response_timeout_, user_agent_, base_headers_}; } template class HttpPropertiesBuilder; diff --git a/libs/common/src/events/event_batch.cpp b/libs/common/src/events/event_batch.cpp index eb44e7298..f0f817532 100644 --- a/libs/common/src/events/event_batch.cpp +++ b/libs/common/src/events/event_batch.cpp @@ -20,8 +20,7 @@ network::detail::HttpRequest const& EventBatch::Request() const { } std::string EventBatch::Target() const { - return (request_.Https() ? "https://" : "http://") + request_.Host() + ":" + - request_.Port() + request_.Path(); + return request_.Url(); } } // namespace launchdarkly::events::detail diff --git a/libs/common/src/network/http_requester.cpp b/libs/common/src/network/http_requester.cpp index e6054fd99..3e421fc3e 100644 --- a/libs/common/src/network/http_requester.cpp +++ b/libs/common/src/network/http_requester.cpp @@ -69,7 +69,8 @@ HttpRequest::HttpRequest(std::string const& url, : properties_(std::move(properties)), method_(method), body_(std::move(body)), - url_(url) { + url_(url), + port_(std::nullopt) { auto uri_components = boost::urls::parse_uri(url); // If the URI cannot be parsed, then the request is not valid. @@ -95,8 +96,6 @@ HttpRequest::HttpRequest(std::string const& url, is_https_ = uri_components->scheme_id() == boost::urls::scheme::https; if (uri_components->has_port()) { port_ = uri_components->port(); - } else { - port_ = is_https_ ? "443" : "80"; } valid_ = true; } @@ -113,7 +112,7 @@ HttpRequest::HttpRequest(HttpRequest& base_request, method_(base_request.method_), body_(std::move(base_request.body_)) {} -std::string const& HttpRequest::Port() const { +std::optional const& HttpRequest::Port() const { return port_; } bool HttpRequest::Https() const { diff --git a/libs/common/tests/http_requester_test.cpp b/libs/common/tests/http_requester_test.cpp index 6c1350090..dddbeb911 100644 --- a/libs/common/tests/http_requester_test.cpp +++ b/libs/common/tests/http_requester_test.cpp @@ -20,20 +20,40 @@ TEST(HttpRequestTests, NormalizesRelativeUrl) { EXPECT_EQ("/ham?egg=true&cheese=true", normalized.Path()); } -TEST(HttpRequestTests, UsesCorrectDefaultPortForSchemes) { - HttpRequest secure("https://some.domain.com/", +TEST(HttpRequestTests, UsesCorrectPort) { + HttpRequest a("scheme://some.domain.com:123", + launchdarkly::network::detail::HttpMethod::kGet, + HttpPropertiesBuilder().Build(), std::nullopt); + + EXPECT_EQ("123", a.Port()); + + HttpRequest b("scheme://some.domain.com:456", + launchdarkly::network::detail::HttpMethod::kGet, + HttpPropertiesBuilder().Build(), std::nullopt); + + EXPECT_EQ("456", b.Port()); + + HttpRequest c("scheme://some.domain.com", + launchdarkly::network::detail::HttpMethod::kGet, + HttpPropertiesBuilder().Build(), std::nullopt); + + EXPECT_FALSE(c.Port()); +} + +TEST(HttpRequestTests, DetectsHttpsFromScheme) { + HttpRequest secure("https://some.domain.com", launchdarkly::network::detail::HttpMethod::kGet, HttpPropertiesBuilder().Build(), std::nullopt); - EXPECT_EQ("443", secure.Port()); + EXPECT_TRUE(secure.Https()); - HttpRequest insecure("http://some.domain.com/", + HttpRequest insecure("http://some.domain.com", launchdarkly::network::detail::HttpMethod::kGet, HttpPropertiesBuilder().Build(), std::nullopt); - EXPECT_EQ("80", insecure.Port()); + EXPECT_FALSE(insecure.Https()); } TEST(HttpRequestTests, CanAppendBasicPath) { diff --git a/libs/server-sent-events/CMakeLists.txt b/libs/server-sent-events/CMakeLists.txt index a7a77548a..a0220b4c7 100644 --- a/libs/server-sent-events/CMakeLists.txt +++ b/libs/server-sent-events/CMakeLists.txt @@ -39,8 +39,7 @@ set(Boost_USE_STATIC_RUNTIME OFF) find_package(Boost 1.80 REQUIRED COMPONENTS url) message(STATUS "LaunchDarkly: using Boost v${Boost_VERSION}") -include(${CMAKE_FILES}/certify.cmake) - +include(${CMAKE_FILES}/foxy.cmake) add_subdirectory(src) diff --git a/libs/server-sent-events/include/launchdarkly/sse/client.hpp b/libs/server-sent-events/include/launchdarkly/sse/client.hpp index 17400b761..af64c7520 100644 --- a/libs/server-sent-events/include/launchdarkly/sse/client.hpp +++ b/libs/server-sent-events/include/launchdarkly/sse/client.hpp @@ -73,6 +73,20 @@ class Builder { */ Builder& read_timeout(std::chrono::milliseconds timeout); + /** + * Specifies the maximum time duration to establish the connection. + * @param timeout + * @return Reference to this builder. + */ + Builder& connect_timeout(std::chrono::milliseconds timeout); + + /** + * Specifies the maximum time duration to write the initial request. + * @param timeout + * @return Reference to this builder. + */ + Builder& write_timeout(std::chrono::milliseconds timeout); + /** * Specify the method for the initial request. The default method is GET. * @param verb The HTTP method. @@ -112,6 +126,8 @@ class Builder { net::any_io_executor executor_; http::request request_; std::optional read_timeout_; + std::optional write_timeout_; + std::optional connect_timeout_; LogCallback logging_cb_; EventReceiver receiver_; }; @@ -124,14 +140,14 @@ class Builder { class Client { public: virtual ~Client() = default; + /** - * Kicks off a connection to the server and begins reading the event stream. - * The provided event receiver and logging callbacks will be invoked from - * the thread that is servicing the Client's executor. - */ + * Asynchronously kicks off an SSE connection to the server and begins + * reading the event stream. */ virtual void run() = 0; + /** - * Closes the stream. + * Asynchronously closes the connection. */ virtual void close() = 0; }; diff --git a/libs/server-sent-events/src/CMakeLists.txt b/libs/server-sent-events/src/CMakeLists.txt index 9a592df8b..07b7a33c8 100644 --- a/libs/server-sent-events/src/CMakeLists.txt +++ b/libs/server-sent-events/src/CMakeLists.txt @@ -11,8 +11,7 @@ add_library(${LIBNAME} backoff.cpp) target_link_libraries(${LIBNAME} PUBLIC OpenSSL::SSL Boost::headers - PRIVATE certify::core Boost::url - ) + PRIVATE Boost::url foxy) add_library(launchdarkly::sse ALIAS ${LIBNAME}) diff --git a/libs/server-sent-events/src/client.cpp b/libs/server-sent-events/src/client.cpp index 9c7b5ecda..fb5ed48e9 100644 --- a/libs/server-sent-events/src/client.cpp +++ b/libs/server-sent-events/src/client.cpp @@ -1,18 +1,14 @@ -#include -#include - #include -#include #include - -#include -#include +#include +#include +#include +#include #include #include #include #include -#include #include #include @@ -21,7 +17,6 @@ #include #include #include -#include namespace launchdarkly::sse { @@ -33,248 +28,151 @@ using tcp = boost::asio::ip::tcp; // from auto const kDefaultUserAgent = BOOST_BEAST_VERSION_STRING; -// The allowed amount of time to connect the socket and perform -// any TLS handshake, if necessary. -const std::chrono::milliseconds kDefaultConnectTimeout = - std::chrono::seconds(15); -// Once connected, the amount of time to send a request and receive the first -// batch of bytes back. -const std::chrono::milliseconds kDefaultResponseTimeout = - std::chrono::seconds(15); - -template -class - Session { // NOLINT(cppcoreguidelines-non-private-member-variables-in-classes) - private: - Derived& derived() { return static_cast(*this); } - http::request req_; - std::chrono::milliseconds connect_timeout_; - std::chrono::milliseconds response_timeout_; - std::optional read_timeout_; +// Time duration used when no timeout is specified (1 year). +auto const kNoTimeout = std::chrono::hours(8760); - protected: - beast::flat_buffer buffer_; - std::string host_; - std::string port_; - tcp::resolver resolver_; - Builder::LogCallback logger_; - - using cb = std::function; - using body = launchdarkly::sse::detail::EventBody; - http::response_parser parser_; +static boost::optional ToOptRef( + std::optional& maybe_val) { + if (maybe_val) { + return maybe_val.value(); + } + return boost::none; +} +class FoxyClient : public Client, + public std::enable_shared_from_this { public: - Session(net::any_io_executor const& exec, - std::string host, - std::string port, - http::request req, - std::chrono::milliseconds connect_timeout, - std::chrono::milliseconds response_timeout, - std::optional read_timeout, - Builder::EventReceiver receiver, - Builder::LogCallback logger) - : req_(std::move(req)), - resolver_(exec), - connect_timeout_(connect_timeout), - response_timeout_(response_timeout), - read_timeout_(std::move(read_timeout)), + FoxyClient(boost::asio::any_io_executor executor, + http::request req, + std::string host, + std::string port, + std::optional connect_timeout, + std::optional read_timeout, + std::optional write_timeout, + Builder::EventReceiver receiver, + Builder::LogCallback logger, + std::optional maybe_ssl) + : ssl_context_(std::move(maybe_ssl)), host_(std::move(host)), port_(std::move(port)), - logger_(std::move(logger)), - parser_() { - parser_.get().body().on_event(std::move(receiver)); + connect_timeout_(connect_timeout), + read_timeout_(read_timeout), + write_timeout_(write_timeout), + req_(std::move(req)), + body_parser_(), + session_(executor, + foxy::session_opts{ + .ssl_ctx = ToOptRef(ssl_context_), + .timeout = connect_timeout.value_or(kNoTimeout)}), + logger_(std::move(logger)) { + // SSE body will never end unless an error occurs, so we shouldn't set a + // size limit. + body_parser_.body_limit(boost::none); + body_parser_.get().body().on_event(std::move(receiver)); } - void fail(beast::error_code ec, char const* what) { - logger_(std::string(what) + ": " + ec.message()); + void fail(boost::system::error_code ec, std::string what) { + logger_("sse-client: " + what + ": " + ec.message()); + do_close(); } - void do_resolve() { - logger_("resolving " + host_ + ":" + port_); - resolver_.async_resolve( + virtual void run() override { + session_.async_connect( host_, port_, - beast::bind_front_handler(&Session::on_resolve, - derived().shared_from_this())); + beast::bind_front_handler(&FoxyClient::on_connect, + shared_from_this())); } - void on_resolve(beast::error_code ec, tcp::resolver::results_type results) { - if (ec) - return fail(ec, "resolve"); - - logger_("connecting (" + std::to_string(connect_timeout_.count()) + - " sec timeout)"); - - beast::get_lowest_layer(derived().stream()) - .expires_after(connect_timeout_); + virtual void close() override { + boost::asio::post(session_.get_executor(), + beast::bind_front_handler(&FoxyClient::do_close, + shared_from_this())); + } - beast::get_lowest_layer(derived().stream()) - .async_connect(results, beast::bind_front_handler( - &Session::on_connect, - derived().shared_from_this())); + void do_close() { + session_.async_shutdown(beast::bind_front_handler(&FoxyClient::on_close, + shared_from_this())); } - void on_connect(beast::error_code ec, - tcp::resolver::results_type::endpoint_type eps) { + void on_close(boost::system::error_code ec) { boost::ignore_unused(ec); } + + void on_connect(boost::system::error_code ec) { if (ec) { return fail(ec, "connect"); } - derived().do_handshake(); - } - - void on_handshake(beast::error_code ec) { - if (ec) - return fail(ec, "handshake"); - - do_write(); - } - - void do_write() { - logger_("making request (" + std::to_string(response_timeout_.count()) + - " sec timeout)"); - - beast::get_lowest_layer(derived().stream()) - .expires_after(response_timeout_); - - http::async_write( - derived().stream(), req_, - beast::bind_front_handler(&Session::on_write, - derived().shared_from_this())); + session_.opts.timeout = write_timeout_.value_or(kNoTimeout); + session_.async_write(req_, + beast::bind_front_handler(&FoxyClient::on_write, + shared_from_this())); } - void on_write(beast::error_code ec, std::size_t) { - if (ec) - return fail(ec, "write"); - - logger_("reading response"); - - http::async_read_some( - derived().stream(), buffer_, parser_, - beast::bind_front_handler(&Session::on_read, - derived().shared_from_this())); - } - - void on_read(beast::error_code ec, std::size_t bytes_transferred) { - boost::ignore_unused(bytes_transferred); + void on_write(boost::system::error_code ec, std::size_t amount) { + boost::ignore_unused(amount); if (ec) { - return fail(ec, "read"); + return fail(ec, "send request"); } - - if (read_timeout_) { - beast::get_lowest_layer(derived().stream()) - .expires_after(*read_timeout_); - } else { - beast::get_lowest_layer(derived().stream()).expires_never(); - }; - - http::async_read_some( - derived().stream(), buffer_, parser_, - beast::bind_front_handler(&Session::on_read, - derived().shared_from_this())); + session_.opts.timeout = read_timeout_.value_or(kNoTimeout); + session_.async_read_header( + body_parser_, beast::bind_front_handler(&FoxyClient::on_headers, + shared_from_this())); } - void do_close() { - logger_("closing"); - beast::get_lowest_layer(derived().stream()).cancel(); - } -}; - -class EncryptedClient : public Client, - public Session, - public std::enable_shared_from_this { - public: - EncryptedClient(net::any_io_executor ex, - ssl::context ctx, - http::request req, - std::string host, - std::string port, - std::optional read_timeout, - Builder::EventReceiver receiver, - Builder::LogCallback logger) - : Session(ex, - std::move(host), - std::move(port), - std::move(req), - kDefaultConnectTimeout, - kDefaultResponseTimeout, - std::move(read_timeout), - std::move(receiver), - std::move(logger)), - ssl_ctx_(std::move(ctx)), - stream_{ex, ssl_ctx_} {} + void on_headers(boost::system::error_code ec, std::size_t amount) { + boost::ignore_unused(amount); + if (ec) { + return fail(ec, "read header"); + } - virtual void run() override { - // Set SNI Hostname (many hosts need this to handshake successfully) - if (!SSL_set_tlsext_host_name(stream_.native_handle(), host_.c_str())) { - beast::error_code ec{static_cast(::ERR_get_error()), - net::error::get_ssl_category()}; - logger_("failed to set TLS host name extension: " + ec.message()); + if (!body_parser_.is_header_done()) { + session_.async_read_header( + body_parser_, beast::bind_front_handler(&FoxyClient::on_headers, + shared_from_this())); return; } - do_resolve(); - } - - virtual void close() override { do_close(); } - - void do_handshake() { - stream_.async_handshake(ssl::stream_base::client, - beast::bind_front_handler( - &EncryptedClient::on_handshake, shared())); - } - - beast::ssl_stream& stream() { return stream_; } - - private: - ssl::context ssl_ctx_; - beast::ssl_stream stream_; - - std::shared_ptr shared() { - return std::static_pointer_cast(shared_from_this()); + auto response = body_parser_.get(); + if (beast::http::to_status_class(response.result()) == + beast::http::status_class::successful) { + session_.async_read(body_parser_, beast::bind_front_handler( + &FoxyClient::on_read_complete, + shared_from_this())); + } else { + return fail(ec, "read response"); + } } -}; -class PlaintextClient : public Client, - public Session, - public std::enable_shared_from_this { - public: - PlaintextClient(net::any_io_executor ex, - http::request req, - std::string host, - std::string port, - std::optional read_timeout, - Builder::EventReceiver receiver, - Builder::LogCallback logger) - : Session(ex, - std::move(host), - std::move(port), - std::move(req), - kDefaultConnectTimeout, - kDefaultResponseTimeout, - read_timeout, - std::move(receiver), - std::move(logger)), - stream_{ex} {} - - virtual void run() override { do_resolve(); } - - void do_handshake() { - // No handshake for plaintext; immediately send the request instead. - do_write(); + void on_read_complete(boost::system::error_code ec, std::size_t amount) { + boost::ignore_unused(amount); + if (ec == boost::asio::error::operation_aborted) { + do_close(); + } else { + return fail(ec, "read body"); + } } - virtual void close() override { do_close(); } - - beast::tcp_stream& stream() { return stream_; } - private: - beast::tcp_stream stream_; + std::optional ssl_context_; + std::string host_; + std::string port_; + std::optional connect_timeout_; + std::optional read_timeout_; + std::optional write_timeout_; + http::request req_; + using cb = std::function; + using body = launchdarkly::sse::detail::EventBody; + http::response_parser body_parser_; + foxy::client_session session_; + Builder::LogCallback logger_; }; Builder::Builder(net::any_io_executor ctx, std::string url) : url_{std::move(url)}, executor_{std::move(ctx)}, - read_timeout_{std::nullopt} { + read_timeout_{std::nullopt}, + write_timeout_{std::nullopt}, + connect_timeout_{std::nullopt}, + logging_cb_([](auto msg) {}) { receiver_ = [](launchdarkly::sse::Event const&) {}; request_.version(11); @@ -294,11 +192,21 @@ Builder& Builder::body(std::string data) { return *this; } +Builder& Builder::connect_timeout(std::chrono::milliseconds timeout) { + connect_timeout_ = timeout; + return *this; +} + Builder& Builder::read_timeout(std::chrono::milliseconds timeout) { read_timeout_ = timeout; return *this; } +Builder& Builder::write_timeout(std::chrono::milliseconds timeout) { + write_timeout_ = timeout; + return *this; +} + Builder& Builder::method(http::verb verb) { request_.method(verb); return *this; @@ -320,49 +228,40 @@ std::shared_ptr Builder::build() { return nullptr; } + auto request = request_; + // Don't send a body unless the method is POST or REPORT - if (!(request_.method() == http::verb::post || - request_.method() == http::verb::report)) { - request_.body() = ""; + if (!(request.method() == http::verb::post || + request.method() == http::verb::report)) { + request.body() = ""; } else { // If it is, then setup Content-Type, only if one wasn't // specified. - if (auto it = request_.find(http::field::content_type); - it == request_.end()) { - request_.set(http::field::content_type, "text/plain"); + if (auto it = request.find(http::field::content_type); + it == request.end()) { + request.set(http::field::content_type, "text/plain"); } } - request_.prepare_payload(); + request.prepare_payload(); std::string host = uri_components->host(); - request_.set(http::field::host, host); - request_.target(uri_components->path()); - - if (uri_components->scheme_id() == boost::urls::scheme::https) { - std::string port = - uri_components->has_port() ? uri_components->port() : "443"; + request.set(http::field::host, host); + request.target(uri_components->path()); - ssl::context ssl_ctx{ssl::context::tlsv12_client}; + std::string service = uri_components->has_port() ? uri_components->port() + : uri_components->scheme(); - ssl_ctx.set_verify_mode(ssl::verify_peer | - ssl::verify_fail_if_no_peer_cert); - - ssl_ctx.set_default_verify_paths(); - boost::certify::enable_native_https_server_verification(ssl_ctx); - - return std::make_shared( - net::make_strand(executor_), std::move(ssl_ctx), request_, host, - port, read_timeout_, receiver_, logging_cb_); - } else { - std::string port = - uri_components->has_port() ? uri_components->port() : "80"; - - return std::make_shared( - net::make_strand(executor_), request_, host, port, read_timeout_, - receiver_, logging_cb_); + std::optional ssl; + if (service == "https") { + ssl = foxy::make_ssl_ctx(ssl::context::tlsv12_client); + ssl->set_default_verify_paths(); } + + return std::make_shared( + net::make_strand(executor_), request, host, service, connect_timeout_, + read_timeout_, write_timeout_, receiver_, logging_cb_, std::move(ssl)); } } // namespace launchdarkly::sse