From 4d8e091af55500eb0317c3189a055631fab70c31 Mon Sep 17 00:00:00 2001 From: Ali Beyad Date: Tue, 1 Oct 2024 15:04:54 -0400 Subject: [PATCH] mobile: Fix bugs in the Apple PAC proxy resolver Signed-off-by: Ali Beyad --- mobile/library/common/BUILD | 1 + .../http/network_configuration/filter.cc | 6 +- mobile/library/common/internal_engine.cc | 8 ++ mobile/library/common/network/BUILD | 4 + .../network/apple_pac_proxy_resolver.cc | 49 +++++++--- .../common/network/apple_pac_proxy_resolver.h | 6 +- .../common/network/apple_proxy_resolver.cc | 41 +++++++-- .../common/network/apple_proxy_resolver.h | 13 ++- .../common/network/proxy_resolver_interface.h | 10 ++ mobile/test/common/proxy/BUILD | 2 - .../proxy/test_apple_api_registration.cc | 6 -- .../proxy/test_apple_pac_proxy_resolver.cc | 92 ------------------- .../proxy/test_apple_pac_proxy_resolver.h | 27 ------ .../common/proxy/test_apple_proxy_resolver.cc | 5 - .../common/proxy/test_apple_proxy_resolver.h | 6 -- .../test_apple_proxy_settings_monitor.cc | 10 +- mobile/test/swift/integration/proxying/BUILD | 17 ++++ .../HTTPRequestUsingPacProxyTest.swift | 91 ++++++++++++++++++ .../proxying/HTTPRequestUsingProxyTest.swift | 59 ------------ 19 files changed, 224 insertions(+), 229 deletions(-) delete mode 100644 mobile/test/common/proxy/test_apple_pac_proxy_resolver.cc delete mode 100644 mobile/test/common/proxy/test_apple_pac_proxy_resolver.h create mode 100644 mobile/test/swift/integration/proxying/HTTPRequestUsingPacProxyTest.swift diff --git a/mobile/library/common/BUILD b/mobile/library/common/BUILD index e945d1b037511..1990b609dc0d7 100644 --- a/mobile/library/common/BUILD +++ b/mobile/library/common/BUILD @@ -31,6 +31,7 @@ envoy_cc_library( "//library/common/http:header_utility_lib", "//library/common/logger:logger_delegate_lib", "//library/common/network:connectivity_manager_lib", + "//library/common/network:proxy_api_lib", "//library/common/stats:utility_lib", "//library/common/types:c_types_lib", "@envoy//envoy/server:lifecycle_notifier_interface", diff --git a/mobile/library/common/extensions/filters/http/network_configuration/filter.cc b/mobile/library/common/extensions/filters/http/network_configuration/filter.cc index 20ae20e09fffe..88bf7cdc1ceb4 100644 --- a/mobile/library/common/extensions/filters/http/network_configuration/filter.cc +++ b/mobile/library/common/extensions/filters/http/network_configuration/filter.cc @@ -113,9 +113,9 @@ NetworkConfigurationFilter::resolveProxy(Http::RequestHeaderMap& request_headers Thread::MainThread::isMainOrTestThread(), "NetworkConfigurationProxy PAC proxy resolver callback not running on main thread."); // This is the callback invoked from the Apple APIs resolving the PAC file URL, which - // gets invoked on the Envoy thread. We keep a weak_ptr to this filter instance - // so that, if the stream is canceled and the filter chain is torn down in the meantime, - // we will fail to aquire the weak_ptr lock and won't execute any callbacks on the resolved + // gets invoked on a separate thread. We keep a weak_ptr to this filter instance so that, + // if the stream is canceled and the filter chain is torn down in the meantime, we will + // fail to aquire the weak_ptr lock and won't execute any callbacks on the resolved // proxies. if (auto filter_ptr = weak_self.lock()) { filter_ptr->onProxyResolutionComplete(Network::ProxySettings::create(proxies)); diff --git a/mobile/library/common/internal_engine.cc b/mobile/library/common/internal_engine.cc index ea6c36f4ad917..ddec37fa7949d 100644 --- a/mobile/library/common/internal_engine.cc +++ b/mobile/library/common/internal_engine.cc @@ -10,6 +10,7 @@ #include "source/common/runtime/runtime_features.h" #include "absl/synchronization/notification.h" +#include "library/common/network/proxy_api.h" #include "library/common/stats/utility.h" namespace Envoy { @@ -131,6 +132,13 @@ envoy_status_t InternalEngine::main(std::shared_ptr opti server_ = main_common->server(); event_dispatcher_ = &server_->dispatcher(); + // If proxy resolution APIs are configured on the Engine, set the main thread's dispatcher + // on the proxy resolver for handling callbacks. + auto* proxy_resolver = static_cast(Api::External::retrieveApi("envoy_proxy_resolver", /*allow_absent=*/true)); + if (proxy_resolver != nullptr) { + proxy_resolver->resolver->setDispatcher(event_dispatcher_); + } + cv_.notifyAll(); } END_TRY diff --git a/mobile/library/common/network/BUILD b/mobile/library/common/network/BUILD index 2efd61a6d191f..f13e2595c36ef 100644 --- a/mobile/library/common/network/BUILD +++ b/mobile/library/common/network/BUILD @@ -108,6 +108,7 @@ envoy_cc_library( repository = "@envoy", deps = [ ":proxy_settings_lib", + "@envoy//envoy/event:dispatcher_interface", ], ) @@ -150,6 +151,9 @@ envoy_cc_library( "//library/common/apple:utility_lib", "//library/common/extensions/cert_validator/platform_bridge:c_types_lib", "//library/common/types:c_types_lib", + "@envoy//envoy/event:dispatcher_interface", + "@envoy//source/common/common:thread_impl_lib_posix", + "@envoy//source/common/common:thread_lib", ], "//conditions:default": [], }), diff --git a/mobile/library/common/network/apple_pac_proxy_resolver.cc b/mobile/library/common/network/apple_pac_proxy_resolver.cc index f8d1fda237200..5b8c7e656d54c 100644 --- a/mobile/library/common/network/apple_pac_proxy_resolver.cc +++ b/mobile/library/common/network/apple_pac_proxy_resolver.cc @@ -13,13 +13,16 @@ namespace Network { namespace { +CFStringRef pacRunLoopMode() { + static const CFStringRef runloop_mode = CFSTR("envoy.PacProxyResolver"); + return runloop_mode; +} + // Creates a CFURLRef from a C++ string URL. CFURLRef createCFURL(const std::string& url_string) { - auto cf_url_string = - CFStringCreateWithCString(kCFAllocatorDefault, url_string.c_str(), kCFStringEncodingUTF8); - auto cf_url = CFURLCreateWithString(kCFAllocatorDefault, cf_url_string, /*baseURL=*/nullptr); - CFRelease(cf_url_string); - return cf_url; + return CFURLCreateWithBytes(kCFAllocatorDefault, + reinterpret_cast(url_string.c_str()), + url_string.length(), kCFStringEncodingUTF8, nullptr); } } // namespace @@ -31,16 +34,15 @@ void proxyAutoConfigurationResultCallback(void* ptr, CFArrayRef cf_proxies, CFEr std::unique_ptr completion_callback( static_cast(ptr)); - std::vector proxies; - if (cf_error != nullptr || cf_proxies == nullptr) { + if (cf_error == nullptr) { ENVOY_BUG(cf_error != nullptr, Apple::toString(CFErrorCopyDescription(cf_error))); - // Treat the error case as if no proxy was configured. Seems to be consistent with what iOS - // system (URLSession) is doing. - (*completion_callback)(proxies); - return; } - for (int i = 0; i < CFArrayGetCount(cf_proxies); i++) { + // Treat the error case as if no proxy was configured. Seems to be consistent with what iOS + // system (URLSession) is doing. + const int num_proxy_entries = cf_proxies != nullptr ? CFArrayGetCount(cf_proxies) : 0; + std::vector proxies; + for (int i = 0; i < num_proxy_entries; i++) { CFDictionaryRef cf_dictionary = static_cast(CFArrayGetValueAtIndex(cf_proxies, i)); CFStringRef cf_proxy_type = @@ -65,6 +67,8 @@ void proxyAutoConfigurationResultCallback(void* ptr, CFArrayRef cf_proxies, CFEr } (*completion_callback)(proxies); + + CFRunLoopStop(CFRunLoopGetCurrent()); } CFRunLoopSourceRef @@ -80,13 +84,28 @@ ApplePacProxyResolver::createPacUrlResolverSource(CFURLRef cf_proxy_autoconfigur } void ApplePacProxyResolver::resolveProxies( - const std::string& target_url, const std::string& proxy_autoconfiguration_file_url, + const std::string& proxy_autoconfiguration_file_url, const std::string& target_url, ProxySettingsResolvedCallback proxy_resolution_completed) { CFURLRef cf_target_url = createCFURL(target_url); CFURLRef cf_proxy_autoconfiguration_file_url = createCFURL(proxy_autoconfiguration_file_url); + if (cf_target_url == nullptr || cf_proxy_autoconfiguration_file_url == nullptr) { + return; + } std::unique_ptr completion_callback = std::make_unique(std::move(proxy_resolution_completed)); + + // Work around . This dummy call to CFNetworkCopyProxiesForURL + // initializes some state within CFNetwork that is required by + // CFNetworkExecuteProxyAutoConfigurationURL. + CFDictionaryRef empty_dictionary = + CFDictionaryCreate(nullptr, nullptr, nullptr, 0, nullptr, nullptr); + CFArrayRef empty_result = CFNetworkCopyProxiesForURL(cf_target_url, empty_dictionary); + CFRelease(empty_dictionary); + if (empty_result) { + CFRelease(empty_result); + } + // According to https://developer.apple.com/documentation/corefoundation/cfstreamclientcontext, // the version must be 0. auto context = std::make_unique( @@ -102,7 +121,9 @@ void ApplePacProxyResolver::resolveProxies( CFRunLoopSourceRef run_loop_source = createPacUrlResolverSource( cf_proxy_autoconfiguration_file_url, cf_target_url, context.release()); - CFRunLoopAddSource(CFRunLoopGetCurrent(), run_loop_source, kCFRunLoopDefaultMode); + CFRunLoopAddSource(CFRunLoopGetCurrent(), run_loop_source, pacRunLoopMode()); + CFRunLoopRunInMode(pacRunLoopMode(), DBL_MAX, false); + CFRunLoopRemoveSource(CFRunLoopGetCurrent(), run_loop_source, pacRunLoopMode()); CFRelease(cf_target_url); CFRelease(cf_proxy_autoconfiguration_file_url); diff --git a/mobile/library/common/network/apple_pac_proxy_resolver.h b/mobile/library/common/network/apple_pac_proxy_resolver.h index 24d3490565065..673e00a6b063b 100644 --- a/mobile/library/common/network/apple_pac_proxy_resolver.h +++ b/mobile/library/common/network/apple_pac_proxy_resolver.h @@ -27,13 +27,13 @@ class ApplePacProxyResolver { /** * Resolves proxy for a given URL using proxy auto configuration file that's hosted at a given * URL. - * @param target_url A request URL to resolve the proxy for. * @param proxy_autoconfiguration_file_url A URL at which a proxy configuration file is hosted. + * @param target_url A request URL to resolve the proxy for. * @param proxy_resolution_completed A function that's called with result proxies as its * arguments when proxy resolution completes. */ - void resolveProxies(const std::string& target_url, - const std::string& proxy_autoconfiguration_file_url, + void resolveProxies(const std::string& proxy_autoconfiguration_file_url, + const std::string& target_url, ProxySettingsResolvedCallback proxy_resolution_completed); protected: diff --git a/mobile/library/common/network/apple_proxy_resolver.cc b/mobile/library/common/network/apple_proxy_resolver.cc index f0bc2c329b5d3..da0d6a2faead3 100644 --- a/mobile/library/common/network/apple_proxy_resolver.cc +++ b/mobile/library/common/network/apple_proxy_resolver.cc @@ -1,6 +1,7 @@ #include "library/common/network/apple_proxy_resolver.h" #include "source/common/common/assert.h" +#include "source/common/common/posix/thread_impl.h" namespace Envoy { namespace Network { @@ -8,7 +9,17 @@ namespace Network { AppleProxyResolver::AppleProxyResolver() : proxy_settings_monitor_( std::make_unique(proxySettingsUpdater())), - pac_proxy_resolver_(std::make_unique()) {} + pac_proxy_resolver_(std::make_unique()), + thread_factory_(Thread::PosixThreadFactory::create()) {} + +AppleProxyResolver::~AppleProxyResolver() { + // Wait for the PAC resolution threads to finish. + for (auto& [_, thread] : pac_resolution_threads_) { + if (thread->joinable()) { + thread->join(); + } + } +} SystemProxySettingsReadCallback AppleProxyResolver::proxySettingsUpdater() { return SystemProxySettingsReadCallback( @@ -23,11 +34,14 @@ void AppleProxyResolver::start() { started_ = true; } +void AppleProxyResolver::setDispatcher(Event::Dispatcher* dispatcher) { dispatcher_ = dispatcher; } + ProxyResolutionResult AppleProxyResolver::resolveProxy(const std::string& target_url_string, std::vector& proxies, ProxySettingsResolvedCallback proxy_resolution_completed) { ASSERT(started_, "AppleProxyResolver not started."); + ASSERT(dispatcher_ != nullptr, "Dispatcher not set on the AppleProxyResolver."); std::string pac_file_url; { @@ -55,11 +69,26 @@ AppleProxyResolver::resolveProxy(const std::string& target_url_string, // implement a caching layer with TTLs for PAC file URL resolution within AppleProxyResolver // itself. ASSERT(!pac_file_url.empty(), "PAC file URL must not be empty if PAC is enabled."); - pac_proxy_resolver_->resolveProxies( - pac_file_url, target_url_string, - [proxy_resolution_completed](const std::vector& proxies) { - proxy_resolution_completed(proxies); - }); + Thread::PosixThreadPtr thread = thread_factory_->createThread( + [this, pac_file_url, target_url_string, callback = std::move(proxy_resolution_completed)] { + pac_proxy_resolver_->resolveProxies( + pac_file_url, target_url_string, + [this, callback](const std::vector& proxies) { + auto pac_resolve_thread_id = thread_factory_->currentPthreadId(); + dispatcher_->post([this, callback, proxies, pac_resolve_thread_id] { + auto thread_handle = pac_resolution_threads_.extract(pac_resolve_thread_id); + if (!thread_handle.empty()) { + thread_handle.mapped()->join(); + } + + callback(proxies); + }); + }); + }, + /* options= */ absl::nullopt, /* crash_on_failure=*/false); + Thread::ThreadId thread_id = thread->pthreadId(); + pac_resolution_threads_[thread_id] = std::move(thread); + return ProxyResolutionResult::ResultInProgress; } diff --git a/mobile/library/common/network/apple_proxy_resolver.h b/mobile/library/common/network/apple_proxy_resolver.h index c40a97f138d2a..877be65725021 100644 --- a/mobile/library/common/network/apple_proxy_resolver.h +++ b/mobile/library/common/network/apple_proxy_resolver.h @@ -6,6 +6,12 @@ #include #include +#include "envoy/event/dispatcher.h" + +#include "source/common/common/posix/thread_impl.h" +#include "source/common/common/thread.h" + +#include "absl/container/flat_hash_map.h" #include "library/common/network/apple_pac_proxy_resolver.h" #include "library/common/network/apple_system_proxy_settings_monitor.h" #include "library/common/network/proxy_resolver_interface.h" @@ -20,7 +26,7 @@ namespace Network { class AppleProxyResolver : public ProxyResolver { public: AppleProxyResolver(); - virtual ~AppleProxyResolver() = default; + virtual ~AppleProxyResolver(); /** * Starts proxy resolver. It needs to be called prior to any proxy resolution attempt. @@ -38,6 +44,8 @@ class AppleProxyResolver : public ProxyResolver { resolveProxy(const std::string& target_url_string, std::vector& proxies, ProxySettingsResolvedCallback proxy_resolution_completed) override; + virtual void setDispatcher(Event::Dispatcher* dispatcher) override; + /* * Supplies a function that updates this instance's proxy settings. */ @@ -49,6 +57,9 @@ class AppleProxyResolver : public ProxyResolver { std::unique_ptr proxy_settings_monitor_; std::unique_ptr pac_proxy_resolver_; absl::optional proxy_settings_; + std::unique_ptr thread_factory_; + absl::flat_hash_map pac_resolution_threads_; + Event::Dispatcher* dispatcher_; absl::Mutex mutex_; bool started_ = false; }; diff --git a/mobile/library/common/network/proxy_resolver_interface.h b/mobile/library/common/network/proxy_resolver_interface.h index 5abedc9c1b328..afe7a90eb3e62 100644 --- a/mobile/library/common/network/proxy_resolver_interface.h +++ b/mobile/library/common/network/proxy_resolver_interface.h @@ -1,5 +1,7 @@ #pragma once +#include "envoy/event/dispatcher.h" + #include "library/common/network/proxy_settings.h" namespace Envoy { @@ -28,6 +30,14 @@ class ProxyResolver { virtual ProxyResolutionResult resolveProxy(const std::string& target_url_string, std::vector& proxies, ProxySettingsResolvedCallback proxy_resolution_completed) PURE; + + /** + * Sets the Event Dispatcher that a proxy resolver instance can use to post callbacks to. + * Typically, this would be the main Envoy thread's dispatcher (which is also the worker thread + * in Envoy Mobile). + * @param dispatcher The event dispatcher. + */ + virtual void setDispatcher(Event::Dispatcher* dispatcher) PURE; }; } // namespace Network diff --git a/mobile/test/common/proxy/BUILD b/mobile/test/common/proxy/BUILD index 6c05ec3608b8f..d721e213c422b 100644 --- a/mobile/test/common/proxy/BUILD +++ b/mobile/test/common/proxy/BUILD @@ -9,7 +9,6 @@ envoy_cc_test_library( srcs = select({ "@envoy//bazel:apple": [ "test_apple_api_registration.cc", - "test_apple_pac_proxy_resolver.cc", "test_apple_proxy_resolver.cc", "test_apple_proxy_settings_monitor.cc", ], @@ -18,7 +17,6 @@ envoy_cc_test_library( hdrs = select({ "@envoy//bazel:apple": [ "test_apple_api_registration.h", - "test_apple_pac_proxy_resolver.h", "test_apple_proxy_resolver.h", "test_apple_proxy_settings_monitor.h", ], diff --git a/mobile/test/common/proxy/test_apple_api_registration.cc b/mobile/test/common/proxy/test_apple_api_registration.cc index 5a8ea1ff6f9cf..e9ecd449fa88b 100644 --- a/mobile/test/common/proxy/test_apple_api_registration.cc +++ b/mobile/test/common/proxy/test_apple_api_registration.cc @@ -1,6 +1,5 @@ #include "test/common/proxy/test_apple_api_registration.h" -#include "test/common/proxy/test_apple_pac_proxy_resolver.h" #include "test/common/proxy/test_apple_proxy_resolver.h" #include "test/common/proxy/test_apple_proxy_settings_monitor.h" @@ -30,11 +29,6 @@ void registerTestAppleProxyResolver(absl::string_view host, int port, const bool test_resolver->setSettingsMonitorForTest( std::make_unique( std::string(host), port, use_pac_resolver, test_resolver->proxySettingsUpdater())); - if (use_pac_resolver) { - // Create a TestApplePacProxyResolver and set the test resolver to use it. - test_resolver->setPacResolverForTest( - std::make_unique(std::string(host), port)); - } // Start the resolver, as we do when registering the envoy_proxy_resolver API. test_resolver->start(); // Create a new test ProxyResolverApi. diff --git a/mobile/test/common/proxy/test_apple_pac_proxy_resolver.cc b/mobile/test/common/proxy/test_apple_pac_proxy_resolver.cc deleted file mode 100644 index 9c46434ce4c70..0000000000000 --- a/mobile/test/common/proxy/test_apple_pac_proxy_resolver.cc +++ /dev/null @@ -1,92 +0,0 @@ -#include "test/common/proxy/test_apple_pac_proxy_resolver.h" - -#include -#include - -#include - -#include "library/common/network/proxy_settings.h" - -namespace Envoy { -namespace Network { - -namespace { - -// Contains state that is passed into the run loop callback. -struct RunLoopSourceInfo { - RunLoopSourceInfo(CFStreamClientContext* _context, const std::string& _host, const int _port) - : context(_context), host(_host), port(_port) {} - - CFStreamClientContext* context; - const std::string& host; - const int port; -}; - -void runLoopCallback(void* info) { - // Wrap in unique_ptr so we release the memory at the end of the function execution. - std::unique_ptr run_loop_info(static_cast(info)); - - const void* keys[] = {kCFProxyTypeKey, kCFProxyHostNameKey, kCFProxyPortNumberKey}; - const void* values[] = { - kCFProxyTypeHTTPS, - CFStringCreateWithCString(kCFAllocatorDefault, run_loop_info->host.c_str(), - kCFStringEncodingUTF8), - CFNumberCreate(kCFAllocatorDefault, kCFNumberIntType, &run_loop_info->port)}; - const int num_pairs = sizeof(keys) / sizeof(CFStringRef); - - CFDictionaryRef settings_dict = CFDictionaryCreate( - kCFAllocatorDefault, static_cast(keys), static_cast(values), - num_pairs, &kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks); - - const void* objects[] = {settings_dict}; - CFArrayRef proxies = CFArrayCreate(kCFAllocatorDefault, static_cast(objects), - /*numValues=*/1, &kCFTypeArrayCallBacks); - - // Wrap in unique_ptr so we release the memory at the end of the function execution. - std::unique_ptr client_context(run_loop_info->context); - - // Invoke the proxy resolution callback that the ApplePacProxyResolver invokes. - Network::proxyAutoConfigurationResultCallback(/*ptr=*/client_context->info, proxies, - /*cf_error=*/nullptr); - - // Release the memory allocated above. - CFRelease(proxies); - CFRelease(settings_dict); - // We don't want to release the first value, since it's a global constant. - for (int i = 1; i < num_pairs; ++i) { - CFRelease(values[i]); - } -} - -} // namespace - -TestApplePacProxyResolver::TestApplePacProxyResolver(const std::string& host, const int port) - : host_(host), port_(port) {} - -CFRunLoopSourceRef TestApplePacProxyResolver::createPacUrlResolverSource( - CFURLRef /*cf_proxy_autoconfiguration_file_url*/, CFURLRef /*cf_target_url*/, - CFStreamClientContext* context) { - auto run_loop_info = std::make_unique(context, host_, port_); - run_loop_context_ = std::make_unique( - CFRunLoopSourceContext{/*version=*/0, - /*info=*/run_loop_info.release(), - /*retain=*/nullptr, - /*release=*/nullptr, - /*copyDescription=*/nullptr, - /*equal=*/nullptr, - /*hash=*/nullptr, - /*schedule=*/nullptr, - /*cancel=*/nullptr, - /*perform=*/runLoopCallback}); - - auto run_loop_source = - CFRunLoopSourceCreate(kCFAllocatorDefault, /*order=*/0, run_loop_context_.get()); - // There are no network events that get triggered to notify this test run loop source that - // it should process a result, so we have to signal it manually. - CFRunLoopSourceSignal(run_loop_source); - - return run_loop_source; -} - -} // namespace Network -} // namespace Envoy diff --git a/mobile/test/common/proxy/test_apple_pac_proxy_resolver.h b/mobile/test/common/proxy/test_apple_pac_proxy_resolver.h deleted file mode 100644 index d1985c967c034..0000000000000 --- a/mobile/test/common/proxy/test_apple_pac_proxy_resolver.h +++ /dev/null @@ -1,27 +0,0 @@ -#pragma once - -#include -#include - -#include "library/common/network/apple_pac_proxy_resolver.h" - -namespace Envoy { -namespace Network { - -class TestApplePacProxyResolver : public Network::ApplePacProxyResolver { -public: - TestApplePacProxyResolver(const std::string& host, const int port); - virtual ~TestApplePacProxyResolver() = default; - -protected: - CFRunLoopSourceRef createPacUrlResolverSource(CFURLRef cf_proxy_autoconfiguration_file_url, - CFURLRef cf_target_url, - CFStreamClientContext* context) override; - - const std::string host_; - const int port_; - std::unique_ptr run_loop_context_; -}; - -} // namespace Network -} // namespace Envoy diff --git a/mobile/test/common/proxy/test_apple_proxy_resolver.cc b/mobile/test/common/proxy/test_apple_proxy_resolver.cc index 283518fef8bc2..b2649ad651177 100644 --- a/mobile/test/common/proxy/test_apple_proxy_resolver.cc +++ b/mobile/test/common/proxy/test_apple_proxy_resolver.cc @@ -8,10 +8,5 @@ void TestAppleProxyResolver::setSettingsMonitorForTest( proxy_settings_monitor_ = std::move(monitor); } -void TestAppleProxyResolver::setPacResolverForTest( - std::unique_ptr&& resolver) { - pac_proxy_resolver_ = std::move(resolver); -} - } // namespace Network } // namespace Envoy diff --git a/mobile/test/common/proxy/test_apple_proxy_resolver.h b/mobile/test/common/proxy/test_apple_proxy_resolver.h index f5543685cb2a5..96053222da8da 100644 --- a/mobile/test/common/proxy/test_apple_proxy_resolver.h +++ b/mobile/test/common/proxy/test_apple_proxy_resolver.h @@ -15,12 +15,6 @@ class TestAppleProxyResolver : public Network::AppleProxyResolver { */ void setSettingsMonitorForTest(std::unique_ptr&& monitor); - - /** - * Resets the PAC URL resolver to the supplied instance. - * For tests only. - */ - void setPacResolverForTest(std::unique_ptr&& resolver); }; } // namespace Network diff --git a/mobile/test/common/proxy/test_apple_proxy_settings_monitor.cc b/mobile/test/common/proxy/test_apple_proxy_settings_monitor.cc index 7c6ff214dc9de..64dfe90e66985 100644 --- a/mobile/test/common/proxy/test_apple_proxy_settings_monitor.cc +++ b/mobile/test/common/proxy/test_apple_proxy_settings_monitor.cc @@ -3,6 +3,8 @@ #include #include +#include "absl/strings/str_cat.h" + namespace Envoy { namespace Network { @@ -44,13 +46,11 @@ CFDictionaryRef TestAppleSystemProxySettingsMonitor::getSystemProxySettingsWithP const void* keys[] = {kCFNetworkProxiesProxyAutoConfigEnable, kCFNetworkProxiesProxyAutoConfigURLString}; + // Interpret the host + port as the location from which to obtain the PAC file. + const std::string pac_file_url = absl::StrCat("http://", host_, ":", port_); const void* values[] = { CFNumberCreate(kCFAllocatorDefault, kCFNumberIntType, &one_), - // The PAC file URL doesn't matter, we don't use it in the tests; we just need it to exist to - // exercise the PAC file URL code path in the tests. - CFStringCreateWithCString(kCFAllocatorDefault, - "http://randomproxysettingsserver.com/random.pac", - kCFStringEncodingUTF8)}; + CFStringCreateWithCString(kCFAllocatorDefault, pac_file_url.c_str(), kCFStringEncodingUTF8)}; int num_pairs = sizeof(keys) / sizeof(CFStringRef); diff --git a/mobile/test/swift/integration/proxying/BUILD b/mobile/test/swift/integration/proxying/BUILD index b891bf3b39c7a..4733cead3fefe 100644 --- a/mobile/test/swift/integration/proxying/BUILD +++ b/mobile/test/swift/integration/proxying/BUILD @@ -21,3 +21,20 @@ envoy_mobile_swift_test( "//test/swift/integration:test_extensions", ], ) + +envoy_mobile_swift_test( + name = "http_request_using_pac_proxy_test", + srcs = [ + "HTTPRequestUsingPacProxyTest.swift", + ], + exec_properties = { + "sandboxNetwork": "standard", + }, + visibility = ["//visibility:public"], + deps = [ + "//library/objective-c:envoy_engine_objc_lib", + "//test/objective-c:envoy_test_api", + "//test/objective-c:envoy_test_server", + "//test/swift/integration:test_extensions", + ], +) diff --git a/mobile/test/swift/integration/proxying/HTTPRequestUsingPacProxyTest.swift b/mobile/test/swift/integration/proxying/HTTPRequestUsingPacProxyTest.swift new file mode 100644 index 0000000000000..a73db72de8e36 --- /dev/null +++ b/mobile/test/swift/integration/proxying/HTTPRequestUsingPacProxyTest.swift @@ -0,0 +1,91 @@ +import Envoy +import EnvoyEngine +import EnvoyTestApi +import EnvoyTestServer +import Foundation +import TestExtensions +import XCTest + +final class HTTPRequestUsingPacProxyTest: XCTestCase { + override static func setUp() { + super.setUp() + register_test_extensions() + } + + override static func tearDown() { + super.tearDown() + // Flush the stdout and stderror to show the print output. + fflush(stdout) + fflush(stderr) + } + + private func executeRequest(engine: Engine, scheme: String, authority: String) -> String? { + let responseHeadersExpectation = + self.expectation(description: "Successful response headers received") + let responseTrailersExpectation = + self.expectation(description: "Successful response trailers received") + + let requestHeaders = RequestHeadersBuilder(method: .get, scheme: scheme, + authority: authority, path: "/") + .build() + + var responseBuffer = Data() + engine.streamClient() + .newStreamPrototype() + .setOnResponseHeaders { responseHeaders, _, _ in + XCTAssertEqual(200, responseHeaders.httpStatus) + responseHeadersExpectation.fulfill() + } + .setOnResponseData { data, _, _ in + responseBuffer.append(contentsOf: data) + } + .setOnResponseTrailers { _, _ in + responseTrailersExpectation.fulfill() + } + .start() + .sendHeaders(requestHeaders, endStream: true) + + let expectations = [responseHeadersExpectation, responseTrailersExpectation] + XCTAssertEqual(XCTWaiter.wait(for: expectations, timeout: 10), .completed) + + return String(data: responseBuffer, encoding: .utf8) + } + + func testHTTPRequestUsingPacProxy() throws { + EnvoyTestServer.startHttpProxyServer() + let proxyPort = EnvoyTestServer.getProxyPort() + EnvoyTestServer.startHttp1PlaintextServer() + let httpPort = EnvoyTestServer.getHttpPort() + + let engineExpectation = self.expectation(description: "Run started engine") + + let engine = EngineBuilder() + .setLogLevel(.info) + .setLogger { _, msg in + print(msg, terminator: "") + } + .setOnEngineRunning { + engineExpectation.fulfill() + } + .respectSystemProxySettings(true) + .build() + + let pacScript = """ + function FindProxyForURL(url, host) { + return "PROXY 127.0.0.1:\(proxyPort)"; + } + """ + EnvoyTestServer.setHeadersAndData("Content-Type", header_value: "application/x-ns-proxy-autoconfig", response_body: pacScript) + EnvoyTestApi.registerTestProxyResolver("127.0.0.1", port: httpPort, usePacResolver: true) + + XCTAssertEqual(XCTWaiter.wait(for: [engineExpectation], timeout: 5), .completed) + + if let respBody = executeRequest(engine: engine, scheme: "http", authority: "neverssl.com") { + XCTAssertGreaterThanOrEqual(respBody.utf8.count, 3900) + } + + engine.terminate() + EnvoyTestServer.shutdownTestProxyServer() + EnvoyTestServer.shutdownTestHttpServer() + } +} diff --git a/mobile/test/swift/integration/proxying/HTTPRequestUsingProxyTest.swift b/mobile/test/swift/integration/proxying/HTTPRequestUsingProxyTest.swift index 48f5d17a089bc..daa302a1bf94b 100644 --- a/mobile/test/swift/integration/proxying/HTTPRequestUsingProxyTest.swift +++ b/mobile/test/swift/integration/proxying/HTTPRequestUsingProxyTest.swift @@ -139,65 +139,6 @@ final class HTTPRequestUsingProxyTest: XCTestCase { EnvoyTestServer.shutdownTestProxyServer() } - // https://github.com/envoyproxy/envoy/issues/33014 - func skipped_testHTTPSRequestUsingPacFileUrlResolver() throws { - EnvoyTestServer.startHttpsProxyServer() - let port = EnvoyTestServer.getProxyPort() - - let engineExpectation = self.expectation(description: "Run started engine") - let responseHeadersExpectation = - self.expectation(description: "Successful response headers received") - let responseBodyExpectation = - self.expectation(description: "Successful response trailers received") - - let engine = EngineBuilder() - .setLogLevel(.debug) - .setLogger { _, msg in - print(msg, terminator: "") - } - .setOnEngineRunning { - engineExpectation.fulfill() - } - .respectSystemProxySettings(true) - .build() - - EnvoyTestApi.registerTestProxyResolver("127.0.0.1", port: port, usePacResolver: true) - - XCTAssertEqual(XCTWaiter.wait(for: [engineExpectation], timeout: 5), .completed) - - let requestHeaders = RequestHeadersBuilder(method: .get, scheme: "https", - authority: "cloud.google.com", path: "/") - .build() - - var responseBuffer = Data() - engine.streamClient() - .newStreamPrototype() - .setOnResponseHeaders { responseHeaders, _, _ in - XCTAssertEqual(200, responseHeaders.httpStatus) - responseHeadersExpectation.fulfill() - } - .setOnResponseData { data, endStream, _ in - responseBuffer.append(contentsOf: data) - if endStream { - responseBodyExpectation.fulfill() - } - } - .setOnResponseTrailers { _, _ in - } - .start() - .sendHeaders(requestHeaders, endStream: true) - - let expectations = [responseHeadersExpectation, responseBodyExpectation] - XCTAssertEqual(XCTWaiter.wait(for: expectations, timeout: 10), .completed) - - if let responseBody = String(data: responseBuffer, encoding: .utf8) { - XCTAssertGreaterThanOrEqual(responseBody.utf8.count, 3900) - } - - engine.terminate() - EnvoyTestServer.shutdownTestProxyServer() - } - func testTwoHTTPRequestsUsingProxy() throws { EnvoyTestServer.startHttpProxyServer() let port = EnvoyTestServer.getProxyPort()