consolidate device ID sanitization and strengthen isPerDeviceMode check

Move all sanitization (strip non-alphanumeric, truncate to 32, fallback
to DEFAULT) into setDeviceId() so it's consistent regardless of call
site. deviceId() now delegates to setDeviceId() and re-reads the
sanitized value.

isPerDeviceMode() now uses QFileInfo::isDir() as a positive filesystem
check — no extension-based heuristics. Tests updated to create real
temp directories instead of using synthetic paths.
This commit is contained in:
Loose Cannon 2026-03-02 12:19:56 -05:00
parent 15246fa231
commit 870bd4af38
4 changed files with 46 additions and 20 deletions

View file

@ -67,22 +67,23 @@ QString KeeShare::deviceId()
// Last resort: use hostname
id = QSysInfo::machineHostName();
}
// Sanitize to [A-Za-z0-9] only
id.remove(QRegularExpression("[^A-Za-z0-9]"));
if (id.isEmpty()) {
id = "DEFAULT";
}
setDeviceId(id);
// Re-read the sanitized value
id = config()->get(Config::KeeShare_DeviceId).toString();
}
return id;
}
void KeeShare::setDeviceId(const QString& id)
{
// Sanitize to [A-Za-z0-9] only and enforce max length
// All sanitization consolidated here: strip non-alphanumeric, enforce max
// length, and fall back to DEFAULT if empty
QString sanitized = id;
sanitized.remove(QRegularExpression("[^A-Za-z0-9]"));
sanitized.truncate(32);
if (sanitized.isEmpty()) {
sanitized = QStringLiteral("DEFAULT");
}
config()->set(Config::KeeShare_DeviceId, sanitized);
}

View file

@ -25,6 +25,7 @@
#include "gui/DatabaseIcons.h"
#include <QDataStream>
#include <QFileInfo>
#include <QTextCodec>
#include <QXmlStreamWriter>
@ -288,9 +289,7 @@ namespace KeeShareSettings
bool Reference::isPerDeviceMode() const
{
return !path.isEmpty()
&& !path.endsWith(".kdbx", Qt::CaseInsensitive)
&& !path.endsWith(".kdbx.share", Qt::CaseInsensitive);
return !path.isEmpty() && QFileInfo(path).isDir();
}
bool Reference::operator<(const Reference& other) const

View file

@ -17,6 +17,7 @@
#include "TestSharing.h"
#include <QDir>
#include <QTest>
#include <QXmlStreamReader>
@ -35,6 +36,14 @@ Q_DECLARE_METATYPE(KeeShareSettings::Certificate)
void TestSharing::initTestCase()
{
QVERIFY(Crypto::init());
m_tempDir = new QTemporaryDir();
QVERIFY(m_tempDir->isValid());
}
void TestSharing::cleanupTestCase()
{
delete m_tempDir;
m_tempDir = nullptr;
}
void TestSharing::testNullObjects()
@ -192,18 +201,28 @@ void TestSharing::testPerDeviceMode_data()
QTest::addColumn<QString>("path");
QTest::addColumn<bool>("expectedPerDevice");
// Classic mode paths (file-based)
// 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 (directory-based)
QTest::newRow("directory path") << "/some/sync/dir" << true;
QTest::newRow("directory trailing slash") << "/some/sync/dir/" << true;
QTest::newRow("relative directory") << "sync/shared" << true;
QTest::newRow("directory with dots") << "/some/path.d/sync" << true;
// Per-device mode paths (real directories on disk)
auto base = m_tempDir->path();
QDir(base).mkpath("syncdir");
QTest::newRow("directory path") << base + "/syncdir" << true;
QDir(base).mkpath("syncdir_slash");
QTest::newRow("directory trailing slash") << base + "/syncdir_slash/" << true;
QDir(base).mkpath("sub/shared");
QTest::newRow("nested directory") << base + "/sub/shared" << true;
QDir(base).mkpath("path.d/sync");
QTest::newRow("directory with dots") << base + "/path.d/sync" << true;
}
void TestSharing::testPerDeviceModeImportExport()
@ -228,16 +247,20 @@ void TestSharing::testPerDeviceModeImportExport_data()
QTest::addColumn<bool>("expectedImporting");
QTest::addColumn<bool>("expectedExporting");
auto base = m_tempDir->path();
QDir(base).mkpath("importexport");
auto dir = base + "/importexport";
QTest::newRow("per-device sync imports")
<< "/some/dir" << int(KeeShareSettings::SynchronizeWith) << true << true;
<< dir << int(KeeShareSettings::SynchronizeWith) << true << true;
QTest::newRow("per-device import only")
<< "/some/dir" << int(KeeShareSettings::ImportFrom) << true << false;
<< dir << int(KeeShareSettings::ImportFrom) << true << false;
QTest::newRow("per-device export only")
<< "/some/dir" << int(KeeShareSettings::ExportTo) << false << true;
<< dir << int(KeeShareSettings::ExportTo) << false << true;
QTest::newRow("classic file sync")
<< "/some/dir/share.kdbx" << int(KeeShareSettings::SynchronizeWith) << true << true;
<< dir + "/share.kdbx" << int(KeeShareSettings::SynchronizeWith) << true << true;
QTest::newRow("inactive per-device")
<< "/some/dir" << int(KeeShareSettings::Inactive) << false << false;
<< dir << int(KeeShareSettings::Inactive) << false << false;
}
const QSharedPointer<Botan::RSA_PrivateKey> TestSharing::stubkey(int index)

View file

@ -19,6 +19,7 @@
#define KEEPASSXC_TESTSHARING_H
#include <QObject>
#include <QTemporaryDir>
namespace Botan
{
@ -30,6 +31,7 @@ class TestSharing : public QObject
private slots:
void initTestCase();
void cleanupTestCase();
void testNullObjects();
void testKeySerialization();
void testReferenceSerialization();
@ -43,6 +45,7 @@ private slots:
private:
const QSharedPointer<Botan::RSA_PrivateKey> stubkey(int index = 0);
QTemporaryDir* m_tempDir = nullptr;
};
#endif // KEEPASSXC_TESTSHARING_H