From c0f29cc790dc64b574229e9506d1d581cfbdb511 Mon Sep 17 00:00:00 2001 From: varjolintu Date: Fri, 22 Nov 2019 13:54:28 +0200 Subject: [PATCH] Show UI warning for invalid URLs --- src/CMakeLists.txt | 1 + src/core/Tools.cpp | 28 +++++++++++++ src/core/Tools.h | 1 + src/gui/URLEdit.cpp | 59 ++++++++++++++++++++++++++++ src/gui/URLEdit.h | 43 ++++++++++++++++++++ src/gui/entry/EditEntryWidget.cpp | 5 +++ src/gui/entry/EditEntryWidgetMain.ui | 8 +++- src/gui/entry/EntryURLModel.cpp | 29 ++++++++++++++ src/gui/entry/EntryURLModel.h | 16 ++++++++ tests/TestBrowser.cpp | 22 ++++++++++- tests/TestBrowser.h | 1 + 11 files changed, 211 insertions(+), 2 deletions(-) create mode 100644 src/gui/URLEdit.cpp create mode 100644 src/gui/URLEdit.h diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 0e3bca7af..af9b9bb58 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -121,6 +121,7 @@ set(keepassx_SOURCES gui/TotpDialog.cpp gui/TotpExportSettingsDialog.cpp gui/DatabaseOpenDialog.cpp + gui/URLEdit.cpp gui/WelcomeWidget.cpp gui/csvImport/CsvImportWidget.cpp gui/csvImport/CsvImportWizard.cpp diff --git a/src/core/Tools.cpp b/src/core/Tools.cpp index 2dbf0093d..7cfc8a55f 100644 --- a/src/core/Tools.cpp +++ b/src/core/Tools.cpp @@ -32,6 +32,7 @@ #include #include #include +#include #include #include @@ -259,6 +260,33 @@ namespace Tools } } + bool checkUrlValid(const QString& urlField) + { + if (urlField.isEmpty()) { + return true; + } + + QUrl url; + if (urlField.contains("://")) { + url = urlField; + } else { + url = QUrl::fromUserInput(urlField); + } + + if (url.scheme() != "file" && url.host().isEmpty()) { + return false; + } + + // Check for illegal characters. Adds also the wildcard * to the list + QRegularExpression re("[<>\\^`{|}\\*]"); + auto match = re.match(urlField); + if (match.hasMatch()) { + return false; + } + + return true; + } + // Escape common regex symbols except for *, ?, and | auto regexEscape = QRegularExpression(R"re(([-[\]{}()+.,\\\/^$#]))re"); diff --git a/src/core/Tools.h b/src/core/Tools.h index 1fa5e6a9a..455b879c2 100644 --- a/src/core/Tools.h +++ b/src/core/Tools.h @@ -41,6 +41,7 @@ namespace Tools bool isBase64(const QByteArray& ba); void sleep(int ms); void wait(int ms); + bool checkUrlValid(const QString& urlField); QString uuidToHex(const QUuid& uuid); QUuid hexToUuid(const QString& uuid); QRegularExpression convertToRegex(const QString& string, diff --git a/src/gui/URLEdit.cpp b/src/gui/URLEdit.cpp new file mode 100644 index 000000000..4dc2a55c2 --- /dev/null +++ b/src/gui/URLEdit.cpp @@ -0,0 +1,59 @@ +/* + * Copyright (C) 2014 Felix Geyer + * Copyright (C) 2019 KeePassXC Team + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 2 or (at your option) + * version 3 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#include "URLEdit.h" + +#include + +#include "core/Config.h" +#include "core/FilePath.h" +#include "core/Tools.h" +#include "gui/Font.h" + +const QColor URLEdit::ErrorColor = QColor(255, 125, 125); + +URLEdit::URLEdit(QWidget* parent) + : QLineEdit(parent) +{ + const QIcon errorIcon = filePath()->icon("status", "dialog-error"); + m_errorAction = addAction(errorIcon, QLineEdit::TrailingPosition); + m_errorAction->setVisible(false); + m_errorAction->setToolTip(tr("Invalid URL")); + + updateStylesheet(); +} + +void URLEdit::enableVerifyMode() +{ + updateStylesheet(); + + connect(this, SIGNAL(textChanged(QString)), SLOT(updateStylesheet())); +} + +void URLEdit::updateStylesheet() +{ + const QString stylesheetTemplate("QLineEdit { background: %1; }"); + + if (!Tools::checkUrlValid(text())) { + setStyleSheet(stylesheetTemplate.arg(ErrorColor.name())); + m_errorAction->setVisible(true); + } else { + m_errorAction->setVisible(false); + setStyleSheet(""); + } +} diff --git a/src/gui/URLEdit.h b/src/gui/URLEdit.h new file mode 100644 index 000000000..11b743b41 --- /dev/null +++ b/src/gui/URLEdit.h @@ -0,0 +1,43 @@ +/* + * Copyright (C) 2014 Felix Geyer + * Copyright (C) 2019 KeePassXC Team + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 2 or (at your option) + * version 3 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#ifndef KEEPASSX_URLEDIT_H +#define KEEPASSX_URLEDIT_H + +#include +#include +#include + +class URLEdit : public QLineEdit +{ + Q_OBJECT + +public: + static const QColor ErrorColor; + + explicit URLEdit(QWidget* parent = nullptr); + void enableVerifyMode(); + +private slots: + void updateStylesheet(); + +private: + QPointer m_errorAction; +}; + +#endif // KEEPASSX_URLEDIT_H diff --git a/src/gui/entry/EditEntryWidget.cpp b/src/gui/entry/EditEntryWidget.cpp index b80a4850d..2f27c9b54 100644 --- a/src/gui/entry/EditEntryWidget.cpp +++ b/src/gui/entry/EditEntryWidget.cpp @@ -167,6 +167,7 @@ void EditEntryWidget::setupMain() #ifdef WITH_XC_NETWORKING connect(m_mainUi->fetchFaviconButton, SIGNAL(clicked()), m_iconsWidget, SLOT(downloadFavicon())); connect(m_mainUi->urlEdit, SIGNAL(textChanged(QString)), m_iconsWidget, SLOT(setUrl(QString))); + m_mainUi->urlEdit->enableVerifyMode(); #endif connect(m_mainUi->expireCheck, SIGNAL(toggled(bool)), m_mainUi->expireDatePicker, SLOT(setEnabled(bool))); connect(m_mainUi->notesEnabled, SIGNAL(toggled(bool)), this, SLOT(toggleHideNotes(bool))); @@ -271,6 +272,10 @@ void EditEntryWidget::setupBrowser() m_additionalURLsDataModel->setEntryAttributes(m_entryAttributes); m_browserUi->additionalURLsView->setModel(m_additionalURLsDataModel); + // Use a custom item delegate to align the icon to the right side + auto iconDelegate = new URLModelIconDelegate(m_browserUi->additionalURLsView); + m_browserUi->additionalURLsView->setItemDelegate(iconDelegate); + // clang-format off connect(m_browserUi->skipAutoSubmitCheckbox, SIGNAL(toggled(bool)), SLOT(updateBrowserModified())); connect(m_browserUi->hideEntryCheckbox, SIGNAL(toggled(bool)), SLOT(updateBrowserModified())); diff --git a/src/gui/entry/EditEntryWidgetMain.ui b/src/gui/entry/EditEntryWidgetMain.ui index 255cd0ab2..54140fcd9 100644 --- a/src/gui/entry/EditEntryWidgetMain.ui +++ b/src/gui/entry/EditEntryWidgetMain.ui @@ -30,7 +30,7 @@ - + Url field @@ -256,6 +256,12 @@
gui/PasswordEdit.h
1 + + URLEdit + QLineEdit +
gui/URLEdit.h
+ 1 + titleEdit diff --git a/src/gui/entry/EntryURLModel.cpp b/src/gui/entry/EntryURLModel.cpp index 3667c78f0..3e6fb839c 100644 --- a/src/gui/entry/EntryURLModel.cpp +++ b/src/gui/entry/EntryURLModel.cpp @@ -19,6 +19,7 @@ #include "EntryURLModel.h" #include "core/Entry.h" +#include "core/FilePath.h" #include "core/Tools.h" #include @@ -26,6 +27,7 @@ EntryURLModel::EntryURLModel(QObject* parent) : QStandardItemModel(parent) , m_entryAttributes(nullptr) + , m_errorIcon(filePath()->icon("status", "dialog-error")) { } @@ -53,6 +55,33 @@ void EntryURLModel::setEntryAttributes(EntryAttributes* entryAttributes) endResetModel(); } +QVariant EntryURLModel::data(const QModelIndex& index, int role) const +{ + if (!index.isValid()) { + return {}; + } + + const auto key = keyByIndex(index); + if (key.isEmpty()) { + return {}; + } + + const auto value = m_entryAttributes->value(key); + const auto urlValid = Tools::checkUrlValid(value); + + if (role == Qt::BackgroundRole && !urlValid) { + return QColor(255, 125, 125); + } else if (role == Qt::DecorationRole && !urlValid) { + return m_errorIcon; + } else if (role == Qt::DisplayRole || role == Qt::EditRole) { + return value; + } else if (role == Qt::ToolTipRole && !urlValid) { + return tr("Invalid URL"); + } + + return {}; +} + bool EntryURLModel::setData(const QModelIndex& index, const QVariant& value, int role) { if (!index.isValid() || role != Qt::EditRole || value.type() != QVariant::String || value.toString().isEmpty()) { diff --git a/src/gui/entry/EntryURLModel.h b/src/gui/entry/EntryURLModel.h index 09344d92a..f9ffa4828 100644 --- a/src/gui/entry/EntryURLModel.h +++ b/src/gui/entry/EntryURLModel.h @@ -20,9 +20,23 @@ #define KEEPASSXC_ENTRYURLMODEL_H #include +#include class EntryAttributes; +class URLModelIconDelegate : public QStyledItemDelegate +{ +public: + using QStyledItemDelegate::QStyledItemDelegate; + +protected: + void initStyleOption(QStyleOptionViewItem* option, const QModelIndex& index) const override + { + QStyledItemDelegate::initStyleOption(option, index); + option->decorationPosition = QStyleOptionViewItem::Right; + } +}; + class EntryURLModel : public QStandardItemModel { Q_OBJECT @@ -32,6 +46,7 @@ public: void setEntryAttributes(EntryAttributes* entryAttributes); void insertRow(const QString& key, const QString& value); bool setData(const QModelIndex& index, const QVariant& value, int role = Qt::EditRole) override; + QVariant data(const QModelIndex& index, int role) const override; QModelIndex indexByKey(const QString& key) const; QString keyByIndex(const QModelIndex& index) const; @@ -41,6 +56,7 @@ private slots: private: QList> m_urls; EntryAttributes* m_entryAttributes; + QIcon m_errorIcon; }; #endif // KEEPASSXC_ENTRYURLMODEL_H diff --git a/tests/TestBrowser.cpp b/tests/TestBrowser.cpp index 938a7e4e5..a3611399e 100644 --- a/tests/TestBrowser.cpp +++ b/tests/TestBrowser.cpp @@ -18,6 +18,7 @@ #include "TestBrowser.h" #include "TestGlobal.h" #include "browser/BrowserSettings.h" +#include "core/Tools.h" #include "crypto/Crypto.h" #include "sodium/crypto_box.h" #include @@ -461,4 +462,23 @@ QList TestBrowser::createEntries(QStringList& urls, Group* root) const } return entries; -} \ No newline at end of file +} +void TestBrowser::testValidURLs() +{ + QHash urls; + urls["https://github.com/login"] = true; + urls["https:///github.com/"] = false; + urls["http://github.com/**//*"] = false; + urls["http://*.github.com/login"] = false; + urls["//github.com"] = true; + urls["github.com/{}<>"] = false; + urls["http:/example.com"] = false; + urls["cmd://C:/Toolchains/msys2/usr/bin/mintty \"ssh jon@192.168.0.1:22\""] = true; + urls["file:///Users/testUser/Code/test.html"] = true; + + QHashIterator i(urls); + while (i.hasNext()) { + i.next(); + QCOMPARE(Tools::checkUrlValid(i.key()), i.value()); + } +} diff --git a/tests/TestBrowser.h b/tests/TestBrowser.h index 8b2dc3e3c..69ba69309 100644 --- a/tests/TestBrowser.h +++ b/tests/TestBrowser.h @@ -47,6 +47,7 @@ private slots: void testSubdomainsAndPaths(); void testSortEntries(); void testGetDatabaseGroups(); + void testValidURLs(); private: QList createEntries(QStringList& urls, Group* root) const;