From ddba24535fb5732c3cb757414cf1a393bd98f693 Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Tue, 9 Nov 2021 10:16:06 -0800 Subject: [PATCH] QFactoryLoader: add setExtraSearchPath() (for QPA plugins' use) This is added specifically for the QPA platform and theme plugins, to honor the QT_QPA_PLATFORM_PLUGIN_PATH environment variable and the (inadvisable) -platformpluginpath command-line argument. This removes the last QFactoryLoader used with an empty path (also the only two that could be reached), which were causing a scan of the application's binary directory whenever the platform plugin path was set. In case of applications installed to /usr/bin, the entire /usr/bin was scanned, which can be qualified as "not good". Fixes: QTBUG-97950 Pick-to: 6.3 Change-Id: Ice04365c72984d07a64dfffd16b47fe1d22f26d3 Reviewed-by: Lars Knoll --- src/corelib/plugin/qfactoryloader.cpp | 34 +++++++++++++++- src/corelib/plugin/qfactoryloader_p.h | 3 +- .../kernel/qplatformintegrationfactory.cpp | 37 ++---------------- src/gui/kernel/qplatformthemefactory.cpp | 38 ++---------------- .../qfactoryloader/tst_qfactoryloader.cpp | 39 +++++++++++++++++-- 5 files changed, 75 insertions(+), 76 deletions(-) diff --git a/src/corelib/plugin/qfactoryloader.cpp b/src/corelib/plugin/qfactoryloader.cpp index 19d858bf8d..0342f1924a 100644 --- a/src/corelib/plugin/qfactoryloader.cpp +++ b/src/corelib/plugin/qfactoryloader.cpp @@ -1,7 +1,7 @@ /**************************************************************************** ** -** Copyright (C) 2016 The Qt Company Ltd. -** Copyright (C) 2018 Intel Corporation. +** Copyright (C) 2021 The Qt Company Ltd. +** Copyright (C) 2022 Intel Corporation. ** Contact: https://www.qt.io/licensing/ ** ** This file is part of the QtCore module of the Qt Toolkit. @@ -140,6 +140,7 @@ public: QList libraryList; QMap keyMap; QString suffix; + QString extraSearchPath; Qt::CaseSensitivity cs; void updateSinglePath(const QString &pluginDir); @@ -276,6 +277,8 @@ void QFactoryLoader::update() d->updateSinglePath(path); } + if (!d->extraSearchPath.isEmpty()) + d->updateSinglePath(d->extraSearchPath); #else Q_D(QFactoryLoader); qCDebug(lcFactoryLoader) << "ignoring" << d->iid @@ -314,6 +317,9 @@ QFactoryLoader::QFactoryLoader(const char *iid, Qt::CaseSensitivity cs) : QObject(*new QFactoryLoaderPrivate) { + Q_ASSERT_X(suffix.startsWith(u'/'), "QFactoryLoader", + "For historical reasons, the suffix must start with '/' (and it can't be empty)"); + moveToThread(QCoreApplicationPrivate::mainThread()); Q_D(QFactoryLoader); d->iid = iid; @@ -334,6 +340,30 @@ QFactoryLoader::QFactoryLoader(const char *iid, #endif } +void QFactoryLoader::setExtraSearchPath(const QString &path) +{ +#if QT_CONFIG(library) + Q_D(QFactoryLoader); + if (d->extraSearchPath == path) + return; // nothing to do + + QMutexLocker locker(qt_factoryloader_mutex()); + QString oldPath = qExchange(d->extraSearchPath, path); + if (oldPath.isEmpty()) { + // easy case, just update this directory + d->updateSinglePath(d->extraSearchPath); + } else { + // must re-scan everything + d->loadedPaths.clear(); + d->libraryList.clear(); + d->keyMap.clear(); + update(); + } +#else + Q_UNUSED(path); +#endif +} + QFactoryLoader::MetaDataList QFactoryLoader::metaData() const { Q_D(const QFactoryLoader); diff --git a/src/corelib/plugin/qfactoryloader_p.h b/src/corelib/plugin/qfactoryloader_p.h index f7033d144f..633cf0a916 100644 --- a/src/corelib/plugin/qfactoryloader_p.h +++ b/src/corelib/plugin/qfactoryloader_p.h @@ -1,7 +1,7 @@ /**************************************************************************** ** ** Copyright (C) 2017 The Qt Company Ltd. -** Copyright (C) 2021 Intel Corporation. +** Copyright (C) 2022 Intel Corporation. ** Contact: https://www.qt.io/licensing/ ** ** This file is part of the QtCore module of the Qt Toolkit. @@ -115,6 +115,7 @@ public: #endif // Q_OS_UNIX && !Q_OS_MAC #endif // QT_CONFIG(library) + void setExtraSearchPath(const QString &path); QMultiMap keyMap() const; int indexOf(const QString &needle) const; diff --git a/src/gui/kernel/qplatformintegrationfactory.cpp b/src/gui/kernel/qplatformintegrationfactory.cpp index 3fcf9014a7..22cdf2ccf8 100644 --- a/src/gui/kernel/qplatformintegrationfactory.cpp +++ b/src/gui/kernel/qplatformintegrationfactory.cpp @@ -51,23 +51,9 @@ QT_BEGIN_NAMESPACE Q_GLOBAL_STATIC_WITH_ARGS(QFactoryLoader, loader, (QPlatformIntegrationFactoryInterface_iid, QLatin1String("/platforms"), Qt::CaseInsensitive)) -#if QT_CONFIG(library) -Q_GLOBAL_STATIC_WITH_ARGS(QFactoryLoader, directLoader, - (QPlatformIntegrationFactoryInterface_iid, QLatin1String(""), Qt::CaseInsensitive)) -#endif // QT_CONFIG(library) - QPlatformIntegration *QPlatformIntegrationFactory::create(const QString &platform, const QStringList ¶mList, int &argc, char **argv, const QString &platformPluginPath) { -#if QT_CONFIG(library) - // Try loading the plugin from platformPluginPath first: - if (!platformPluginPath.isEmpty()) { - QCoreApplication::addLibraryPath(platformPluginPath); - if (QPlatformIntegration *ret = qLoadPlugin(directLoader(), platform, paramList, argc, argv)) - return ret; - } -#else - Q_UNUSED(platformPluginPath); -#endif + loader->setExtraSearchPath(platformPluginPath); return qLoadPlugin(loader(), platform, paramList, argc, argv); } @@ -80,25 +66,8 @@ QPlatformIntegration *QPlatformIntegrationFactory::create(const QString &platfor QStringList QPlatformIntegrationFactory::keys(const QString &platformPluginPath) { - QStringList list; -#if QT_CONFIG(library) - if (!platformPluginPath.isEmpty()) { - QCoreApplication::addLibraryPath(platformPluginPath); - list = directLoader()->keyMap().values(); - if (!list.isEmpty()) { - const QString postFix = QLatin1String(" (from ") - + QDir::toNativeSeparators(platformPluginPath) - + QLatin1Char(')'); - const QStringList::iterator end = list.end(); - for (QStringList::iterator it = list.begin(); it != end; ++it) - (*it).append(postFix); - } - } -#else - Q_UNUSED(platformPluginPath); -#endif - list.append(loader()->keyMap().values()); - return list; + loader->setExtraSearchPath(platformPluginPath); + return loader->keyMap().values(); } QT_END_NAMESPACE diff --git a/src/gui/kernel/qplatformthemefactory.cpp b/src/gui/kernel/qplatformthemefactory.cpp index 447d385abe..0d7163cdd4 100644 --- a/src/gui/kernel/qplatformthemefactory.cpp +++ b/src/gui/kernel/qplatformthemefactory.cpp @@ -51,25 +51,11 @@ QT_BEGIN_NAMESPACE Q_GLOBAL_STATIC_WITH_ARGS(QFactoryLoader, loader, (QPlatformThemeFactoryInterface_iid, QLatin1String("/platformthemes"), Qt::CaseInsensitive)) -#if QT_CONFIG(library) -Q_GLOBAL_STATIC_WITH_ARGS(QFactoryLoader, directLoader, - (QPlatformThemeFactoryInterface_iid, QLatin1String(""), Qt::CaseInsensitive)) -#endif - QPlatformTheme *QPlatformThemeFactory::create(const QString& key, const QString &platformPluginPath) { QStringList paramList = key.split(QLatin1Char(':')); const QString platform = paramList.takeFirst().toLower(); -#if QT_CONFIG(library) - // Try loading the plugin from platformPluginPath first: - if (!platformPluginPath.isEmpty()) { - QCoreApplication::addLibraryPath(platformPluginPath); - if (QPlatformTheme *ret = qLoadPlugin(directLoader(), platform, paramList)) - return ret; - } -#else - Q_UNUSED(platformPluginPath); -#endif + loader->setExtraSearchPath(platformPluginPath); return qLoadPlugin(loader(), platform, paramList); } @@ -81,26 +67,8 @@ QPlatformTheme *QPlatformThemeFactory::create(const QString& key, const QString */ QStringList QPlatformThemeFactory::keys(const QString &platformPluginPath) { - QStringList list; - -#if QT_CONFIG(library) - if (!platformPluginPath.isEmpty()) { - QCoreApplication::addLibraryPath(platformPluginPath); - list += directLoader()->keyMap().values(); - if (!list.isEmpty()) { - const QString postFix = QLatin1String(" (from ") - + QDir::toNativeSeparators(platformPluginPath) - + QLatin1Char(')'); - const QStringList::iterator end = list.end(); - for (QStringList::iterator it = list.begin(); it != end; ++it) - (*it).append(postFix); - } - } -#else - Q_UNUSED(platformPluginPath); -#endif - list += loader()->keyMap().values(); - return list; + loader->setExtraSearchPath(platformPluginPath); + return loader->keyMap().values(); } QT_END_NAMESPACE diff --git a/tests/auto/corelib/plugin/qfactoryloader/tst_qfactoryloader.cpp b/tests/auto/corelib/plugin/qfactoryloader/tst_qfactoryloader.cpp index 9fa61804b3..862877fa6f 100644 --- a/tests/auto/corelib/plugin/qfactoryloader/tst_qfactoryloader.cpp +++ b/tests/auto/corelib/plugin/qfactoryloader/tst_qfactoryloader.cpp @@ -47,11 +47,13 @@ class tst_QFactoryLoader : public QObject QSharedPointer directory; #endif + QString binFolder; public slots: void initTestCase(); private slots: void usingTwoFactoriesFromSameDir(); + void extraSearchPath(); }; static const char binFolderC[] = "bin"; @@ -64,15 +66,17 @@ void tst_QFactoryLoader::initTestCase() QVERIFY(directory->isValid()); QVERIFY2(QDir::setCurrent(directory->path()), qPrintable("Could not chdir to " + directory->path())); #endif - const QString binFolder = QFINDTESTDATA(binFolderC); + binFolder = QFINDTESTDATA(binFolderC); QVERIFY2(!binFolder.isEmpty(), "Unable to locate 'bin' folder"); -#if QT_CONFIG(library) - QCoreApplication::setLibraryPaths(QStringList(QFileInfo(binFolder).absolutePath())); -#endif } void tst_QFactoryLoader::usingTwoFactoriesFromSameDir() { +#if QT_CONFIG(library) + // set the library path to contain the directory where the 'bin' dir is located + QCoreApplication::setLibraryPaths( { QFileInfo(binFolder).absolutePath() }); +#endif + const QString suffix = QLatin1Char('/') + QLatin1String(binFolderC); QFactoryLoader loader1(PluginInterface1_iid, suffix); @@ -92,5 +96,32 @@ void tst_QFactoryLoader::usingTwoFactoriesFromSameDir() QCOMPARE(plugin2->pluginName(), QLatin1String("Plugin2 ok")); } +void tst_QFactoryLoader::extraSearchPath() +{ +#if defined(Q_OS_ANDROID) && !QT_CONFIG(library) + QSKIP("Test not applicable in this configuration."); +#else + QCoreApplication::setLibraryPaths(QStringList()); + + QString absoluteBinPath = QFileInfo(binFolder).absoluteFilePath(); + QFactoryLoader loader1(PluginInterface1_iid, "/nonexistent"); + + // it shouldn't have scanned anything because we haven't given it a path yet + QVERIFY(loader1.metaData().isEmpty()); + + loader1.setExtraSearchPath(absoluteBinPath); + PluginInterface1 *plugin1 = qobject_cast(loader1.instance(0)); + QVERIFY2(plugin1, + qPrintable(QString::fromLatin1("Cannot load plugin '%1'") + .arg(QLatin1String(PluginInterface1_iid)))); + + QCOMPARE(plugin1->pluginName(), QLatin1String("Plugin1 ok")); + + // check that it forgets that plugin + loader1.setExtraSearchPath(QString()); + QVERIFY(loader1.metaData().isEmpty()); +#endif +} + QTEST_MAIN(tst_QFactoryLoader) #include "tst_qfactoryloader.moc"