Merge pull request #5332 from boundlessgeo/auth_mutex

[auth] Add mutexes to protect access to authentication config cache from multiple threads
qgis · Oct 11, 2017 · eb5c559 · eb5c559
2 parents 0b5d07c + 057ddc2
commit eb5c559
Show file tree

Hide file tree

Showing 10 changed files with 40 additions and 5 deletions.
diff --git a/src/auth/basic/qgsauthbasicmethod.cpp b/src/auth/basic/qgsauthbasicmethod.cpp
@@ -21,6 +21,7 @@
 #include "qgslogger.h"
 
 #include <QNetworkProxy>
+#include <QMutexLocker>
 
 static const QString AUTH_METHOD_KEY = QStringLiteral( "Basic" );
 static const QString AUTH_METHOD_DESCRIPTION = QStringLiteral( "Basic authentication" );
@@ -169,6 +170,7 @@ void QgsAuthBasicMethod::clearCachedConfig( const QString &authcfg )
 
 QgsAuthMethodConfig QgsAuthBasicMethod::getMethodConfig( const QString &authcfg, bool fullconfig )
 {
+  QMutexLocker locker( &mConfigMutex );
   QgsAuthMethodConfig mconfig;
 
   // check if it is cached
@@ -187,19 +189,22 @@ QgsAuthMethodConfig QgsAuthBasicMethod::getMethodConfig( const QString &authcfg,
   }
 
   // cache bundle
+  locker.unlock();
   putMethodConfig( authcfg, mconfig );
 
   return mconfig;
 }
 
 void QgsAuthBasicMethod::putMethodConfig( const QString &authcfg, const QgsAuthMethodConfig &mconfig )
 {
+  QMutexLocker locker( &mConfigMutex );
   QgsDebugMsg( QString( "Putting basic config for authcfg: %1" ).arg( authcfg ) );
   sAuthConfigCache.insert( authcfg, mconfig );
 }
 
 void QgsAuthBasicMethod::removeMethodConfig( const QString &authcfg )
 {
+  QMutexLocker locker( &mConfigMutex );
   if ( sAuthConfigCache.contains( authcfg ) )
   {
     sAuthConfigCache.remove( authcfg );

diff --git a/src/auth/basic/qgsauthbasicmethod.h b/src/auth/basic/qgsauthbasicmethod.h
@@ -18,6 +18,7 @@
 #define QGSAUTHBASICMETHOD_H
 
 #include <QObject>
+#include <QMutex>
 
 #include "qgsauthconfig.h"
 #include "qgsauthmethod.h"
@@ -61,6 +62,8 @@ class QgsAuthBasicMethod : public QgsAuthMethod
     QString escapeUserPass( const QString &val, QChar delim = '\'' ) const;
 
     static QMap<QString, QgsAuthMethodConfig> sAuthConfigCache;
+
+    QMutex mConfigMutex;
 };
 
 #endif // QGSAUTHBASICMETHOD_H
diff --git a/src/auth/identcert/qgsauthidentcertmethod.cpp b/src/auth/identcert/qgsauthidentcertmethod.cpp
@@ -25,6 +25,7 @@
 #include <QSslConfiguration>
 #include <QSslError>
 #endif
+#include <QMutexLocker>
 
 #include "qgsauthcertutils.h"
 #include "qgsauthmanager.h"
@@ -50,6 +51,7 @@ QgsAuthIdentCertMethod::QgsAuthIdentCertMethod()
 
 QgsAuthIdentCertMethod::~QgsAuthIdentCertMethod()
 {
+  QMutexLocker locker( &mConfigMutex );
   qDeleteAll( sPkiConfigBundleCache );
   sPkiConfigBundleCache.clear();
 }
@@ -219,6 +221,7 @@ void QgsAuthIdentCertMethod::updateMethodConfig( QgsAuthMethodConfig &mconfig )
 
 QgsPkiConfigBundle *QgsAuthIdentCertMethod::getPkiConfigBundle( const QString &authcfg )
 {
+  QMutexLocker locker( &mConfigMutex );
   QgsPkiConfigBundle *bundle = nullptr;
 
   // check if it is cached
@@ -263,6 +266,7 @@ QgsPkiConfigBundle *QgsAuthIdentCertMethod::getPkiConfigBundle( const QString &a
 
   bundle = new QgsPkiConfigBundle( mconfig, clientcert, clientkey );
 
+  locker.unlock();
   // cache bundle
   putPkiConfigBundle( authcfg, bundle );
 
@@ -271,12 +275,14 @@ QgsPkiConfigBundle *QgsAuthIdentCertMethod::getPkiConfigBundle( const QString &a
 
 void QgsAuthIdentCertMethod::putPkiConfigBundle( const QString &authcfg, QgsPkiConfigBundle *pkibundle )
 {
+  QMutexLocker locker( &mConfigMutex );
   QgsDebugMsg( QString( "Putting PKI bundle for authcfg %1" ).arg( authcfg ) );
   sPkiConfigBundleCache.insert( authcfg, pkibundle );
 }
 
 void QgsAuthIdentCertMethod::removePkiConfigBundle( const QString &authcfg )
 {
+  QMutexLocker locker( &mConfigMutex );
   if ( sPkiConfigBundleCache.contains( authcfg ) )
   {
     QgsPkiConfigBundle *pkibundle = sPkiConfigBundleCache.take( authcfg );

diff --git a/src/auth/identcert/qgsauthidentcertmethod.h b/src/auth/identcert/qgsauthidentcertmethod.h
@@ -18,6 +18,7 @@
 #define QGSAUTHIDENTCERTMETHOD_H
 
 #include <QObject>
+#include <QMutex>
 
 #include "qgsauthconfig.h"
 #include "qgsauthmethod.h"
@@ -57,6 +58,8 @@ class QgsAuthIdentCertMethod : public QgsAuthMethod
     void removePkiConfigBundle( const QString &authcfg );
 
     static QMap<QString, QgsPkiConfigBundle *> sPkiConfigBundleCache;
+
+    QMutex mConfigMutex;
 };
 
 #endif // QGSAUTHIDENTCERTMETHOD_H
diff --git a/src/auth/pkipaths/qgsauthpkipathsmethod.cpp b/src/auth/pkipaths/qgsauthpkipathsmethod.cpp
@@ -25,6 +25,7 @@
 #include <QSslConfiguration>
 #include <QSslError>
 #endif
+#include <QMutexLocker>
 
 #include "qgsauthcertutils.h"
 #include "qgsauthmanager.h"
@@ -51,6 +52,7 @@ QgsAuthPkiPathsMethod::QgsAuthPkiPathsMethod()
 
 QgsAuthPkiPathsMethod::~QgsAuthPkiPathsMethod()
 {
+  QMutexLocker locker( &mConfigMutex );
   qDeleteAll( sPkiConfigBundleCache );
   sPkiConfigBundleCache.clear();
 }
@@ -223,6 +225,7 @@ void QgsAuthPkiPathsMethod::updateMethodConfig( QgsAuthMethodConfig &mconfig )
 
 QgsPkiConfigBundle *QgsAuthPkiPathsMethod::getPkiConfigBundle( const QString &authcfg )
 {
+  QMutexLocker locker( &mConfigMutex );
   QgsPkiConfigBundle *bundle = nullptr;
 
   // check if it is cached
@@ -265,6 +268,7 @@ QgsPkiConfigBundle *QgsAuthPkiPathsMethod::getPkiConfigBundle( const QString &au
 
   bundle = new QgsPkiConfigBundle( mconfig, clientcert, clientkey );
 
+  locker.unlock();
   // cache bundle
   putPkiConfigBundle( authcfg, bundle );
 
@@ -273,12 +277,14 @@ QgsPkiConfigBundle *QgsAuthPkiPathsMethod::getPkiConfigBundle( const QString &au
 
 void QgsAuthPkiPathsMethod::putPkiConfigBundle( const QString &authcfg, QgsPkiConfigBundle *pkibundle )
 {
+  QMutexLocker locker( &mConfigMutex );
   QgsDebugMsg( QString( "Putting PKI bundle for authcfg %1" ).arg( authcfg ) );
   sPkiConfigBundleCache.insert( authcfg, pkibundle );
 }
 
 void QgsAuthPkiPathsMethod::removePkiConfigBundle( const QString &authcfg )
 {
+  QMutexLocker locker( &mConfigMutex );
   if ( sPkiConfigBundleCache.contains( authcfg ) )
   {
     QgsPkiConfigBundle *pkibundle = sPkiConfigBundleCache.take( authcfg );

diff --git a/src/auth/pkipaths/qgsauthpkipathsmethod.h b/src/auth/pkipaths/qgsauthpkipathsmethod.h
@@ -18,6 +18,7 @@
 #define QGSAUTHPKIPATHSMETHOD_H
 
 #include <QObject>
+#include <QMutex>
 
 #include "qgsauthconfig.h"
 #include "qgsauthmethod.h"
@@ -57,6 +58,9 @@ class QgsAuthPkiPathsMethod : public QgsAuthMethod
     void removePkiConfigBundle( const QString &authcfg );
 
     static QMap<QString, QgsPkiConfigBundle *> sPkiConfigBundleCache;
+
+    QMutex mConfigMutex;
+
 };
 
 #endif // QGSAUTHPKIPATHSMETHOD_H
diff --git a/src/auth/pkipkcs12/qgsauthpkcs12method.cpp b/src/auth/pkipkcs12/qgsauthpkcs12method.cpp
@@ -25,6 +25,7 @@
 #include <QSslConfiguration>
 #include <QSslError>
 #endif
+#include <QMutexLocker>
 
 #include "qgsauthcertutils.h"
 #include "qgsauthmanager.h"
@@ -51,6 +52,7 @@ QgsAuthPkcs12Method::QgsAuthPkcs12Method()
 
 QgsAuthPkcs12Method::~QgsAuthPkcs12Method()
 {
+  QMutexLocker locker( &mConfigMutex );
   qDeleteAll( sPkiConfigBundleCache );
   sPkiConfigBundleCache.clear();
 }
@@ -221,6 +223,7 @@ void QgsAuthPkcs12Method::updateMethodConfig( QgsAuthMethodConfig &mconfig )
 
 QgsPkiConfigBundle *QgsAuthPkcs12Method::getPkiConfigBundle( const QString &authcfg )
 {
+  QMutexLocker locker( &mConfigMutex );
   QgsPkiConfigBundle *bundle = nullptr;
 
   // check if it is cached
@@ -271,6 +274,7 @@ QgsPkiConfigBundle *QgsAuthPkcs12Method::getPkiConfigBundle( const QString &auth
 
   bundle = new QgsPkiConfigBundle( mconfig, clientcert, clientkey );
 
+  locker.unlock();
   // cache bundle
   putPkiConfigBundle( authcfg, bundle );
 
@@ -279,12 +283,14 @@ QgsPkiConfigBundle *QgsAuthPkcs12Method::getPkiConfigBundle( const QString &auth
 
 void QgsAuthPkcs12Method::putPkiConfigBundle( const QString &authcfg, QgsPkiConfigBundle *pkibundle )
 {
+  QMutexLocker locker( &mConfigMutex );
   QgsDebugMsg( QString( "Putting PKI bundle for authcfg %1" ).arg( authcfg ) );
   sPkiConfigBundleCache.insert( authcfg, pkibundle );
 }
 
 void QgsAuthPkcs12Method::removePkiConfigBundle( const QString &authcfg )
 {
+  QMutexLocker locker( &mConfigMutex );
   if ( sPkiConfigBundleCache.contains( authcfg ) )
   {
     QgsPkiConfigBundle *pkibundle = sPkiConfigBundleCache.take( authcfg );

diff --git a/src/auth/pkipkcs12/qgsauthpkcs12method.h b/src/auth/pkipkcs12/qgsauthpkcs12method.h
@@ -18,6 +18,7 @@
 #define QGSAUTHPKCS12METHOD_H
 
 #include <QObject>
+#include <QMutex>
 
 #include "qgsauthconfig.h"
 #include "qgsauthmethod.h"
@@ -58,6 +59,8 @@ class QgsAuthPkcs12Method : public QgsAuthMethod
     void removePkiConfigBundle( const QString &authcfg );
 
     static QMap<QString, QgsPkiConfigBundle *> sPkiConfigBundleCache;
+
+    QMutex mConfigMutex;
 };
 
 #endif // QGSAUTHPKCS12METHOD_H
diff --git a/src/gui/qgsmessagelogviewer.cpp b/src/gui/qgsmessagelogviewer.cpp
@@ -28,7 +28,7 @@
 #include <QToolTip>
 #include <QPlainTextEdit>
 #include <QScrollBar>
-
+#include <QDebug>
 
 QgsMessageLogViewer::QgsMessageLogViewer( QWidget *parent, Qt::WindowFlags fl )
   : QDialog( parent, fl )
@@ -57,8 +57,7 @@ void QgsMessageLogViewer::logMessage( const QString &message, const QString &tag
     cleanedTag = tr( "General" );
 
   int i;
-  for ( i = 0; i < tabWidget->count() && tabWidget->tabText( i ) != cleanedTag; i++ )
-    ;
+  for ( i = 0; i < tabWidget->count() && tabWidget->tabText( i ).remove( QChar( '&' ) ) != cleanedTag; i++ );
 
   QPlainTextEdit *w = nullptr;
   if ( i < tabWidget->count() )

diff --git a/src/providers/wms/qgswmscapabilities.h b/src/providers/wms/qgswmscapabilities.h
@@ -508,12 +508,12 @@ struct QgsWmsAuthorization
     {
       return QgsAuthManager::instance()->updateNetworkRequest( request, mAuthCfg );
     }
-    else if ( !mUserName.isNull() || !mPassword.isNull() )
+    else if ( !mUserName.isEmpty() || !mPassword.isEmpty() )
     {
       request.setRawHeader( "Authorization", "Basic " + QStringLiteral( "%1:%2" ).arg( mUserName, mPassword ).toLatin1().toBase64() );
     }
 
-    if ( !mReferer.isNull() )
+    if ( !mReferer.isEmpty() )
     {
       request.setRawHeader( "Referer", QStringLiteral( "%1" ).arg( mReferer ).toLatin1() );
     }