From 737af8798f4693ee3bd461c1b7faa212f3b74e8b Mon Sep 17 00:00:00 2001 From: Loose Cannon Date: Mon, 2 Mar 2026 15:52:59 -0500 Subject: [PATCH] isPerDeviceMode takes baseDir context, fix import result path Change isPerDeviceMode(const QString&) to isPerDeviceMode(const QDir& baseDir) so the method resolves this->path against the caller-provided base directory. This fixes silent misclassification when reference paths are relative, since QFileInfo previously resolved against process CWD instead of the database directory. Override Result.path with the actual device file path in importPerDeviceShares so status messages show which specific container failed, not just the sync directory. Add regression tests for relative directory and file paths. --- src/keeshare/KeeShareSettings.cpp | 9 +++-- src/keeshare/KeeShareSettings.h | 4 ++- src/keeshare/ShareObserver.cpp | 19 ++++++---- .../group/EditGroupWidgetKeeShare.cpp | 6 ++-- tests/TestSharing.cpp | 35 +++++++++++-------- 5 files changed, 48 insertions(+), 25 deletions(-) diff --git a/src/keeshare/KeeShareSettings.cpp b/src/keeshare/KeeShareSettings.cpp index 33425a85f..558601ee5 100644 --- a/src/keeshare/KeeShareSettings.cpp +++ b/src/keeshare/KeeShareSettings.cpp @@ -25,6 +25,7 @@ #include "gui/DatabaseIcons.h" #include +#include #include #include #include @@ -287,9 +288,13 @@ namespace KeeShareSettings return (type & ImportFrom) != 0 && !path.isEmpty(); } - bool Reference::isPerDeviceMode() const + bool Reference::isPerDeviceMode(const QDir& baseDir) const { - return !path.isEmpty() && QFileInfo(path).isDir(); + if (path.isEmpty()) { + return false; + } + const QString resolvedPath = baseDir.absoluteFilePath(path); + return QFileInfo(resolvedPath).isDir(); } bool Reference::operator<(const Reference& other) const diff --git a/src/keeshare/KeeShareSettings.h b/src/keeshare/KeeShareSettings.h index dd786a315..1a72118ec 100644 --- a/src/keeshare/KeeShareSettings.h +++ b/src/keeshare/KeeShareSettings.h @@ -21,6 +21,8 @@ #include #include +class QDir; + namespace Botan { class Private_Key; @@ -133,7 +135,7 @@ namespace KeeShareSettings bool isValid() const; bool isExporting() const; bool isImporting() const; - bool isPerDeviceMode() const; + bool isPerDeviceMode(const QDir& baseDir) const; bool operator<(const Reference& other) const; bool operator==(const Reference& other) const; diff --git a/src/keeshare/ShareObserver.cpp b/src/keeshare/ShareObserver.cpp index e391b91a4..e80c3d813 100644 --- a/src/keeshare/ShareObserver.cpp +++ b/src/keeshare/ShareObserver.cpp @@ -102,6 +102,8 @@ void ShareObserver::reinitialize() QMap imported; QMap exported; + const QDir baseDir = QFileInfo(m_db->filePath()).absoluteDir(); + for (const auto& share : shares) { auto group = share.first; auto& reference = share.second; @@ -113,7 +115,7 @@ void ShareObserver::reinitialize() if (!reference.path.isEmpty() && reference.type != KeeShareSettings::Inactive) { const auto newResolvedPath = resolvePath(reference.path, m_db); - if (reference.isPerDeviceMode()) { + if (reference.isPerDeviceMode(baseDir)) { // Per-device mode: watch the directory for changes auto dirWatcher = QSharedPointer::create(); if (QDir(newResolvedPath).exists()) { @@ -137,10 +139,10 @@ void ShareObserver::reinitialize() if (reference.isImporting()) { imported[reference.path] << group->name(); + const auto resolvedDir = resolvePath(reference.path, m_db); - if (reference.isPerDeviceMode()) { + if (reference.isPerDeviceMode(baseDir)) { // Per-device mode: import from all device files in the directory - const auto resolvedDir = resolvePath(reference.path, m_db); const auto results = importPerDeviceShares(resolvedDir, reference, group); for (const auto& result : results) { if (!result.isValid()) { @@ -260,7 +262,8 @@ void ShareObserver::handleDirectoryUpdated(const QString& dirPath) return; } auto reference = KeeShare::referenceOf(group); - if (!reference.isImporting() || !reference.isPerDeviceMode()) { + const QDir handleBaseDir = QFileInfo(m_db->filePath()).absoluteDir(); + if (!reference.isImporting() || !reference.isPerDeviceMode(handleBaseDir)) { return; } @@ -326,7 +329,9 @@ QList ShareObserver::importPerDeviceShares( continue; // Skip own device's file } const auto filePath = dir.absoluteFilePath(fileName); - results << ShareImport::containerInto(filePath, reference, targetGroup); + auto result = ShareImport::containerInto(filePath, reference, targetGroup); + result.path = filePath; + results << result; } return results; } @@ -398,11 +403,13 @@ QList ShareObserver::exportShares() return results; } + const QDir exportBaseDir = QFileInfo(m_db->filePath()).absoluteDir(); + for (auto it = references.cbegin(); it != references.cend(); ++it) { auto reference = it.value().first(); const QString resolvedPath = resolvePath(reference.config.path, m_db); - if (reference.config.isPerDeviceMode()) { + if (reference.config.isPerDeviceMode(exportBaseDir)) { // Per-device mode: export to {directory}/{DEVICE_ID}.kdbx QDir dir(resolvedPath); if (!dir.exists()) { diff --git a/src/keeshare/group/EditGroupWidgetKeeShare.cpp b/src/keeshare/group/EditGroupWidgetKeeShare.cpp index 62ab6c98f..2a0dc7cda 100644 --- a/src/keeshare/group/EditGroupWidgetKeeShare.cpp +++ b/src/keeshare/group/EditGroupWidgetKeeShare.cpp @@ -24,6 +24,7 @@ #include "keeshare/KeeShare.h" #include +#include #include EditGroupWidgetKeeShare::EditGroupWidgetKeeShare(QWidget* parent) @@ -110,8 +111,9 @@ void EditGroupWidgetKeeShare::updateSharingState() // Custom message for active KeeShare reference const auto reference = KeeShare::referenceOf(m_temporaryGroup); + const QDir uiBaseDir = QFileInfo(m_database->filePath()).absoluteDir(); if (!reference.path.isEmpty()) { - if (reference.isPerDeviceMode()) { + if (reference.isPerDeviceMode(uiBaseDir)) { // Per-device mode: path is a directory, show info message m_ui->messageWidget->showMessage( tr("Per-device sync mode: each device writes its own container in this directory.\n" @@ -151,7 +153,7 @@ void EditGroupWidgetKeeShare::updateSharingState() conflictExport |= other.isExporting() && reference.isExporting(); // In per-device mode, import+export to the same directory is expected // (export writes own device file, import reads other devices' files) - if (!reference.isPerDeviceMode()) { + if (!reference.isPerDeviceMode(uiBaseDir)) { cycleImportExport |= (other.isImporting() && reference.isExporting()) || (other.isExporting() && reference.isImporting()); } diff --git a/tests/TestSharing.cpp b/tests/TestSharing.cpp index c89035c77..27884703f 100644 --- a/tests/TestSharing.cpp +++ b/tests/TestSharing.cpp @@ -187,42 +187,49 @@ void TestSharing::testSettingsSerialization_data() void TestSharing::testPerDeviceMode() { QFETCH(QString, path); + QFETCH(QString, baseDir); QFETCH(bool, expectedPerDevice); KeeShareSettings::Reference reference; reference.path = path; reference.type = KeeShareSettings::SynchronizeWith; - QCOMPARE(reference.isPerDeviceMode(), expectedPerDevice); + QCOMPARE(reference.isPerDeviceMode(QDir(baseDir)), expectedPerDevice); } void TestSharing::testPerDeviceMode_data() { QTest::addColumn("path"); + QTest::addColumn("baseDir"); QTest::addColumn("expectedPerDevice"); - // Classic mode paths (file-based — don't need to exist on disk) - QTest::newRow("kdbx file") << "/some/path/share.kdbx" << false; - QTest::newRow("kdbx.share file") << "/some/path/share.kdbx.share" << false; - QTest::newRow("KDBX uppercase") << "/some/path/share.KDBX" << false; - QTest::newRow("KDBX.SHARE uppercase") << "/some/path/share.KDBX.SHARE" << false; - QTest::newRow("empty path") << "" << false; - QTest::newRow("nonexistent path") << "/nonexistent/path/nowhere" << false; - - // Per-device mode paths (real directories on disk) auto base = m_tempDir->path(); + // Classic mode paths (file-based — don't need to exist on disk) + QTest::newRow("kdbx file") << "/some/path/share.kdbx" << base << false; + QTest::newRow("kdbx.share file") << "/some/path/share.kdbx.share" << base << false; + QTest::newRow("KDBX uppercase") << "/some/path/share.KDBX" << base << false; + QTest::newRow("KDBX.SHARE uppercase") << "/some/path/share.KDBX.SHARE" << base << false; + QTest::newRow("empty path") << "" << base << false; + QTest::newRow("nonexistent path") << "/nonexistent/path/nowhere" << base << false; + + // Per-device mode paths (real directories on disk — absolute) QDir(base).mkpath("syncdir"); - QTest::newRow("directory path") << base + "/syncdir" << true; + QTest::newRow("directory path") << base + "/syncdir" << base << true; QDir(base).mkpath("syncdir_slash"); - QTest::newRow("directory trailing slash") << base + "/syncdir_slash/" << true; + QTest::newRow("directory trailing slash") << base + "/syncdir_slash/" << base << true; QDir(base).mkpath("sub/shared"); - QTest::newRow("nested directory") << base + "/sub/shared" << true; + QTest::newRow("nested directory") << base + "/sub/shared" << base << true; QDir(base).mkpath("path.d/sync"); - QTest::newRow("directory with dots") << base + "/path.d/sync" << true; + QTest::newRow("directory with dots") << base + "/path.d/sync" << base << true; + + // Per-device mode with relative path (regression test: must resolve against baseDir) + QDir(base).mkpath("reldir"); + QTest::newRow("relative directory") << "reldir" << base << true; + QTest::newRow("relative file") << "share.kdbx" << base << false; } void TestSharing::testPerDeviceModeImportExport()