From 3788cf5b0f7a3189db72c9754f3919757e10907b Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Fri, 8 Nov 2024 15:36:24 -0800 Subject: [PATCH 01/19] add HttpProxy getter on HttpProperties --- .../config/shared/built/http_properties.hpp | 6 +++++- .../launchdarkly/config/shared/defaults.hpp | 3 ++- libs/common/src/config/http_properties.cpp | 20 ++++++++++++------- 3 files changed, 20 insertions(+), 9 deletions(-) diff --git a/libs/common/include/launchdarkly/config/shared/built/http_properties.hpp b/libs/common/include/launchdarkly/config/shared/built/http_properties.hpp index a21e7143f..b443fcc39 100644 --- a/libs/common/include/launchdarkly/config/shared/built/http_properties.hpp +++ b/libs/common/include/launchdarkly/config/shared/built/http_properties.hpp @@ -30,7 +30,8 @@ class HttpProperties final { std::chrono::milliseconds write_timeout, std::chrono::milliseconds response_timeout, std::map base_headers, - TlsOptions tls); + TlsOptions tls, + std::optional http_proxy); [[nodiscard]] std::chrono::milliseconds ConnectTimeout() const; [[nodiscard]] std::chrono::milliseconds ReadTimeout() const; @@ -41,6 +42,8 @@ class HttpProperties final { [[nodiscard]] TlsOptions const& Tls() const; + [[nodiscard]] std::optional HttpProxy() const; + private: std::chrono::milliseconds connect_timeout_; std::chrono::milliseconds read_timeout_; @@ -48,6 +51,7 @@ class HttpProperties final { std::chrono::milliseconds response_timeout_; std::map base_headers_; TlsOptions tls_; + std::optional http_proxy_; // TODO: Proxy. }; diff --git a/libs/common/include/launchdarkly/config/shared/defaults.hpp b/libs/common/include/launchdarkly/config/shared/defaults.hpp index 0995eaa52..ca19b06da 100644 --- a/libs/common/include/launchdarkly/config/shared/defaults.hpp +++ b/libs/common/include/launchdarkly/config/shared/defaults.hpp @@ -57,7 +57,8 @@ struct Defaults { std::chrono::seconds{10}, std::chrono::seconds{10}, std::map(), - TLS()}; + TLS(), + std::nullopt}; } static auto StreamingConfig() -> shared::built::StreamingConfig { diff --git a/libs/common/src/config/http_properties.cpp b/libs/common/src/config/http_properties.cpp index 07c4213ec..df1b0e8d3 100644 --- a/libs/common/src/config/http_properties.cpp +++ b/libs/common/src/config/http_properties.cpp @@ -4,15 +4,14 @@ namespace launchdarkly::config::shared::built { -TlsOptions::TlsOptions(TlsOptions::VerifyMode verify_mode, +TlsOptions::TlsOptions(VerifyMode const verify_mode, std::optional ca_bundle_path) : verify_mode_(verify_mode), ca_bundle_path_(std::move(ca_bundle_path)) {} -TlsOptions::TlsOptions(TlsOptions::VerifyMode verify_mode) +TlsOptions::TlsOptions(VerifyMode const verify_mode) : TlsOptions(verify_mode, std::nullopt) {} -TlsOptions::TlsOptions() - : TlsOptions(TlsOptions::VerifyMode::kVerifyPeer, std::nullopt) {} +TlsOptions::TlsOptions() : TlsOptions(VerifyMode::kVerifyPeer, std::nullopt) {} TlsOptions::VerifyMode TlsOptions::PeerVerifyMode() const { return verify_mode_; @@ -27,13 +26,15 @@ HttpProperties::HttpProperties(std::chrono::milliseconds connect_timeout, std::chrono::milliseconds write_timeout, std::chrono::milliseconds response_timeout, std::map base_headers, - TlsOptions tls) + TlsOptions tls, + std::optional http_proxy) : connect_timeout_(connect_timeout), read_timeout_(read_timeout), write_timeout_(write_timeout), response_timeout_(response_timeout), base_headers_(std::move(base_headers)), - tls_(std::move(tls)) {} + tls_(std::move(tls)), + http_proxy_(std::move(http_proxy)) {} std::chrono::milliseconds HttpProperties::ConnectTimeout() const { return connect_timeout_; @@ -59,11 +60,16 @@ TlsOptions const& HttpProperties::Tls() const { return tls_; } +std::optional HttpProperties::HttpProxy() const { + return http_proxy_; +} + 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.Tls() == rhs.Tls(); + lhs.BaseHeaders() == rhs.BaseHeaders() && lhs.Tls() == rhs.Tls() && + lhs.HttpProxy() == rhs.HttpProxy(); } bool operator==(TlsOptions const& lhs, TlsOptions const& rhs) { From 1d1a6398b12e66fdc7a2436c84ecc7340e82b1bc Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Fri, 8 Nov 2024 15:42:30 -0800 Subject: [PATCH 02/19] sort asio_requester's constructor order --- .../launchdarkly/config/shared/built/http_properties.hpp | 2 -- .../include/launchdarkly/network/asio_requester.hpp | 9 +++------ 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/libs/common/include/launchdarkly/config/shared/built/http_properties.hpp b/libs/common/include/launchdarkly/config/shared/built/http_properties.hpp index b443fcc39..313db415d 100644 --- a/libs/common/include/launchdarkly/config/shared/built/http_properties.hpp +++ b/libs/common/include/launchdarkly/config/shared/built/http_properties.hpp @@ -52,8 +52,6 @@ class HttpProperties final { std::map base_headers_; TlsOptions tls_; std::optional http_proxy_; - - // TODO: Proxy. }; bool operator==(HttpProperties const& lhs, HttpProperties const& rhs); diff --git a/libs/internal/include/launchdarkly/network/asio_requester.hpp b/libs/internal/include/launchdarkly/network/asio_requester.hpp index a4078953c..eb9f4b958 100644 --- a/libs/internal/include/launchdarkly/network/asio_requester.hpp +++ b/libs/internal/include/launchdarkly/network/asio_requester.hpp @@ -10,8 +10,6 @@ #include #include #include -#include -#include #include #include "foxy/client_session.hpp" @@ -19,7 +17,6 @@ #include #include #include -#include #include #include @@ -158,10 +155,10 @@ class FoxyClient connect_timeout_(connect_timeout), response_timeout_(response_timeout), handler_(std::move(handler)), + resp_(), session_(exec, - launchdarkly::foxy::session_opts{ - ToOptRef(ssl_context_.get()), connect_timeout_}), - resp_() {} + foxy::session_opts{ToOptRef(ssl_context_.get()), + connect_timeout_}) {} void Run() { session_.async_connect(host_, port_, From e25e63751f4ef1f0a49b7c4ab8d084180eac2712 Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Fri, 8 Nov 2024 16:37:10 -0800 Subject: [PATCH 03/19] plumb http proxy config throughout network libs --- .../builders/http_properties_builder.hpp | 8 ++ .../launchdarkly/config/shared/defaults.hpp | 3 +- .../src/config/http_properties_builder.cpp | 15 ++- .../launchdarkly/network/asio_requester.hpp | 14 ++- .../include/launchdarkly/sse/client.hpp | 9 ++ libs/server-sent-events/src/client.cpp | 104 +++++++++++++----- 6 files changed, 121 insertions(+), 32 deletions(-) diff --git a/libs/common/include/launchdarkly/config/shared/builders/http_properties_builder.hpp b/libs/common/include/launchdarkly/config/shared/builders/http_properties_builder.hpp index dbe312df8..2f2de60ec 100644 --- a/libs/common/include/launchdarkly/config/shared/builders/http_properties_builder.hpp +++ b/libs/common/include/launchdarkly/config/shared/builders/http_properties_builder.hpp @@ -178,6 +178,13 @@ class HttpPropertiesBuilder { */ HttpPropertiesBuilder& Tls(TlsBuilder builder); + /** + * Sets an HTTP proxy URL. + * @param http_proxy The proxy, for example 'http://proxy.example.com:8080'. + * @return A reference to this builder. + */ + HttpPropertiesBuilder& HttpProxy(std::string http_proxy); + /** * Build a set of HttpProperties. * @return The built properties. @@ -193,6 +200,7 @@ class HttpPropertiesBuilder { std::string wrapper_version_; std::map base_headers_; TlsBuilder tls_; + std::optional http_proxy_; }; } // namespace launchdarkly::config::shared::builders diff --git a/libs/common/include/launchdarkly/config/shared/defaults.hpp b/libs/common/include/launchdarkly/config/shared/defaults.hpp index ca19b06da..d104f2775 100644 --- a/libs/common/include/launchdarkly/config/shared/defaults.hpp +++ b/libs/common/include/launchdarkly/config/shared/defaults.hpp @@ -106,7 +106,8 @@ struct Defaults { std::chrono::seconds{10}, std::chrono::seconds{10}, std::map(), - TLS()}; + TLS(), + std::nullopt}; } static auto StreamingConfig() -> built::StreamingConfig { diff --git a/libs/common/src/config/http_properties_builder.cpp b/libs/common/src/config/http_properties_builder.cpp index 836eeb397..44d85bc9a 100644 --- a/libs/common/src/config/http_properties_builder.cpp +++ b/libs/common/src/config/http_properties_builder.cpp @@ -51,6 +51,7 @@ HttpPropertiesBuilder::HttpPropertiesBuilder( response_timeout_ = properties.ResponseTimeout(); base_headers_ = properties.BaseHeaders(); tls_ = properties.Tls(); + http_proxy_ = properties.HttpProxy(); } template @@ -121,6 +122,13 @@ HttpPropertiesBuilder& HttpPropertiesBuilder::Tls( return *this; } +template +HttpPropertiesBuilder& HttpPropertiesBuilder::HttpProxy( + std::string http_proxy) { + http_proxy_ = std::move(http_proxy); + return *this; +} + template built::HttpProperties HttpPropertiesBuilder::Build() const { if (!wrapper_name_.empty()) { @@ -128,10 +136,11 @@ built::HttpProperties HttpPropertiesBuilder::Build() const { headers_with_wrapper["X-LaunchDarkly-Wrapper"] = wrapper_name_ + "/" + wrapper_version_; return {connect_timeout_, read_timeout_, write_timeout_, - response_timeout_, headers_with_wrapper, tls_.Build()}; + response_timeout_, headers_with_wrapper, tls_.Build(), + http_proxy_}; } - return {connect_timeout_, read_timeout_, write_timeout_, - response_timeout_, base_headers_, tls_.Build()}; + return {connect_timeout_, read_timeout_, write_timeout_, response_timeout_, + base_headers_, tls_.Build(), http_proxy_}; } template class TlsBuilder; diff --git a/libs/internal/include/launchdarkly/network/asio_requester.hpp b/libs/internal/include/launchdarkly/network/asio_requester.hpp index eb9f4b958..86b51f995 100644 --- a/libs/internal/include/launchdarkly/network/asio_requester.hpp +++ b/libs/internal/include/launchdarkly/network/asio_requester.hpp @@ -132,6 +132,7 @@ class FoxyClient std::shared_ptr ssl_context_; std::string host_; std::string port_; + std::optional http_proxy_; http::request req_; std::chrono::milliseconds connect_timeout_; std::chrono::milliseconds response_timeout_; @@ -144,6 +145,7 @@ class FoxyClient std::shared_ptr ssl_context, std::string host, std::string port, + std::optional http_proxy, http::request req, std::chrono::milliseconds connect_timeout, std::chrono::milliseconds response_timeout, @@ -151,6 +153,7 @@ class FoxyClient : ssl_context_(std::move(ssl_context)), host_(std::move(host)), port_(std::move(port)), + http_proxy_(std::move(http_proxy)), req_(std::move(req)), connect_timeout_(connect_timeout), response_timeout_(response_timeout), @@ -161,7 +164,11 @@ class FoxyClient connect_timeout_}) {} void Run() { - session_.async_connect(host_, port_, + std::string host = host_; + if (http_proxy_) { + host = *http_proxy_; + } + session_.async_connect(host, port_, beast::bind_front_handler(&FoxyClient::OnConnect, shared_from_this())); } @@ -344,8 +351,9 @@ class AsioRequester { } std::make_shared( - exec, std::move(ssl), request->Host(), service, beast_request, - properties.ConnectTimeout(), properties.ResponseTimeout(), + exec, std::move(ssl), request->Host(), service, std::nullopt, + beast_request, properties.ConnectTimeout(), + properties.ResponseTimeout(), [exec, callback, request, this, redirect_count](auto res) { NeedsRedirect(res) ? InnerRequest(exec, MakeRedirectRequest(*request, res), diff --git a/libs/server-sent-events/include/launchdarkly/sse/client.hpp b/libs/server-sent-events/include/launchdarkly/sse/client.hpp index 2bf93989c..dc3999efe 100644 --- a/libs/server-sent-events/include/launchdarkly/sse/client.hpp +++ b/libs/server-sent-events/include/launchdarkly/sse/client.hpp @@ -46,6 +46,14 @@ class Builder { */ Builder(net::any_io_executor ioc, std::string url); + /** + * Sets an HTTP proxy. Requests will be made to this proxy + * instead of the URL passed in the constructor. + * @param proxy The http proxy. + * @return Reference to this builder. + */ + Builder& http_proxy(std::string proxy); + /** * Add a custom header to the initial request. The following headers * are added by default and can be overridden: @@ -163,6 +171,7 @@ class Builder { private: std::string url_; + std::optional http_proxy_; net::any_io_executor executor_; http::request request_; std::optional read_timeout_; diff --git a/libs/server-sent-events/src/client.cpp b/libs/server-sent-events/src/client.cpp index 72e7c8492..bb4d9213c 100644 --- a/libs/server-sent-events/src/client.cpp +++ b/libs/server-sent-events/src/client.cpp @@ -504,6 +504,7 @@ class FoxyClient : public Client, Builder::Builder(net::any_io_executor ctx, std::string url) : url_{std::move(url)}, + http_proxy_{std::nullopt}, executor_{std::move(ctx)}, read_timeout_{std::nullopt}, write_timeout_{std::nullopt}, @@ -521,6 +522,11 @@ Builder::Builder(net::any_io_executor ctx, std::string url) request_.set(http::field::cache_control, "no-cache"); } +Builder& Builder::http_proxy(std::string proxy) { + http_proxy_ = std::move(proxy); + return *this; +} + Builder& Builder::header(std::string const& name, std::string const& value) { request_.set(name, value); return *this; @@ -585,22 +591,43 @@ Builder& Builder::custom_ca_file(std::string path) { return *this; } +std::string host_with_optional_port( + boost::system::result const& url) { + if (url->has_port() && url->port() != "80" && url->port() != "443") { + std::string host = url->host(); + host.append(":"); + host.append(url->port()); + return host; + } + return url->host(); +} + +bool validate_scheme(boost::system::result const& url) { + if (!url->has_scheme()) { + return false; + } + return url->scheme_id() == boost::urls::scheme::http || + url->scheme_id() == boost::urls::scheme::https; +} + std::shared_ptr Builder::build() { auto uri_components = boost::urls::parse_uri(url_); if (!uri_components) { return nullptr; } + if (!validate_scheme(uri_components)) { + return nullptr; + } + auto request = request_; + request.body() = ""; - // If this isn't a post or report, ensure the body is empty. - 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 content_header = request.find(http::field::content_type); + if (request.method() == http::verb::post || + request.method() == http::verb::report) { + // If this is a post or report, set the content type (if not already + // specified.) + if (auto const content_header = request.find(http::field::content_type); content_header == request.end()) { request.set(http::field::content_type, "text/plain"); } @@ -608,24 +635,50 @@ std::shared_ptr Builder::build() { request.prepare_payload(); - std::string host = uri_components->host(); - - request.set(http::field::host, host); + request.set(http::field::host, host_with_optional_port(uri_components)); request.target(uri_components->encoded_target()); - if (uri_components->has_scheme()) { - if (!(uri_components->scheme_id() == boost::urls::scheme::http || - uri_components->scheme_id() == boost::urls::scheme::https)) { + // We need to open a TCP socket to a host and port. The resolver takes + // either a port number, or a "service name". So if the uri has a port + // number, we'll use that - otherwise, we'll assume the service name is the + // scheme. + + std::string tcp_host = uri_components->host(); + std::string tcp_port = uri_components->has_port() + ? uri_components->port() + : uri_components->scheme(); + + // Only allow http_proxy if the original request is HTTP. This is an + // artificial limitation before we support HTTP CONNECT. + if (http_proxy_ && + uri_components->scheme_id() == boost::urls::scheme::http) { + auto http_proxy_components = boost::urls::parse_uri(*http_proxy_); + if (!http_proxy_components) { + return nullptr; + } + + // An HTTP proxy talks http to the client. Setting https scheme + // indicates a misconfiguration. + if (http_proxy_components->has_scheme() && + http_proxy_components->scheme_id() != boost::urls::scheme::http) { return nullptr; } - } - // The resolver accepts either a port number or a service name. If the - // URL specifies a port, use that - otherwise, pass in the scheme as the - // service name (which will be either http or https due to the check - // above.) - std::string service = uri_components->has_port() ? uri_components->port() - : uri_components->scheme(); + // Although a proxy might talk HTTP on port 443, that would be + // very weird and is likely a misconfiguration. + if (http_proxy_components->has_port() && + http_proxy_components->port() == "443") { + return nullptr; + } + + tcp_host = http_proxy_components->host(); + + // If they didn't specify a port, then the uri is of the form + // [http://]foo.bar at this point. Use port 80 if not specified. + tcp_port = http_proxy_components->has_port() + ? http_proxy_components->port() + : "80"; + } std::optional ssl; if (uri_components->scheme_id() == boost::urls::scheme::https) { @@ -644,10 +697,11 @@ std::shared_ptr Builder::build() { } } - return std::make_shared( - net::make_strand(executor_), request, host, service, connect_timeout_, - read_timeout_, write_timeout_, initial_reconnect_delay_, receiver_, - logging_cb_, error_cb_, std::move(ssl)); + return std::make_shared(net::make_strand(executor_), request, + tcp_host, tcp_port, connect_timeout_, + read_timeout_, write_timeout_, + initial_reconnect_delay_, receiver_, + logging_cb_, error_cb_, std::move(ssl)); } } // namespace launchdarkly::sse From 1755b3fe21bae9fdc198c774800dd611692172a8 Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Fri, 8 Nov 2024 17:25:24 -0800 Subject: [PATCH 04/19] http proxy support in eventsource client --- libs/client-sdk/src/data_sources/streaming_data_source.cpp | 4 ++++ .../sources/streaming/streaming_data_source.cpp | 4 ++++ libs/server-sent-events/src/client.cpp | 5 +++-- 3 files changed, 11 insertions(+), 2 deletions(-) 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 d28b36677..add7a1f57 100644 --- a/libs/client-sdk/src/data_sources/streaming_data_source.cpp +++ b/libs/client-sdk/src/data_sources/streaming_data_source.cpp @@ -121,6 +121,10 @@ void StreamingDataSource::Start() { client_builder.custom_ca_file(*ca_file); } + if (http_config_.HttpProxy()) { + client_builder.http_proxy(*http_config_.HttpProxy()); + } + auto weak_self = weak_from_this(); client_builder.receiver([weak_self](launchdarkly::sse::Event const& event) { diff --git a/libs/server-sdk/src/data_systems/background_sync/sources/streaming/streaming_data_source.cpp b/libs/server-sdk/src/data_systems/background_sync/sources/streaming/streaming_data_source.cpp index 778eef153..f831b34d9 100644 --- a/libs/server-sdk/src/data_systems/background_sync/sources/streaming/streaming_data_source.cpp +++ b/libs/server-sdk/src/data_systems/background_sync/sources/streaming/streaming_data_source.cpp @@ -119,6 +119,10 @@ void StreamingDataSource::StartAsync( client_builder.custom_ca_file(*ca_file); } + if (http_config_.HttpProxy()) { + client_builder.http_proxy(*http_config_.HttpProxy()); + } + auto weak_self = weak_from_this(); client_builder.receiver([weak_self](launchdarkly::sse::Event const& event) { diff --git a/libs/server-sent-events/src/client.cpp b/libs/server-sent-events/src/client.cpp index bb4d9213c..5d78f5968 100644 --- a/libs/server-sent-events/src/client.cpp +++ b/libs/server-sent-events/src/client.cpp @@ -659,8 +659,7 @@ std::shared_ptr Builder::build() { // An HTTP proxy talks http to the client. Setting https scheme // indicates a misconfiguration. - if (http_proxy_components->has_scheme() && - http_proxy_components->scheme_id() != boost::urls::scheme::http) { + if (http_proxy_components->scheme_id() != boost::urls::scheme::http) { return nullptr; } @@ -678,6 +677,8 @@ std::shared_ptr Builder::build() { tcp_port = http_proxy_components->has_port() ? http_proxy_components->port() : "80"; + + request.target(url_); } std::optional ssl; From 036d62d0be661432bbda7b6e1aa52b5ce01cc2ec Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Tue, 12 Nov 2024 11:37:22 -0800 Subject: [PATCH 05/19] fix bug in handling of http proxy parsing --- .../launchdarkly/network/asio_requester.hpp | 35 ++++++------- .../launchdarkly/network/http_requester.hpp | 5 ++ libs/internal/src/network/http_requester.cpp | 51 +++++++++++++++---- 3 files changed, 63 insertions(+), 28 deletions(-) diff --git a/libs/internal/include/launchdarkly/network/asio_requester.hpp b/libs/internal/include/launchdarkly/network/asio_requester.hpp index 86b51f995..7c60411e5 100644 --- a/libs/internal/include/launchdarkly/network/asio_requester.hpp +++ b/libs/internal/include/launchdarkly/network/asio_requester.hpp @@ -69,20 +69,26 @@ static http::request MakeBeastRequest( HttpRequest const& request) { http::request beast_request; - beast_request.method(ConvertMethod(request.Method())); - auto body = request.Body(); - if (body) { + if (auto const body = request.Body()) { beast_request.body() = body.value(); } else { beast_request.body() = ""; } + + beast_request.prepare_payload(); + + beast_request.method(ConvertMethod(request.Method())); + if (request.Path().empty()) { beast_request.target("/"); } else { beast_request.target(request.Path()); } - beast_request.prepare_payload(); + if (request.HttpProxyHost()) { + beast_request.target(request.Url()); + } + beast_request.set(http::field::host, request.Host()); auto const& properties = request.Properties(); @@ -132,7 +138,6 @@ class FoxyClient std::shared_ptr ssl_context_; std::string host_; std::string port_; - std::optional http_proxy_; http::request req_; std::chrono::milliseconds connect_timeout_; std::chrono::milliseconds response_timeout_; @@ -145,7 +150,6 @@ class FoxyClient std::shared_ptr ssl_context, std::string host, std::string port, - std::optional http_proxy, http::request req, std::chrono::milliseconds connect_timeout, std::chrono::milliseconds response_timeout, @@ -153,7 +157,6 @@ class FoxyClient : ssl_context_(std::move(ssl_context)), host_(std::move(host)), port_(std::move(port)), - http_proxy_(std::move(http_proxy)), req_(std::move(req)), connect_timeout_(connect_timeout), response_timeout_(response_timeout), @@ -164,11 +167,7 @@ class FoxyClient connect_timeout_}) {} void Run() { - std::string host = host_; - if (http_proxy_) { - host = *http_proxy_; - } - session_.async_connect(host, port_, + session_.async_connect(host_, port_, beast::bind_front_handler(&FoxyClient::OnConnect, shared_from_this())); } @@ -342,8 +341,11 @@ class AsioRequester { const auto& properties = request->Properties(); - std::string service = - request->Port().value_or(request->Https() ? "https" : "http"); + std::string tcp_host = + request->HttpProxyHost().value_or(request->Host()); + + std::string tcp_port = request->HttpProxyPort().value_or( + request->Port().value_or(request->Https() ? "443" : "80")); std::shared_ptr ssl; if (request->Https()) { @@ -351,9 +353,8 @@ class AsioRequester { } std::make_shared( - exec, std::move(ssl), request->Host(), service, std::nullopt, - beast_request, properties.ConnectTimeout(), - properties.ResponseTimeout(), + exec, std::move(ssl), tcp_host, tcp_port, beast_request, + properties.ConnectTimeout(), properties.ResponseTimeout(), [exec, callback, request, this, redirect_count](auto res) { NeedsRedirect(res) ? InnerRequest(exec, MakeRedirectRequest(*request, res), diff --git a/libs/internal/include/launchdarkly/network/http_requester.hpp b/libs/internal/include/launchdarkly/network/http_requester.hpp index 26084eb25..2c24ccc34 100644 --- a/libs/internal/include/launchdarkly/network/http_requester.hpp +++ b/libs/internal/include/launchdarkly/network/http_requester.hpp @@ -96,6 +96,9 @@ class HttpRequest { [[nodiscard]] bool Https() const; + [[nodiscard]] std::optional const& HttpProxyHost() const; + [[nodiscard]] std::optional const& HttpProxyPort() const; + /** * Indicates if a request is valid. Meaning that it has correctly formed * data that can be used to make an http request. @@ -126,6 +129,8 @@ class HttpRequest { std::string host_; std::optional port_; std::string path_; + std::optional http_proxy_host_; + std::optional http_proxy_port_; std::map params_; bool is_https_; bool valid_; diff --git a/libs/internal/src/network/http_requester.cpp b/libs/internal/src/network/http_requester.cpp index 67b917ce3..86fdd13a8 100644 --- a/libs/internal/src/network/http_requester.cpp +++ b/libs/internal/src/network/http_requester.cpp @@ -33,10 +33,10 @@ HttpResult::HeadersType const& HttpResult::Headers() const { HttpResult::HttpResult(HttpResult::StatusCode status, std::optional body, HttpResult::HeadersType headers) - : status_(status), + : error_(false), + status_(status), body_(std::move(body)), - headers_(std::move(headers)), - error_(false) {} + headers_(std::move(headers)) {} bool HttpResult::IsError() const { return error_; @@ -47,7 +47,7 @@ std::optional const& HttpResult::ErrorMessage() const { } HttpResult::HttpResult(std::optional error_message) - : error_message_(std::move(error_message)), error_(true), status_(0) {} + : error_(true), error_message_(std::move(error_message)), status_(0) {} HttpMethod HttpRequest::Method() const { return method_; @@ -73,14 +73,26 @@ std::string const& HttpRequest::Url() const { return url_; } +std::optional const& HttpRequest::HttpProxyHost() const { + return http_proxy_host_; +} + +std::optional const& HttpRequest::HttpProxyPort() const { + return http_proxy_port_; +} + +// What I'm doing: supporting HTTP proxy in the AsioEventRequestor. +// It's annoying that we have usage in SSE client but also here that both needs +// to be updated. + HttpRequest::HttpRequest(std::string const& url, HttpMethod method, config::shared::built::HttpProperties properties, HttpRequest::BodyType body) - : properties_(std::move(properties)), + : url_(url), method_(method), body_(std::move(body)), - url_(url), + properties_(std::move(properties)), port_(std::nullopt) { auto uri_components = boost::urls::parse_uri(url); @@ -108,20 +120,37 @@ HttpRequest::HttpRequest(std::string const& url, if (uri_components->has_port()) { port_ = uri_components->port(); } + + if (properties_.HttpProxy()) { + auto const http_proxy = properties_.HttpProxy().value(); + auto http_proxy_uri_components = boost::urls::parse_uri(http_proxy); + if (!http_proxy_uri_components) { + valid_ = false; + return; + } + + http_proxy_host_ = http_proxy_uri_components->host(); + if (http_proxy_uri_components->has_port()) { + http_proxy_port_ = http_proxy_uri_components->port(); + } + } + valid_ = true; } HttpRequest::HttpRequest(HttpRequest& base_request, config::shared::built::HttpProperties properties) - : properties_(std::move(properties)), + : url_(base_request.url_), + method_(base_request.method_), + body_(std::move(base_request.body_)), + properties_(std::move(properties)), host_(base_request.host_), port_(base_request.port_), path_(base_request.path_), + http_proxy_host_(base_request.http_proxy_host_), + http_proxy_port_(base_request.http_proxy_port_), is_https_(base_request.is_https_), - valid_(base_request.valid_), - url_(base_request.url_), - method_(base_request.method_), - body_(std::move(base_request.body_)) {} + valid_(base_request.valid_) {} std::optional const& HttpRequest::Port() const { return port_; From 05a47f475fb75669a657f0ccbce40629c8886ac4 Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Tue, 12 Nov 2024 12:56:25 -0800 Subject: [PATCH 06/19] add support for getting http_proxy from env --- .../src/config/http_properties_builder.cpp | 27 ++++++++++++++----- .../launchdarkly/network/asio_requester.hpp | 24 ++++++++++++++--- libs/internal/src/network/http_requester.cpp | 5 ++++ libs/server-sent-events/src/client.cpp | 8 +++--- 4 files changed, 49 insertions(+), 15 deletions(-) diff --git a/libs/common/src/config/http_properties_builder.cpp b/libs/common/src/config/http_properties_builder.cpp index 44d85bc9a..8fa08b8c4 100644 --- a/libs/common/src/config/http_properties_builder.cpp +++ b/libs/common/src/config/http_properties_builder.cpp @@ -129,18 +129,31 @@ HttpPropertiesBuilder& HttpPropertiesBuilder::HttpProxy( return *this; } +static std::optional HttpProxyFromEnv() { + if (char const* http_proxy = std::getenv("http_proxy")) { + if (strlen(http_proxy) > 0) { + return http_proxy; + } + } + return std::nullopt; +} + template built::HttpProperties HttpPropertiesBuilder::Build() const { + std::map headers = base_headers_; + if (!wrapper_name_.empty()) { - std::map headers_with_wrapper(base_headers_); - headers_with_wrapper["X-LaunchDarkly-Wrapper"] = + headers["X-LaunchDarkly-Wrapper"] = wrapper_name_ + "/" + wrapper_version_; - return {connect_timeout_, read_timeout_, write_timeout_, - response_timeout_, headers_with_wrapper, tls_.Build(), - http_proxy_}; } - return {connect_timeout_, read_timeout_, write_timeout_, response_timeout_, - base_headers_, tls_.Build(), http_proxy_}; + + return {connect_timeout_, + read_timeout_, + write_timeout_, + response_timeout_, + std::move(headers), + tls_.Build(), + http_proxy_ ? *http_proxy_ : HttpProxyFromEnv()}; } template class TlsBuilder; diff --git a/libs/internal/include/launchdarkly/network/asio_requester.hpp b/libs/internal/include/launchdarkly/network/asio_requester.hpp index 7c60411e5..46fcd3400 100644 --- a/libs/internal/include/launchdarkly/network/asio_requester.hpp +++ b/libs/internal/include/launchdarkly/network/asio_requester.hpp @@ -65,6 +65,17 @@ static http::verb ConvertMethod(HttpMethod method) { launchdarkly::detail::unreachable(); } +static std::string HostWithOptionalPort( + std::string host, + std::optional const& port) { + if (!port || (*port == "80" || *port == "443")) { + return host; + } + host.append(":"); + host.append(*port); + return host; +} + static http::request MakeBeastRequest( HttpRequest const& request) { http::request beast_request; @@ -85,15 +96,18 @@ static http::request MakeBeastRequest( beast_request.target(request.Path()); } + // If we have an HTTP proxy set, we need to pass the entire path + // in the HTTP request's target (so the proxy knows where to send it). if (request.HttpProxyHost()) { beast_request.target(request.Url()); } - beast_request.set(http::field::host, request.Host()); + beast_request.set(http::field::host, + HostWithOptionalPort(request.Host(), request.Port())); auto const& properties = request.Properties(); - for (auto const& pair : request.Properties().BaseHeaders()) { + for (auto const& pair : properties.BaseHeaders()) { beast_request.set(pair.first, pair.second); } @@ -348,7 +362,11 @@ class AsioRequester { request->Port().value_or(request->Https() ? "443" : "80")); std::shared_ptr ssl; - if (request->Https()) { + if (request->Https() && !request->HttpProxyHost()) { + // If the user requested an HTTP proxy, then we will be making + // an HTTP request to the proxy (and then the proxy will make + // an HTTPS request on our behalf to the target.) So, don't + // use the SSL client if we have HTTP proxy configured. ssl = this->ssl_ctx_; } diff --git a/libs/internal/src/network/http_requester.cpp b/libs/internal/src/network/http_requester.cpp index 86fdd13a8..318db02a9 100644 --- a/libs/internal/src/network/http_requester.cpp +++ b/libs/internal/src/network/http_requester.cpp @@ -130,6 +130,11 @@ HttpRequest::HttpRequest(std::string const& url, } http_proxy_host_ = http_proxy_uri_components->host(); + if (http_proxy_host_ && *http_proxy_host_ == "") { + // This will be true if there is no scheme, like 'localhost:8080'. + valid_ = false; + return; + } if (http_proxy_uri_components->has_port()) { http_proxy_port_ = http_proxy_uri_components->port(); } diff --git a/libs/server-sent-events/src/client.cpp b/libs/server-sent-events/src/client.cpp index 5d78f5968..91353f623 100644 --- a/libs/server-sent-events/src/client.cpp +++ b/libs/server-sent-events/src/client.cpp @@ -648,10 +648,7 @@ std::shared_ptr Builder::build() { ? uri_components->port() : uri_components->scheme(); - // Only allow http_proxy if the original request is HTTP. This is an - // artificial limitation before we support HTTP CONNECT. - if (http_proxy_ && - uri_components->scheme_id() == boost::urls::scheme::http) { + if (http_proxy_) { auto http_proxy_components = boost::urls::parse_uri(*http_proxy_); if (!http_proxy_components) { return nullptr; @@ -682,7 +679,8 @@ std::shared_ptr Builder::build() { } std::optional ssl; - if (uri_components->scheme_id() == boost::urls::scheme::https) { + if (!http_proxy_ && + uri_components->scheme_id() == boost::urls::scheme::https) { ssl = launchdarkly::foxy::make_ssl_ctx(ssl::context::tlsv12_client); ssl->set_default_verify_paths(); From 59db06ddae02367a00e83dc0ae05875f43cb41c3 Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Tue, 12 Nov 2024 13:17:40 -0800 Subject: [PATCH 07/19] docs --- .../client_side/bindings/c/config/builder.h | 22 +++++++++++++++++++ libs/client-sdk/src/bindings/c/builder.cpp | 9 ++++++++ .../builders/http_properties_builder.hpp | 22 +++++++++++++------ .../src/config/http_properties_builder.cpp | 2 +- .../streaming/streaming_data_source.cpp | 4 ---- libs/server-sent-events/src/client.cpp | 7 ------ 6 files changed, 47 insertions(+), 19 deletions(-) diff --git a/libs/client-sdk/include/launchdarkly/client_side/bindings/c/config/builder.h b/libs/client-sdk/include/launchdarkly/client_side/bindings/c/config/builder.h index 81c5cb64c..cc8a7c9ba 100644 --- a/libs/client-sdk/include/launchdarkly/client_side/bindings/c/config/builder.h +++ b/libs/client-sdk/include/launchdarkly/client_side/bindings/c/config/builder.h @@ -421,6 +421,28 @@ LDClientConfigBuilder_HttpProperties_Header(LDClientConfigBuilder b, char const* key, char const* value); +/** + * Specifies an HTTP proxy which the client should use to communicate + * with LaunchDarkly. + * + * SDK <-- HTTP, plaintext --> Proxy <-- HTTPS --> LaunchDarkly + * + * This setting affects streaming mode, polling mode, and event delivery. + * The argument should be of the form: 'http://proxy.example.com:8080'. + * + * The scheme must be 'http' and the port is optional (80 if not + * specified.) + * + * The SDK respects the 'http_proxy' environment variable as an alternative + * to this method. If both are set, this method takes precedence. + * + * @param b Client config builder. Must not be NULL. + * @param http_proxy HTTP proxy URL. Must not be NULL. + */ +LD_EXPORT(void) +LDClientConfigBuilder_HttpProperties_HttpProxy(LDClientConfigBuilder b, + char const* http_proxy); + /** * Sets the TLS options builder. The builder is automatically freed. * diff --git a/libs/client-sdk/src/bindings/c/builder.cpp b/libs/client-sdk/src/bindings/c/builder.cpp index 7994207e4..6de3bb647 100644 --- a/libs/client-sdk/src/bindings/c/builder.cpp +++ b/libs/client-sdk/src/bindings/c/builder.cpp @@ -311,6 +311,15 @@ LDClientConfigBuilder_HttpProperties_Header(LDClientConfigBuilder b, TO_BUILDER(b)->HttpProperties().Header(key, value); } +LD_EXPORT(void) +LDClientConfigBuilder_HttpProperties_HttpProxy(LDClientConfigBuilder b, + char const* http_proxy) { + LD_ASSERT_NOT_NULL(b); + LD_ASSERT_NOT_NULL(http_proxy); + + TO_BUILDER(b)->HttpProperties().HttpProxy(http_proxy); +} + LD_EXPORT(void) LDClientConfigBuilder_HttpProperties_Tls( LDClientConfigBuilder b, diff --git a/libs/common/include/launchdarkly/config/shared/builders/http_properties_builder.hpp b/libs/common/include/launchdarkly/config/shared/builders/http_properties_builder.hpp index 2f2de60ec..ffe1a5f3e 100644 --- a/libs/common/include/launchdarkly/config/shared/builders/http_properties_builder.hpp +++ b/libs/common/include/launchdarkly/config/shared/builders/http_properties_builder.hpp @@ -179,16 +179,24 @@ class HttpPropertiesBuilder { HttpPropertiesBuilder& Tls(TlsBuilder builder); /** - * Sets an HTTP proxy URL. - * @param http_proxy The proxy, for example 'http://proxy.example.com:8080'. - * @return A reference to this builder. + * Specifies an HTTP proxy which the client should use to communicate + * with LaunchDarkly. + * + * SDK <-- HTTP, plaintext --> Proxy <-- HTTPS --> LaunchDarkly + * + * This setting affects streaming mode, polling mode, and event delivery. + * The argument should be of the form: 'http://proxy.example.com:8080'. + * + * The scheme must be 'http' and the port is optional (80 if not + * specified.) + * + * The SDK respects the 'http_proxy' environment variable as an alternative + * to this method. If both are set, this method takes precedence. + * + * @param http_proxy HTTP proxy URL. */ HttpPropertiesBuilder& HttpProxy(std::string http_proxy); - /** - * Build a set of HttpProperties. - * @return The built properties. - */ [[nodiscard]] built::HttpProperties Build() const; private: diff --git a/libs/common/src/config/http_properties_builder.cpp b/libs/common/src/config/http_properties_builder.cpp index 8fa08b8c4..3ddfb6651 100644 --- a/libs/common/src/config/http_properties_builder.cpp +++ b/libs/common/src/config/http_properties_builder.cpp @@ -153,7 +153,7 @@ built::HttpProperties HttpPropertiesBuilder::Build() const { response_timeout_, std::move(headers), tls_.Build(), - http_proxy_ ? *http_proxy_ : HttpProxyFromEnv()}; + http_proxy_.has_value() ? http_proxy_ : HttpProxyFromEnv()}; } template class TlsBuilder; diff --git a/libs/server-sdk/src/data_systems/background_sync/sources/streaming/streaming_data_source.cpp b/libs/server-sdk/src/data_systems/background_sync/sources/streaming/streaming_data_source.cpp index f831b34d9..778eef153 100644 --- a/libs/server-sdk/src/data_systems/background_sync/sources/streaming/streaming_data_source.cpp +++ b/libs/server-sdk/src/data_systems/background_sync/sources/streaming/streaming_data_source.cpp @@ -119,10 +119,6 @@ void StreamingDataSource::StartAsync( client_builder.custom_ca_file(*ca_file); } - if (http_config_.HttpProxy()) { - client_builder.http_proxy(*http_config_.HttpProxy()); - } - auto weak_self = weak_from_this(); client_builder.receiver([weak_self](launchdarkly::sse::Event const& event) { diff --git a/libs/server-sent-events/src/client.cpp b/libs/server-sent-events/src/client.cpp index 91353f623..cd39354b0 100644 --- a/libs/server-sent-events/src/client.cpp +++ b/libs/server-sent-events/src/client.cpp @@ -660,13 +660,6 @@ std::shared_ptr Builder::build() { return nullptr; } - // Although a proxy might talk HTTP on port 443, that would be - // very weird and is likely a misconfiguration. - if (http_proxy_components->has_port() && - http_proxy_components->port() == "443") { - return nullptr; - } - tcp_host = http_proxy_components->host(); // If they didn't specify a port, then the uri is of the form From 967b1251595b001bc1d99bdb37083add86045952 Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Tue, 12 Nov 2024 13:21:25 -0800 Subject: [PATCH 08/19] fix build --- libs/common/src/config/http_properties_builder.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/libs/common/src/config/http_properties_builder.cpp b/libs/common/src/config/http_properties_builder.cpp index 3ddfb6651..e68dd4ed2 100644 --- a/libs/common/src/config/http_properties_builder.cpp +++ b/libs/common/src/config/http_properties_builder.cpp @@ -1,9 +1,10 @@ -#include - #include #include #include +#include +#include + namespace launchdarkly::config::shared::builders { template From 352283d3d3264d00bf8f6bccaa7a884e1de086f0 Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Tue, 12 Nov 2024 14:05:09 -0800 Subject: [PATCH 09/19] only enable for client side SDK --- .../builders/http_properties_builder.hpp | 17 ++++++++++++++++- .../src/config/http_properties_builder.cpp | 19 ++++++++++--------- 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/libs/common/include/launchdarkly/config/shared/builders/http_properties_builder.hpp b/libs/common/include/launchdarkly/config/shared/builders/http_properties_builder.hpp index ffe1a5f3e..8f6496283 100644 --- a/libs/common/include/launchdarkly/config/shared/builders/http_properties_builder.hpp +++ b/libs/common/include/launchdarkly/config/shared/builders/http_properties_builder.hpp @@ -7,9 +7,18 @@ #include #include +#include namespace launchdarkly::config::shared::builders { +// namespace detail { +// template +// struct is_client_sdk : std::false_type {}; +// +// template <> +// struct is_client_sdk : std::true_type {}; +// } // namespace detail + /** * Class used for building TLS options used within HttpProperties. * @tparam SDK The SDK type to build options for. This affects the default @@ -179,7 +188,10 @@ class HttpPropertiesBuilder { HttpPropertiesBuilder& Tls(TlsBuilder builder); /** - * Specifies an HTTP proxy which the client should use to communicate + * NOTE: This method and the associated 'http_proxy' environment variable + * are only available for the client-side SDK. + * + * Specifies an HTTP proxy which the SDK should use to communicate * with LaunchDarkly. * * SDK <-- HTTP, plaintext --> Proxy <-- HTTPS --> LaunchDarkly @@ -195,6 +207,9 @@ class HttpPropertiesBuilder { * * @param http_proxy HTTP proxy URL. */ + template < + typename T = SDK, + std::enable_if_t, int> = 0> HttpPropertiesBuilder& HttpProxy(std::string http_proxy); [[nodiscard]] built::HttpProperties Build() const; diff --git a/libs/common/src/config/http_properties_builder.cpp b/libs/common/src/config/http_properties_builder.cpp index e68dd4ed2..3fde4038b 100644 --- a/libs/common/src/config/http_properties_builder.cpp +++ b/libs/common/src/config/http_properties_builder.cpp @@ -1,9 +1,7 @@ #include #include -#include #include -#include namespace launchdarkly::config::shared::builders { @@ -124,6 +122,7 @@ HttpPropertiesBuilder& HttpPropertiesBuilder::Tls( } template +template , int>> HttpPropertiesBuilder& HttpPropertiesBuilder::HttpProxy( std::string http_proxy) { http_proxy_ = std::move(http_proxy); @@ -148,13 +147,15 @@ built::HttpProperties HttpPropertiesBuilder::Build() const { wrapper_name_ + "/" + wrapper_version_; } - return {connect_timeout_, - read_timeout_, - write_timeout_, - response_timeout_, - std::move(headers), - tls_.Build(), - http_proxy_.has_value() ? http_proxy_ : HttpProxyFromEnv()}; + std::optional http_proxy; + + if constexpr (std::is_same_v) { + http_proxy = http_proxy_.has_value() ? http_proxy_ : HttpProxyFromEnv(); + } + + return {connect_timeout_, read_timeout_, write_timeout_, + response_timeout_, std::move(headers), tls_.Build(), + http_proxy}; } template class TlsBuilder; From fa0e186ede2314645a5dbc7f2ca1618a02373fc0 Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Tue, 12 Nov 2024 14:10:50 -0800 Subject: [PATCH 10/19] simplify tempalte usage --- .../shared/builders/http_properties_builder.hpp | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/libs/common/include/launchdarkly/config/shared/builders/http_properties_builder.hpp b/libs/common/include/launchdarkly/config/shared/builders/http_properties_builder.hpp index 8f6496283..a152d2828 100644 --- a/libs/common/include/launchdarkly/config/shared/builders/http_properties_builder.hpp +++ b/libs/common/include/launchdarkly/config/shared/builders/http_properties_builder.hpp @@ -11,14 +11,6 @@ namespace launchdarkly::config::shared::builders { -// namespace detail { -// template -// struct is_client_sdk : std::false_type {}; -// -// template <> -// struct is_client_sdk : std::true_type {}; -// } // namespace detail - /** * Class used for building TLS options used within HttpProperties. * @tparam SDK The SDK type to build options for. This affects the default @@ -207,9 +199,8 @@ class HttpPropertiesBuilder { * * @param http_proxy HTTP proxy URL. */ - template < - typename T = SDK, - std::enable_if_t, int> = 0> + template , int> = 0> HttpPropertiesBuilder& HttpProxy(std::string http_proxy); [[nodiscard]] built::HttpProperties Build() const; From 09a077c0561a7a4427a92a102318c1b02bfa0b48 Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Tue, 12 Nov 2024 14:28:05 -0800 Subject: [PATCH 11/19] simplications --- .../include/launchdarkly/network/asio_requester.hpp | 2 ++ libs/server-sent-events/src/client.cpp | 8 +++++--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/libs/internal/include/launchdarkly/network/asio_requester.hpp b/libs/internal/include/launchdarkly/network/asio_requester.hpp index 46fcd3400..50bb7f340 100644 --- a/libs/internal/include/launchdarkly/network/asio_requester.hpp +++ b/libs/internal/include/launchdarkly/network/asio_requester.hpp @@ -65,6 +65,8 @@ static http::verb ConvertMethod(HttpMethod method) { launchdarkly::detail::unreachable(); } +// Returns the host with a port appended, if the port is not 80 or 443. +// Although not strictly necessary, it is meant to match what browsers do. static std::string HostWithOptionalPort( std::string host, std::optional const& port) { diff --git a/libs/server-sent-events/src/client.cpp b/libs/server-sent-events/src/client.cpp index cd39354b0..815275d93 100644 --- a/libs/server-sent-events/src/client.cpp +++ b/libs/server-sent-events/src/client.cpp @@ -591,6 +591,8 @@ Builder& Builder::custom_ca_file(std::string path) { return *this; } +// Returns the host with a port appended, if the port is not 80 or 443. +// Although not strictly necessary, it is meant to match what browsers do. std::string host_with_optional_port( boost::system::result const& url) { if (url->has_port() && url->port() != "80" && url->port() != "443") { @@ -662,11 +664,11 @@ std::shared_ptr Builder::build() { tcp_host = http_proxy_components->host(); - // If they didn't specify a port, then the uri is of the form - // [http://]foo.bar at this point. Use port 80 if not specified. + // If they didn't specify a port, use the scheme as the service + // (which we've enforced to be http above.) tcp_port = http_proxy_components->has_port() ? http_proxy_components->port() - : "80"; + : http_proxy_components->scheme(); request.target(url_); } From a9e140553545b7192d68277a8f53e13ebbafde40 Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Tue, 12 Nov 2024 14:53:28 -0800 Subject: [PATCH 12/19] define HttpProxy method in header --- .../config/shared/builders/http_properties_builder.hpp | 5 ++++- libs/common/src/config/http_properties_builder.cpp | 8 -------- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/libs/common/include/launchdarkly/config/shared/builders/http_properties_builder.hpp b/libs/common/include/launchdarkly/config/shared/builders/http_properties_builder.hpp index a152d2828..afd83ff6e 100644 --- a/libs/common/include/launchdarkly/config/shared/builders/http_properties_builder.hpp +++ b/libs/common/include/launchdarkly/config/shared/builders/http_properties_builder.hpp @@ -201,7 +201,10 @@ class HttpPropertiesBuilder { */ template , int> = 0> - HttpPropertiesBuilder& HttpProxy(std::string http_proxy); + HttpPropertiesBuilder& HttpProxy(std::string http_proxy) { + http_proxy_ = std::move(http_proxy); + return *this; + } [[nodiscard]] built::HttpProperties Build() const; diff --git a/libs/common/src/config/http_properties_builder.cpp b/libs/common/src/config/http_properties_builder.cpp index 3fde4038b..f4081c3dc 100644 --- a/libs/common/src/config/http_properties_builder.cpp +++ b/libs/common/src/config/http_properties_builder.cpp @@ -121,14 +121,6 @@ HttpPropertiesBuilder& HttpPropertiesBuilder::Tls( return *this; } -template -template , int>> -HttpPropertiesBuilder& HttpPropertiesBuilder::HttpProxy( - std::string http_proxy) { - http_proxy_ = std::move(http_proxy); - return *this; -} - static std::optional HttpProxyFromEnv() { if (char const* http_proxy = std::getenv("http_proxy")) { if (strlen(http_proxy) > 0) { From 251832799133d4cf43a0cc882cb1046ed3352ab7 Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Mon, 18 Nov 2024 10:49:13 -0800 Subject: [PATCH 13/19] fix REPORT method in client side streaming requests --- libs/server-sent-events/src/client.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libs/server-sent-events/src/client.cpp b/libs/server-sent-events/src/client.cpp index 815275d93..a522fe309 100644 --- a/libs/server-sent-events/src/client.cpp +++ b/libs/server-sent-events/src/client.cpp @@ -623,7 +623,6 @@ std::shared_ptr Builder::build() { } auto request = request_; - request.body() = ""; if (request.method() == http::verb::post || request.method() == http::verb::report) { @@ -633,6 +632,9 @@ std::shared_ptr Builder::build() { content_header == request.end()) { request.set(http::field::content_type, "text/plain"); } + } else { + // If this is *not* a post or report, then make sure no body is sent. + request.body() = ""; } request.prepare_payload(); From 11384dfb42087a72f05d1484b1ef77a7216152c8 Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Mon, 18 Nov 2024 11:03:59 -0800 Subject: [PATCH 14/19] add contract test capability --- .../client-contract-tests/src/entity_manager.cpp | 6 ++++++ .../data-model/include/data_model/data_model.hpp | 10 +++++++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/contract-tests/client-contract-tests/src/entity_manager.cpp b/contract-tests/client-contract-tests/src/entity_manager.cpp index 58eef6cfd..ac1d2587b 100644 --- a/contract-tests/client-contract-tests/src/entity_manager.cpp +++ b/contract-tests/client-contract-tests/src/entity_manager.cpp @@ -38,6 +38,12 @@ std::optional EntityManager::create(ConfigParams const& in) { .PollingBaseUrl(default_endpoints.PollingBaseUrl()) .StreamingBaseUrl(default_endpoints.StreamingBaseUrl()); + if (in.proxy) { + if (in.proxy->httpProxy) { + config_builder.HttpProperties().HttpProxy(*in.proxy->httpProxy); + } + } + if (in.serviceEndpoints) { if (in.serviceEndpoints->streaming) { endpoints.StreamingBaseUrl(*in.serviceEndpoints->streaming); diff --git a/contract-tests/data-model/include/data_model/data_model.hpp b/contract-tests/data-model/include/data_model/data_model.hpp index 12ca2e28d..7346f0e41 100644 --- a/contract-tests/data-model/include/data_model/data_model.hpp +++ b/contract-tests/data-model/include/data_model/data_model.hpp @@ -37,6 +37,12 @@ NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE_WITH_DEFAULT(ConfigTLSParams, skipVerifyPeer, customCAFile); +struct ConfigProxyParams { + std::optional httpProxy; +}; + +NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE_WITH_DEFAULT(ConfigProxyParams, httpProxy); + struct ConfigStreamingParams { std::optional baseUri; std::optional initialRetryDelayMs; @@ -118,6 +124,7 @@ struct ConfigParams { std::optional clientSide; std::optional tags; std::optional tls; + std::optional proxy; }; NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE_WITH_DEFAULT(ConfigParams, @@ -130,7 +137,8 @@ NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE_WITH_DEFAULT(ConfigParams, serviceEndpoints, clientSide, tags, - tls); + tls, + proxy); struct ContextSingleParams { std::optional kind; From 0b9fc7269e168e4e17e0e492764d72fcfe00b38d Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Mon, 18 Nov 2024 11:40:36 -0800 Subject: [PATCH 15/19] add test capability --- contract-tests/client-contract-tests/src/main.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/contract-tests/client-contract-tests/src/main.cpp b/contract-tests/client-contract-tests/src/main.cpp index 3c4fc4883..4e71fcf69 100644 --- a/contract-tests/client-contract-tests/src/main.cpp +++ b/contract-tests/client-contract-tests/src/main.cpp @@ -46,7 +46,8 @@ int main(int argc, char* argv[]) { srv.add_capability("tls:verify-peer"); srv.add_capability("tls:skip-verify-peer"); srv.add_capability("tls:custom-ca"); - + srv.add_capability("http-proxy"); + net::signal_set signals{ioc, SIGINT, SIGTERM}; boost::asio::spawn(ioc.get_executor(), [&](auto yield) mutable { From f13b133805a99832b3a616c00f00b58a119bf2c9 Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Thu, 21 Nov 2024 17:05:10 -0800 Subject: [PATCH 16/19] refactor to http proxy builder --- .../client_side/bindings/c/config/builder.h | 23 ++++++ libs/client-sdk/src/bindings/c/builder.cpp | 32 ++++++-- .../data_sources/streaming_data_source.cpp | 4 +- .../builders/http_properties_builder.hpp | 77 ++++++++++++------- .../config/shared/built/http_properties.hpp | 21 ++++- .../launchdarkly/config/shared/defaults.hpp | 8 +- libs/common/src/config/http_properties.cpp | 28 +++++-- .../src/config/http_properties_builder.cpp | 62 ++++++++++----- libs/internal/src/network/http_requester.cpp | 4 +- 9 files changed, 193 insertions(+), 66 deletions(-) diff --git a/libs/client-sdk/include/launchdarkly/client_side/bindings/c/config/builder.h b/libs/client-sdk/include/launchdarkly/client_side/bindings/c/config/builder.h index cc8a7c9ba..dc91f9487 100644 --- a/libs/client-sdk/include/launchdarkly/client_side/bindings/c/config/builder.h +++ b/libs/client-sdk/include/launchdarkly/client_side/bindings/c/config/builder.h @@ -24,6 +24,9 @@ typedef struct _LDPersistenceCustomBuilder* LDPersistenceCustomBuilder; typedef struct _LDClientHttpPropertiesTlsBuilder* LDClientHttpPropertiesTlsBuilder; +typedef struct _LDClientHttpPropertiesProxyBuilder* + LDClientHttpPropertiesProxyBuilder; + typedef void (*SetFn)(char const* storage_namespace, char const* key, char const* data, @@ -421,6 +424,26 @@ LDClientConfigBuilder_HttpProperties_Header(LDClientConfigBuilder b, char const* key, char const* value); +/** + * Creates a new proxy options builder for the HttpProperties builder. + * + * If not passed into the HttpProperties + * builder, must be manually freed with LDClientHttpPropertiesProxyBuilder_Free. + * + * @return New builder for proxy options. + */ +LD_EXPORT(LDClientHttpPropertiesProxyBuilder) +LDClientHttpPropertiesProxyBuilder_New(void); + +/** + * Frees a proxy options builder. Do not call if the builder was consumed by + * the HttpProperties builder. + * + * @param b Builder to free. + */ +LD_EXPORT(void) +LDClientHttpPropertiesProxyBuilder_Free(LDClientHttpPropertiesProxyBuilder b); + /** * Specifies an HTTP proxy which the client should use to communicate * with LaunchDarkly. diff --git a/libs/client-sdk/src/bindings/c/builder.cpp b/libs/client-sdk/src/bindings/c/builder.cpp index 6de3bb647..019a1b14d 100644 --- a/libs/client-sdk/src/bindings/c/builder.cpp +++ b/libs/client-sdk/src/bindings/c/builder.cpp @@ -41,11 +41,18 @@ using namespace launchdarkly::client_side; #define FROM_CUSTOM_PERSISTENCE_BUILDER(ptr) \ (reinterpret_cast(ptr)) -#define TO_TLS_BUILDER(ptr) (reinterpret_cast(ptr)) +#define TO_TLS_BUILDER(ptr) \ + (reinterpret_cast(ptr)) #define FROM_TLS_BUILDER(ptr) \ (reinterpret_cast(ptr)) +#define TO_PROXY_BUILDER(ptr) \ + (reinterpret_cast(ptr)) + +#define FROM_PROXY_BUILDER(ptr) \ + (reinterpret_cast(ptr)) + class PersistenceImplementationWrapper : public IPersistence { public: explicit PersistenceImplementationWrapper(LDPersistence impl) @@ -311,13 +318,26 @@ LDClientConfigBuilder_HttpProperties_Header(LDClientConfigBuilder b, TO_BUILDER(b)->HttpProperties().Header(key, value); } +LD_EXPORT(LDClientHttpPropertiesProxyBuilder) +LDClientHttpPropertiesProxyBuilder_New(void) { + return FROM_PROXY_BUILDER(new HttpPropertiesBuilder::ProxyBuilder()); +} + LD_EXPORT(void) -LDClientConfigBuilder_HttpProperties_HttpProxy(LDClientConfigBuilder b, - char const* http_proxy) { +LDClientHttpPropertiesProxyBuilder_Free(LDClientHttpPropertiesProxyBuilder b) { + delete TO_PROXY_BUILDER(b); +} + +LD_EXPORT(void) +LDClientConfigBuilder_HttpProperties_HttpProxy( + LDClientConfigBuilder b, + LDClientHttpPropertiesProxyBuilder proxy_builder) { LD_ASSERT_NOT_NULL(b); - LD_ASSERT_NOT_NULL(http_proxy); + LD_ASSERT_NOT_NULL(proxy_builder); + + TO_BUILDER(b)->HttpProperties().Proxy(*TO_PROXY_BUILDER(proxy_builder)); - TO_BUILDER(b)->HttpProperties().HttpProxy(http_proxy); + LDClientHttpPropertiesProxyBuilder_Free(proxy_builder); } LD_EXPORT(void) @@ -353,7 +373,7 @@ LDClientHttpPropertiesTlsBuilder_CustomCAFile( LD_EXPORT(LDClientHttpPropertiesTlsBuilder) LDClientHttpPropertiesTlsBuilder_New(void) { - return FROM_TLS_BUILDER(new TlsBuilder()); + return FROM_TLS_BUILDER(new HttpPropertiesBuilder::TlsBuilder()); } LD_EXPORT(void) 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 add7a1f57..cb51ab1c4 100644 --- a/libs/client-sdk/src/data_sources/streaming_data_source.cpp +++ b/libs/client-sdk/src/data_sources/streaming_data_source.cpp @@ -121,8 +121,8 @@ void StreamingDataSource::Start() { client_builder.custom_ca_file(*ca_file); } - if (http_config_.HttpProxy()) { - client_builder.http_proxy(*http_config_.HttpProxy()); + if (http_config_.Proxy().HttpProxy()) { + client_builder.http_proxy(*http_config_.Proxy().HttpProxy()); } auto weak_self = weak_from_this(); diff --git a/libs/common/include/launchdarkly/config/shared/builders/http_properties_builder.hpp b/libs/common/include/launchdarkly/config/shared/builders/http_properties_builder.hpp index afd83ff6e..6be7359db 100644 --- a/libs/common/include/launchdarkly/config/shared/builders/http_properties_builder.hpp +++ b/libs/common/include/launchdarkly/config/shared/builders/http_properties_builder.hpp @@ -66,6 +66,48 @@ class TlsBuilder { enum built::TlsOptions::VerifyMode verify_mode_; std::optional custom_ca_file_; }; + +template +class ProxyBuilder { + public: + ProxyBuilder(); + + ProxyBuilder(built::ProxyOptions const& proxy); + + /** + * NOTE: This method and the associated 'http_proxy' environment variable + * are only available for the client-side SDK. + * + * Specifies an HTTP proxy which the SDK should use to communicate + * with LaunchDarkly. + * + * SDK <-- HTTP, plaintext --> Proxy <-- HTTPS --> LaunchDarkly + * + * This setting affects streaming mode, polling mode, and event delivery. + * The argument should be of the form: 'http://proxy.example.com:8080'. + * + * The scheme must be 'http' and the port is optional (80 if not + * specified.) + * + * The SDK respects the 'http_proxy' environment variable as an alternative + * to this method. If both are set, this method takes precedence. + * + * @param http_proxy HTTP proxy URL. + */ + template , int> = 0> + ProxyBuilder& HttpProxy(std::string http_proxy) { + http_proxy_ = std::move(http_proxy); + return *this; + } + + [[nodiscard]] built::ProxyOptions Build() const; + + private: + std::optional http_proxy_; + std::optional https_proxy_; +}; + /** * Class used for building a set of HttpProperties. * @tparam SDK The SDK type to build properties for. This affects the default @@ -74,6 +116,9 @@ class TlsBuilder { template class HttpPropertiesBuilder { public: + using TlsBuilder = TlsBuilder; + using ProxyBuilder = ProxyBuilder; + /** * Construct a new HttpPropertiesBuilder. The builder will use the default * properties based on the SDK type. Setting a property will override @@ -177,34 +222,14 @@ class HttpPropertiesBuilder { * @param builder The TLS property builder. * @return A reference to this builder. */ - HttpPropertiesBuilder& Tls(TlsBuilder builder); + HttpPropertiesBuilder& Tls(TlsBuilder builder); /** - * NOTE: This method and the associated 'http_proxy' environment variable - * are only available for the client-side SDK. - * - * Specifies an HTTP proxy which the SDK should use to communicate - * with LaunchDarkly. - * - * SDK <-- HTTP, plaintext --> Proxy <-- HTTPS --> LaunchDarkly - * - * This setting affects streaming mode, polling mode, and event delivery. - * The argument should be of the form: 'http://proxy.example.com:8080'. - * - * The scheme must be 'http' and the port is optional (80 if not - * specified.) * - * The SDK respects the 'http_proxy' environment variable as an alternative - * to this method. If both are set, this method takes precedence. - * - * @param http_proxy HTTP proxy URL. + * @param builder Sets the builder for proxy properties. + * @return A reference to this builder. */ - template , int> = 0> - HttpPropertiesBuilder& HttpProxy(std::string http_proxy) { - http_proxy_ = std::move(http_proxy); - return *this; - } + HttpPropertiesBuilder& Proxy(ProxyBuilder builder); [[nodiscard]] built::HttpProperties Build() const; @@ -216,8 +241,8 @@ class HttpPropertiesBuilder { std::string wrapper_name_; std::string wrapper_version_; std::map base_headers_; - TlsBuilder tls_; - std::optional http_proxy_; + TlsBuilder tls_; + ProxyBuilder proxy_; }; } // namespace launchdarkly::config::shared::builders diff --git a/libs/common/include/launchdarkly/config/shared/built/http_properties.hpp b/libs/common/include/launchdarkly/config/shared/built/http_properties.hpp index 313db415d..a194a1def 100644 --- a/libs/common/include/launchdarkly/config/shared/built/http_properties.hpp +++ b/libs/common/include/launchdarkly/config/shared/built/http_properties.hpp @@ -23,6 +23,20 @@ class TlsOptions final { std::optional ca_bundle_path_; }; +class ProxyOptions final { + public: + ProxyOptions(std::optional http_proxy, + std::optional https_proxy); + + ProxyOptions() = default; + [[nodiscard]] std::optional HttpProxy() const; + [[nodiscard]] std::optional HttpsProxy() const; + + private: + std::optional http_proxy_; + std::optional https_proxy_; +}; + class HttpProperties final { public: HttpProperties(std::chrono::milliseconds connect_timeout, @@ -31,7 +45,7 @@ class HttpProperties final { std::chrono::milliseconds response_timeout, std::map base_headers, TlsOptions tls, - std::optional http_proxy); + ProxyOptions proxy); [[nodiscard]] std::chrono::milliseconds ConnectTimeout() const; [[nodiscard]] std::chrono::milliseconds ReadTimeout() const; @@ -42,7 +56,7 @@ class HttpProperties final { [[nodiscard]] TlsOptions const& Tls() const; - [[nodiscard]] std::optional HttpProxy() const; + [[nodiscard]] ProxyOptions const& Proxy() const; private: std::chrono::milliseconds connect_timeout_; @@ -51,10 +65,11 @@ class HttpProperties final { std::chrono::milliseconds response_timeout_; std::map base_headers_; TlsOptions tls_; - std::optional http_proxy_; + ProxyOptions proxy_; }; bool operator==(HttpProperties const& lhs, HttpProperties const& rhs); +bool operator==(ProxyOptions const& lhs, ProxyOptions const& rhs); bool operator==(TlsOptions const& lhs, TlsOptions const& rhs); } // namespace launchdarkly::config::shared::built diff --git a/libs/common/include/launchdarkly/config/shared/defaults.hpp b/libs/common/include/launchdarkly/config/shared/defaults.hpp index d104f2775..a7e9ff0d8 100644 --- a/libs/common/include/launchdarkly/config/shared/defaults.hpp +++ b/libs/common/include/launchdarkly/config/shared/defaults.hpp @@ -51,6 +51,8 @@ struct Defaults { static auto TLS() -> shared::built::TlsOptions { return {}; } + static auto NoProxy() -> built::ProxyOptions { return {}; } + static auto HttpProperties() -> shared::built::HttpProperties { return {std::chrono::seconds{10}, std::chrono::seconds{10}, @@ -58,7 +60,7 @@ struct Defaults { std::chrono::seconds{10}, std::map(), TLS(), - std::nullopt}; + NoProxy()}; } static auto StreamingConfig() -> shared::built::StreamingConfig { @@ -100,6 +102,8 @@ struct Defaults { static auto TLS() -> shared::built::TlsOptions { return {}; } + static auto NoProxy() -> built::ProxyOptions { return {}; } + static auto HttpProperties() -> built::HttpProperties { return {std::chrono::seconds{10}, std::chrono::seconds{10}, @@ -107,7 +111,7 @@ struct Defaults { std::chrono::seconds{10}, std::map(), TLS(), - std::nullopt}; + NoProxy()}; } static auto StreamingConfig() -> built::StreamingConfig { diff --git a/libs/common/src/config/http_properties.cpp b/libs/common/src/config/http_properties.cpp index df1b0e8d3..a1172f5f6 100644 --- a/libs/common/src/config/http_properties.cpp +++ b/libs/common/src/config/http_properties.cpp @@ -21,20 +21,33 @@ std::optional const& TlsOptions::CustomCAFile() const { return ca_bundle_path_; } +ProxyOptions::ProxyOptions(std::optional http_proxy, + std::optional https_proxy) + : http_proxy_(std::move(http_proxy)), + https_proxy_(std::move(https_proxy)) {} + +std::optional ProxyOptions::HttpProxy() const { + return http_proxy_; +} + +std::optional ProxyOptions::HttpsProxy() const { + return https_proxy_; +} + HttpProperties::HttpProperties(std::chrono::milliseconds connect_timeout, std::chrono::milliseconds read_timeout, std::chrono::milliseconds write_timeout, std::chrono::milliseconds response_timeout, std::map base_headers, TlsOptions tls, - std::optional http_proxy) + ProxyOptions proxy) : connect_timeout_(connect_timeout), read_timeout_(read_timeout), write_timeout_(write_timeout), response_timeout_(response_timeout), base_headers_(std::move(base_headers)), tls_(std::move(tls)), - http_proxy_(std::move(http_proxy)) {} + proxy_(std::move(proxy)) {} std::chrono::milliseconds HttpProperties::ConnectTimeout() const { return connect_timeout_; @@ -60,8 +73,8 @@ TlsOptions const& HttpProperties::Tls() const { return tls_; } -std::optional HttpProperties::HttpProxy() const { - return http_proxy_; +ProxyOptions const& HttpProperties::Proxy() const { + return proxy_; } bool operator==(HttpProperties const& lhs, HttpProperties const& rhs) { @@ -69,7 +82,7 @@ bool operator==(HttpProperties const& lhs, HttpProperties const& rhs) { lhs.WriteTimeout() == rhs.WriteTimeout() && lhs.ConnectTimeout() == rhs.ConnectTimeout() && lhs.BaseHeaders() == rhs.BaseHeaders() && lhs.Tls() == rhs.Tls() && - lhs.HttpProxy() == rhs.HttpProxy(); + lhs.Proxy() == rhs.Proxy(); } bool operator==(TlsOptions const& lhs, TlsOptions const& rhs) { @@ -77,4 +90,9 @@ bool operator==(TlsOptions const& lhs, TlsOptions const& rhs) { lhs.CustomCAFile() == rhs.CustomCAFile(); } +bool operator==(ProxyOptions const& lhs, ProxyOptions const& rhs) { + return lhs.HttpProxy() == rhs.HttpProxy() && + lhs.HttpsProxy() == rhs.HttpsProxy(); +} + } // namespace launchdarkly::config::shared::built diff --git a/libs/common/src/config/http_properties_builder.cpp b/libs/common/src/config/http_properties_builder.cpp index f4081c3dc..f0fcd6e55 100644 --- a/libs/common/src/config/http_properties_builder.cpp +++ b/libs/common/src/config/http_properties_builder.cpp @@ -37,6 +37,33 @@ built::TlsOptions TlsBuilder::Build() const { return {verify_mode_, custom_ca_file_}; } +template +ProxyBuilder::ProxyBuilder() : ProxyBuilder(Defaults::NoProxy()) {} + +template +ProxyBuilder::ProxyBuilder(built::ProxyOptions const& proxy) { + http_proxy_ = proxy.HttpProxy(); + https_proxy_ = proxy.HttpsProxy(); +} + +static std::optional StringFromEnv(char const* env) { + if (char const* proxy = std::getenv(env)) { + if (strlen(proxy) > 0) { + return proxy; + } + } + return std::nullopt; +} + +template +built::ProxyOptions ProxyBuilder::Build() const { + if constexpr (std::is_same_v) { + return {http_proxy_ ? http_proxy_ : StringFromEnv("http_proxy"), + https_proxy_ ? https_proxy_ : StringFromEnv("https_proxy")}; + } + return Defaults::NoProxy(); +} + template HttpPropertiesBuilder::HttpPropertiesBuilder() : HttpPropertiesBuilder(shared::Defaults::HttpProperties()) {} @@ -50,7 +77,7 @@ HttpPropertiesBuilder::HttpPropertiesBuilder( response_timeout_ = properties.ResponseTimeout(); base_headers_ = properties.BaseHeaders(); tls_ = properties.Tls(); - http_proxy_ = properties.HttpProxy(); + proxy_ = properties.Proxy(); } template @@ -116,18 +143,16 @@ HttpPropertiesBuilder& HttpPropertiesBuilder::Header( template HttpPropertiesBuilder& HttpPropertiesBuilder::Tls( - TlsBuilder builder) { + TlsBuilder builder) { tls_ = std::move(builder); return *this; } -static std::optional HttpProxyFromEnv() { - if (char const* http_proxy = std::getenv("http_proxy")) { - if (strlen(http_proxy) > 0) { - return http_proxy; - } - } - return std::nullopt; +template +HttpPropertiesBuilder& HttpPropertiesBuilder::Proxy( + ProxyBuilder builder) { + proxy_ = std::move(builder); + return *this; } template @@ -139,20 +164,17 @@ built::HttpProperties HttpPropertiesBuilder::Build() const { wrapper_name_ + "/" + wrapper_version_; } - std::optional http_proxy; - - if constexpr (std::is_same_v) { - http_proxy = http_proxy_.has_value() ? http_proxy_ : HttpProxyFromEnv(); - } - return {connect_timeout_, read_timeout_, write_timeout_, response_timeout_, std::move(headers), tls_.Build(), - http_proxy}; + proxy_.Build()}; } -template class TlsBuilder; -template class TlsBuilder; +template class TlsBuilder; +template class TlsBuilder; + +template class ProxyBuilder; +template class ProxyBuilder; -template class HttpPropertiesBuilder; -template class HttpPropertiesBuilder; +template class HttpPropertiesBuilder; +template class HttpPropertiesBuilder; } // namespace launchdarkly::config::shared::builders diff --git a/libs/internal/src/network/http_requester.cpp b/libs/internal/src/network/http_requester.cpp index 318db02a9..54f5fc4fd 100644 --- a/libs/internal/src/network/http_requester.cpp +++ b/libs/internal/src/network/http_requester.cpp @@ -121,8 +121,8 @@ HttpRequest::HttpRequest(std::string const& url, port_ = uri_components->port(); } - if (properties_.HttpProxy()) { - auto const http_proxy = properties_.HttpProxy().value(); + if (properties_.Proxy().HttpProxy()) { + auto const http_proxy = properties_.Proxy().HttpProxy().value(); auto http_proxy_uri_components = boost::urls::parse_uri(http_proxy); if (!http_proxy_uri_components) { valid_ = false; From 98407f1d6ee817c25a1bbd40e41241437fff2771 Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Thu, 21 Nov 2024 18:03:42 -0800 Subject: [PATCH 17/19] add unit test for binding --- .../src/entity_manager.cpp | 4 ++- .../client_side/bindings/c/config/builder.h | 34 ++++++++++++------ libs/client-sdk/src/bindings/c/builder.cpp | 12 ++++++- libs/client-sdk/tests/client_config_test.cpp | 35 ++++++++++++++++++- 4 files changed, 72 insertions(+), 13 deletions(-) diff --git a/contract-tests/client-contract-tests/src/entity_manager.cpp b/contract-tests/client-contract-tests/src/entity_manager.cpp index ac1d2587b..493f2789f 100644 --- a/contract-tests/client-contract-tests/src/entity_manager.cpp +++ b/contract-tests/client-contract-tests/src/entity_manager.cpp @@ -40,7 +40,9 @@ std::optional EntityManager::create(ConfigParams const& in) { if (in.proxy) { if (in.proxy->httpProxy) { - config_builder.HttpProperties().HttpProxy(*in.proxy->httpProxy); + config_builder.HttpProperties().Proxy( + HttpPropertiesBuilder::ProxyBuilder().HttpProxy( + *in.proxy->httpProxy)); } } diff --git a/libs/client-sdk/include/launchdarkly/client_side/bindings/c/config/builder.h b/libs/client-sdk/include/launchdarkly/client_side/bindings/c/config/builder.h index dc91f9487..501adf95d 100644 --- a/libs/client-sdk/include/launchdarkly/client_side/bindings/c/config/builder.h +++ b/libs/client-sdk/include/launchdarkly/client_side/bindings/c/config/builder.h @@ -425,7 +425,7 @@ LDClientConfigBuilder_HttpProperties_Header(LDClientConfigBuilder b, char const* value); /** - * Creates a new proxy options builder for the HttpProperties builder. + * Creates a new ProxyBuilder for the HttpProperties builder. * * If not passed into the HttpProperties * builder, must be manually freed with LDClientHttpPropertiesProxyBuilder_Free. @@ -436,7 +436,7 @@ LD_EXPORT(LDClientHttpPropertiesProxyBuilder) LDClientHttpPropertiesProxyBuilder_New(void); /** - * Frees a proxy options builder. Do not call if the builder was consumed by + * Frees a ProxyBuilder. Do not call if the builder was consumed by * the HttpProperties builder. * * @param b Builder to free. @@ -445,10 +445,7 @@ LD_EXPORT(void) LDClientHttpPropertiesProxyBuilder_Free(LDClientHttpPropertiesProxyBuilder b); /** - * Specifies an HTTP proxy which the client should use to communicate - * with LaunchDarkly. - * - * SDK <-- HTTP, plaintext --> Proxy <-- HTTPS --> LaunchDarkly + * Specifies an HTTP proxy which the client should use for any HTTP requests. * * This setting affects streaming mode, polling mode, and event delivery. * The argument should be of the form: 'http://proxy.example.com:8080'. @@ -462,12 +459,29 @@ LDClientHttpPropertiesProxyBuilder_Free(LDClientHttpPropertiesProxyBuilder b); * @param b Client config builder. Must not be NULL. * @param http_proxy HTTP proxy URL. Must not be NULL. */ + LD_EXPORT(void) -LDClientConfigBuilder_HttpProperties_HttpProxy(LDClientConfigBuilder b, - char const* http_proxy); +LDClientHttpPropertiesProxyBuilder_HttpProxy( + LDClientHttpPropertiesProxyBuilder b, + char const* http_proxy); + +/** + * Sets the ProxyBuilder. The builder is automatically freed. + * + * WARNING: Do not call any other LDClientHttpPropertiesProxyBuilder function on + * the provided LDClientHttpPropertiesProxyBuilder after calling this function. + * It is undefined behavior. + * + * @param b Client config builder. Must not be NULL. + * @param proxy_builder The ProxyBuilder. Must not be NULL. + */ +LD_EXPORT(void) +LDClientConfigBuilder_HttpProperties_Proxy( + LDClientConfigBuilder b, + LDClientHttpPropertiesProxyBuilder proxy_builder); /** - * Sets the TLS options builder. The builder is automatically freed. + * Sets the TlsBuilder. The builder is automatically freed. * * WARNING: Do not call any other * LDClientHttpPropertiesTlsBuilder function on the provided @@ -475,7 +489,7 @@ LDClientConfigBuilder_HttpProperties_HttpProxy(LDClientConfigBuilder b, * It is undefined behavior. * * @param b Client config builder. Must not be NULL. - * @param tls_builder The TLS options builder. Must not be NULL. + * @param tls_builder The TlsBuilder. Must not be NULL. */ LD_EXPORT(void) LDClientConfigBuilder_HttpProperties_Tls( diff --git a/libs/client-sdk/src/bindings/c/builder.cpp b/libs/client-sdk/src/bindings/c/builder.cpp index 019a1b14d..b7d589a09 100644 --- a/libs/client-sdk/src/bindings/c/builder.cpp +++ b/libs/client-sdk/src/bindings/c/builder.cpp @@ -329,7 +329,7 @@ LDClientHttpPropertiesProxyBuilder_Free(LDClientHttpPropertiesProxyBuilder b) { } LD_EXPORT(void) -LDClientConfigBuilder_HttpProperties_HttpProxy( +LDClientConfigBuilder_HttpProperties_Proxy( LDClientConfigBuilder b, LDClientHttpPropertiesProxyBuilder proxy_builder) { LD_ASSERT_NOT_NULL(b); @@ -340,6 +340,16 @@ LDClientConfigBuilder_HttpProperties_HttpProxy( LDClientHttpPropertiesProxyBuilder_Free(proxy_builder); } +LD_EXPORT(void) +LDClientHttpPropertiesProxyBuilder_HttpProxy( + LDClientHttpPropertiesProxyBuilder b, + char const* http_proxy) { + LD_ASSERT_NOT_NULL(b); + LD_ASSERT_NOT_NULL(http_proxy); + + TO_PROXY_BUILDER(b)->HttpProxy(http_proxy); +} + LD_EXPORT(void) LDClientConfigBuilder_HttpProperties_Tls( LDClientConfigBuilder b, diff --git a/libs/client-sdk/tests/client_config_test.cpp b/libs/client-sdk/tests/client_config_test.cpp index df94f48d8..883f4ec6b 100644 --- a/libs/client-sdk/tests/client_config_test.cpp +++ b/libs/client-sdk/tests/client_config_test.cpp @@ -89,7 +89,15 @@ TEST(ClientConfigBindings, AllConfigs) { LDClientHttpPropertiesTlsBuilder_New(); LDClientHttpPropertiesTlsBuilder_Free(tls_builder2); - LDClientConfigBuilder_Logging_Disable(builder); + LDClientHttpPropertiesProxyBuilder proxy_builder = + LDClientHttpPropertiesProxyBuilder_New(); + LDClientHttpPropertiesProxyBuilder_HttpProxy(proxy_builder, + "http://proxy:8080"); + LDClientConfigBuilder_HttpProperties_Proxy(builder, proxy_builder); + + LDClientHttpPropertiesProxyBuilder proxy_builder2 = + LDClientHttpPropertiesProxyBuilder_New(); + LDClientHttpPropertiesProxyBuilder_Free(proxy_builder2); LDLoggingBasicBuilder log_builder = LDLoggingBasicBuilder_New(); LDLoggingBasicBuilder_Level(log_builder, LD_LOG_WARN); @@ -263,3 +271,28 @@ TEST(ClientConfigBindings, CustomLogger) { ASSERT_EQ(args.write->level, LD_LOG_ERROR); ASSERT_EQ(args.write->msg, "hello"); } + +TEST(ClientConfigBindings, ProxyOptions) { + using namespace launchdarkly; + + LDClientConfigBuilder builder = LDClientConfigBuilder_New("sdk-123"); + + LDClientHttpPropertiesProxyBuilder proxy_builder = + LDClientHttpPropertiesProxyBuilder_New(); + + LDClientHttpPropertiesProxyBuilder_HttpProxy(proxy_builder, + "http://proxy.com:8080"); + + LDClientConfigBuilder_HttpProperties_Proxy(builder, proxy_builder); + + LDClientConfig config = nullptr; + LDStatus status = LDClientConfigBuilder_Build(builder, &config); + ASSERT_TRUE(LDStatus_Ok(status)); + + auto client_config = reinterpret_cast(config); + + ASSERT_EQ(client_config->HttpProperties().Proxy().HttpProxy(), + "http://proxy.com:8080"); + + LDClientConfig_Free(config); +} From 06dff6d53eba02a86d220131ee1aabc9f7905b54 Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Thu, 21 Nov 2024 18:10:43 -0800 Subject: [PATCH 18/19] rename getters to drop proxy suffix --- libs/client-sdk/src/data_sources/streaming_data_source.cpp | 4 ++-- libs/client-sdk/tests/client_config_test.cpp | 2 +- .../launchdarkly/config/shared/built/http_properties.hpp | 4 ++-- libs/common/src/config/http_properties.cpp | 7 +++---- libs/common/src/config/http_properties_builder.cpp | 4 ++-- libs/internal/src/network/http_requester.cpp | 4 ++-- 6 files changed, 12 insertions(+), 13 deletions(-) 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 cb51ab1c4..b5beef411 100644 --- a/libs/client-sdk/src/data_sources/streaming_data_source.cpp +++ b/libs/client-sdk/src/data_sources/streaming_data_source.cpp @@ -121,8 +121,8 @@ void StreamingDataSource::Start() { client_builder.custom_ca_file(*ca_file); } - if (http_config_.Proxy().HttpProxy()) { - client_builder.http_proxy(*http_config_.Proxy().HttpProxy()); + if (http_config_.Proxy().Http()) { + client_builder.http_proxy(*http_config_.Proxy().Http()); } auto weak_self = weak_from_this(); diff --git a/libs/client-sdk/tests/client_config_test.cpp b/libs/client-sdk/tests/client_config_test.cpp index 883f4ec6b..992b44c1e 100644 --- a/libs/client-sdk/tests/client_config_test.cpp +++ b/libs/client-sdk/tests/client_config_test.cpp @@ -291,7 +291,7 @@ TEST(ClientConfigBindings, ProxyOptions) { auto client_config = reinterpret_cast(config); - ASSERT_EQ(client_config->HttpProperties().Proxy().HttpProxy(), + ASSERT_EQ(client_config->HttpProperties().Proxy().Http(), "http://proxy.com:8080"); LDClientConfig_Free(config); diff --git a/libs/common/include/launchdarkly/config/shared/built/http_properties.hpp b/libs/common/include/launchdarkly/config/shared/built/http_properties.hpp index a194a1def..8abbcf488 100644 --- a/libs/common/include/launchdarkly/config/shared/built/http_properties.hpp +++ b/libs/common/include/launchdarkly/config/shared/built/http_properties.hpp @@ -29,8 +29,8 @@ class ProxyOptions final { std::optional https_proxy); ProxyOptions() = default; - [[nodiscard]] std::optional HttpProxy() const; - [[nodiscard]] std::optional HttpsProxy() const; + [[nodiscard]] std::optional Http() const; + [[nodiscard]] std::optional Https() const; private: std::optional http_proxy_; diff --git a/libs/common/src/config/http_properties.cpp b/libs/common/src/config/http_properties.cpp index a1172f5f6..066c69a5b 100644 --- a/libs/common/src/config/http_properties.cpp +++ b/libs/common/src/config/http_properties.cpp @@ -26,11 +26,11 @@ ProxyOptions::ProxyOptions(std::optional http_proxy, : http_proxy_(std::move(http_proxy)), https_proxy_(std::move(https_proxy)) {} -std::optional ProxyOptions::HttpProxy() const { +std::optional ProxyOptions::Http() const { return http_proxy_; } -std::optional ProxyOptions::HttpsProxy() const { +std::optional ProxyOptions::Https() const { return https_proxy_; } @@ -91,8 +91,7 @@ bool operator==(TlsOptions const& lhs, TlsOptions const& rhs) { } bool operator==(ProxyOptions const& lhs, ProxyOptions const& rhs) { - return lhs.HttpProxy() == rhs.HttpProxy() && - lhs.HttpsProxy() == rhs.HttpsProxy(); + return lhs.Http() == rhs.Http() && lhs.Https() == rhs.Https(); } } // namespace launchdarkly::config::shared::built diff --git a/libs/common/src/config/http_properties_builder.cpp b/libs/common/src/config/http_properties_builder.cpp index f0fcd6e55..dbac21a77 100644 --- a/libs/common/src/config/http_properties_builder.cpp +++ b/libs/common/src/config/http_properties_builder.cpp @@ -42,8 +42,8 @@ ProxyBuilder::ProxyBuilder() : ProxyBuilder(Defaults::NoProxy()) {} template ProxyBuilder::ProxyBuilder(built::ProxyOptions const& proxy) { - http_proxy_ = proxy.HttpProxy(); - https_proxy_ = proxy.HttpsProxy(); + http_proxy_ = proxy.Http(); + https_proxy_ = proxy.Https(); } static std::optional StringFromEnv(char const* env) { diff --git a/libs/internal/src/network/http_requester.cpp b/libs/internal/src/network/http_requester.cpp index 54f5fc4fd..1af0118be 100644 --- a/libs/internal/src/network/http_requester.cpp +++ b/libs/internal/src/network/http_requester.cpp @@ -121,8 +121,8 @@ HttpRequest::HttpRequest(std::string const& url, port_ = uri_components->port(); } - if (properties_.Proxy().HttpProxy()) { - auto const http_proxy = properties_.Proxy().HttpProxy().value(); + if (properties_.Proxy().Http()) { + auto const http_proxy = properties_.Proxy().Http().value(); auto http_proxy_uri_components = boost::urls::parse_uri(http_proxy); if (!http_proxy_uri_components) { valid_ = false; From 03f588ac2203426b7ba7c49fda93f43e45329442 Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Thu, 12 Dec 2024 10:28:03 -0800 Subject: [PATCH 19/19] attempt to fix compilation on Linux --- .../client-contract-tests/src/entity_manager.cpp | 3 +-- libs/client-sdk/src/bindings/c/builder.cpp | 10 ++++------ libs/common/include/launchdarkly/config/client.hpp | 1 + .../shared/builders/http_properties_builder.hpp | 11 ++++------- libs/common/src/config/http_properties_builder.cpp | 4 ++-- 5 files changed, 12 insertions(+), 17 deletions(-) diff --git a/contract-tests/client-contract-tests/src/entity_manager.cpp b/contract-tests/client-contract-tests/src/entity_manager.cpp index 493f2789f..d345837f6 100644 --- a/contract-tests/client-contract-tests/src/entity_manager.cpp +++ b/contract-tests/client-contract-tests/src/entity_manager.cpp @@ -41,8 +41,7 @@ std::optional EntityManager::create(ConfigParams const& in) { if (in.proxy) { if (in.proxy->httpProxy) { config_builder.HttpProperties().Proxy( - HttpPropertiesBuilder::ProxyBuilder().HttpProxy( - *in.proxy->httpProxy)); + ProxyBuilder().HttpProxy(*in.proxy->httpProxy)); } } diff --git a/libs/client-sdk/src/bindings/c/builder.cpp b/libs/client-sdk/src/bindings/c/builder.cpp index b7d589a09..8001d2fd3 100644 --- a/libs/client-sdk/src/bindings/c/builder.cpp +++ b/libs/client-sdk/src/bindings/c/builder.cpp @@ -41,14 +41,12 @@ using namespace launchdarkly::client_side; #define FROM_CUSTOM_PERSISTENCE_BUILDER(ptr) \ (reinterpret_cast(ptr)) -#define TO_TLS_BUILDER(ptr) \ - (reinterpret_cast(ptr)) +#define TO_TLS_BUILDER(ptr) (reinterpret_cast(ptr)) #define FROM_TLS_BUILDER(ptr) \ (reinterpret_cast(ptr)) -#define TO_PROXY_BUILDER(ptr) \ - (reinterpret_cast(ptr)) +#define TO_PROXY_BUILDER(ptr) (reinterpret_cast(ptr)) #define FROM_PROXY_BUILDER(ptr) \ (reinterpret_cast(ptr)) @@ -320,7 +318,7 @@ LDClientConfigBuilder_HttpProperties_Header(LDClientConfigBuilder b, LD_EXPORT(LDClientHttpPropertiesProxyBuilder) LDClientHttpPropertiesProxyBuilder_New(void) { - return FROM_PROXY_BUILDER(new HttpPropertiesBuilder::ProxyBuilder()); + return FROM_PROXY_BUILDER(new ProxyBuilder()); } LD_EXPORT(void) @@ -383,7 +381,7 @@ LDClientHttpPropertiesTlsBuilder_CustomCAFile( LD_EXPORT(LDClientHttpPropertiesTlsBuilder) LDClientHttpPropertiesTlsBuilder_New(void) { - return FROM_TLS_BUILDER(new HttpPropertiesBuilder::TlsBuilder()); + return FROM_TLS_BUILDER(new TlsBuilder()); } LD_EXPORT(void) diff --git a/libs/common/include/launchdarkly/config/client.hpp b/libs/common/include/launchdarkly/config/client.hpp index dd288a82a..16194d88e 100644 --- a/libs/common/include/launchdarkly/config/client.hpp +++ b/libs/common/include/launchdarkly/config/client.hpp @@ -23,6 +23,7 @@ using DataSourceBuilder = config::shared::builders::DataSourceBuilder; using LoggingBuilder = config::shared::builders::LoggingBuilder; using PersistenceBuilder = config::shared::builders::PersistenceBuilder; using TlsBuilder = config::shared::builders::TlsBuilder; +using ProxyBuilder = config::shared::builders::ProxyBuilder; using Config = config::Config; diff --git a/libs/common/include/launchdarkly/config/shared/builders/http_properties_builder.hpp b/libs/common/include/launchdarkly/config/shared/builders/http_properties_builder.hpp index 6be7359db..fa46ff313 100644 --- a/libs/common/include/launchdarkly/config/shared/builders/http_properties_builder.hpp +++ b/libs/common/include/launchdarkly/config/shared/builders/http_properties_builder.hpp @@ -116,9 +116,6 @@ class ProxyBuilder { template class HttpPropertiesBuilder { public: - using TlsBuilder = TlsBuilder; - using ProxyBuilder = ProxyBuilder; - /** * Construct a new HttpPropertiesBuilder. The builder will use the default * properties based on the SDK type. Setting a property will override @@ -222,14 +219,14 @@ class HttpPropertiesBuilder { * @param builder The TLS property builder. * @return A reference to this builder. */ - HttpPropertiesBuilder& Tls(TlsBuilder builder); + HttpPropertiesBuilder& Tls(TlsBuilder builder); /** * * @param builder Sets the builder for proxy properties. * @return A reference to this builder. */ - HttpPropertiesBuilder& Proxy(ProxyBuilder builder); + HttpPropertiesBuilder& Proxy(ProxyBuilder builder); [[nodiscard]] built::HttpProperties Build() const; @@ -241,8 +238,8 @@ class HttpPropertiesBuilder { std::string wrapper_name_; std::string wrapper_version_; std::map base_headers_; - TlsBuilder tls_; - ProxyBuilder proxy_; + TlsBuilder tls_; + ProxyBuilder proxy_; }; } // namespace launchdarkly::config::shared::builders diff --git a/libs/common/src/config/http_properties_builder.cpp b/libs/common/src/config/http_properties_builder.cpp index dbac21a77..e098fbbd8 100644 --- a/libs/common/src/config/http_properties_builder.cpp +++ b/libs/common/src/config/http_properties_builder.cpp @@ -143,14 +143,14 @@ HttpPropertiesBuilder& HttpPropertiesBuilder::Header( template HttpPropertiesBuilder& HttpPropertiesBuilder::Tls( - TlsBuilder builder) { + TlsBuilder builder) { tls_ = std::move(builder); return *this; } template HttpPropertiesBuilder& HttpPropertiesBuilder::Proxy( - ProxyBuilder builder) { + ProxyBuilder builder) { proxy_ = std::move(builder); return *this; }