Fix searchEntries and add some non-GUI tests

keepassxreboot · Oct 6, 2019 · 6f06d97 · 6f06d97
1 parent 7671697
commit 6f06d97
Show file tree

Hide file tree

Showing 4 changed files with 75 additions and 36 deletions.
diff --git a/src/browser/BrowserService.cpp b/src/browser/BrowserService.cpp
@@ -53,6 +53,8 @@ static const QString KEEPASSHTTP_GROUP_NAME = QStringLiteral("KeePassHttp Passwo
 // Extra entry related options saved in custom data
 const QString BrowserService::OPTION_SKIP_AUTO_SUBMIT = QStringLiteral("BrowserSkipAutoSubmit");
 const QString BrowserService::OPTION_HIDE_ENTRY = QStringLiteral("BrowserHideEntry");
+// Multiple URL's
+const QString BrowserService::ADDITIONAL_URL = QStringLiteral("KP2A_URL");
 
 BrowserService::BrowserService(DatabaseTabWidget* parent)
     : m_dbTabWidget(parent)
@@ -588,51 +590,31 @@ BrowserService::searchEntries(const QSharedPointer<Database>& db, const QString&
         return entries;
     }
 
-    auto handleURL = [&](const QString entryUrl) {
-        QUrl entryQUrl(entryUrl);
-        QString entryScheme = entryQUrl.scheme();
-        QUrl qUrl(url);
-
-        // Ignore entry if port or scheme defined in the URL doesn't match
-        if ((entryQUrl.port() > 0 && entryQUrl.port() != qUrl.port())
-            || (browserSettings()->matchUrlScheme() && !entryScheme.isEmpty()
-                && entryScheme.compare(qUrl.scheme()) != 0)) {
-            return false;
-        }
-
-        // Filter to match hostname in URL field
-        if ((!entryUrl.isEmpty() && hostname.contains(entryUrl))
-            || (matchUrlScheme(entryUrl) && hostname.endsWith(entryQUrl.host()))) {
-            return true;
-        }
-        return false;
-    };
-
-    // Search host from URL fields
-    for (auto* entry : EntrySearcher().search(baseDomain(hostname), rootGroup)) {
-        if (!handleURL(entry->url())) {
-            continue;
-        }
-
-        entries.append(entry);
-    }
-
-    // Search for additional URL's starting with KP2A_URL
-    for (const auto group : rootGroup->groupsRecursive(true)) {
+    for (const auto& group : rootGroup->groupsRecursive(true)) {
         if (group->isRecycled() || !group->resolveSearchingEnabled()) {
             continue;
         }
 
-        for (auto* entry : group->entries()) {
-            if (entry->isRecycled() || !entry->attributes()->keys().contains("KP2A_URL")) {
+        for (auto* entry: group->entries()) {
+            if (entry->isRecycled()) {
                 continue;
             }
 
-            for (const auto& key : entry->attributes()->keys()) {
-                if (key.startsWith("KP2A_URL") && handleURL(entry->attributes()->value(key))) {
-                    entries.append(entry);
+            // Search for additional URL's starting with KP2A_URL
+            if (entry->attributes()->keys().contains(ADDITIONAL_URL)) {
+                for (const auto& key : entry->attributes()->keys()) {
+                    if (key.startsWith(ADDITIONAL_URL) && handleURL(entry->attributes()->value(key), hostname, url)) {
+                        entries.append(entry);
+                        continue;
+                    }
                 }
             }
+
+            if (!handleURL(entry->url(), hostname, url)) {
+                continue;
+            }
+
+            entries.append(entry);
         }
     }
 
@@ -1010,6 +992,26 @@ bool BrowserService::removeFirstDomain(QString& hostname)
     return false;
 }
 
+bool BrowserService::handleURL(const QString& entryUrl, const QString& hostname, const QString& url) {
+    QUrl entryQUrl(entryUrl);
+    QString entryScheme = entryQUrl.scheme();
+    QUrl qUrl(url);
+
+    // Ignore entry if port or scheme defined in the URL doesn't match
+    if ((entryQUrl.port() > 0 && entryQUrl.port() != qUrl.port())
+        || (browserSettings()->matchUrlScheme() && !entryScheme.isEmpty()
+            && entryScheme.compare(qUrl.scheme()) != 0)) {
+        return false;
+    }
+
+    // Filter to match hostname in URL field
+    if ((!entryUrl.isEmpty() && hostname.contains(entryUrl))
+        || (matchUrlScheme(entryUrl) && hostname.endsWith(entryQUrl.host()))) {
+        return true;
+    }
+    return false;
+};
+
 /**
  * Gets the base domain of URL.
  *

diff --git a/src/browser/BrowserService.h b/src/browser/BrowserService.h
@@ -74,6 +74,7 @@ class BrowserService : public QObject
     static const QString LEGACY_ASSOCIATE_KEY_PREFIX;
     static const QString OPTION_SKIP_AUTO_SUBMIT;
     static const QString OPTION_HIDE_ENTRY;
+    static const QString ADDITIONAL_URL;
 
 public slots:
     QJsonArray findMatchingEntries(const QString& id,
@@ -129,6 +130,7 @@ public slots:
     sortPriority(const Entry* entry, const QString& host, const QString& submitUrl, const QString& baseSubmitUrl) const;
     bool matchUrlScheme(const QString& url);
     bool removeFirstDomain(QString& hostname);
+    bool handleURL(const QString& entryUrl, const QString& hostname, const QString& url);
     QString baseDomain(const QString& url) const;
     QSharedPointer<Database> getDatabase();
     QSharedPointer<Database> selectedDatabase();

diff --git a/tests/TestBrowser.cpp b/tests/TestBrowser.cpp
@@ -245,6 +245,40 @@ void TestBrowser::testSearchEntriesWithPort()
     QCOMPARE(result[0]->url(), QString("http://127.0.0.1:443"));
 }
 
+void TestBrowser::testSearchEntriesWithAdditionalURLs()
+{
+    auto db = QSharedPointer<Database>::create();
+    auto* root = db->rootGroup();
+
+    QList<Entry*> entries;
+    QList<QString> urls;
+    urls.push_back("https://github.com/");
+    urls.push_back("https://www.example.com");
+    urls.push_back("http://domain.com");
+
+    for (int i = 0; i < urls.length(); ++i) {
+        auto entry = new Entry();
+        entry->setGroup(root);
+        entry->beginUpdate();
+        entry->setUrl(urls[i]);
+        entry->setUsername(QString("User %1").arg(i));
+        entry->endUpdate();
+        entries.push_back(entry);
+    }
+
+    // Add an additional URL to the first entry
+    entries.first()->attributes()->set(BrowserService::ADDITIONAL_URL, "https://keepassxc.org");
+
+    auto result = m_browserService->searchEntries(db, "github.com", "https://github.com"); // db, hostname, url
+    QCOMPARE(result.length(), 1);
+    QCOMPARE(result[0]->url(), QString("https://github.com/"));
+
+    // Search the additional URL. It should return the same entry
+    auto additionalResult = m_browserService->searchEntries(db, "keepassxc.org", "https://keepassxc.org");
+    QCOMPARE(additionalResult.length(), 1);
+    QCOMPARE(additionalResult[0]->url(), QString("https://github.com/"));
+}
+
 void TestBrowser::testSortEntries()
 {
     auto db = QSharedPointer<Database>::create();

diff --git a/tests/TestBrowser.h b/tests/TestBrowser.h
@@ -42,6 +42,7 @@ private slots:
     void testSortPriority();
     void testSearchEntries();
     void testSearchEntriesWithPort();
+    void testSearchEntriesWithAdditionalURLs();
     void testSortEntries();
     void testGetDatabaseGroups();