Skip to content

Commit 42503b8

Browse files
kunlingliokunlinglio
committed
LibProxy+Services: Fix proxy being ignored in some cases
- Update system proxy detect sequence in `Request.cpp` to make it compatible with disk cache - Clean up remaining debug message - Correct returned proxy type in `ProxyMacOS.cpp`
1 parent 1218cad commit 42503b8

5 files changed

Lines changed: 74 additions & 28 deletions

File tree

‎Libraries/LibProxy/Proxy.cpp‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,11 @@ static Atomic<ProxyMode> g_proxy_mode { ProxyMode::System };
1616

1717
void set_proxy_mode(ProxyMode const& new_mode)
1818
{
19-
dbgln("Setting proxy mode to {}", print_proxy_mode(new_mode));
2019
g_proxy_mode.store(new_mode);
2120
}
2221

2322
ProxyMode proxy_mode()
2423
{
25-
dbgln("Getting proxy mode: {}", print_proxy_mode(g_proxy_mode.load()));
2624
return g_proxy_mode.load();
2725
}
2826

‎Libraries/LibProxy/Proxy.h‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,21 @@ struct ProxyData {
4949
bool operator==(ProxyData const&) const = default;
5050

5151
static ErrorOr<ProxyData> from_url(URL::URL const&);
52+
53+
ALWAYS_INLINE String print_type() const
54+
{
55+
switch (type) {
56+
case Type::Direct:
57+
return "direct"_string;
58+
case Type::SOCKS5:
59+
return "socks5"_string;
60+
case Type::HTTP:
61+
return "http"_string;
62+
case Type::HTTPS:
63+
return "https"_string;
64+
}
65+
VERIFY_NOT_REACHED();
66+
}
5267
};
5368

5469
Vector<ProxyData> get_proxies_for_url(URL::URL const&);
@@ -58,5 +73,6 @@ Vector<ProxyData> get_proxies_from_mac_system_configuration(URL::URL const&);
5873
#endif
5974

6075
// FIXME: Add more platform-specific proxy retrieval functions here (e.g. Android, BSD, Windows).
76+
// FIXME: Add support to Proxy auto-configuration (PAC) js script here.
6177

6278
}

‎Libraries/LibProxy/ProxyMacOS.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,9 @@ Vector<ProxyData> get_proxies_from_mac_system_configuration(URL::URL const& url)
3737
auto check_proxy = [&](CFStringRef enable_key, CFStringRef host_key, CFStringRef port_key, ProxyData::Type type) {
3838
CFNumberRef enabled = (CFNumberRef)CFDictionaryGetValue(proxies, enable_key);
3939
if (enabled) {
40-
int val = 0;
41-
CFNumberGetValue(enabled, kCFNumberIntType, &val);
42-
if (val) {
40+
int enabled_val = 0;
41+
CFNumberGetValue(enabled, kCFNumberIntType, &enabled_val);
42+
if (enabled_val) {
4343
CFStringRef host = (CFStringRef)CFDictionaryGetValue(proxies, host_key);
4444
CFNumberRef port = (CFNumberRef)CFDictionaryGetValue(proxies, port_key);
4545
if (host && port) {
@@ -58,7 +58,7 @@ Vector<ProxyData> get_proxies_from_mac_system_configuration(URL::URL const& url)
5858
if (url.scheme() == "http") {
5959
check_proxy(kSCPropNetProxiesHTTPEnable, kSCPropNetProxiesHTTPProxy, kSCPropNetProxiesHTTPPort, ProxyData::Type::HTTP);
6060
} else if (url.scheme() == "https") {
61-
check_proxy(kSCPropNetProxiesHTTPSEnable, kSCPropNetProxiesHTTPSProxy, kSCPropNetProxiesHTTPSPort, ProxyData::Type::HTTPS);
61+
check_proxy(kSCPropNetProxiesHTTPSEnable, kSCPropNetProxiesHTTPSProxy, kSCPropNetProxiesHTTPSPort, ProxyData::Type::HTTP);
6262
}
6363

6464
check_proxy(kSCPropNetProxiesSOCKSEnable, kSCPropNetProxiesSOCKSProxy, kSCPropNetProxiesSOCKSPort, ProxyData::Type::SOCKS5);

‎Services/RequestServer/Request.cpp‎

Lines changed: 53 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -386,7 +386,7 @@ NonnullOwnPtr<Request> Request::connect(
386386
{
387387
auto request = adopt_own(*new Request { request_id, client, curl_multi, resolver, move(url) });
388388
request->m_connect_cache_level = cache_level;
389-
request->transition_to_state(State::DNSLookup);
389+
request->transition_to_network_lookup();
390390
return request;
391391
}
392392

@@ -583,6 +583,25 @@ void Request::process()
583583
}
584584
}
585585

586+
void Request::transition_to_network_lookup()
587+
{
588+
if (Proxy::use_system_proxy()) {
589+
auto proxies = Proxy::get_proxies_for_url(m_url);
590+
if (!proxies.is_empty()) {
591+
m_proxy_data = proxies.first(); // FIXME: Support multiple proxies (e.g. fallback proxies).
592+
if (!m_proxy_data.is_direct()) {
593+
if (m_type == RequestType::Connect)
594+
transition_to_state(State::Connect);
595+
else
596+
transition_to_state(State::RetrieveCookie);
597+
return;
598+
}
599+
}
600+
}
601+
602+
transition_to_state(State::DNSLookup);
603+
}
604+
586605
void Request::handle_initial_state()
587606
{
588607
// Check for resource substitution before anything else.
@@ -610,7 +629,7 @@ void Request::handle_initial_state()
610629
m_client.start_revalidation_request({}, m_method, m_url, m_request_headers, m_request_body, m_include_credentials);
611630

612631
if (is_revalidation_request())
613-
transition_to_state(State::DNSLookup);
632+
transition_to_network_lookup();
614633
else
615634
transition_to_state(State::ReadCache);
616635
} else if (m_type == RequestType::BackgroundRevalidation) {
@@ -651,19 +670,7 @@ void Request::handle_initial_state()
651670
return;
652671
}
653672

654-
if (Proxy::use_system_proxy()) {
655-
auto proxies = Proxy::get_proxies_for_url(m_url);
656-
if (!proxies.is_empty()) {
657-
m_proxy_data = proxies.first(); // FIXME: Support multiple proxies (e.g. fallback proxies).
658-
if (!m_proxy_data.is_direct()) {
659-
dbgln("Using system proxy {}:{} for {}", m_proxy_data.host, m_proxy_data.port, m_url.serialize());
660-
transition_to_state(State::RetrieveCookie);
661-
return;
662-
}
663-
}
664-
}
665-
666-
transition_to_state(State::DNSLookup);
673+
transition_to_network_lookup();
667674
}
668675

669676
void Request::handle_read_cache_state()
@@ -867,16 +874,40 @@ void Request::handle_connect_state()
867874
set_option(CURLOPT_CONNECTTIMEOUT, s_connect_timeout_seconds);
868875
set_option(CURLOPT_CONNECT_ONLY, 1L);
869876

870-
// Pre-populate the multi's hostcache so libcurl skips its threaded resolver entirely.
871-
VERIFY(m_dns_result);
872-
auto formatted_address = build_curl_resolve_list(*m_dns_result, m_url.serialized_host(), m_url.port_or_default());
873-
if (curl_slist* resolve_list = curl_slist_append(nullptr, formatted_address.characters())) {
874-
set_option(CURLOPT_RESOLVE, resolve_list);
875-
m_curl_string_lists.append(resolve_list);
876-
} else {
877+
switch (m_proxy_data.type) {
878+
case Proxy::ProxyData::Type::Direct:
879+
break;
880+
case Proxy::ProxyData::Type::HTTP:
881+
set_option(CURLOPT_PROXYTYPE, CURLPROXY_HTTP);
882+
set_option(CURLOPT_PROXY, m_proxy_data.host.to_byte_string().characters());
883+
set_option(CURLOPT_PROXYPORT, static_cast<long>(m_proxy_data.port));
884+
break;
885+
case Proxy::ProxyData::Type::HTTPS:
886+
set_option(CURLOPT_PROXYTYPE, CURLPROXY_HTTPS);
887+
set_option(CURLOPT_PROXY, m_proxy_data.host.to_byte_string().characters());
888+
set_option(CURLOPT_PROXYPORT, static_cast<long>(m_proxy_data.port));
889+
break;
890+
case Proxy::ProxyData::Type::SOCKS5:
891+
set_option(CURLOPT_PROXYTYPE, CURLPROXY_SOCKS5_HOSTNAME);
892+
set_option(CURLOPT_PROXY, m_proxy_data.host.to_byte_string().characters());
893+
set_option(CURLOPT_PROXYPORT, static_cast<long>(m_proxy_data.port));
894+
break;
895+
default:
877896
VERIFY_NOT_REACHED();
878897
}
879898

899+
if (m_proxy_data.is_direct()) {
900+
// Pre-populate the multi's hostcache so libcurl skips its threaded resolver entirely.
901+
VERIFY(m_dns_result);
902+
auto formatted_address = build_curl_resolve_list(*m_dns_result, m_url.serialized_host(), m_url.port_or_default());
903+
if (curl_slist* resolve_list = curl_slist_append(nullptr, formatted_address.characters())) {
904+
set_option(CURLOPT_RESOLVE, resolve_list);
905+
m_curl_string_lists.append(resolve_list);
906+
} else {
907+
VERIFY_NOT_REACHED();
908+
}
909+
}
910+
880911
mark_lifecycle_event(this, &WireStats::curl_added_at);
881912
auto result = curl_multi_add_handle(m_curl_multi_handle, m_curl_easy_handle);
882913
VERIFY(result == CURLM_OK);

‎Services/RequestServer/Request.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@ class Request final : public HTTP::CacheRequest {
143143
URL::URL url);
144144

145145
void transition_to_state(State);
146+
void transition_to_network_lookup();
146147
void process();
147148

148149
void handle_initial_state();

0 commit comments

Comments
 (0)