diff --git a/src/format/KeePass2Reader.cpp b/src/format/KeePass2Reader.cpp index e1f65306..6c35e37a 100644 --- a/src/format/KeePass2Reader.cpp +++ b/src/format/KeePass2Reader.cpp @@ -38,7 +38,8 @@ KeePass2Reader::KeePass2Reader() Database* KeePass2Reader::readDatabase(QIODevice* device, const CompositeKey& key) { - m_db = new Database(); + QScopedPointer db(new Database()); + m_db = db.data(); m_device = device; m_error = false; m_errorStr = QString(); @@ -48,13 +49,13 @@ Database* KeePass2Reader::readDatabase(QIODevice* device, const CompositeKey& ke quint32 signature1 = Endian::readUInt32(m_device, KeePass2::BYTEORDER, &ok); if (!ok || signature1 != KeePass2::SIGNATURE_1) { - raiseError("1"); + raiseError(tr("Not a KeePass database.")); return 0; } quint32 signature2 = Endian::readUInt32(m_device, KeePass2::BYTEORDER, &ok); if (!ok || signature2 != KeePass2::SIGNATURE_2) { - raiseError("2"); + raiseError(tr("Not a KeePass database.")); return 0; } @@ -62,11 +63,11 @@ Database* KeePass2Reader::readDatabase(QIODevice* device, const CompositeKey& ke quint32 expectedVersion = KeePass2::FILE_VERSION & KeePass2::FILE_VERSION_CRITICAL_MASK; // TODO do we support old Kdbx versions? if (!ok || (version != expectedVersion)) { - raiseError("3"); + raiseError(tr("Unsupported KeePass database version.")); return 0; } - while (readHeaderField() && !error()) { + while (readHeaderField() && !hasError()) { } // TODO check if all header fields have been parsed @@ -84,7 +85,7 @@ Database* KeePass2Reader::readDatabase(QIODevice* device, const CompositeKey& ke QByteArray realStart = cipherStream.read(32); if (realStart != m_streamStartBytes) { - raiseError("4"); + raiseError(tr("Wrong key or database file is corrupt.")); return 0; } @@ -117,28 +118,41 @@ Database* KeePass2Reader::readDatabase(QIODevice* device, const CompositeKey& ke KeePass2XmlReader xmlReader; xmlReader.readDatabase(xmlDevice, m_db, &randomStream); - // TODO forward error messages from xmlReader - return m_db; + + if (xmlReader.hasError()) { + raiseError(xmlReader.errorString()); + return 0; + } + + return db.take(); } Database* KeePass2Reader::readDatabase(const QString& filename, const CompositeKey& key) { QFile file(filename); - file.open(QFile::ReadOnly); - Database* db = readDatabase(&file, key); - // TODO check for QFile errors - return db; + if (!file.open(QFile::ReadOnly)) { + raiseError(file.errorString()); + return 0; + } + + QScopedPointer db(readDatabase(&file, key)); + + if (file.error() != QFile::NoError) { + raiseError(file.errorString()); + return 0; + } + + return db.take(); } -bool KeePass2Reader::error() +bool KeePass2Reader::hasError() { return m_error; } QString KeePass2Reader::errorString() { - // TODO - return QString(); + return m_errorStr; } void KeePass2Reader::setSaveXml(bool save) @@ -153,9 +167,8 @@ QByteArray KeePass2Reader::xmlData() void KeePass2Reader::raiseError(const QString& str) { - // TODO - qWarning("KeePass2Reader error: %s", qPrintable(str)); m_error = true; + m_errorStr = str; } bool KeePass2Reader::readHeaderField() diff --git a/src/format/KeePass2Reader.h b/src/format/KeePass2Reader.h index fd872168..86d2337c 100644 --- a/src/format/KeePass2Reader.h +++ b/src/format/KeePass2Reader.h @@ -33,7 +33,7 @@ public: KeePass2Reader(); Database* readDatabase(QIODevice* device, const CompositeKey& key); Database* readDatabase(const QString& filename, const CompositeKey& key); - bool error(); + bool hasError(); QString errorString(); void setSaveXml(bool save); QByteArray xmlData(); diff --git a/src/format/KeePass2XmlReader.cpp b/src/format/KeePass2XmlReader.cpp index 275c7413..4c4efa1d 100644 --- a/src/format/KeePass2XmlReader.cpp +++ b/src/format/KeePass2XmlReader.cpp @@ -34,6 +34,7 @@ KeePass2XmlReader::KeePass2XmlReader() void KeePass2XmlReader::readDatabase(QIODevice* device, Database* db, KeePass2RandomStream* randomStream) { + m_xml.clear(); m_xml.setDevice(device); m_db = db; @@ -48,9 +49,9 @@ void KeePass2XmlReader::readDatabase(QIODevice* device, Database* db, KeePass2Ra } } - // TODO check if m_tmpParent doesn't have entries if (!m_xml.error() && !m_tmpParent->children().isEmpty()) { - raiseError(); + qWarning("KeePass2XmlReader::readDatabase: found %d invalid entry references", + m_tmpParent->children().size()); } delete m_tmpParent; @@ -70,7 +71,7 @@ Database* KeePass2XmlReader::readDatabase(const QString& filename) return readDatabase(&file); } -bool KeePass2XmlReader::error() +bool KeePass2XmlReader::hasError() { return m_xml.hasError(); } @@ -300,7 +301,7 @@ Group* KeePass2XmlReader::parseGroup() if (m_xml.name() == "UUID") { Uuid uuid = readUuid(); if (uuid.isNull()) { - raiseError(); + raiseError(1); } else { group = getGroup(uuid); @@ -315,7 +316,7 @@ Group* KeePass2XmlReader::parseGroup() else if (m_xml.name() == "IconID") { int iconId = readNumber(); if (iconId < 0) { - raiseError(); + raiseError(2); } else { if (iconId >= databaseIcons()->iconCount()) { @@ -352,7 +353,7 @@ Group* KeePass2XmlReader::parseGroup() group->setAutoTypeEnabled(Group::Disable); } else { - raiseError(); + raiseError(3); } } @@ -369,7 +370,7 @@ Group* KeePass2XmlReader::parseGroup() group->setSearchingEnabled(Group::Disable); } else { - raiseError(); + raiseError(4); } } else if (m_xml.name() == "LastTopVisibleEntry") { @@ -419,7 +420,7 @@ void KeePass2XmlReader::parseDeletedObject() if (m_xml.name() == "UUID") { Uuid uuid = readUuid(); if (uuid.isNull()) { - raiseError(); + raiseError(5); } else { delObj.uuid = uuid; @@ -445,7 +446,7 @@ Entry* KeePass2XmlReader::parseEntry(bool history) if (m_xml.name() == "UUID") { Uuid uuid = readUuid(); if (uuid.isNull()) { - raiseError(); + raiseError(6); } else { if (history) { @@ -460,7 +461,7 @@ Entry* KeePass2XmlReader::parseEntry(bool history) else if (m_xml.name() == "IconID") { int iconId = readNumber(); if (iconId < 0) { - raiseError(); + raiseError(7); } else { entry->setIcon(iconId); @@ -498,7 +499,7 @@ Entry* KeePass2XmlReader::parseEntry(bool history) } else if (m_xml.name() == "History") { if (history) { - raiseError(); + raiseError(8); } else { parseEntryHistory(entry); @@ -532,7 +533,7 @@ void KeePass2XmlReader::parseEntryString(Entry *entry) value = m_randomStream->process(QByteArray::fromBase64(value.toAscii())); } else { - raiseError(); + raiseError(9); } } @@ -679,7 +680,7 @@ bool KeePass2XmlReader::readBool() return false; } else { - raiseError(); + raiseError(10); return false; } } @@ -690,7 +691,7 @@ QDateTime KeePass2XmlReader::readDateTime() QDateTime dt = QDateTime::fromString(str, Qt::ISODate); if (!dt.isValid()) { - raiseError(); + raiseError(11); } return dt; @@ -705,7 +706,7 @@ QColor KeePass2XmlReader::readColor() } if (colorStr.length() != 7 || colorStr[0] != '#') { - raiseError(); + raiseError(12); return QColor(); } @@ -715,7 +716,7 @@ QColor KeePass2XmlReader::readColor() bool ok; int rgbPart = rgbPartStr.toInt(&ok, 16); if (!ok || rgbPart > 255) { - raiseError(); + raiseError(13); return QColor(); } @@ -738,7 +739,7 @@ int KeePass2XmlReader::readNumber() bool ok; int result = readString().toInt(&ok); if (!ok) { - raiseError(); + raiseError(14); } return result; } @@ -747,7 +748,7 @@ Uuid KeePass2XmlReader::readUuid() { QByteArray uuidBin = readBinary(); if (uuidBin.length() != Uuid::LENGTH) { - raiseError(); + raiseError(15); return Uuid(); } else { @@ -798,9 +799,9 @@ Entry* KeePass2XmlReader::getEntry(const Uuid& uuid) return entry; } -void KeePass2XmlReader::raiseError() +void KeePass2XmlReader::raiseError(int internalNumber) { - m_xml.raiseError(tr("Invalid database file")); + m_xml.raiseError(tr("Invalid database file (error no %1).").arg(QString::number(internalNumber))); } void KeePass2XmlReader::skipCurrentElement() diff --git a/src/format/KeePass2XmlReader.h b/src/format/KeePass2XmlReader.h index cd1c6f04..1d03a3a2 100644 --- a/src/format/KeePass2XmlReader.h +++ b/src/format/KeePass2XmlReader.h @@ -42,7 +42,7 @@ public: Database* readDatabase(QIODevice* device); void readDatabase(QIODevice* device, Database* db, KeePass2RandomStream* randomStream = 0); Database* readDatabase(const QString& filename); - bool error(); + bool hasError(); QString errorString(); private: @@ -75,7 +75,7 @@ private: Group* getGroup(const Uuid& uuid); Entry* getEntry(const Uuid& uuid); - void raiseError(); + void raiseError(int internalNumber); void skipCurrentElement(); QXmlStreamReader m_xml; diff --git a/tests/TestKeePass2Reader.cpp b/tests/TestKeePass2Reader.cpp index 4a37bd19..67fbdcac 100644 --- a/tests/TestKeePass2Reader.cpp +++ b/tests/TestKeePass2Reader.cpp @@ -41,7 +41,7 @@ void TestKeePass2Reader::testNonAscii() KeePass2Reader* reader = new KeePass2Reader(); Database* db = reader->readDatabase(filename, key); QVERIFY(db); - QVERIFY(!reader->error()); + QVERIFY(!reader->hasError()); QCOMPARE(db->metadata()->name(), QString("NonAsciiTest")); delete db; @@ -56,7 +56,7 @@ void TestKeePass2Reader::testCompressed() KeePass2Reader* reader = new KeePass2Reader(); Database* db = reader->readDatabase(filename, key); QVERIFY(db); - QVERIFY(!reader->error()); + QVERIFY(!reader->hasError()); QCOMPARE(db->metadata()->name(), QString("Compressed")); delete db; @@ -71,7 +71,7 @@ void TestKeePass2Reader::testProtectedStrings() KeePass2Reader* reader = new KeePass2Reader(); Database* db = reader->readDatabase(filename, key); QVERIFY(db); - QVERIFY(!reader->error()); + QVERIFY(!reader->hasError()); QCOMPARE(db->metadata()->name(), QString("Protected Strings Test")); Entry* entry = db->rootGroup()->entries().at(0); diff --git a/tests/TestKeePass2Writer.cpp b/tests/TestKeePass2Writer.cpp index 3513b92d..a844b004 100644 --- a/tests/TestKeePass2Writer.cpp +++ b/tests/TestKeePass2Writer.cpp @@ -60,7 +60,7 @@ void TestKeePass2Writer::initTestCase() buffer.seek(0); KeePass2Reader reader; m_dbTest = reader.readDatabase(&buffer, key); - QVERIFY(!reader.error()); + QVERIFY(!reader.hasError()); QVERIFY(m_dbTest); } diff --git a/tests/TestKeePass2XmlReader.cpp b/tests/TestKeePass2XmlReader.cpp index 641e777f..12b2dafd 100644 --- a/tests/TestKeePass2XmlReader.cpp +++ b/tests/TestKeePass2XmlReader.cpp @@ -71,7 +71,7 @@ void TestKeePass2XmlReader::initTestCase() QString xmlFile = QString(KEEPASSX_TEST_DATA_DIR).append("/NewDatabase.xml"); m_db = reader->readDatabase(xmlFile); QVERIFY(m_db); - QVERIFY(!reader->error()); + QVERIFY(!reader->hasError()); } void TestKeePass2XmlReader::testMetadata() diff --git a/tests/TestKeys.cpp b/tests/TestKeys.cpp index 44a09771..6fd9967d 100644 --- a/tests/TestKeys.cpp +++ b/tests/TestKeys.cpp @@ -77,7 +77,7 @@ void TestKeys::testFileKey() Database* db = reader.readDatabase(dbFilename, compositeKey); QVERIFY(db); - QVERIFY(!reader.error()); + QVERIFY(!reader.hasError()); QCOMPARE(db->metadata()->name(), QString("%1 Database").arg(name)); delete db; @@ -122,7 +122,7 @@ void TestKeys::testCreateFileKey() KeePass2Reader reader; Database* dbRead = reader.readDatabase(&dbBuffer, compositeKey); QVERIFY(dbRead); - QVERIFY(!reader.error()); + QVERIFY(!reader.hasError()); QCOMPARE(dbRead->metadata()->name(), dbName); delete dbRead; } diff --git a/utils/kdbx-extract.cpp b/utils/kdbx-extract.cpp index 44abfc4c..0e3be692 100644 --- a/utils/kdbx-extract.cpp +++ b/utils/kdbx-extract.cpp @@ -57,7 +57,7 @@ int main(int argc, char **argv) reader.setSaveXml(true); reader.readDatabase(&dbFile, key); - if (reader.error()) { + if (reader.hasError()) { qCritical("Error while reading the database."); return 1; }