diff --git a/src/core/Database.cpp b/src/core/Database.cpp index b818a9034..944597f56 100644 --- a/src/core/Database.cpp +++ b/src/core/Database.cpp @@ -54,7 +54,7 @@ Database::Database() s_uuidMap.insert(m_uuid, this); - connect(m_metadata, SIGNAL(metadataModified()), this, SLOT(markAsModified())); + connect(m_metadata, SIGNAL(metadataModified()), SLOT(markAsModified())); connect(m_timer, SIGNAL(timeout()), SIGNAL(databaseModified())); connect(this, SIGNAL(databaseOpened()), SLOT(updateCommonUsernames())); connect(this, SIGNAL(databaseSaved()), SLOT(updateCommonUsernames())); @@ -189,7 +189,7 @@ bool Database::save(QString* error, bool atomic, bool backup) /** * Save the database to a specific file. * - * If atmoic is false, this function uses QTemporaryFile instead of QSaveFile + * If atomic is false, this function uses QTemporaryFile instead of QSaveFile * due to a bug in Qt (https://bugreports.qt.io/browse/QTBUG-57299) that may * prevent the QSaveFile from renaming itself when using Dropbox, Google Drive, * or OneDrive. diff --git a/tests/TestCli.cpp b/tests/TestCli.cpp index 8bc865799..586c39be1 100644 --- a/tests/TestCli.cpp +++ b/tests/TestCli.cpp @@ -144,36 +144,32 @@ void TestCli::init() m_stdinFile.reset(new TemporaryFile()); m_stdinFile->open(); - m_stdinHandle = fdopen(m_stdinFile->handle(), "r+"); - Utils::STDIN = m_stdinHandle; + Utils::STDIN = fdopen(m_stdinFile->handle(), "r+"); m_stdoutFile.reset(new TemporaryFile()); m_stdoutFile->open(); - m_stdoutHandle = fdopen(m_stdoutFile->handle(), "r+"); - Utils::STDOUT = m_stdoutHandle; + Utils::STDOUT = fdopen(m_stdoutFile->handle(), "r+"); m_stderrFile.reset(new TemporaryFile()); m_stderrFile->open(); - m_stderrHandle = fdopen(m_stderrFile->handle(), "r+"); - Utils::STDERR = m_stderrHandle; + Utils::STDERR = fdopen(m_stderrFile->handle(), "r+"); } void TestCli::cleanup() { m_dbFile.reset(); - m_dbFile2.reset(); + m_keyFileProtectedDbFile.reset(); + m_keyFileProtectedNoPasswordDbFile.reset(); + m_yubiKeyProtectedDbFile.reset(); m_stdinFile.reset(); - m_stdinHandle = stdin; Utils::STDIN = stdin; m_stdoutFile.reset(); Utils::STDOUT = stdout; - m_stdoutHandle = stdout; m_stderrFile.reset(); - m_stderrHandle = stderr; Utils::STDERR = stderr; } @@ -184,8 +180,8 @@ void TestCli::cleanupTestCase() QSharedPointer TestCli::readTestDatabase() const { Utils::Test::setNextPassword("a"); - auto db = QSharedPointer(Utils::unlockDatabase(m_dbFile->fileName(), true, "", "", m_stdoutHandle)); - m_stdoutFile->seek(ftell(m_stdoutHandle)); // re-synchronize handles + auto db = QSharedPointer(Utils::unlockDatabase(m_dbFile->fileName(), true, "", "", Utils::STDOUT)); + m_stdoutFile->seek(ftell(Utils::STDOUT)); // re-synchronize handles return db; } @@ -282,17 +278,25 @@ void TestCli::testAdd() addCmd.execute({"add", "-q", "-u", "newuser", "-g", "-L", "20", m_dbFile->fileName(), "/newentry-quiet"}); m_stdoutFile->seek(pos); m_stderrFile->seek(posErr); - QCOMPARE(m_stdoutFile->readAll(), QByteArray("")); QCOMPARE(m_stderrFile->readAll(), QByteArray("")); + QCOMPARE(m_stdoutFile->readAll(), QByteArray("")); db = readTestDatabase(); entry = db->rootGroup()->findEntryByPath("/newentry-quiet"); QVERIFY(entry); QCOMPARE(entry->password().size(), 20); + pos = m_stdoutFile->pos(); + posErr = m_stderrFile->pos(); Utils::Test::setNextPassword("a"); Utils::Test::setNextPassword("newpassword"); addCmd.execute( {"add", "-u", "newuser2", "--url", "https://example.net/", "-p", m_dbFile->fileName(), "/newuser-entry2"}); + m_stdoutFile->seek(pos); + m_stderrFile->seek(posErr); + m_stdoutFile->readLine(); // skip password prompt + m_stdoutFile->readLine(); // skip password input + QCOMPARE(m_stderrFile->readAll(), QByteArray("")); + QCOMPARE(m_stdoutFile->readAll(), QByteArray("Successfully added entry newuser-entry2.\n")); db = readTestDatabase(); entry = db->rootGroup()->findEntryByPath("/newuser-entry2"); @@ -309,8 +313,8 @@ void TestCli::testAdd() m_stdoutFile->seek(pos); m_stderrFile->seek(posErr); m_stdoutFile->readLine(); // skip password prompt - QCOMPARE(m_stdoutFile->readAll(), QByteArray("Successfully added entry newuser-entry3.\n")); QCOMPARE(m_stderrFile->readAll(), QByteArray("")); + QCOMPARE(m_stdoutFile->readAll(), QByteArray("Successfully added entry newuser-entry3.\n")); db = readTestDatabase(); entry = db->rootGroup()->findEntryByPath("/newuser-entry3"); @@ -339,8 +343,8 @@ void TestCli::testAdd() m_stdoutFile->seek(pos); m_stderrFile->seek(posErr); m_stdoutFile->readLine(); // skip password prompt - QCOMPARE(m_stdoutFile->readAll(), QByteArray("Successfully added entry newuser-entry4.\n")); QCOMPARE(m_stderrFile->readAll(), QByteArray("")); + QCOMPARE(m_stdoutFile->readAll(), QByteArray("Successfully added entry newuser-entry4.\n")); db = readTestDatabase(); entry = db->rootGroup()->findEntryByPath("/newuser-entry4"); @@ -541,7 +545,7 @@ void TestCli::testCreate() QScopedPointer testDir(new QTemporaryDir()); - QString databaseFilename = testDir->path() + "testCreate1.kdbx"; + QString databaseFilename = testDir->path() + "/testCreate1.kdbx"; // Password Utils::Test::setNextPassword("a"); createCmd.execute({"create", databaseFilename}); @@ -568,8 +572,8 @@ void TestCli::testCreate() QCOMPARE(m_stderrFile->readAll(), errorMessage.toUtf8()); // Testing with keyfile creation - QString databaseFilename2 = testDir->path() + "testCreate2.kdbx"; - QString keyfilePath = testDir->path() + "keyfile.txt"; + QString databaseFilename2 = testDir->path() + "/testCreate2.kdbx"; + QString keyfilePath = testDir->path() + "/keyfile.txt"; pos = m_stdoutFile->pos(); errPos = m_stderrFile->pos(); Utils::Test::setNextPassword("a"); @@ -586,7 +590,7 @@ void TestCli::testCreate() QVERIFY(db2); // Testing with existing keyfile - QString databaseFilename3 = testDir->path() + "testCreate3.kdbx"; + QString databaseFilename3 = testDir->path() + "/testCreate3.kdbx"; pos = m_stdoutFile->pos(); errPos = m_stderrFile->pos(); Utils::Test::setNextPassword("a"); diff --git a/tests/TestCli.h b/tests/TestCli.h index 46fff944b..bd0f9fc3f 100644 --- a/tests/TestCli.h +++ b/tests/TestCli.h @@ -91,9 +91,6 @@ private: QScopedPointer m_stdoutFile; QScopedPointer m_stderrFile; QScopedPointer m_stdinFile; - FILE* m_stdoutHandle = stdout; - FILE* m_stderrHandle = stderr; - FILE* m_stdinHandle = stdin; }; #endif // KEEPASSXC_TESTCLI_H diff --git a/tests/TestDatabase.cpp b/tests/TestDatabase.cpp index 960578872..c3a3a8c42 100644 --- a/tests/TestDatabase.cpp +++ b/tests/TestDatabase.cpp @@ -23,7 +23,6 @@ #include "config-keepassx-tests.h" #include "core/Metadata.h" -#include "core/Tools.h" #include "crypto/Crypto.h" #include "format/KeePass2Writer.h" #include "keys/PasswordKey.h" @@ -31,6 +30,8 @@ QTEST_GUILESS_MAIN(TestDatabase) +static QString dbFileName = QStringLiteral(KEEPASSX_TEST_DATA_DIR).append("/NewDatabase.kdbx"); + void TestDatabase::initTestCase() { QVERIFY(Crypto::init()); @@ -45,7 +46,7 @@ void TestDatabase::testOpen() auto key = QSharedPointer::create(); key->addKey(QSharedPointer::create("a")); - bool ok = db->open(QString(KEEPASSX_TEST_DATA_DIR).append("/NewDatabase.kdbx"), key); + bool ok = db->open(dbFileName, key); QVERIFY(ok); QVERIFY(db->isInitialized()); @@ -57,16 +58,8 @@ void TestDatabase::testOpen() void TestDatabase::testSave() { - QByteArray data; - QFile sourceDbFile(QString(KEEPASSX_TEST_DATA_DIR).append("/NewDatabase.kdbx")); - QVERIFY(sourceDbFile.open(QIODevice::ReadOnly)); - QVERIFY(Tools::readAllFromDevice(&sourceDbFile, data)); - sourceDbFile.close(); - TemporaryFile tempFile; - QVERIFY(tempFile.open()); - QCOMPARE(tempFile.write(data), static_cast((data.size()))); - tempFile.close(); + QVERIFY(tempFile.copyFromFile(dbFileName)); auto db = QSharedPointer::create(); auto key = QSharedPointer::create(); @@ -79,15 +72,63 @@ void TestDatabase::testSave() // Test safe saves db->metadata()->setName("test"); QVERIFY(db->isModified()); - - // Test unsafe saves - QVERIFY2(db->save(&error, false, false), error.toLatin1()); - QVERIFY2(db->save(&error), error.toLatin1()); QVERIFY(!db->isModified()); + // Test unsafe saves + db->metadata()->setName("test2"); + QVERIFY2(db->save(&error, false, false), error.toLatin1()); + QVERIFY(!db->isModified()); + // Test save backups + db->metadata()->setName("test3"); QVERIFY2(db->save(&error, true, true), error.toLatin1()); + QVERIFY(!db->isModified()); + + // Confirm backup exists and then delete it + auto re = QRegularExpression("(\\.[^.]+)$"); + auto match = re.match(tempFile.fileName()); + auto backupFilePath = tempFile.fileName(); + backupFilePath = backupFilePath.replace(re, "") + ".old" + match.captured(1); + QVERIFY(QFile::exists(backupFilePath)); + QFile::remove(backupFilePath); + QVERIFY(!QFile::exists(backupFilePath)); +} + +void TestDatabase::testSignals() +{ + TemporaryFile tempFile; + QVERIFY(tempFile.copyFromFile(dbFileName)); + + auto db = QSharedPointer::create(); + auto key = QSharedPointer::create(); + key->addKey(QSharedPointer::create("a")); + + QSignalSpy spyFilePathChanged(db.data(), SIGNAL(filePathChanged(const QString&, const QString&))); + QString error; + bool ok = db->open(tempFile.fileName(), key, &error); + QVERIFY(ok); + QCOMPARE(spyFilePathChanged.count(), 1); + + QSignalSpy spyModified(db.data(), SIGNAL(databaseModified())); + db->metadata()->setName("test1"); + QTRY_COMPARE(spyModified.count(), 1); + + QSignalSpy spySaved(db.data(), SIGNAL(databaseSaved())); + QVERIFY(db->save(&error)); + QCOMPARE(spySaved.count(), 1); + + QSignalSpy spyFileChanged(db.data(), SIGNAL(databaseFileChanged())); + QVERIFY(tempFile.copyFromFile(dbFileName)); + QTRY_COMPARE(spyFileChanged.count(), 1); + QTRY_VERIFY(!db->isModified()); + + db->metadata()->setName("test2"); + QTRY_VERIFY(db->isModified()); + + QSignalSpy spyDiscarded(db.data(), SIGNAL(databaseDiscarded())); + QVERIFY(db->open(tempFile.fileName(), key, &error)); + QCOMPARE(spyDiscarded.count(), 1); } void TestDatabase::testEmptyRecycleBinOnDisabled() diff --git a/tests/TestDatabase.h b/tests/TestDatabase.h index b5df8690f..dc377ef05 100644 --- a/tests/TestDatabase.h +++ b/tests/TestDatabase.h @@ -29,6 +29,7 @@ private slots: void initTestCase(); void testOpen(); void testSave(); + void testSignals(); void testEmptyRecycleBinOnDisabled(); void testEmptyRecycleBinOnNotCreated(); void testEmptyRecycleBinOnEmpty(); diff --git a/tests/gui/TestGui.cpp b/tests/gui/TestGui.cpp index af5107288..ca208db01 100644 --- a/tests/gui/TestGui.cpp +++ b/tests/gui/TestGui.cpp @@ -75,6 +75,8 @@ QTEST_MAIN(TestGui) +static QString dbFileName = QStringLiteral(KEEPASSX_TEST_DATA_DIR).append("/NewDatabase.kdbx"); + void TestGui::initTestCase() { QVERIFY(Crypto::init()); @@ -91,26 +93,19 @@ void TestGui::initTestCase() m_mainWindow.reset(new MainWindow()); Bootstrap::restoreMainWindowState(*m_mainWindow); + Bootstrap::bootstrapApplication(); m_tabWidget = m_mainWindow->findChild("tabWidget"); m_mainWindow->show(); - - // Load the NewDatabase.kdbx file into temporary storage - QFile sourceDbFile(QString(KEEPASSX_TEST_DATA_DIR).append("/NewDatabase.kdbx")); - QVERIFY(sourceDbFile.open(QIODevice::ReadOnly)); - QVERIFY(Tools::readAllFromDevice(&sourceDbFile, m_dbData)); - sourceDbFile.close(); } // Every test starts with opening the temp database void TestGui::init() { - m_dbFile.reset(new TemporaryFile()); - // Write the temp storage to a temp database file for use in our tests - QVERIFY(m_dbFile->open()); - QCOMPARE(m_dbFile->write(m_dbData), static_cast((m_dbData.size()))); - m_dbFileName = QFileInfo(m_dbFile->fileName()).fileName(); - m_dbFilePath = m_dbFile->fileName(); - m_dbFile->close(); + // Copy the test database file to the temporary file + QVERIFY(m_dbFile.copyFromFile(dbFileName)); + + m_dbFileName = QFileInfo(m_dbFile.fileName()).fileName(); + m_dbFilePath = m_dbFile.fileName(); // make sure window is activated or focus tests may fail m_mainWindow->activateWindow(); @@ -145,13 +140,11 @@ void TestGui::cleanup() if (m_dbWidget) { delete m_dbWidget; } - - m_dbFile->remove(); } void TestGui::cleanupTestCase() { - m_dbFile->remove(); + m_dbFile.remove(); } void TestGui::testSettingsDefaultTabOrder() @@ -333,19 +326,10 @@ void TestGui::testAutoreloadDatabase() { config()->set("AutoReloadOnChange", false); - // Load the MergeDatabase.kdbx file into temporary storage - QByteArray unmodifiedMergeDatabase; - QFile mergeDbFile(QString(KEEPASSX_TEST_DATA_DIR).append("/MergeDatabase.kdbx")); - QVERIFY(mergeDbFile.open(QIODevice::ReadOnly)); - QVERIFY(Tools::readAllFromDevice(&mergeDbFile, unmodifiedMergeDatabase)); - mergeDbFile.close(); - // Test accepting new file in autoreload MessageBox::setNextAnswer(MessageBox::Yes); // Overwrite the current database with the temp data - QVERIFY(m_dbFile->open()); - QVERIFY(m_dbFile->write(unmodifiedMergeDatabase, static_cast(unmodifiedMergeDatabase.size()))); - m_dbFile->close(); + QVERIFY(m_dbFile.copyFromFile(QString(KEEPASSX_TEST_DATA_DIR).append("/MergeDatabase.kdbx"))); QTRY_VERIFY(m_db != m_dbWidget->database()); m_db = m_dbWidget->database(); @@ -360,10 +344,8 @@ void TestGui::testAutoreloadDatabase() // Test rejecting new file in autoreload MessageBox::setNextAnswer(MessageBox::No); - // Overwrite the current temp database with a new file - QVERIFY(m_dbFile->open()); - QVERIFY(m_dbFile->write(unmodifiedMergeDatabase, static_cast(unmodifiedMergeDatabase.size()))); - m_dbFile->close(); + // Overwrite the current database with the temp data + QVERIFY(m_dbFile.copyFromFile(QString(KEEPASSX_TEST_DATA_DIR).append("/MergeDatabase.kdbx"))); // Ensure the merge did not take place QCOMPARE(m_db->rootGroup()->findChildByName("General")->entries().size(), 0); @@ -382,9 +364,7 @@ void TestGui::testAutoreloadDatabase() // This is saying yes to merging the entries MessageBox::setNextAnswer(MessageBox::Merge); // Overwrite the current database with the temp data - QVERIFY(m_dbFile->open()); - QVERIFY(m_dbFile->write(unmodifiedMergeDatabase, static_cast(unmodifiedMergeDatabase.size()))); - m_dbFile->close(); + QVERIFY(m_dbFile.copyFromFile(QString(KEEPASSX_TEST_DATA_DIR).append("/MergeDatabase.kdbx"))); QTRY_VERIFY(m_db != m_dbWidget->database()); m_db = m_dbWidget->database(); @@ -1279,9 +1259,8 @@ void TestGui::testDragAndDropKdbxFiles() QCOMPARE(m_tabWidget->count(), openedDatabasesCount); - const QString goodDatabaseFilePath(QString(KEEPASSX_TEST_DATA_DIR).append("/NewDatabase.kdbx")); QMimeData goodMimeData; - goodMimeData.setUrls({QUrl::fromLocalFile(goodDatabaseFilePath)}); + goodMimeData.setUrls({QUrl::fromLocalFile(dbFileName)}); QDragEnterEvent goodDragEvent(QPoint(1, 1), Qt::LinkAction, &goodMimeData, Qt::LeftButton, Qt::NoModifier); qApp->notify(m_mainWindow.data(), &goodDragEvent); QCOMPARE(goodDragEvent.isAccepted(), true); diff --git a/tests/gui/TestGui.h b/tests/gui/TestGui.h index 22f779936..b7798d0b2 100644 --- a/tests/gui/TestGui.h +++ b/tests/gui/TestGui.h @@ -91,8 +91,7 @@ private: QPointer m_tabWidget; QPointer m_dbWidget; QSharedPointer m_db; - QByteArray m_dbData; - QScopedPointer m_dbFile; + TemporaryFile m_dbFile; QString m_dbFileName; QString m_dbFilePath; }; diff --git a/tests/util/TemporaryFile.cpp b/tests/util/TemporaryFile.cpp index 476313b02..19622faed 100644 --- a/tests/util/TemporaryFile.cpp +++ b/tests/util/TemporaryFile.cpp @@ -17,7 +17,7 @@ #include "TemporaryFile.h" -#ifdef Q_OS_WIN +#include TemporaryFile::TemporaryFile() : TemporaryFile(nullptr) @@ -47,9 +47,35 @@ TemporaryFile::TemporaryFile(const QString& templateName, QObject* parent) tmp.close(); } +TemporaryFile::~TemporaryFile() +{ + remove(); +} + bool TemporaryFile::open() { return QFile::open(QIODevice::ReadWrite); } -#endif +bool TemporaryFile::copyFromFile(const QString& otherFileName) +{ + close(); + if (!open(QFile::WriteOnly | QFile::Truncate)) { + return false; + } + + QFile otherFile(otherFileName); + if (!otherFile.open(QFile::ReadOnly)) { + close(); + return false; + } + + QByteArray data; + while(!(data = otherFile.read(1024)).isEmpty()) { + write(data); + } + + otherFile.close(); + close(); + return true; +} diff --git a/tests/util/TemporaryFile.h b/tests/util/TemporaryFile.h index 1fca110a5..3c1cc6aa0 100644 --- a/tests/util/TemporaryFile.h +++ b/tests/util/TemporaryFile.h @@ -20,31 +20,20 @@ #include -#ifdef Q_OS_WIN -/** - * QTemporaryFile does not actually close a file when close() is - * called, which causes the file to be locked on Windows. - * This class extends a QFile with the extra functionality - * of a QTemporaryFile to circumvent this problem. - */ class TemporaryFile : public QFile -#else -class TemporaryFile : public QTemporaryFile -#endif { Q_OBJECT -#ifdef Q_OS_WIN public: TemporaryFile(); explicit TemporaryFile(const QString& templateName); explicit TemporaryFile(QObject* parent); TemporaryFile(const QString& templateName, QObject* parent); - ~TemporaryFile() override = default; + ~TemporaryFile() override; using QFile::open; bool open(); -#endif + bool copyFromFile(const QString& otherFileName); }; #endif // KEEPASSXC_TEMPORARYFILE_H