Fix empty strings in proxy exclude list results in proxy being skipped for ALL hosts

nyalldawson · nyalldawson · commit cf1cf0fe455a · 2019-01-30T05:08:39.000+11:00
Fixes #20213
diff --git a/src/app/qgsoptions.cpp b/src/app/qgsoptions.cpp
@@ -360,6 +360,9 @@ QgsOptions::QgsOptions( QWidget *parent, Qt::WindowFlags fl, const QList<QgsOpti
   const QStringList excludedUrlPathList = mSettings->value( QStringLiteral( "proxy/proxyExcludedUrls" ) ).toStringList();
   for ( const QString &path : excludedUrlPathList )
   {
+    if ( path.trimmed().isEmpty() )
+      continue;
+
     QListWidgetItem *newItem = new QListWidgetItem( mExcludeUrlListWidget );
     newItem->setText( path );
     newItem->setFlags( Qt::ItemIsEditable | Qt::ItemIsEnabled | Qt::ItemIsSelectable );
@@ -1386,9 +1389,12 @@ void QgsOptions::saveOptions()
 
   //url to exclude from proxys
   QStringList excludedUrls;
+  excludedUrls.reserve( mExcludeUrlListWidget->count() );
   for ( int i = 0; i < mExcludeUrlListWidget->count(); ++i )
   {
-    excludedUrls << mExcludeUrlListWidget->item( i )->text();
+    const QString host = mExcludeUrlListWidget->item( i )->text();
+    if ( !host.trimmed().isEmpty() )
+      excludedUrls << host;
   }
   mSettings->setValue( QStringLiteral( "proxy/proxyExcludedUrls" ), excludedUrls );
 
diff --git a/src/core/qgsnetworkaccessmanager.cpp b/src/core/qgsnetworkaccessmanager.cpp
@@ -76,7 +76,7 @@ class QgsNetworkProxyFactory : public QNetworkProxyFactory
 
       Q_FOREACH ( const QString &exclude, nam->excludeList() )
       {
-        if ( url.startsWith( exclude ) )
+        if ( !exclude.trimmed().isEmpty() && url.startsWith( exclude ) )
         {
           QgsDebugMsgLevel( QStringLiteral( "using default proxy for %1 [exclude %2]" ).arg( url, exclude ), 4 );
           return QList<QNetworkProxy>() << QNetworkProxy();
@@ -172,6 +172,13 @@ void QgsNetworkAccessManager::setFallbackProxyAndExcludes( const QNetworkProxy &
 
   mFallbackProxy = proxy;
   mExcludedURLs = excludes;
+  // remove empty records from excludes list -- these would otherwise match ANY url, so the proxy would always be skipped!
+  mExcludedURLs.erase( std::remove_if( mExcludedURLs.begin(), mExcludedURLs.end(), // clazy:exclude=detaching-member
+                                       []( const QString & url )
+  {
+    return url.trimmed().isEmpty();
+  } ), mExcludedURLs.end() ); // clazy:exclude=detaching-member
+
 }
 
 QNetworkReply *QgsNetworkAccessManager::createRequest( QNetworkAccessManager::Operation op, const QNetworkRequest &req, QIODevice *outgoingData )
@@ -448,7 +455,7 @@ void QgsNetworkAccessManager::setupDefaultProxyAndCache( Qt::ConnectionType conn
   bool proxyEnabled = settings.value( QStringLiteral( "proxy/proxyEnabled" ), false ).toBool();
   if ( proxyEnabled )
   {
-    excludes = settings.value( QStringLiteral( "proxy/proxyExcludedUrls" ), "" ).toStringList();
+    excludes = settings.value( QStringLiteral( "proxy/proxyExcludedUrls" ), QStringList() ).toStringList();
 
     //read type, host, port, user, passw from settings
     QString proxyHost = settings.value( QStringLiteral( "proxy/proxyHost" ), "" ).toString();
diff --git a/tests/src/core/testqgsnetworkaccessmanager.cpp b/tests/src/core/testqgsnetworkaccessmanager.cpp
@@ -82,6 +82,7 @@ class TestQgsNetworkAccessManager : public QObject
     void cleanupTestCase();// will be called after the last testfunction was executed.
     void init();// will be called before each testfunction is executed.
     void cleanup();// will be called after every testfunction.
+    void testProxyExcludeList();
     void fetchEmptyUrl(); //test fetching blank url
     void fetchBadUrl(); //test fetching bad url
     void fetchEncodedContent(); //test fetching url content encoded as utf-8
@@ -118,6 +119,20 @@ void TestQgsNetworkAccessManager::cleanup()
 {
 }
 
+void TestQgsNetworkAccessManager::testProxyExcludeList()
+{
+  QgsNetworkAccessManager manager;
+  QNetworkProxy fallback( QNetworkProxy::HttpProxy, QStringLiteral( "babies_first_proxy" ) );
+  manager.setFallbackProxyAndExcludes( fallback, QStringList() << QStringLiteral( "intranet" ) << QStringLiteral( "something_else" ) );
+  QCOMPARE( manager.fallbackProxy().hostName(), QStringLiteral( "babies_first_proxy" ) );
+  QCOMPARE( manager.excludeList(), QStringList() << QStringLiteral( "intranet" ) << QStringLiteral( "something_else" ) );
+
+  QgsNetworkAccessManager manager2;
+  manager2.setFallbackProxyAndExcludes( fallback, QStringList() << QStringLiteral( "intranet" ) << "" );
+  // empty strings MUST be filtered from this list - otherwise they match all hosts!
+  QCOMPARE( manager2.excludeList(), QStringList() << QStringLiteral( "intranet" ) );
+}
+
 void TestQgsNetworkAccessManager::fetchEmptyUrl()
 {
   QObject context;
