From 490e92167db2b23e79d802e7b59cfe18ae3f8759 Mon Sep 17 00:00:00 2001 From: Jonathan White Date: Wed, 7 Feb 2018 07:10:56 -0500 Subject: [PATCH] Replace qhttp client with curl for favicon downloading (#1460) Replace qhttp client with curl for favicon downloading --- .travis.yml | 47 ---------- Dockerfile | 5 +- ci/trusty/Dockerfile | 1 + snapcraft.yaml | 1 + src/CMakeLists.txt | 20 ++--- src/gui/EditWidgetIcons.cpp | 173 +++++++++++++----------------------- src/gui/EditWidgetIcons.h | 18 +--- src/gui/MainWindow.cpp | 10 +-- src/http/CMakeLists.txt | 6 +- 9 files changed, 87 insertions(+), 194 deletions(-) delete mode 100644 .travis.yml diff --git a/.travis.yml b/.travis.yml deleted file mode 100644 index 7183e19f..00000000 --- a/.travis.yml +++ /dev/null @@ -1,47 +0,0 @@ -language: cpp -sudo: required -dist: trusty -# FIXME : remove when (https://github.com/google/sanitizers/issues/837) is resolved. -group: deprecated-2017Q3 -services: [docker] - -os: - - linux -# - osx - -# Define clang compiler without any frills -compiler: - - clang - - gcc - -env: - - CONFIG=Release ASAN_OPTIONS=detect_odr_violation=1 - - CONFIG=Debug ASAN_OPTIONS=detect_odr_violation=1 - -git: - depth: 3 - -before_install: - - if [ "$TRAVIS_OS_NAME" = "linux" ]; then sudo apt-get -qq update; fi - - if [ "$TRAVIS_OS_NAME" = "linux" ]; then sudo apt-get -qq install cmake3 libclang-common-3.5-dev libxi-dev qtbase5-dev libqt5x11extras5-dev qttools5-dev qttools5-dev-tools libgcrypt20-dev zlib1g-dev libxtst-dev xvfb libyubikey-dev libykpers-1-dev; fi - - if [ "$TRAVIS_OS_NAME" = "osx" ]; then brew update; fi - - if [ "$TRAVIS_OS_NAME" = "osx" ]; then brew ls | grep -wq cmake || brew install cmake; fi - - if [ "$TRAVIS_OS_NAME" = "osx" ]; then brew ls | grep -wq qt5 || brew install qt5; fi - - if [ "$TRAVIS_OS_NAME" = "osx" ]; then brew ls | grep -wq libgcrypt || brew install libgcrypt; fi - -before_script: - - if [ "$TRAVIS_OS_NAME" = "osx" ]; then CMAKE_ARGS="-DCMAKE_PREFIX_PATH=/usr/local/opt/qt5"; fi - - mkdir build && pushd build - -script: - - cmake -DCMAKE_BUILD_TYPE=${CONFIG} -DWITH_GUI_TESTS=ON -DWITH_ASAN=ON -DWITH_XC_HTTP=ON -DWITH_XC_AUTOTYPE=ON -DWITH_XC_YUBIKEY=ON -DWITH_XC_SSHAGENT=ON $CMAKE_ARGS .. - - make -j2 - - if [ "$TRAVIS_OS_NAME" = "linux" ]; then make test ARGS+="-E testgui --output-on-failure"; fi - - if [ "$TRAVIS_OS_NAME" = "linux" ]; then ASAN_OPTIONS=${ASAN_OPTIONS}:leak_check_at_exit=0 xvfb-run -a --server-args="-screen 0 800x600x24" make test ARGS+="-R testgui --output-on-failure"; fi - - if [ "$TRAVIS_OS_NAME" = "osx" ]; then make test ARGS+="--output-on-failure"; fi - -# Generate snapcraft build when merging into master/develop branches -#after_success: -# - popd -# - "[[ $DEPLOY = 1 ]] && [[ $CONFIG = Release ]] && [[ $TRAVIS_BRANCH =~ (master|develop) ]] && [[ $TRAVIS_PULL_REQUEST = false ]] \ -# && docker run -v $(pwd):/cwd snapcore/snapcraft sh -c 'cd /cwd && apt update && snapcraft'" diff --git a/Dockerfile b/Dockerfile index 91eb6f39..0a5c464b 100644 --- a/Dockerfile +++ b/Dockerfile @@ -16,7 +16,7 @@ FROM ubuntu:14.04 -ENV REBUILD_COUNTER=4 +ENV REBUILD_COUNTER=5 ENV QT5_VERSION=59 ENV QT5_PPA_VERSION=${QT5_VERSION}2 @@ -51,7 +51,8 @@ RUN set -x \ libxtst-dev \ mesa-common-dev \ libyubikey-dev \ - libykpers-1-dev + libykpers-1-dev \ + libcurl4-openssl-dev ENV CMAKE_PREFIX_PATH="/opt/qt${QT5_VERSION}/lib/cmake" ENV CMAKE_INCLUDE_PATH="/opt/libgcrypt20-18/include:/opt/gpg-error-127/include" diff --git a/ci/trusty/Dockerfile b/ci/trusty/Dockerfile index cdaba3a0..5ef9cac2 100644 --- a/ci/trusty/Dockerfile +++ b/ci/trusty/Dockerfile @@ -43,6 +43,7 @@ RUN set -x \ libgcrypt20-18-dev \ libargon2-0-dev \ libsodium-dev \ + libcurl4-openssl-dev \ qt${QT5_VERSION}base \ qt${QT5_VERSION}tools \ qt${QT5_VERSION}x11extras \ diff --git a/snapcraft.yaml b/snapcraft.yaml index d614c354..81d4ae3b 100644 --- a/snapcraft.yaml +++ b/snapcraft.yaml @@ -42,6 +42,7 @@ parts: - libxtst-dev - libyubikey-dev - libykpers-1-dev + - libcurl4-openssl-dev - libsodium-dev stage-packages: - dbus diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 4b7c07cd..f01780ba 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -199,13 +199,9 @@ add_feature_info(KeePassHTTP WITH_XC_HTTP "Browser integration compatible with C add_feature_info(SSHAgent WITH_XC_SSHAGENT "SSH agent integration compatible with KeeAgent") add_feature_info(YubiKey WITH_XC_YUBIKEY "YubiKey HMAC-SHA1 challenge-response") -if(WITH_XC_HTTP) - add_subdirectory(http) - set(keepasshttp_LIB keepasshttp) -endif() +add_subdirectory(http) if(WITH_XC_NETWORKING) - add_subdirectory(http/qhttp) - set(keepassxcnetwork_LIB qhttp Qt5::Network) + find_package(CURL REQUIRED) endif() set(BROWSER_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR}/browser) @@ -251,23 +247,21 @@ endif() add_library(autotype STATIC ${autotype_SOURCES}) target_link_libraries(autotype Qt5::Core Qt5::Widgets) -set(autotype_LIB autotype) - add_library(keepassx_core STATIC ${keepassx_SOURCES}) set_target_properties(keepassx_core PROPERTIES COMPILE_DEFINITIONS KEEPASSX_BUILDING_CORE) target_link_libraries(keepassx_core + autotype + ${keepassxchttp_LIB} ${keepassxcbrowser_LIB} - ${keepasshttp_LIB} - ${keepassxcnetwork_LIB} - ${autotype_LIB} ${sshagent_LIB} - ${YUBIKEY_LIBRARIES} - ${ZXCVBN_LIBRARIES} Qt5::Core Qt5::Network Qt5::Concurrent Qt5::Widgets + ${CURL_LIBRARIES} + ${YUBIKEY_LIBRARIES} + ${ZXCVBN_LIBRARIES} ${ARGON2_LIBRARIES} ${GCRYPT_LIBRARIES} ${GPGERROR_LIBRARIES} diff --git a/src/gui/EditWidgetIcons.cpp b/src/gui/EditWidgetIcons.cpp index 6f5e7ee1..9a73c293 100644 --- a/src/gui/EditWidgetIcons.cpp +++ b/src/gui/EditWidgetIcons.cpp @@ -31,10 +31,9 @@ #include "gui/MessageBox.h" #ifdef WITH_XC_NETWORKING -#include "http/qhttp/qhttpclient.hpp" -#include "http/qhttp/qhttpclientresponse.hpp" - -using namespace qhttp::client; +#include +#include "core/AsyncTask.h" +#undef MessageBox #endif IconStruct::IconStruct() @@ -49,10 +48,6 @@ EditWidgetIcons::EditWidgetIcons(QWidget* parent) , m_database(nullptr) , m_defaultIconModel(new DefaultIconModel(this)) , m_customIconModel(new CustomIconModel(this)) -#ifdef WITH_XC_NETWORKING - , m_fallbackToGoogle(true) - , m_redirectCount(0) -#endif { m_ui->setupUi(this); @@ -88,17 +83,14 @@ IconStruct EditWidgetIcons::state() QModelIndex index = m_ui->defaultIconsView->currentIndex(); if (index.isValid()) { iconStruct.number = index.row(); - } - else { + } else { Q_ASSERT(false); } - } - else { + } else { QModelIndex index = m_ui->customIconsView->currentIndex(); if (index.isValid()) { iconStruct.uuid = m_customIconModel->uuidFromIndex(m_ui->customIconsView->currentIndex()); - } - else { + } else { iconStruct.number = -1; } } @@ -129,14 +121,12 @@ void EditWidgetIcons::load(const Uuid& currentUuid, Database* database, const Ic int iconNumber = iconStruct.number; m_ui->defaultIconsView->setCurrentIndex(m_defaultIconModel->index(iconNumber, 0)); m_ui->defaultIconsRadio->setChecked(true); - } - else { + } else { QModelIndex index = m_customIconModel->indexFromUuid(iconUuid); if (index.isValid()) { m_ui->customIconsView->setCurrentIndex(index); m_ui->customIconsRadio->setChecked(true); - } - else { + } else { m_ui->defaultIconsView->setCurrentIndex(m_defaultIconModel->index(0, 0)); m_ui->defaultIconsRadio->setChecked(true); } @@ -148,7 +138,6 @@ void EditWidgetIcons::setUrl(const QString& url) #ifdef WITH_XC_NETWORKING m_url = url; m_ui->faviconButton->setVisible(!url.isEmpty()); - resetFaviconDownload(); #else Q_UNUSED(url); m_ui->faviconButton->setVisible(false); @@ -158,107 +147,75 @@ void EditWidgetIcons::setUrl(const QString& url) void EditWidgetIcons::downloadFavicon() { #ifdef WITH_XC_NETWORKING + m_ui->faviconButton->setDisabled(true); + QUrl url = QUrl(m_url); url.setPath("/favicon.ico"); - fetchFavicon(url); + // Attempt to simply load the favicon.ico file + QImage image = fetchFavicon(url); + if (!image.isNull()) { + addCustomIcon(image); + } else if (config()->get("security/IconDownloadFallbackToGoogle", false).toBool()) { + QUrl faviconUrl = QUrl("https://www.google.com/s2/favicons"); + faviconUrl.setQuery("domain=" + QUrl::toPercentEncoding(url.host())); + // Attempt to load favicon from Google + image = fetchFavicon(faviconUrl); + if (!image.isNull()) { + addCustomIcon(image); + } else { + emit messageEditEntry(tr("Unable to fetch favicon."), MessageWidget::Error); + } + } else { + emit messageEditEntry(tr("Unable to fetch favicon.") + "\n" + + tr("Hint: You can enable Google as a fallback under Tools>Settings>Security"), + MessageWidget::Error); + } + + m_ui->faviconButton->setDisabled(false); #endif } #ifdef WITH_XC_NETWORKING -void EditWidgetIcons::fetchFavicon(const QUrl& url) +namespace { +std::size_t writeCurlResponse(char* ptr, std::size_t size, std::size_t nmemb, void* data) { - if (nullptr == m_httpClient) { - m_httpClient = new QHttpClient(this); - } + QByteArray* response = static_cast(data); + std::size_t realsize = size * nmemb; + response->append(ptr, realsize); + return realsize; +} +} - bool requestMade = m_httpClient->request(qhttp::EHTTP_GET, url, [this, url](QHttpResponse* response) { - if (m_database == nullptr) { - return; - } +QImage EditWidgetIcons::fetchFavicon(const QUrl& url) +{ + QImage image; + CURL* curl = curl_easy_init(); + if (curl) { + QByteArray imagedata; + QByteArray baUrl = url.url().toLatin1(); - response->collectData(); - response->onEnd([this, response, &url]() { - int status = response->status(); - if (200 == status) { - QImage image; - image.loadFromData(response->collectedData()); + curl_easy_setopt(curl, CURLOPT_URL, baUrl.data()); + curl_easy_setopt(curl, CURLOPT_MAXREDIRS, 5L); + curl_easy_setopt(curl, CURLOPT_REDIR_PROTOCOLS, CURLPROTO_HTTP | CURLPROTO_HTTPS); + curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, 1L); + curl_easy_setopt(curl, CURLOPT_CONNECTTIMEOUT, 5L); + curl_easy_setopt(curl, CURLOPT_FAILONERROR, 1L); + curl_easy_setopt(curl, CURLOPT_WRITEDATA, &imagedata); + curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, &writeCurlResponse); - if (!image.isNull()) { - addCustomIcon(image); - resetFaviconDownload(); - } else { - fetchFaviconFromGoogle(url.host()); - } - } else if (301 == status || 302 == status) { - // Check if server has sent a redirect - QUrl possibleRedirectUrl(response->headers().value("location", "")); - if (!possibleRedirectUrl.isEmpty() && possibleRedirectUrl != m_redirectUrl && m_redirectCount < 3) { - resetFaviconDownload(false); - m_redirectUrl = possibleRedirectUrl; - ++m_redirectCount; - fetchFavicon(m_redirectUrl); - } else { - // website is trying to redirect to itself or - // maximum number of redirects has been reached, fall back to Google - fetchFaviconFromGoogle(url.host()); - } - } else { - fetchFaviconFromGoogle(url.host()); - } + // Perform the request in another thread + CURLcode result = AsyncTask::runAndWaitForFuture([curl]() { + return curl_easy_perform(curl); }); - }); - if (!requestMade) { - resetFaviconDownload(); - return; - } - - m_httpClient->setConnectingTimeOut(5000, [this]() { - QUrl tempurl = QUrl(m_url); - if (tempurl.scheme() == "http") { - resetFaviconDownload(); - emit messageEditEntry(tr("Unable to fetch favicon.") + "\n" + - tr("Hint: You can enable Google as a fallback under Tools>Settings>Security"), - MessageWidget::Error); - } else { - tempurl.setScheme("http"); - m_url = tempurl.url(); - tempurl.setPath("/favicon.ico"); - fetchFavicon(tempurl); + if (result == CURLE_OK) { + image.loadFromData(imagedata); } - }); - m_ui->faviconButton->setDisabled(true); -} - -void EditWidgetIcons::fetchFaviconFromGoogle(const QString& domain) -{ - if (config()->get("security/IconDownloadFallbackToGoogle", false).toBool() && m_fallbackToGoogle) { - resetFaviconDownload(); - m_fallbackToGoogle = false; - QUrl faviconUrl = QUrl("https://www.google.com/s2/favicons"); - faviconUrl.setQuery("domain=" + QUrl::toPercentEncoding(domain)); - fetchFavicon(faviconUrl); - } else { - resetFaviconDownload(); - emit messageEditEntry(tr("Unable to fetch favicon."), MessageWidget::Error); - } -} - -void EditWidgetIcons::resetFaviconDownload(bool clearRedirect) -{ - if (clearRedirect) { - m_redirectUrl.clear(); - m_redirectCount = 0; + curl_easy_cleanup(curl); } - if (nullptr != m_httpClient) { - m_httpClient->deleteLater(); - m_httpClient = nullptr; - } - - m_fallbackToGoogle = true; - m_ui->faviconButton->setDisabled(false); + return image; } #endif @@ -281,7 +238,7 @@ void EditWidgetIcons::addCustomIconFromFile() } } -void EditWidgetIcons::addCustomIcon(const QImage &icon) +void EditWidgetIcons::addCustomIcon(const QImage& icon) { if (m_database) { Uuid uuid = m_database->metadata()->findCustomIcon(icon); @@ -392,8 +349,7 @@ void EditWidgetIcons::updateWidgetsDefaultIcons(bool check) QModelIndex index = m_ui->defaultIconsView->currentIndex(); if (!index.isValid()) { m_ui->defaultIconsView->setCurrentIndex(m_defaultIconModel->index(0, 0)); - } - else { + } else { m_ui->defaultIconsView->setCurrentIndex(index); } m_ui->customIconsView->selectionModel()->clearSelection(); @@ -408,8 +364,7 @@ void EditWidgetIcons::updateWidgetsCustomIcons(bool check) QModelIndex index = m_ui->customIconsView->currentIndex(); if (!index.isValid()) { m_ui->customIconsView->setCurrentIndex(m_customIconModel->index(0, 0)); - } - else { + } else { m_ui->customIconsView->setCurrentIndex(index); } m_ui->defaultIconsView->selectionModel()->clearSelection(); diff --git a/src/gui/EditWidgetIcons.h b/src/gui/EditWidgetIcons.h index 796dd593..7b5edf80 100644 --- a/src/gui/EditWidgetIcons.h +++ b/src/gui/EditWidgetIcons.h @@ -32,14 +32,6 @@ class Database; class DefaultIconModel; class CustomIconModel; -#ifdef WITH_XC_NETWORKING -namespace qhttp { - namespace client { - class QHttpClient; - } -} -#endif - namespace Ui { class EditWidgetIcons; } @@ -74,9 +66,7 @@ signals: private slots: void downloadFavicon(); #ifdef WITH_XC_NETWORKING - void fetchFavicon(const QUrl& url); - void fetchFaviconFromGoogle(const QString& domain); - void resetFaviconDownload(bool clearRedirect = true); + QImage fetchFavicon(const QUrl& url); #endif void addCustomIconFromFile(); void addCustomIcon(const QImage& icon); @@ -93,12 +83,6 @@ private: QString m_url; DefaultIconModel* const m_defaultIconModel; CustomIconModel* const m_customIconModel; -#ifdef WITH_XC_NETWORKING - QUrl m_redirectUrl; - bool m_fallbackToGoogle; - unsigned short m_redirectCount; - qhttp::client::QHttpClient* m_httpClient = nullptr; -#endif Q_DISABLE_COPY(EditWidgetIcons) }; diff --git a/src/gui/MainWindow.cpp b/src/gui/MainWindow.cpp index def6e657..6fad6585 100644 --- a/src/gui/MainWindow.cpp +++ b/src/gui/MainWindow.cpp @@ -69,7 +69,7 @@ class HttpPlugin: public ISettingsPage { public: - HttpPlugin(DatabaseTabWidget * tabWidget) + HttpPlugin(DatabaseTabWidget* tabWidget) { m_service = new Service(tabWidget); } @@ -88,18 +88,18 @@ public: QWidget * createWidget() override { - OptionDialog * dlg = new OptionDialog(); + OptionDialog* dlg = new OptionDialog(); QObject::connect(dlg, SIGNAL(removeSharedEncryptionKeys()), m_service, SLOT(removeSharedEncryptionKeys())); QObject::connect(dlg, SIGNAL(removeStoredPermissions()), m_service, SLOT(removeStoredPermissions())); return dlg; } - void loadSettings(QWidget * widget) override + void loadSettings(QWidget* widget) override { qobject_cast(widget)->loadSettings(); } - void saveSettings(QWidget * widget) override + void saveSettings(QWidget* widget) override { qobject_cast(widget)->saveSettings(); if (HttpSettings::isEnabled()) @@ -108,7 +108,7 @@ public: m_service->stop(); } private: - Service *m_service; + Service* m_service; }; #endif diff --git a/src/http/CMakeLists.txt b/src/http/CMakeLists.txt index db9f5dfb..962f78c3 100644 --- a/src/http/CMakeLists.txt +++ b/src/http/CMakeLists.txt @@ -1,4 +1,6 @@ if(WITH_XC_HTTP) + add_subdirectory(qhttp) + include_directories(${CMAKE_CURRENT_SOURCE_DIR} ${CMAKE_CURRENT_BINARY_DIR}) set(keepasshttp_SOURCES @@ -13,5 +15,7 @@ if(WITH_XC_HTTP) ) add_library(keepasshttp STATIC ${keepasshttp_SOURCES}) - target_link_libraries(keepasshttp qhttp Qt5::Core Qt5::Concurrent Qt5::Widgets Qt5::Network) + target_link_libraries(keepasshttp PUBLIC qhttp Qt5::Core Qt5::Concurrent Qt5::Widgets Qt5::Network) + + set(keepassxchttp_LIB keepasshttp PARENT_SCOPE) endif()