From bb573b83ace5a8039b2d0f5192192496e7937308 Mon Sep 17 00:00:00 2001
From: Kai Uwe Broulik <kde@privat.broulik.de>
Date: Fri, 31 Jan 2020 14:33:23 +0100
Subject: [PATCH] [Task Manager] Remove strict URL handling

The code tried hard to ignore garbage URLs, as Qt is quite lenient, e.g. QUrl("Garbage Url") is still valid.
There is no way to change the strictness of a QUrl after creation,
so the code would enforce it by doing QUrl strictUrl(inputUrl.toString(), QUrl::StrictMode).
However, toString() defaults to PrettyDecoded which avoids percent-encoding and keeps spaces
in tact which is not a valid thing to have in a strict URL.
Effectively, we want to ensure a URL is either a valid path to a local file,
or one of the special applications (for menu ids), or preferred for preferred applications, like web browser,

BUG: 385727
FIXED-IN: 5.18.0

Differential Revision: https://phabricator.kde.org/D26941
---
 .../autotests/launchertasksmodeltest.cpp      | 20 +++++++++++++++++++
 libtaskmanager/launchertasksmodel.cpp         | 14 ++++++-------
 libtaskmanager/launchertasksmodel_p.h         | 19 ++++++++++++++++--
 3 files changed, 44 insertions(+), 9 deletions(-)

diff --git a/libtaskmanager/autotests/launchertasksmodeltest.cpp b/libtaskmanager/autotests/launchertasksmodeltest.cpp
index 2110bd689..f5ce382d6 100644
--- a/libtaskmanager/autotests/launchertasksmodeltest.cpp
+++ b/libtaskmanager/autotests/launchertasksmodeltest.cpp
@@ -36,6 +36,7 @@ class LauncherTasksModelTest : public QObject
 
         void shouldRoundTripLauncherUrlList();
         void shouldIgnoreInvalidUrls();
+        void shouldAcceptSpaces();
         void shouldRejectDuplicates();
         void shouldAddRemoveLauncher();
         void shouldReturnValidLauncherPositions();
@@ -91,6 +92,25 @@ void LauncherTasksModelTest::shouldIgnoreInvalidUrls()
     QCOMPARE(m.launcherList(), QStringList());
 }
 
+void LauncherTasksModelTest::shouldAcceptSpaces()
+{
+    LauncherTasksModel m;
+
+    const QStringList urlStrings{
+        QLatin1String("applications:App with spaces.desktop")
+    };
+
+    QSignalSpy launcherListChangedSpy(&m, &LauncherTasksModel::launcherListChanged);
+    QVERIFY(launcherListChangedSpy.isValid());
+
+    const bool added = m.requestAddLauncher(QUrl(urlStrings.at(0)));
+
+    QVERIFY(added);
+    QCOMPARE(launcherListChangedSpy.count(), 1);
+
+    QCOMPARE(m.launcherList(), QStringList() << urlStrings.at(0));
+}
+
 void LauncherTasksModelTest::shouldRejectDuplicates()
 {
     LauncherTasksModel m;
diff --git a/libtaskmanager/launchertasksmodel.cpp b/libtaskmanager/launchertasksmodel.cpp
index 2069370a5..d6e89c159 100644
--- a/libtaskmanager/launchertasksmodel.cpp
+++ b/libtaskmanager/launchertasksmodel.cpp
@@ -143,15 +143,13 @@ AppData LauncherTasksModel::Private::appData(const QUrl &url)
 
 bool LauncherTasksModel::Private::requestAddLauncherToActivities(const QUrl &_url, const QStringList &_activities)
 {
-    // isValid() for the passed-in URL might return true if it was
-    // constructed in TolerantMode, but we want to reject invalid URLs.
-    QUrl url(_url.toString(), QUrl::StrictMode);
-    const auto activities = ActivitiesSet::fromList(_activities);
-
-    if (url.isEmpty() || !url.isValid()) {
+    QUrl url(_url);
+    if (!isValidLauncherUrl(url)) {
         return false;
     }
 
+    const auto activities = ActivitiesSet::fromList(_activities);
+
     if (url.isLocalFile() && KDesktopFile::isDesktopFile(url.toLocalFile())) {
         KDesktopFile f(url.toLocalFile());
 
@@ -409,7 +407,9 @@ void LauncherTasksModel::setLauncherList(const QStringList &serializedLaunchers)
         auto activities = ActivitiesSet::fromList(_activities);
 
         // Is url is not valid, ignore it
-        if (!url.isValid()) continue;
+        if (!isValidLauncherUrl(url)) {
+            continue;
+        }
 
         // If we have a null uuid, it means we are on all activities
         // and we should contain only the null uuid
diff --git a/libtaskmanager/launchertasksmodel_p.h b/libtaskmanager/launchertasksmodel_p.h
index 10e5e97dd..63eca3d42 100644
--- a/libtaskmanager/launchertasksmodel_p.h
+++ b/libtaskmanager/launchertasksmodel_p.h
@@ -29,10 +29,25 @@ namespace TaskManager {
 
 #define NULL_UUID "00000000-0000-0000-0000-000000000000"
 
+inline static bool isValidLauncherUrl(const QUrl &url)
+{
+    if (url.isEmpty() || !url.isValid()) {
+        return false;
+    }
+
+    if (!url.isLocalFile()
+            && url.scheme() != QLatin1String("applications")
+            && url.scheme() != QLatin1String("preferred")) {
+        return false;
+    }
+
+    return true;
+}
+
 inline static std::pair<QUrl, QStringList> deserializeLauncher(const QString &serializedLauncher)
 {
     QStringList activities;
-    QUrl url(serializedLauncher, QUrl::StrictMode);
+    QUrl url(serializedLauncher);
 
     // The storage format is: [list of activity ids]\nURL
     // The activity IDs list can not be empty, it at least needs
@@ -47,7 +62,7 @@ inline static std::pair<QUrl, QStringList> deserializeLauncher(const QString &se
             activities = serializedLauncher.mid(1, activitiesBlockEnd - 1).split(",", QString::SkipEmptyParts);
 
             if (!activities.isEmpty()) {
-                url = QUrl(serializedLauncher.mid(activitiesBlockEnd + 2), QUrl::StrictMode);
+                url = QUrl(serializedLauncher.mid(activitiesBlockEnd + 2));
             }
         }
     }