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..a48c49f01b 100644 --- a/src/gui/UrlTools.cpp +++ b/src/gui/UrlTools.cpp @@ -20,43 +20,34 @@ #include #include #include +#include +#include #endif #include #include -const QString UrlTools::URL_WILDCARD = "1kpxcwc1"; - -Q_GLOBAL_STATIC(UrlTools, s_urlTools) +#include -UrlTools* UrlTools::instance() -{ - return s_urlTools; -} +const QString UrlTools::URL_WILDCARD = "1kpxcwc1"; -QUrl UrlTools::convertVariantToUrl(const QVariant& var) const +#if defined(KPXC_FEATURE_NETWORK) || defined(KPXC_FEATURE_BROWSER) +QUrl UrlTools::getRedirectTarget(QNetworkReply* reply) { QUrl url; + QVariant var = reply->attribute(QNetworkRequest::RedirectionTargetAttribute); if (var.canConvert()) { url = var.toUrl(); } return url; } -#if defined(KPXC_FEATURE_NETWORK) || defined(KPXC_FEATURE_BROWSER) -QUrl UrlTools::getRedirectTarget(QNetworkReply* reply) const -{ - QVariant var = reply->attribute(QNetworkRequest::RedirectionTargetAttribute); - QUrl url = convertVariantToUrl(var); - return url; -} - /** * Gets the base domain of URL or hostname. * * 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 +76,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 +103,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 +112,30 @@ 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) { - url = url.trimmed(); - if (url.endsWith("/")) { - url.remove(url.length() - 1, 1); + QString trimUrl(QString& url) + { + url = url.trimmed().replace("*", UrlTools::URL_WILDCARD); + 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. +// 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(first); + const auto secondUrl = trimUrl(second); if (firstUrl == secondUrl) { return true; } @@ -149,7 +143,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)) { @@ -169,14 +163,14 @@ bool UrlTools::isUrlValid(const QString& urlField, bool looseComparison) const // 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("*.*")) { return false; } - url.replace("*", UrlTools::URL_WILDCARD); + url.replace("*", URL_WILDCARD); } } @@ -193,16 +187,16 @@ bool UrlTools::isUrlValid(const QString& urlField, bool looseComparison) const #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; } } #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; @@ -211,8 +205,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\^#|/:<>\?@\[\]\\])"); + static const QRegularExpression re(R"([\s\^#|/:<>\?@\[\]\\])"); return re.match(domain).hasMatch(); } diff --git a/src/gui/UrlTools.h b/src/gui/UrlTools.h index 4b601d3497..60db7cb656 100644 --- a/src/gui/UrlTools.h +++ b/src/gui/UrlTools.h @@ -19,43 +19,25 @@ #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 -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; + bool isUrlIdentical(QString first, QString second); + bool isUrlValid(const QString& urlField, bool looseComparison = false); + bool domainHasIllegalCharacters(const QString& domain); - static const QString URL_WILDCARD; - -private: - QUrl convertVariantToUrl(const QVariant& var) const; - -private: - Q_DISABLE_COPY(UrlTools); -}; - -static inline UrlTools* urlTools() -{ - return UrlTools::instance(); -} + extern const QString URL_WILDCARD; +} // namespace UrlTools #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