Fix support for referenced URL fields

keepassxreboot · Nov 12, 2022 · b4ef66c · b4ef66c
1 parent afc7dcd
commit b4ef66c
Show file tree

Hide file tree

Showing 5 changed files with 52 additions and 26 deletions.
diff --git a/src/browser/BrowserService.cpp b/src/browser/BrowserService.cpp
@@ -864,7 +864,7 @@ QList<Entry*> BrowserService::sortEntries(QList<Entry*>& pwEntries, const QStrin
     // Build map of prioritized entries
     QMultiMap<int, Entry*> priorities;
     for (auto* entry : pwEntries) {
-        priorities.insert(sortPriority(getEntryURLs(entry), siteUrl, formUrl), entry);
+        priorities.insert(sortPriority(entry->getAllUrls(), siteUrl, formUrl), entry);
     }
 
     auto keys = priorities.uniqueKeys();
@@ -1342,21 +1342,6 @@ bool BrowserService::checkLegacySettings(QSharedPointer<Database> db)
     return dialogResult == MessageBox::Yes;
 }
 
-QStringList BrowserService::getEntryURLs(const Entry* entry)
-{
-    QStringList urlList;
-    urlList << entry->url();
-
-    // Handle additional URL's
-    for (const auto& key : entry->attributes()->keys()) {
-        if (key.startsWith(ADDITIONAL_URL)) {
-            urlList << entry->attributes()->value(key);
-        }
-    }
-
-    return urlList;
-}
-
 void BrowserService::hideWindow() const
 {
     if (m_prevWindowState == WindowState::Minimized) {

diff --git a/src/browser/BrowserService.h b/src/browser/BrowserService.h
@@ -158,7 +158,6 @@ private slots:
     QString getDatabaseRootUuid();
     QString getDatabaseRecycleBinUuid();
     bool checkLegacySettings(QSharedPointer<Database> db);
-    QStringList getEntryURLs(const Entry* entry);
     void hideWindow() const;
     void raiseWindow(const bool force = false);
 

diff --git a/src/core/Entry.cpp b/src/core/Entry.cpp
@@ -372,16 +372,18 @@ QString Entry::url() const
 QStringList Entry::getAllUrls() const
 {
     QStringList urlList;
+    auto entryUrl = url();
 
-    if (!url().isEmpty()) {
-        urlList << url();
+    if (!entryUrl.isEmpty()) {
+        urlList << (EntryAttributes::matchReference(entryUrl).hasMatch() ? resolveMultiplePlaceholders(entryUrl)
+                                                                         : entryUrl);
     }
 
     for (const auto& key : m_attributes->keys()) {
         if (key.startsWith("KP2A_URL")) {
             auto additionalUrl = m_attributes->value(key);
             if (!additionalUrl.isEmpty()) {
-                urlList << additionalUrl;
+                urlList << resolveMultiplePlaceholders(additionalUrl);
             }
         }
     }
@@ -1432,7 +1434,7 @@ QString Entry::resolveUrl(const QString& url) const
         }
 
         // No URL in this command
-        return QString("");
+        return {};
     }
 
     if (!newUrl.isEmpty() && !newUrl.contains("://")) {

diff --git a/tests/TestBrowser.cpp b/tests/TestBrowser.cpp
@@ -1,5 +1,5 @@
 /*
- *  Copyright (C) 2021 KeePassXC Team <[email protected]>
+ *  Copyright (C) 2022 KeePassXC Team <[email protected]>
  *
  *  This program is free software: you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -168,8 +168,7 @@ void TestBrowser::testSortPriority()
     QScopedPointer<Entry> entry(new Entry());
     entry->setUrl(entryUrl);
 
-    QCOMPARE(m_browserService->sortPriority(m_browserService->getEntryURLs(entry.data()), siteUrl, formUrl),
-             expectedScore);
+    QCOMPARE(m_browserService->sortPriority(entry->getAllUrls(), siteUrl, formUrl), expectedScore);
 }
 
 void TestBrowser::testSortPriority_data()
@@ -310,7 +309,7 @@ void TestBrowser::testSearchEntriesByUUID()
 
     for (Entry* entry : entries) {
         QString testUrl = "keepassxc://by-uuid/" + entry->uuidToHex();
-        /* Look for an entry with that UUID. First using handleEntry, then through the search */
+        /* Look for an entry with that UUID. First using shouldIncludeEntry, then through the search */
         QCOMPARE(m_browserService->shouldIncludeEntry(entry, testUrl, ""), true);
         auto result = m_browserService->searchEntries(db, testUrl, "");
         QCOMPARE(result.length(), 1);
@@ -337,6 +336,46 @@ void TestBrowser::testSearchEntriesByUUID()
     }
 }
 
+void TestBrowser::testSearchEntriesByReference()
+{
+    auto db = QSharedPointer<Database>::create();
+    auto* root = db->rootGroup();
+
+    /* The URLs don't really matter for this test, we just need some entries */
+    QStringList urls = {"https://subdomain.example.com",
+                        "example.com", // Only includes a partial URL for references
+                        "https://another.domain.com", // Additional URL as full reference
+                        "https://subdomain.somesite.com", // Additional URL as partial reference
+                        "", // Full reference will be added to https://subdomain.example.com
+                        "" // Partial reference will be added to https://subdomain.example.com
+                        "https://www.notincluded.com"}; // Should not show in search
+    auto entries = createEntries(urls, root);
+
+    auto firstEntryUuid = entries.first()->uuidToHex();
+    auto secondEntryUuid = entries[1]->uuidToHex();
+    auto fullReference = QString("{REF:A@I:%1}").arg(firstEntryUuid);
+    auto partialReference = QString("https://subdomain.{REF:A@I:%1}").arg(secondEntryUuid);
+    entries[2]->attributes()->set(BrowserService::ADDITIONAL_URL, fullReference);
+    entries[3]->attributes()->set(BrowserService::ADDITIONAL_URL, partialReference);
+    entries[4]->setUrl(fullReference);
+    entries[5]->setUrl(partialReference);
+
+    auto result = m_browserService->searchEntries(db, "https://subdomain.example.com", "");
+    QCOMPARE(result.length(), 6);
+    QCOMPARE(result[0]->url(), urls[0]);
+    QCOMPARE(result[1]->url(), urls[1]);
+    QCOMPARE(result[2]->url(), urls[2]);
+    QCOMPARE(result[2]->resolveMultiplePlaceholders(result[2]->attributes()->value(BrowserService::ADDITIONAL_URL)),
+             urls[0]);
+    QCOMPARE(result[3]->url(), urls[3]);
+    QCOMPARE(result[3]->resolveMultiplePlaceholders(result[3]->attributes()->value(BrowserService::ADDITIONAL_URL)),
+             urls[0]);
+    QCOMPARE(result[4]->url(), fullReference);
+    QCOMPARE(result[4]->resolveMultiplePlaceholders(result[4]->url()), urls[0]); // Should be resolved to the main entry
+    QCOMPARE(result[5]->url(), partialReference);
+    QCOMPARE(result[5]->resolveMultiplePlaceholders(result[5]->url()), urls[0]); // Should be resolved to the main entry
+}
+
 void TestBrowser::testSearchEntriesWithPort()
 {
     auto db = QSharedPointer<Database>::create();

diff --git a/tests/TestBrowser.h b/tests/TestBrowser.h
@@ -1,5 +1,5 @@
 /*
- *  Copyright (C) 2021 KeePassXC Team <[email protected]>
+ *  Copyright (C) 2022 KeePassXC Team <[email protected]>
  *
  *  This program is free software: you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -44,6 +44,7 @@ private slots:
     void testSearchEntries();
     void testSearchEntriesByPath();
     void testSearchEntriesByUUID();
+    void testSearchEntriesByReference();
     void testSearchEntriesWithPort();
     void testSearchEntriesWithAdditionalURLs();
     void testInvalidEntries();