Skip to content

Commit

Permalink
Fix URL matching
Browse files Browse the repository at this point in the history
  • Loading branch information
varjolintu committed Nov 8, 2019
1 parent 329701a commit ac79177
Show file tree
Hide file tree
Showing 4 changed files with 168 additions and 43 deletions.
79 changes: 60 additions & 19 deletions src/browser/BrowserService.cpp
View file
Expand Up @@ -18,6 +18,7 @@
*/

#include <QCheckBox>
#include <QHostAddress>
#include <QInputDialog>
#include <QJsonArray>
#include <QMessageBox>
Expand Down Expand Up @@ -600,17 +601,20 @@ BrowserService::searchEntries(const QSharedPointer<Database>& db, const QString&
continue;
}

auto domain = baseDomain(hostname);

// 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)) {
if (key.startsWith(ADDITIONAL_URL)
&& handleURL(entry->attributes()->value(key), domain, url)) {
entries.append(entry);
continue;
}
}
}

if (!handleURL(entry->url(), hostname, url)) {
if (!handleURL(entry->url(), domain, url)) {
continue;
}

Expand Down Expand Up @@ -928,12 +932,15 @@ int BrowserService::sortPriority(const Entry* entry,
{
QUrl url(entry->url());
if (url.scheme().isEmpty()) {
url.setScheme("http");
url.setScheme("https");
}
const QString entryURL = url.toString(QUrl::StripTrailingSlash);
const QString baseEntryURL =
url.toString(QUrl::StripTrailingSlash | QUrl::RemovePath | QUrl::RemoveQuery | QUrl::RemoveFragment);

if (!url.host().contains(".") && url.host() != "localhost") {
return 0;
}
if (submitUrl == entryURL) {
return 100;
}
Expand Down Expand Up @@ -970,7 +977,7 @@ int BrowserService::sortPriority(const Entry* entry,
return 0;
}

bool BrowserService::matchUrlScheme(const QString& url)
bool BrowserService::schemeFound(const QString& url)
{
QUrl address(url);
return !address.scheme().isEmpty();
Expand All @@ -995,21 +1002,49 @@ bool BrowserService::removeFirstDomain(QString& hostname)

bool BrowserService::handleURL(const QString& entryUrl, const QString& hostname, const QString& url)
{
QUrl entryQUrl(entryUrl);
QString entryScheme = entryQUrl.scheme();
if (entryUrl.isEmpty()) {
return false;
}

QUrl entryQUrl;
if (entryUrl.contains("://")) {
entryQUrl = entryUrl;
} else {
entryQUrl = QUrl::fromUserInput(entryUrl);

if (browserSettings()->matchUrlScheme()) {
entryQUrl.setScheme("https");
}
}

// URL host validation fails
if (browserSettings()->matchUrlScheme() && entryQUrl.host().isEmpty()) {
return false;
}

// Match port, if used
QUrl qUrl(url);
if (entryQUrl.port() > 0 && entryQUrl.port() != qUrl.port()) {
return false;
}

// 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)) {
// Match scheme
if (browserSettings()->matchUrlScheme() && !entryQUrl.scheme().isEmpty() && entryQUrl.scheme().compare(qUrl.scheme()) != 0) {
return false;
}

// Check for illegal characters
QRegularExpression re("[<>\\^`{|}]");
auto match = re.match(entryUrl);
if (match.hasMatch()) {
return false;
}

// Filter to match hostname in URL field
if ((!entryUrl.isEmpty() && hostname.contains(entryUrl))
|| (matchUrlScheme(entryUrl) && hostname.endsWith(entryQUrl.host()))) {
if (entryQUrl.host().endsWith(hostname)) {
return true;
}

return false;
};

Expand All @@ -1018,19 +1053,25 @@ bool BrowserService::handleURL(const QString& entryUrl, const QString& hostname,
*
* Returns the base domain, e.g. https://another.example.co.uk -> example.co.uk
*/
QString BrowserService::baseDomain(const QString& url) const
QString BrowserService::baseDomain(const QString& hostname) const
{
QUrl qurl = QUrl::fromUserInput(url);
QString hostname = qurl.host();
QUrl qurl = QUrl::fromUserInput(hostname);
QString host = qurl.host();

// If the hostname is an IP address, return it directly
QHostAddress hostAddress(hostname);
if (!hostAddress.isNull()) {
return hostname;
}

if (hostname.isEmpty() || !hostname.contains(qurl.topLevelDomain())) {
if (host.isEmpty() || !host.contains(qurl.topLevelDomain())) {
return {};
}

// Remove the top level domain part from the hostname, e.g. https://another.example.co.uk -> https://another.example
hostname.chop(qurl.topLevelDomain().length());
host.chop(qurl.topLevelDomain().length());
// Split the URL and select the last part, e.g. https://another.example -> example
QString baseDomain = hostname.split('.').last();
QString baseDomain = host.split('.').last();
// Append the top level domain back to the URL, e.g. example -> example.co.uk
baseDomain.append(qurl.topLevelDomain());
return baseDomain;
Expand Down Expand Up @@ -1070,7 +1111,7 @@ QSharedPointer<Database> BrowserService::selectedDatabase()
if (res == QDialog::Accepted) {
const auto selectedDatabase = browserEntrySaveDialog.getSelected();
if (selectedDatabase.length() > 0) {
int index = selectedDatabase[0]->data(Qt::UserRole).toUInt();
int index = selectedDatabase[0]->data(Qt::UserRole).toInt();
return databaseWidgets[index]->database();
}
} else {
Expand Down Expand Up @@ -1188,7 +1229,7 @@ void BrowserService::raiseWindow(const bool force)
m_prevWindowState = WindowState::Minimized;
}
#ifdef Q_OS_MACOS
Q_UNUSED(force);
Q_UNUSED(force)

if (macUtils()->isHidden()) {
m_prevWindowState = WindowState::Hidden;
Expand Down
4 changes: 2 additions & 2 deletions src/browser/BrowserService.h
View file
Expand Up @@ -128,10 +128,10 @@ public slots:
Group* getDefaultEntryGroup(const QSharedPointer<Database>& selectedDb = {});
int
sortPriority(const Entry* entry, const QString& host, const QString& submitUrl, const QString& baseSubmitUrl) const;
bool matchUrlScheme(const QString& url);
bool schemeFound(const QString& url);
bool removeFirstDomain(QString& hostname);
bool handleURL(const QString& entryUrl, const QString& hostname, const QString& url);
QString baseDomain(const QString& url) const;
QString baseDomain(const QString& hostname) const;
QSharedPointer<Database> getDatabase();
QSharedPointer<Database> selectedDatabase();
QJsonArray getChildrenFromGroup(Group* group);
Expand Down
126 changes: 104 additions & 22 deletions tests/TestBrowser.cpp
View file
Expand Up @@ -147,7 +147,7 @@ void TestBrowser::testSortPriority()
entry6->setUrl("http://github.com/login");
entry7->setUrl("github.com");
entry8->setUrl("github.com/login");
entry9->setUrl("https://github");
entry9->setUrl("https://github"); // Invalid URL
entry10->setUrl("github.com");

// The extension uses the submitUrl as default for comparison
Expand All @@ -170,7 +170,7 @@ void TestBrowser::testSortPriority()
QCOMPARE(res6, 0);
QCOMPARE(res7, 0);
QCOMPARE(res8, 0);
QCOMPARE(res9, 90);
QCOMPARE(res9, 0);
QCOMPARE(res10, 0);
}

Expand All @@ -188,7 +188,7 @@ void TestBrowser::testSearchEntries()
urls.push_back("http://github.com/login");
urls.push_back("github.com");
urls.push_back("github.com/login");
urls.push_back("https://github");
urls.push_back("https://github"); // Invalid URL
urls.push_back("github.com");

for (int i = 0; i < urls.length(); ++i) {
Expand All @@ -202,24 +202,23 @@ void TestBrowser::testSearchEntries()

browserSettings()->setMatchUrlScheme(false);
auto result = m_browserService->searchEntries(db, "github.com", "https://github.com"); // db, hostname, url
QCOMPARE(result.length(), 7);

QCOMPARE(result.length(), 9);
QCOMPARE(result[0]->url(), QString("https://github.com/login_page"));
QCOMPARE(result[1]->url(), QString("https://github.com/login"));
QCOMPARE(result[2]->url(), QString("https://github.com/"));
QCOMPARE(result[3]->url(), QString("http://github.com"));
QCOMPARE(result[4]->url(), QString("http://github.com/login"));
QCOMPARE(result[5]->url(), QString("github.com"));
QCOMPARE(result[6]->url(), QString("github.com"));
QCOMPARE(result[3]->url(), QString("github.com/login"));
QCOMPARE(result[4]->url(), QString("http://github.com"));
QCOMPARE(result[5]->url(), QString("http://github.com/login"));

// With matching there should be only 5 results
// With matching there should be only 3 results + 4 without a scheme
browserSettings()->setMatchUrlScheme(true);
result = m_browserService->searchEntries(db, "github.com", "https://github.com"); // db, hostname, url
QCOMPARE(result.length(), 5);
QCOMPARE(result.length(), 7);
QCOMPARE(result[0]->url(), QString("https://github.com/login_page"));
QCOMPARE(result[1]->url(), QString("https://github.com/login"));
QCOMPARE(result[2]->url(), QString("https://github.com/"));
QCOMPARE(result[3]->url(), QString("github.com"));
QCOMPARE(result[4]->url(), QString("github.com"));
QCOMPARE(result[3]->url(), QString("github.com/login"));
}

void TestBrowser::testSearchEntriesWithPort()
Expand All @@ -242,7 +241,7 @@ void TestBrowser::testSearchEntriesWithPort()

auto result = m_browserService->searchEntries(db, "127.0.0.1", "http://127.0.0.1:443"); // db, hostname, url
QCOMPARE(result.length(), 1);
QCOMPARE(result[0]->url(), QString("http://127.0.0.1:443"));
QCOMPARE(result[0]->url(), urls[0]);
}

void TestBrowser::testSearchEntriesWithAdditionalURLs()
Expand Down Expand Up @@ -271,12 +270,94 @@ void TestBrowser::testSearchEntriesWithAdditionalURLs()

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/"));
QCOMPARE(result[0]->url(), urls[0]);

// 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/"));
QCOMPARE(additionalResult[0]->url(), urls[0]);
}

void TestBrowser::testInvalidEntries()
{
auto db = QSharedPointer<Database>::create();
auto* root = db->rootGroup();

QList<QString> urls;
urls.push_back("https://github.com/login");
urls.push_back("https:///github.com/"); // Extra '/'
urls.push_back("http://github.com/**//*");
urls.push_back("http://*.github.com/login");
urls.push_back("//github.com"); // fromUserInput() corrects this one.
urls.push_back("github.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();
}

browserSettings()->setMatchUrlScheme(true);
auto result = m_browserService->searchEntries(db, "github.com", "https://github.com"); // db, hostname, url
QCOMPARE(result.length(), 2);
QCOMPARE(result[0]->url(), QString("https://github.com/login"));
QCOMPARE(result[1]->url(), QString("//github.com"));

// Test the URL's directly
QCOMPARE(m_browserService->handleURL(urls[0], "github.com", "https://github.com"), true);
QCOMPARE(m_browserService->handleURL(urls[1], "github.com", "https://github.com"), false);
QCOMPARE(m_browserService->handleURL(urls[2], "github.com", "https://github.com"), false);
QCOMPARE(m_browserService->handleURL(urls[3], "github.com", "https://github.com"), false);
QCOMPARE(m_browserService->handleURL(urls[4], "github.com", "https://github.com"), true);
QCOMPARE(m_browserService->handleURL(urls[5], "github.com", "https://github.com"), false);
}

void TestBrowser::testSubdomainsAndPaths()
{
auto db = QSharedPointer<Database>::create();
auto* root = db->rootGroup();

QList<QString> urls;
urls.push_back("https://www.github.com/login/page.xml");
urls.push_back("https://login.github.com/");
urls.push_back("https://github.com");
urls.push_back("http://www.github.com");
urls.push_back("http://login.github.com/pathtonowhere");
urls.push_back(".github.com"); // Invalid URL
urls.push_back("www.github.com/");
urls.push_back("https://github"); // Invalid URL

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();
}

browserSettings()->setMatchUrlScheme(false);
auto result = m_browserService->searchEntries(db, "github.com", "https://github.com"); // db, hostname, url

QCOMPARE(result.length(), 6);
QCOMPARE(result[0]->url(), urls[0]);
QCOMPARE(result[1]->url(), urls[1]);
QCOMPARE(result[2]->url(), urls[2]);
QCOMPARE(result[3]->url(), urls[3]);
QCOMPARE(result[4]->url(), urls[4]);
QCOMPARE(result[5]->url(), urls[6]);

// With matching there should be only 3 results
browserSettings()->setMatchUrlScheme(true);
result = m_browserService->searchEntries(db, "github.com", "https://github.com"); // db, hostname, url
QCOMPARE(result.length(), 4);
QCOMPARE(result[0]->url(), urls[0]);
QCOMPARE(result[1]->url(), urls[1]);
QCOMPARE(result[2]->url(), urls[2]);
QCOMPARE(result[3]->url(), urls[6]);
}

void TestBrowser::testSortEntries()
Expand All @@ -293,7 +374,7 @@ void TestBrowser::testSortEntries()
urls.push_back("http://github.com/login");
urls.push_back("github.com");
urls.push_back("github.com/login");
urls.push_back("https://github");
urls.push_back("https://github"); // Invalid URL
urls.push_back("github.com");

QList<Entry*> entries;
Expand All @@ -313,12 +394,13 @@ void TestBrowser::testSortEntries()
QCOMPARE(result.size(), 10);
QCOMPARE(result[0]->username(), QString("User 2"));
QCOMPARE(result[0]->url(), QString("https://github.com/"));
QCOMPARE(result[1]->username(), QString("User 8"));
QCOMPARE(result[1]->url(), QString("https://github"));
QCOMPARE(result[2]->username(), QString("User 0"));
QCOMPARE(result[2]->url(), QString("https://github.com/login_page"));
QCOMPARE(result[3]->username(), QString("User 1"));
QCOMPARE(result[3]->url(), QString("https://github.com/login"));
QCOMPARE(result[1]->username(), QString("User 0"));
QCOMPARE(result[1]->url(), QString("https://github.com/login_page"));
QCOMPARE(result[2]->username(), QString("User 1"));
QCOMPARE(result[2]->url(), QString("https://github.com/login"));
QCOMPARE(result[3]->username(), QString("User 3"));
QCOMPARE(result[3]->url(), QString("github.com/login"));

}

void TestBrowser::testGetDatabaseGroups()
Expand Down
2 changes: 2 additions & 0 deletions tests/TestBrowser.h
View file
Expand Up @@ -43,6 +43,8 @@ private slots:
void testSearchEntries();
void testSearchEntriesWithPort();
void testSearchEntriesWithAdditionalURLs();
void testInvalidEntries();
void testSubdomainsAndPaths();
void testSortEntries();
void testGetDatabaseGroups();

Expand Down

0 comments on commit ac79177

Please sign in to comment.