From b5c8c48a87a19618b23d6d4fc5035915a9322117 Mon Sep 17 00:00:00 2001
From: Kai Koehne <kai.koehne@qt.io>
Date: Thu, 20 Jul 2017 16:14:47 +0200
Subject: [PATCH] Fix logic to check for proxy settings
The previous code tried to find out whether a user has set an
application proxy by checking the type of the applicationProxy.
This is wrong, because a system proxy will actually also change the
applicationProxy type.
Instead, we now rely on QNetworkProxyFactory::usesSystemConfiguration
to decide whether to use QtNetwork's application proxy, or Chromium's
logic for the system proxy. We also save the state of
QNetworkProxy::useSystemConfiguration to be able to track changes.
[ChangeLog][Networking] Fixed an issue where system proxy settings were
not picked up correctly.
Task-number: QTBUG-61910
Change-Id: I1d9af3f6006ba187266fe50c645f425a46632e41
Reviewed-by: Peter Varga <pvarga@inf.u-szeged.hu>
---
src/core/proxy_config_service_qt.cpp | 92 ++++++++++++++++++----------
src/core/proxy_config_service_qt.h | 6 +-
2 files changed, 63 insertions(+), 35 deletions(-)
diff --git a/src/core/proxy_config_service_qt.cpp b/src/core/proxy_config_service_qt.cpp
index b7ed83624..491131e42 100644
--- a/src/core/proxy_config_service_qt.cpp
+++ b/src/core/proxy_config_service_qt.cpp
@@ -36,6 +36,9 @@
** $QT_END_LICENSE$
**
****************************************************************************/
+
+
+//================ Based on ChromeProxyConfigService =======================
// Copyright (c) 2011 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
@@ -67,11 +70,10 @@ net::ProxyServer ProxyConfigServiceQt::fromQNetworkProxy(const QNetworkProxy &qt
return net::ProxyServer(proxyScheme, net::HostPortPair(qtProxy.hostName().toStdString(), qtProxy.port()));
}
-//================ Based on ChromeProxyConfigService =======================
-
ProxyConfigServiceQt::ProxyConfigServiceQt(std::unique_ptr<ProxyConfigService> baseService)
: m_baseService(baseService.release()),
- m_registeredObserver(false)
+ m_registeredObserver(false),
+ m_usesSystemConfiguration(false)
{
}
@@ -83,7 +85,6 @@ ProxyConfigServiceQt::~ProxyConfigServiceQt()
void ProxyConfigServiceQt::AddObserver(net::ProxyConfigService::Observer *observer)
{
- RegisterObserver();
m_observers.AddObserver(observer);
}
@@ -94,28 +95,31 @@ void ProxyConfigServiceQt::RemoveObserver(net::ProxyConfigService::Observer *obs
net::ProxyConfigService::ConfigAvailability ProxyConfigServiceQt::GetLatestProxyConfig(net::ProxyConfig *config)
{
- RegisterObserver();
-
- // Ask the base service if available.
- net::ProxyConfig systemConfig;
- ConfigAvailability systemAvailability = net::ProxyConfigService::CONFIG_UNSET;
- if (m_baseService.get())
- systemAvailability = m_baseService->GetLatestProxyConfig(&systemConfig);
+#if QT_VERSION >= QT_VERSION_CHECK(5, 8, 0)
+ m_usesSystemConfiguration = QNetworkProxyFactory::usesSystemConfiguration();
+#endif
+ if (m_usesSystemConfiguration) {
+ // Use Chromium's base service to retrieve system settings
+ net::ProxyConfig systemConfig;
+ ConfigAvailability systemAvailability = net::ProxyConfigService::CONFIG_UNSET;
+ if (m_baseService.get())
+ systemAvailability = m_baseService->GetLatestProxyConfig(&systemConfig);
+ *config = systemConfig;
+ // make sure to get updates via OnProxyConfigChanged
+ RegisterObserver();
+ return systemAvailability;
+ }
+ // Use QNetworkProxy::applicationProxy settings
const QNetworkProxy &qtProxy = QNetworkProxy::applicationProxy();
if (qtProxy == m_qtApplicationProxy && !m_qtProxyConfig.proxy_rules().empty()) {
+ // no changes
*config = m_qtProxyConfig;
return CONFIG_VALID;
}
+
m_qtApplicationProxy = qtProxy;
m_qtProxyConfig = net::ProxyConfig();
-#if QT_VERSION >= QT_VERSION_CHECK(5, 7, 0)
- if (qtProxy.type() == QNetworkProxy::NoProxy
- && QNetworkProxyFactory::usesSystemConfiguration()) {
- *config = systemConfig;
- return systemAvailability;
- }
-#endif
net::ProxyConfig::ProxyRules qtRules;
net::ProxyServer server = fromQNetworkProxy(qtProxy);
@@ -145,31 +149,51 @@ net::ProxyConfigService::ConfigAvailability ProxyConfigServiceQt::GetLatestProxy
void ProxyConfigServiceQt::OnLazyPoll()
{
- if (m_qtApplicationProxy != QNetworkProxy::applicationProxy()) {
- net::ProxyConfig unusedConfig;
- OnProxyConfigChanged(unusedConfig, CONFIG_VALID);
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+
+#if QT_VERSION >= QT_VERSION_CHECK(5, 8, 0)
+ // We need to update if
+ // - setUseSystemConfiguration() was called in between
+ // - user changed application proxy
+ if (m_usesSystemConfiguration != QNetworkProxyFactory::usesSystemConfiguration()
+ || (!m_usesSystemConfiguration && m_qtApplicationProxy != QNetworkProxy::applicationProxy())) {
+ Update();
+ } else if (m_usesSystemConfiguration) {
+ if (m_baseService.get())
+ m_baseService->OnLazyPoll();
}
- if (m_baseService.get())
- m_baseService->OnLazyPoll();
+#else
+ if (m_qtApplicationProxy != QNetworkProxy::applicationProxy())
+ Update();
+#endif
}
-
+// Called when the base service changed
void ProxyConfigServiceQt::OnProxyConfigChanged(const net::ProxyConfig &config, ConfigAvailability availability)
{
- Q_UNUSED(config);
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+ Q_UNUSED(config);
- if (m_qtApplicationProxy != QNetworkProxy::applicationProxy()
- || m_qtApplicationProxy.type() == QNetworkProxy::NoProxy) {
- net::ProxyConfig actual_config;
- availability = GetLatestProxyConfig(&actual_config);
- if (availability == CONFIG_PENDING)
- return;
- for (net::ProxyConfigService::Observer &observer: m_observers)
- observer.OnProxyConfigChanged(actual_config, availability);
- }
+ if (!m_usesSystemConfiguration)
+ return;
+
+ Update();
+}
+
+// Update our observers
+void ProxyConfigServiceQt::Update()
+{
+ net::ProxyConfig actual_config;
+ ConfigAvailability availability = GetLatestProxyConfig(&actual_config);
+ if (availability == CONFIG_PENDING)
+ return;
+ for (net::ProxyConfigService::Observer &observer: m_observers)
+ observer.OnProxyConfigChanged(actual_config, availability);
}
+// Register ourselves as observer of the base service.
+// This has to be done on the IO thread, and therefore cannot be done
+// in the constructor.
void ProxyConfigServiceQt::RegisterObserver()
{
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
diff --git a/src/core/proxy_config_service_qt.h b/src/core/proxy_config_service_qt.h
index f2f9a2210..7be3289d0 100644
--- a/src/core/proxy_config_service_qt.h
+++ b/src/core/proxy_config_service_qt.h
@@ -69,13 +69,17 @@ private:
void OnProxyConfigChanged(const net::ProxyConfig& config,
ConfigAvailability availability) override;
+ // Retrieve new proxy settings and notify observers.
+ void Update();
+
// Makes sure that the observer registration with the base service is set up.
void RegisterObserver();
std::unique_ptr<net::ProxyConfigService> m_baseService;
base::ObserverList<net::ProxyConfigService::Observer, true> m_observers;
- // Keep the last QNetworkProxy::applicationProxy state around.
+ // Keep the last state around.
+ bool m_usesSystemConfiguration;
QNetworkProxy m_qtApplicationProxy;
net::ProxyConfig m_qtProxyConfig;
--
GitLab