From bb158100dd6b7c694142a1c5395ba121e55c9236 Mon Sep 17 00:00:00 2001 From: Robert Griebl Date: Mon, 21 Mar 2022 23:42:44 +0100 Subject: Handle an empty or invalid documentDir correctly Not specifying the documentDir, but enabling the installer would - unexpectedly - use $PWD as documentDir. The code does now follow the documentation: if not set, no document directories will be created on package installation. Also, we now fail early if the documentDir and installationDir are sub-directories of each other, as that would lead to mayhem on PackageManager::cleanupBrokenInstallations(). Change-Id: Id8d89f82bf0d63c771f3d86a963c66681f43f195 Fixes: QTBUG-101881 Reviewed-by: Bernd Weimer (cherry picked from commit bc06a04dd845d039ba802f94b5eae57519ec63a8) Reviewed-by: Qt Cherry-pick Bot --- doc/configuration.qdoc | 3 +++ src/main-lib/main.cpp | 9 +++++++++ src/manager-lib/deinstallationtask.cpp | 2 +- src/manager-lib/installationtask.cpp | 2 +- src/manager-lib/packagemanager.cpp | 5 +++-- 5 files changed, 17 insertions(+), 4 deletions(-) diff --git a/doc/configuration.qdoc b/doc/configuration.qdoc index ad9a2f98..9f9a124a 100644 --- a/doc/configuration.qdoc +++ b/doc/configuration.qdoc @@ -180,6 +180,9 @@ or across multiple config files, the final value is resolved based on these rule \li string \li The base directory for per-package document storage directories. (default: empty/disabled) + + Please note, that if you do not set a \c documentDir here, no per-application + document directories will be created when installing new packages. \row \li [\c applications/installationDirMountPoint] \li string diff --git a/src/main-lib/main.cpp b/src/main-lib/main.cpp index f7cba36a..2cbe7ec2 100644 --- a/src/main-lib/main.cpp +++ b/src/main-lib/main.cpp @@ -542,6 +542,15 @@ void Main::setupInstaller(bool devMode, bool allowUnsigned, const QStringList &c "even automatically switching to C.UTF-8 or en_US.UTF-8 failed."; } + // make sure the installation and document dirs are valid + Q_ASSERT(!m_installationDir.isEmpty()); + const auto instPath = QDir(m_installationDir).canonicalPath(); + const auto docPath = m_documentDir.isEmpty() ? QString { } + : QDir(m_documentDir).canonicalPath(); + + if (!docPath.isEmpty() && (instPath.startsWith(docPath) || docPath.startsWith(instPath))) + throw Exception("either installationDir or documentDir cannot be a sub-directory of the other"); + // we only output these deployment warnings, if we are on embedded, applicationUserIdSeparation // is enabled and either... if (isRunningOnEmbedded() && userIdSeparation && userIdSeparation(nullptr, nullptr, nullptr)) { diff --git a/src/manager-lib/deinstallationtask.cpp b/src/manager-lib/deinstallationtask.cpp index 02fbda20..4de582a1 100644 --- a/src/manager-lib/deinstallationtask.cpp +++ b/src/manager-lib/deinstallationtask.cpp @@ -114,7 +114,7 @@ void DeinstallationTask::execute() ScopedRenamer docDirRename; ScopedRenamer appDirRename; - if (!m_keepDocuments) { + if (!m_keepDocuments || !m_documentPath.isEmpty()) { if (!docDirRename.rename(QDir(m_documentPath).absoluteFilePath(packageId()), ScopedRenamer::NameToNameMinus)) { throw Exception(Error::IO, "could not rename %1 to %1-").arg(docDirRename.baseName()); diff --git a/src/manager-lib/installationtask.cpp b/src/manager-lib/installationtask.cpp index 7cdd2555..c9b487d2 100644 --- a/src/manager-lib/installationtask.cpp +++ b/src/manager-lib/installationtask.cpp @@ -420,7 +420,7 @@ void InstallationTask::finishInstallation() Q_DECL_NOEXCEPT_EXPR(false) reportFile.close(); // create the document directories when installing (not needed on updates) - if (mode == Installation) { + if ((mode == Installation) && !m_documentPath.isEmpty()) { // this package may have been installed earlier and the document directory may not have been removed if (!documentDirectory.cd(m_packageId)) { if (!documentDirCreator.create(documentDirectory.absoluteFilePath(m_packageId))) diff --git a/src/manager-lib/packagemanager.cpp b/src/manager-lib/packagemanager.cpp index 8a70b465..c174665f 100644 --- a/src/manager-lib/packagemanager.cpp +++ b/src/manager-lib/packagemanager.cpp @@ -836,7 +836,7 @@ QVariantMap PackageManager::installationLocation() const */ QVariantMap PackageManager::documentLocation() const { - return locationMap(d->documentPath); + return d->documentPath.isEmpty() ? QVariantMap { } : locationMap(d->documentPath); } bool PackageManager::isPackageInstallationActive(const QString &packageId) const @@ -896,7 +896,8 @@ void PackageManager::cleanupBrokenInstallations() Q_DECL_NOEXCEPT_EXPR(false) if (valid) { validPaths.insert(d->installationPath, pkg->id() + QDir::separator()); - validPaths.insert(d->documentPath, pkg->id() + QDir::separator()); + if (!d->documentPath.isEmpty()) + validPaths.insert(d->documentPath, pkg->id() + QDir::separator()); } else { if (startingPackageRemoval(pkg->id())) { if (finishedPackageInstall(pkg->id())) -- cgit v1.2.1