From 8b6c8a6967d3d4d436cb9061914361c3b89fc1d1 Mon Sep 17 00:00:00 2001 From: Adrien Ollier Date: Mon, 20 Apr 2026 15:42:53 +0200 Subject: [PATCH 1/6] refactor(urltools): UrlTools is now a namespace UrlTools does not need to inherit from QObject or even be a class. It is a collection of regular functions. --- src/browser/BrowserService.cpp | 2 +- src/browser/PasskeyUtils.cpp | 6 +-- src/gui/IconDownloader.cpp | 4 +- src/gui/URLEdit.cpp | 2 +- src/gui/UrlTools.cpp | 58 ++++++++++++++-------------- src/gui/UrlTools.h | 35 +++++------------ src/gui/entry/EntryURLModel.cpp | 4 +- tests/TestUrlTools.cpp | 67 ++++++++++++++------------------- tests/TestUrlTools.h | 6 --- 9 files changed, 75 insertions(+), 109 deletions(-) diff --git a/src/browser/BrowserService.cpp b/src/browser/BrowserService.cpp index 6079658138..81aa06d6b9 100644 --- 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 index b28c94dbbc..a67b7cb338 100644 --- 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 index b07856862f..50663b7d99 100644 --- 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 index 2d13501eaa..33d15e2605 100644 --- 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 index fafebd7b9c..6a0de5982c 100644 --- 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()) { + url = var.toUrl(); + } + return url; -QUrl UrlTools::convertVariantToUrl(const QVariant& var) const -{ - QUrl url; - if (var.canConvert()) { - 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,27 +117,29 @@ 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; } @@ -149,7 +147,7 @@ bool UrlTools::isUrlIdentical(const QString& first, const QString& second) const 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 index 4b601d3497..9371cbaf94 100644 --- a/src/gui/UrlTools.h +++ b/src/gui/UrlTools.h @@ -26,36 +26,19 @@ #include #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 index d0201562b8..8dacf1625b 100644 --- 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 index ae059d2287..c6fbf979f7 100644 --- 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 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 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 index c2ba770b88..5fbffe6d54 100644 --- 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 m_urlTools; }; #endif // KEEPASSXC_TESTURLTOOLS_H From 402c153ac13438e8753a56a98d0d110e3cb421fa Mon Sep 17 00:00:00 2001 From: Adrien Ollier Date: Mon, 20 Apr 2026 22:32:57 +0200 Subject: [PATCH 2/6] refactor(urltools): Make regular expression static domainHasIllegalCharacters() constructs a QRegularExpression on every call. Since the pattern is constant, make it static const QRegularExpression to avoid repeated regex compilation in hot paths (e.g., domain validation during browsing/passkey operations). --- src/gui/UrlTools.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gui/UrlTools.cpp b/src/gui/UrlTools.cpp index 6a0de5982c..3f35d0cf76 100644 --- a/src/gui/UrlTools.cpp +++ b/src/gui/UrlTools.cpp @@ -211,6 +211,6 @@ bool UrlTools::isUrlValid(const QString& urlField, bool looseComparison) bool UrlTools::domainHasIllegalCharacters(const QString& domain) { - const QRegularExpression re(R"([\s\^#|/:<>\?@\[\]\\])"); + static const QRegularExpression re(R"([\s\^#|/:<>\?@\[\]\\])"); return re.match(domain).hasMatch(); } From 2ed81456434f72e92491e57013a410f0743ac975 Mon Sep 17 00:00:00 2001 From: Adrien Ollier Date: Tue, 21 Apr 2026 09:51:10 +0200 Subject: [PATCH 3/6] refactor(urltools): Remove the unnecessary convertVariantToUrl function format code --- src/gui/UrlTools.cpp | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/src/gui/UrlTools.cpp b/src/gui/UrlTools.cpp index 3f35d0cf76..8a333f8b4a 100644 --- a/src/gui/UrlTools.cpp +++ b/src/gui/UrlTools.cpp @@ -26,23 +26,14 @@ const QString UrlTools::URL_WILDCARD = "1kpxcwc1"; -namespace { - QUrl convertVariantToUrl(const QVariant& var) - { - QUrl url; - if (var.canConvert()) { - url = var.toUrl(); - } - return url; - - } -} - #if defined(KPXC_FEATURE_NETWORK) || defined(KPXC_FEATURE_BROWSER) QUrl UrlTools::getRedirectTarget(QNetworkReply* reply) { + QUrl url; QVariant var = reply->attribute(QNetworkRequest::RedirectionTargetAttribute); - QUrl url = convertVariantToUrl(var); + if (var.canConvert()) { + url = var.toUrl(); + } return url; } @@ -119,14 +110,15 @@ bool UrlTools::isIpAddress(const QString& host) namespace { - QString trimUrl(QString&& url) { + QString trimUrl(QString&& url) + { url = url.trimmed(); if (url.endsWith('/')) { url.chop(1); // Removes the last character } return url; } -} +} // namespace // Returns true if URLs are identical. Paths with "/" are removed during comparison. // URLs without scheme reverts to https. From 7f922f3dc331251cf824798327bf1dfaa4ee78b5 Mon Sep 17 00:00:00 2001 From: Adrien Ollier Date: Tue, 21 Apr 2026 10:40:40 +0200 Subject: [PATCH 4/6] refactor(urltools): Include What You Use --- src/gui/UrlTools.cpp | 4 ++++ src/gui/UrlTools.h | 7 +++---- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/gui/UrlTools.cpp b/src/gui/UrlTools.cpp index 8a333f8b4a..06b500d592 100644 --- a/src/gui/UrlTools.cpp +++ b/src/gui/UrlTools.cpp @@ -20,10 +20,14 @@ #include #include #include +#include +#include #endif #include #include +#include + const QString UrlTools::URL_WILDCARD = "1kpxcwc1"; #if defined(KPXC_FEATURE_NETWORK) || defined(KPXC_FEATURE_BROWSER) diff --git a/src/gui/UrlTools.h b/src/gui/UrlTools.h index 9371cbaf94..bdc7e07ca6 100644 --- a/src/gui/UrlTools.h +++ b/src/gui/UrlTools.h @@ -19,11 +19,10 @@ #define KEEPASSXC_URLTOOLS_H #include "config-keepassx.h" -#include -#include -#include +#include #if defined(KPXC_FEATURE_NETWORK) || defined(KPXC_FEATURE_BROWSER) -#include +#include +class QNetworkReply; #endif namespace UrlTools From 6a34b1aa36ef9d9ae38418e7e6d0e5a0150d5c68 Mon Sep 17 00:00:00 2001 From: Adrien Ollier Date: Tue, 21 Apr 2026 10:47:36 +0200 Subject: [PATCH 5/6] refactor(urltools): Use URL_WILDCARD directly in implementation --- src/gui/UrlTools.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/gui/UrlTools.cpp b/src/gui/UrlTools.cpp index 06b500d592..b12fa2fb1e 100644 --- a/src/gui/UrlTools.cpp +++ b/src/gui/UrlTools.cpp @@ -134,8 +134,8 @@ bool UrlTools::isUrlIdentical(QString first, QString second) } // Replace URL wildcards for comparison if found - const auto&& firstUrl = trimUrl(std::move(first.replace("*", UrlTools::URL_WILDCARD))); - const auto&& secondUrl = trimUrl(std::move(second.replace("*", UrlTools::URL_WILDCARD))); + const auto&& firstUrl = trimUrl(std::move(first.replace("*", URL_WILDCARD))); + const auto&& secondUrl = trimUrl(std::move(second.replace("*", URL_WILDCARD))); if (firstUrl == secondUrl) { return true; } @@ -170,7 +170,7 @@ bool UrlTools::isUrlValid(const QString& urlField, bool looseComparison) return false; } - url.replace("*", UrlTools::URL_WILDCARD); + url.replace("*", URL_WILDCARD); } } @@ -187,9 +187,9 @@ bool UrlTools::isUrlValid(const QString& urlField, bool looseComparison) #if defined(KPXC_FEATURE_NETWORK) || defined(KPXC_FEATURE_BROWSER) // Prevent TLD wildcards - if (looseComparison && url.contains(UrlTools::URL_WILDCARD)) { + if (looseComparison && url.contains(URL_WILDCARD)) { const auto tld = getTopLevelDomainFromUrl(url); - if (tld.contains(UrlTools::URL_WILDCARD) || qUrl.host() == QString("%1.%2").arg(UrlTools::URL_WILDCARD, tld)) { + if (tld.contains(URL_WILDCARD) || qUrl.host() == QString("%1.%2").arg(URL_WILDCARD, tld)) { return false; } } From b089b546c296acc4680f8cf28be6f16c36811179 Mon Sep 17 00:00:00 2001 From: Adrien Ollier Date: Sat, 25 Apr 2026 12:42:27 +0200 Subject: [PATCH 6/6] refactor(urltools): some more inprovements - code format - use pure Qt functions for the string handling - use chop to remove the last character of a QString - make another QRegularExpression static const --- src/gui/UrlTools.cpp | 12 ++++++------ src/gui/UrlTools.h | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/gui/UrlTools.cpp b/src/gui/UrlTools.cpp index b12fa2fb1e..a48c49f01b 100644 --- a/src/gui/UrlTools.cpp +++ b/src/gui/UrlTools.cpp @@ -114,9 +114,9 @@ bool UrlTools::isIpAddress(const QString& host) namespace { - QString trimUrl(QString&& url) + QString trimUrl(QString& url) { - url = url.trimmed(); + url = url.trimmed().replace("*", UrlTools::URL_WILDCARD); if (url.endsWith('/')) { url.chop(1); // Removes the last character } @@ -134,8 +134,8 @@ bool UrlTools::isUrlIdentical(QString first, QString second) } // Replace URL wildcards for comparison if found - const auto&& firstUrl = trimUrl(std::move(first.replace("*", URL_WILDCARD))); - const auto&& secondUrl = trimUrl(std::move(second.replace("*", URL_WILDCARD))); + const auto firstUrl = trimUrl(first); + const auto secondUrl = trimUrl(second); if (firstUrl == secondUrl) { return true; } @@ -163,7 +163,7 @@ bool UrlTools::isUrlValid(const QString& urlField, bool looseComparison) // Get the URL inside "" url.remove(0, 1); - url.remove(url.length() - 1, 1); + url.chop(1); } else { // Do not allow URL with just wildcards, or double wildcards if (url.length() == url.count("*") || url.contains("**") || url.contains("*.*")) { @@ -196,7 +196,7 @@ bool UrlTools::isUrlValid(const QString& urlField, bool looseComparison) #endif // Check for illegal characters. Adds also the wildcard * to the list - QRegularExpression re("[<>\\^`{|}\\*]"); + static const QRegularExpression re("[<>\\^`{|}\\*]"); auto match = re.match(url); if (match.hasMatch()) { return false; diff --git a/src/gui/UrlTools.h b/src/gui/UrlTools.h index bdc7e07ca6..60db7cb656 100644 --- a/src/gui/UrlTools.h +++ b/src/gui/UrlTools.h @@ -38,6 +38,6 @@ namespace UrlTools bool domainHasIllegalCharacters(const QString& domain); extern const QString URL_WILDCARD; -} +} // namespace UrlTools #endif // KEEPASSXC_URLTOOLS_H