Replace qhttp client with curl for favicon downloading (#1460)

Replace qhttp client with curl for favicon downloading

.travis.yml

@@ -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'"

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"

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 \

snapcraft.yaml

@@ -42,6 +42,7 @@ parts:
       - libxtst-dev
       - libyubikey-dev
       - libykpers-1-dev
+      - libcurl4-openssl-dev
       - libsodium-dev
     stage-packages:
       - dbus

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)
-    add_subdirectory(http)
-    set(keepasshttp_LIB keepasshttp)
-endif()
 if(WITH_XC_NETWORKING)
-    add_subdirectory(http/qhttp)
+  find_package(CURL REQUIRED)
-    set(keepassxcnetwork_LIB qhttp Qt5::Network)
 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}

src/gui/EditWidgetIcons.cpp

@@ -31,10 +31,9 @@
 #include "gui/MessageBox.h"
 
 #ifdef WITH_XC_NETWORKING
-#include "http/qhttp/qhttpclient.hpp"
+#include <curl/curl.h>
-#include "http/qhttp/qhttpclientresponse.hpp"
+#include "core/AsyncTask.h"
-
+#undef MessageBox
-using namespace qhttp::client;
 #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) {
+    QByteArray* response = static_cast<QByteArray*>(data);
-        m_httpClient = new QHttpClient(this);
+    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) {
+QImage EditWidgetIcons::fetchFavicon(const QUrl& url)
-        if (m_database == nullptr) {
+{
-            return;
+    QImage image;
-        }
+    CURL* curl = curl_easy_init();
+    if (curl) {
+        QByteArray imagedata;
+        QByteArray baUrl = url.url().toLatin1();
 
-        response->collectData();
+        curl_easy_setopt(curl, CURLOPT_URL, baUrl.data());
-        response->onEnd([this, response, &url]() {
+        curl_easy_setopt(curl, CURLOPT_MAXREDIRS, 5L);
-            int status = response->status();
+        curl_easy_setopt(curl, CURLOPT_REDIR_PROTOCOLS, CURLPROTO_HTTP | CURLPROTO_HTTPS);
-            if (200 == status) {
+        curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, 1L);
-                QImage image;
+        curl_easy_setopt(curl, CURLOPT_CONNECTTIMEOUT, 5L);
-                image.loadFromData(response->collectedData());
+        curl_easy_setopt(curl, CURLOPT_FAILONERROR, 1L);
+        curl_easy_setopt(curl, CURLOPT_WRITEDATA, &imagedata);
+        curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, &writeCurlResponse);
 
-                if (!image.isNull()) {
+        // Perform the request in another thread
-                    addCustomIcon(image);
+        CURLcode result = AsyncTask::runAndWaitForFuture([curl]() {
-                    resetFaviconDownload();
+            return curl_easy_perform(curl);
-                } 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());
-            }
         });
-    });
 
-    if (!requestMade) {
+        if (result == CURLE_OK) {
-        resetFaviconDownload();
+            image.loadFromData(imagedata);
-        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);
         }
-    });
 
-    m_ui->faviconButton->setDisabled(true);
+        curl_easy_cleanup(curl);
-}
-
-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;
     }
 
-    if (nullptr != m_httpClient) {
+    return image;
-        m_httpClient->deleteLater();
-        m_httpClient = nullptr;
-    }
-
-    m_fallbackToGoogle = true;
-    m_ui->faviconButton->setDisabled(false);
 }
 #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();

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);
+    QImage fetchFavicon(const QUrl& url);
-    void fetchFaviconFromGoogle(const QString& domain);
-    void resetFaviconDownload(bool clearRedirect = true);
 #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)
 };

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<OptionDialog*>(widget)->loadSettings();
     }
 
-    void saveSettings(QWidget * widget) override
+    void saveSettings(QWidget* widget) override
     {
         qobject_cast<OptionDialog*>(widget)->saveSettings();
         if (HttpSettings::isEnabled())
@@ -108,7 +108,7 @@ public:
             m_service->stop();
     }
 private:
-    Service *m_service;
+    Service* m_service;
 };
 #endif
 

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