Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a URL that preserves the URL path when trying to resolve favicons. #8565

Merged
merged 3 commits into from
Oct 20, 2022
Merged

Add a URL that preserves the URL path when trying to resolve favicons. #8565

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
378d928
Add a URL that preserves the URL path when trying to resolve favicons.
libklein Oct 10, 2022
002330a
Merge branch 'develop' into hotfix/8564-fix-favicon-download
libklein Oct 12, 2022
08cffcf
Merge branch 'develop' into hotfix/8564-fix-favicon-download
libklein Oct 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/gui/IconDownloader.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,13 @@ void IconDownloader::setUrl(const QString& entryUrl)
}
}

// Add a pull that preserves the query if there is one.
if (!url.path().isEmpty()) {
// Appends /favicon.ico to the last segment of the path.
// stem/something/ will become stem/something/favicon.ico, and stem/something will become stem/favicon.ico
m_urlsToTry.append(url.resolved(QUrl("./favicon.ico")));
}

// Add a direct pull of the website's own favicon.ico file
QUrl favicon_url = url;
favicon_url.setPath("/favicon.ico");
Expand Down
18 changes: 13 additions & 5 deletions tests/TestIconDownloader.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,11 @@ void TestIconDownloader::testIconDownloader_data()
QTest::newRow("Unsupported schema") << "ftp://google.com" << QStringList{};
QTest::newRow("Missing schema") << "keepassxc.org" << QStringList{"https://keepassxc.org/favicon.ico"};
QTest::newRow("Missing host") << "https:///register" << QStringList{};
QTest::newRow("URL with path") << "https://keepassxc.org/register/here" << QStringList{keepassxc_favicon};
QTest::newRow("URL with path") << "https://keepassxc.org/register/here/"
<< QStringList{"https://keepassxc.org/register/here/favicon.ico", keepassxc_favicon};
QTest::newRow("URL with path and query")
<< "https://keepassxc.org/register/here?login=me" << QStringList{keepassxc_favicon};
<< "https://keepassxc.org/register/here?login=me"
<< QStringList{"https://keepassxc.org/register/favicon.ico", keepassxc_favicon};
QTest::newRow("URL with port") << "https://keepassxc.org:8080"
<< QStringList{"https://keepassxc.org:8080/favicon.ico"};
QTest::newRow("2nd level domain") << "https://login.keepassxc.org"
Expand All @@ -54,11 +56,14 @@ void TestIconDownloader::testIconDownloader_data()
<< QStringList{"https://134.130.155.184/favicon.ico"};
QTest::newRow("Raw IP") << "134.130.155.184" << QStringList{"https://134.130.155.184/favicon.ico"};
QTest::newRow("Raw IP with schema and path")
<< "https://134.130.155.184/with/path" << QStringList{"https://134.130.155.184/favicon.ico"};
<< "https://134.130.155.184/with/path/"
<< QStringList{"https://134.130.155.184/with/path/favicon.ico", "https://134.130.155.184/favicon.ico"};
QTest::newRow("Raw IP with schema (https), path, and port")
<< "https://134.130.155.184:8080/test" << QStringList{"https://134.130.155.184:8080/favicon.ico"};
<< "https://134.130.155.184:8080/test/"
<< QStringList{"https://134.130.155.184:8080/test/favicon.ico", "https://134.130.155.184:8080/favicon.ico"};
QTest::newRow("Raw IP with schema (http), path, and port")
<< "134.130.155.184:8080/test" << QStringList{"https://134.130.155.184:8080/favicon.ico"};
<< "134.130.155.184:8080/test/"
<< QStringList{"https://134.130.155.184:8080/test/favicon.ico", "https://134.130.155.184:8080/favicon.ico"};
QTest::newRow("URL with username and password")
<< "https://user:password@keepassxc.org" << QStringList{"https://user:password@keepassxc.org/favicon.ico"};
QTest::newRow("URL with username and password, several subdomains")
Expand All @@ -68,4 +73,7 @@ void TestIconDownloader::testIconDownloader_data()
"https://keepassxc.org/favicon.ico"};
QTest::newRow("Raw IP with username and password")
<< "https://user:password@134.130.155.184" << QStringList{"https://user:password@134.130.155.184/favicon.ico"};
QTest::newRow("Relative path should be preserved")
<< "https://test.com/rel-path/"
<< QStringList{"https://test.com/rel-path/favicon.ico", "https://test.com/favicon.ico"};
}