refactor(urltools): UrlTools is now a namespace

aollier · aollier · commit 682d6f1ead37 · 2026-04-20T16:24:42.000+02:00
UrlTools does not need to inherit from QObject or even be a class.
It is a collection of regular functions.
diff --git a/src/browser/BrowserService.cpp b/src/browser/BrowserService.cpp
@@ -1542,7 +1542,7 @@ bool BrowserService::handleURL(const QString& entryUrl,
     }
 
     // Match the base domain
-    if (urlTools()->getBaseDomainFromUrl(siteQUrl.host()) != urlTools()->getBaseDomainFromUrl(entryQUrl.host())) {
+    if (UrlTools::getBaseDomainFromUrl(siteQUrl.host()) != UrlTools::getBaseDomainFromUrl(entryQUrl.host())) {
         return false;
     }
 
diff --git a/src/browser/PasskeyUtils.cpp b/src/browser/PasskeyUtils.cpp
@@ -232,11 +232,11 @@ bool PasskeyUtils::isRegistrableDomainSuffix(const QString& hostSuffixString, co
         return false;
     }
 
-    if (hostSuffix == urlTools()->getTopLevelDomainFromUrl(hostSuffix)) {
+    if (hostSuffix == UrlTools::getTopLevelDomainFromUrl(hostSuffix)) {
         return false;
     }
 
-    const auto originalPublicSuffix = urlTools()->getTopLevelDomainFromUrl(originalHost);
+    const auto originalPublicSuffix = UrlTools::getTopLevelDomainFromUrl(originalHost);
     if (originalPublicSuffix.isEmpty()) {
         return false;
     }
@@ -256,7 +256,7 @@ bool PasskeyUtils::isDomain(const QString& hostName) const
 {
     const auto domain = QUrl::fromUserInput(hostName).host();
     return !domain.isEmpty() && !domain.endsWith('.') && Tools::isAsciiString(domain)
-           && !urlTools()->domainHasIllegalCharacters(domain) && !urlTools()->isIpAddress(hostName);
+           && !UrlTools::domainHasIllegalCharacters(domain) && !UrlTools::isIpAddress(hostName);
 }
 
 bool PasskeyUtils::isUserVerificationValid(const QString& userVerification) const
diff --git a/src/gui/IconDownloader.cpp b/src/gui/IconDownloader.cpp
@@ -84,7 +84,7 @@ void IconDownloader::setUrl(const QString& entryUrl)
     // Determine the second-level domain, if available
     QString secondLevelDomain;
     if (!hostIsIp) {
-        secondLevelDomain = urlTools()->getBaseDomainFromUrl(url.toString());
+        secondLevelDomain = UrlTools::getBaseDomainFromUrl(url.toString());
     }
 
     // Start with the "fallback" url (if enabled) to try to get the best favicon
@@ -172,7 +172,7 @@ void IconDownloader::fetchFinished()
     QString url = m_url;
 
     bool error = (m_reply->error() != QNetworkReply::NoError);
-    QUrl redirectTarget = urlTools()->getRedirectTarget(m_reply);
+    QUrl redirectTarget = UrlTools::getRedirectTarget(m_reply);
 
     m_reply->deleteLater();
     m_reply = nullptr;
diff --git a/src/gui/URLEdit.cpp b/src/gui/URLEdit.cpp
@@ -50,7 +50,7 @@ void URLEdit::updateStylesheet(const QString& url)
     const QString stylesheetTemplate("QLineEdit { background: %1; }");
     const auto resolvedUrl = m_entry ? m_entry->resolveMultiplePlaceholders(url) : url;
 
-    if (!urlTools()->isUrlValid(resolvedUrl)) {
+    if (!UrlTools::isUrlValid(resolvedUrl)) {
         const StateColorPalette statePalette;
         const auto color = statePalette.color(StateColorPalette::ColorRole::Error);
         setStyleSheet(stylesheetTemplate.arg(color.name()));
diff --git a/src/gui/UrlTools.cpp b/src/gui/UrlTools.cpp
@@ -26,24 +26,20 @@
 
 const QString UrlTools::URL_WILDCARD = "1kpxcwc1";
 
-Q_GLOBAL_STATIC(UrlTools, s_urlTools)
-
-UrlTools* UrlTools::instance()
-{
-    return s_urlTools;
-}
+namespace {
+    QUrl convertVariantToUrl(const QVariant& var)
+    {
+        QUrl url;
+        if (var.canConvert<QUrl>()) {
+            url = var.toUrl();
+        }
+        return url;
 
-QUrl UrlTools::convertVariantToUrl(const QVariant& var) const
-{
-    QUrl url;
-    if (var.canConvert<QUrl>()) {
-        url = var.toUrl();
     }
-    return url;
 }
 
 #if defined(KPXC_FEATURE_NETWORK) || defined(KPXC_FEATURE_BROWSER)
-QUrl UrlTools::getRedirectTarget(QNetworkReply* reply) const
+QUrl UrlTools::getRedirectTarget(QNetworkReply* reply)
 {
     QVariant var = reply->attribute(QNetworkRequest::RedirectionTargetAttribute);
     QUrl url = convertVariantToUrl(var);
@@ -56,7 +52,7 @@ QUrl UrlTools::getRedirectTarget(QNetworkReply* reply) const
  * Returns the base domain, e.g. https://another.example.co.uk -> example.co.uk
  * Up-to-date list can be found: https://publicsuffix.org/list/public_suffix_list.dat
  */
-QString UrlTools::getBaseDomainFromUrl(const QString& url) const
+QString UrlTools::getBaseDomainFromUrl(const QString& url)
 {
     auto qUrl = QUrl::fromUserInput(url);
 
@@ -85,7 +81,7 @@ QString UrlTools::getBaseDomainFromUrl(const QString& url) const
  *
  * Returns the TLD e.g. https://another.example.co.uk -> co.uk
  */
-QString UrlTools::getTopLevelDomainFromUrl(const QString& url) const
+QString UrlTools::getTopLevelDomainFromUrl(const QString& url)
 {
     auto host = QUrl::fromUserInput(url).host();
     if (isIpAddress(host)) {
@@ -112,7 +108,7 @@ QString UrlTools::getTopLevelDomainFromUrl(const QString& url) const
     return host;
 }
 
-bool UrlTools::isIpAddress(const QString& host) const
+bool UrlTools::isIpAddress(const QString& host)
 {
     // Handle IPv6 host with brackets, e.g [::1]
     const auto hostAddress = host.startsWith('[') && host.endsWith(']') ? host.mid(1, host.length() - 2) : host;
@@ -121,35 +117,37 @@ bool UrlTools::isIpAddress(const QString& host) const
 }
 #endif
 
-// Returns true if URLs are identical. Paths with "/" are removed during comparison.
-// URLs without scheme reverts to https.
-// Special handling is needed because QUrl::matches() with QUrl::StripTrailingSlash does not strip "/" paths.
-bool UrlTools::isUrlIdentical(const QString& first, const QString& second) const
+namespace
 {
-    auto trimUrl = [](QString url) {
+    QString trimUrl(QString&& url) {
         url = url.trimmed();
-        if (url.endsWith("/")) {
-            url.remove(url.length() - 1, 1);
+        if (url.endsWith('/')) {
+            url.chop(1); // Removes the last character
         }
-
         return url;
-    };
+    }
+}
 
+// Returns true if URLs are identical. Paths with "/" are removed during comparison.
+// URLs without scheme reverts to https.
+// Special handling is needed because QUrl::matches() with QUrl::StripTrailingSlash does not strip "/" paths.
+bool UrlTools::isUrlIdentical(QString first, QString second)
+{
     if (first.isEmpty() || second.isEmpty()) {
         return false;
     }
 
     // Replace URL wildcards for comparison if found
-    const auto firstUrl = trimUrl(QString(first).replace("*", UrlTools::URL_WILDCARD));
-    const auto secondUrl = trimUrl(QString(second).replace("*", UrlTools::URL_WILDCARD));
+    const auto&& firstUrl = trimUrl(std::move(first.replace("*", UrlTools::URL_WILDCARD)));
+    const auto&& secondUrl = trimUrl(std::move(second.replace("*", UrlTools::URL_WILDCARD)));
     if (firstUrl == secondUrl) {
         return true;
     }
 
     return QUrl(firstUrl).matches(QUrl(secondUrl), QUrl::StripTrailingSlash);
 }
 
-bool UrlTools::isUrlValid(const QString& urlField, bool looseComparison) const
+bool UrlTools::isUrlValid(const QString& urlField, bool looseComparison)
 {
     if (urlField.isEmpty() || urlField.startsWith("cmd://", Qt::CaseInsensitive)
         || urlField.startsWith("kdbx://", Qt::CaseInsensitive) || urlField.startsWith("{REF:A", Qt::CaseInsensitive)) {
@@ -211,8 +209,8 @@ bool UrlTools::isUrlValid(const QString& urlField, bool looseComparison) const
     return true;
 }
 
-bool UrlTools::domainHasIllegalCharacters(const QString& domain) const
+bool UrlTools::domainHasIllegalCharacters(const QString& domain)
 {
-    QRegularExpression re(R"([\s\^#|/:<>\?@\[\]\\])");
+    const QRegularExpression re(R"([\s\^#|/:<>\?@\[\]\\])");
     return re.match(domain).hasMatch();
 }
diff --git a/src/gui/UrlTools.h b/src/gui/UrlTools.h
@@ -26,36 +26,19 @@
 #include <QNetworkReply>
 #endif
 
-class UrlTools : public QObject
+namespace UrlTools
 {
-    Q_OBJECT
-
-public:
-    explicit UrlTools() = default;
-    static UrlTools* instance();
-
 #if defined(KPXC_FEATURE_NETWORK) || defined(KPXC_FEATURE_BROWSER)
-    QUrl getRedirectTarget(QNetworkReply* reply) const;
-    QString getBaseDomainFromUrl(const QString& url) const;
-    QString getTopLevelDomainFromUrl(const QString& url) const;
-    bool isIpAddress(const QString& host) const;
+    QUrl getRedirectTarget(QNetworkReply* reply);
+    QString getBaseDomainFromUrl(const QString& url);
+    QString getTopLevelDomainFromUrl(const QString& url);
+    bool isIpAddress(const QString& host);
 #endif
-    bool isUrlIdentical(const QString& first, const QString& second) const;
-    bool isUrlValid(const QString& urlField, bool looseComparison = false) const;
-    bool domainHasIllegalCharacters(const QString& domain) const;
-
-    static const QString URL_WILDCARD;
+    bool isUrlIdentical(QString first, QString second);
+    bool isUrlValid(const QString& urlField, bool looseComparison = false);
+    bool domainHasIllegalCharacters(const QString& domain);
 
-private:
-    QUrl convertVariantToUrl(const QVariant& var) const;
-
-private:
-    Q_DISABLE_COPY(UrlTools);
-};
-
-static inline UrlTools* urlTools()
-{
-    return UrlTools::instance();
+    extern const QString URL_WILDCARD;
 }
 
 #endif // KEEPASSXC_URLTOOLS_H
diff --git a/src/gui/entry/EntryURLModel.cpp b/src/gui/entry/EntryURLModel.cpp
@@ -67,14 +67,14 @@ QVariant EntryURLModel::data(const QModelIndex& index, int role) const
     }
 
     const auto value = m_entryAttributes->value(key);
-    const auto urlValid = urlTools()->isUrlValid(value, true);
+    const auto urlValid = UrlTools::isUrlValid(value, true);
 
     // Check for duplicate URLs in the attribute list. Excludes the current key/value from the comparison.
     auto customAttributeKeys = m_entryAttributes->customKeys().filter(EntryAttributes::AdditionalUrlAttribute);
     customAttributeKeys.removeOne(key);
 
     const auto duplicateUrl =
-        m_entryAttributes->values(customAttributeKeys).contains(value) || urlTools()->isUrlIdentical(value, m_entryUrl);
+        m_entryAttributes->values(customAttributeKeys).contains(value) || UrlTools::isUrlIdentical(value, m_entryUrl);
     if (role == Qt::BackgroundRole && (!urlValid || duplicateUrl)) {
         StateColorPalette statePalette;
         return statePalette.color(StateColorPalette::ColorRole::Error);
diff --git a/tests/TestUrlTools.cpp b/tests/TestUrlTools.cpp
@@ -20,15 +20,6 @@
 
 QTEST_GUILESS_MAIN(TestUrlTools)
 
-void TestUrlTools::initTestCase()
-{
-    m_urlTools = urlTools();
-}
-
-void TestUrlTools::init()
-{
-}
-
 void TestUrlTools::testTopLevelDomain()
 {
     // Create list of URLs and expected TLD responses
@@ -48,7 +39,7 @@ void TestUrlTools::testTopLevelDomain()
     };
 
     for (const auto& u : tldUrls) {
-        QCOMPARE(urlTools()->getTopLevelDomainFromUrl(u.first), u.second);
+        QCOMPARE(UrlTools::getTopLevelDomainFromUrl(u.first), u.second);
     }
 
     // Create list of URLs and expected base URL responses
@@ -66,7 +57,7 @@ void TestUrlTools::testTopLevelDomain()
     };
 
     for (const auto& u : baseUrls) {
-        QCOMPARE(urlTools()->getBaseDomainFromUrl(u.first), u.second);
+        QCOMPARE(UrlTools::getBaseDomainFromUrl(u.first), u.second);
     }
 }
 
@@ -84,32 +75,32 @@ void TestUrlTools::testIsIpAddress()
     auto host10 = "::";
     auto host11 = "[2001:20::1]";
 
-    QVERIFY(!urlTools()->isIpAddress(host1));
-    QVERIFY(urlTools()->isIpAddress(host2));
-    QVERIFY(!urlTools()->isIpAddress(host3));
-    QVERIFY(urlTools()->isIpAddress(host4));
-    QVERIFY(urlTools()->isIpAddress(host5));
-    QVERIFY(urlTools()->isIpAddress(host6));
-    QVERIFY(urlTools()->isIpAddress(host7));
-    QVERIFY(!urlTools()->isIpAddress(host8));
-    QVERIFY(urlTools()->isIpAddress(host9));
-    QVERIFY(urlTools()->isIpAddress(host10));
-    QVERIFY(urlTools()->isIpAddress(host11));
+    QVERIFY(!UrlTools::isIpAddress(host1));
+    QVERIFY(UrlTools::isIpAddress(host2));
+    QVERIFY(!UrlTools::isIpAddress(host3));
+    QVERIFY(UrlTools::isIpAddress(host4));
+    QVERIFY(UrlTools::isIpAddress(host5));
+    QVERIFY(UrlTools::isIpAddress(host6));
+    QVERIFY(UrlTools::isIpAddress(host7));
+    QVERIFY(!UrlTools::isIpAddress(host8));
+    QVERIFY(UrlTools::isIpAddress(host9));
+    QVERIFY(UrlTools::isIpAddress(host10));
+    QVERIFY(UrlTools::isIpAddress(host11));
 }
 
 void TestUrlTools::testIsUrlIdentical()
 {
-    QVERIFY(urlTools()->isUrlIdentical("https://example.com", "https://example.com"));
-    QVERIFY(urlTools()->isUrlIdentical("https://example.com", "  https://example.com  "));
-    QVERIFY(!urlTools()->isUrlIdentical("https://example.com", "https://example2.com"));
-    QVERIFY(!urlTools()->isUrlIdentical("https://example.com/", "https://example.com/#login"));
-    QVERIFY(urlTools()->isUrlIdentical("https://example.com", "https://example.com/"));
-    QVERIFY(urlTools()->isUrlIdentical("https://example.com/", "https://example.com"));
-    QVERIFY(urlTools()->isUrlIdentical("https://example.com/  ", "  https://example.com"));
-    QVERIFY(!urlTools()->isUrlIdentical("https://example.com/", "  example.com"));
-    QVERIFY(urlTools()->isUrlIdentical("https://example.com/path/to/nowhere", "https://example.com/path/to/nowhere/"));
-    QVERIFY(!urlTools()->isUrlIdentical("https://example.com/", "://example.com/"));
-    QVERIFY(urlTools()->isUrlIdentical("ftp://127.0.0.1/", "ftp://127.0.0.1"));
+    QVERIFY(UrlTools::isUrlIdentical("https://example.com", "https://example.com"));
+    QVERIFY(UrlTools::isUrlIdentical("https://example.com", "  https://example.com  "));
+    QVERIFY(!UrlTools::isUrlIdentical("https://example.com", "https://example2.com"));
+    QVERIFY(!UrlTools::isUrlIdentical("https://example.com/", "https://example.com/#login"));
+    QVERIFY(UrlTools::isUrlIdentical("https://example.com", "https://example.com/"));
+    QVERIFY(UrlTools::isUrlIdentical("https://example.com/", "https://example.com"));
+    QVERIFY(UrlTools::isUrlIdentical("https://example.com/  ", "  https://example.com"));
+    QVERIFY(!UrlTools::isUrlIdentical("https://example.com/", "  example.com"));
+    QVERIFY(UrlTools::isUrlIdentical("https://example.com/path/to/nowhere", "https://example.com/path/to/nowhere/"));
+    QVERIFY(!UrlTools::isUrlIdentical("https://example.com/", "://example.com/"));
+    QVERIFY(UrlTools::isUrlIdentical("ftp://127.0.0.1/", "ftp://127.0.0.1"));
 }
 
 void TestUrlTools::testIsUrlValid()
@@ -130,7 +121,7 @@ void TestUrlTools::testIsUrlValid()
     QHashIterator<QString, bool> i(urls);
     while (i.hasNext()) {
         i.next();
-        QCOMPARE(urlTools()->isUrlValid(i.key()), i.value());
+        QCOMPARE(UrlTools::isUrlValid(i.key()), i.value());
     }
 }
 
@@ -165,13 +156,13 @@ void TestUrlTools::testIsUrlValidWithLooseComparison()
     QHashIterator<QString, bool> i(urls);
     while (i.hasNext()) {
         i.next();
-        QCOMPARE(urlTools()->isUrlValid(i.key(), true), i.value());
+        QCOMPARE(UrlTools::isUrlValid(i.key(), true), i.value());
     }
 }
 
 void TestUrlTools::testDomainHasIllegalCharacters()
 {
-    QVERIFY(!urlTools()->domainHasIllegalCharacters("example.com"));
-    QVERIFY(urlTools()->domainHasIllegalCharacters("domain has spaces.com"));
-    QVERIFY(urlTools()->domainHasIllegalCharacters("example#|.com"));
+    QVERIFY(!UrlTools::domainHasIllegalCharacters("example.com"));
+    QVERIFY(UrlTools::domainHasIllegalCharacters("domain has spaces.com"));
+    QVERIFY(UrlTools::domainHasIllegalCharacters("example#|.com"));
 }
diff --git a/tests/TestUrlTools.h b/tests/TestUrlTools.h
@@ -27,17 +27,11 @@ class TestUrlTools : public QObject
     Q_OBJECT
 
 private slots:
-    void initTestCase();
-    void init();
-
     void testTopLevelDomain();
     void testIsIpAddress();
     void testIsUrlIdentical();
     void testIsUrlValid();
     void testIsUrlValidWithLooseComparison();
     void testDomainHasIllegalCharacters();
-
-private:
-    QPointer<UrlTools> m_urlTools;
 };
 #endif // KEEPASSXC_TESTURLTOOLS_H

Original file line number	Diff line number	Diff line change
`@@ -1542,7 +1542,7 @@ bool BrowserService::handleURL(const QString& entryUrl,`
`1542`	`1542`	`}`
`1543`	`1543`
`1544`	`1544`	`// Match the base domain`
`1545`		`- if (urlTools()->getBaseDomainFromUrl(siteQUrl.host()) != urlTools()->getBaseDomainFromUrl(entryQUrl.host())) {`
	`1545`	`+ if (UrlTools::getBaseDomainFromUrl(siteQUrl.host()) != UrlTools::getBaseDomainFromUrl(entryQUrl.host())) {`
`1546`	`1546`	`return false;`
`1547`	`1547`	`}`
`1548`	`1548`
Original file line number	Diff line number	Diff line change
`@@ -232,11 +232,11 @@ bool PasskeyUtils::isRegistrableDomainSuffix(const QString& hostSuffixString, co`
`232`	`232`	`return false;`
`233`	`233`	`}`
`234`	`234`
`235`		`- if (hostSuffix == urlTools()->getTopLevelDomainFromUrl(hostSuffix)) {`
	`235`	`+ if (hostSuffix == UrlTools::getTopLevelDomainFromUrl(hostSuffix)) {`
`236`	`236`	`return false;`
`237`	`237`	`}`
`238`	`238`
`239`		`- const auto originalPublicSuffix = urlTools()->getTopLevelDomainFromUrl(originalHost);`
	`239`	`+ const auto originalPublicSuffix = UrlTools::getTopLevelDomainFromUrl(originalHost);`
`240`	`240`	`if (originalPublicSuffix.isEmpty()) {`
`241`	`241`	`return false;`
`242`	`242`	`}`
`@@ -256,7 +256,7 @@ bool PasskeyUtils::isDomain(const QString& hostName) const`
`256`	`256`	`{`
`257`	`257`	`const auto domain = QUrl::fromUserInput(hostName).host();`
`258`	`258`	`return !domain.isEmpty() && !domain.endsWith('.') && Tools::isAsciiString(domain)`
`259`		`- && !urlTools()->domainHasIllegalCharacters(domain) && !urlTools()->isIpAddress(hostName);`
	`259`	`+ && !UrlTools::domainHasIllegalCharacters(domain) && !UrlTools::isIpAddress(hostName);`
`260`	`260`	`}`
`261`	`261`
`262`	`262`	`bool PasskeyUtils::isUserVerificationValid(const QString& userVerification) const`