Skip to content

Commit

Permalink
kde-frameworks/kio: OpenUrlJob: handle all text scripts consistently
Browse files Browse the repository at this point in the history
Upstream commit fdd7c47c85d5d6dbf21e05e7a0d6afcf383f1d24

KDE-Bug: https://bugs.kde.org/show_bug.cgi?id=425829
Package-Manager: Portage-3.0.8, Repoman-3.0.1
Signed-off-by: Andreas Sturmlechner <[email protected]>
  • Loading branch information
a17r committed Sep 29, 2020
1 parent 017d20b commit 79bc863
Show file tree
Hide file tree
Showing 2 changed files with 314 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -0,0 +1,310 @@
From fdd7c47c85d5d6dbf21e05e7a0d6afcf383f1d24 Mon Sep 17 00:00:00 2001
From: Ahmad Samir <[email protected]>
Date: Tue, 15 Sep 2020 20:06:49 +0200
Subject: [PATCH] OpenUrlJob: handle all text scripts consistently

Previously we only handled application/x-shellscript, but there are other
scripts; a script is technically a file that inherits both text/plain and
application/x-executable, e.g. .sh, .csh, .py, perl scripts ...etc. Treat
all those mime types the way we handled shell scripts:
- if it's not a local url, or isn't executable we open it in the preferred
text editor
- if it's executable either show the OpenOrExecute dialog or execute
directly depending on how the job is configured

The mimetype world is a confusing one:
- Executables, this includes .exe files (MS Windows); and "application/x-executable"
and "application/x-sharedlib", this depends on various parameters (e.g.
stripped executables are x-sharedlib, the same executable if not stripped
is x-executable...)
- Scripts: shell, python, perl... etc scripts, which are text files that
can be executed or opened as text.

Adjust the unit test.

BUG: 425829
BUG: 425177
FIXED-IN: 5.75
---
autotests/openurljobtest.cpp | 56 +++++++++++++++++++++++--------
autotests/openurljobtest.h | 2 ++
src/gui/openurljob.cpp | 65 ++++++++++++++++++++++--------------
3 files changed, 85 insertions(+), 38 deletions(-)

diff --git a/autotests/openurljobtest.cpp b/autotests/openurljobtest.cpp
index 2f2ef8ad..ed2211a8 100644
--- a/autotests/openurljobtest.cpp
+++ b/autotests/openurljobtest.cpp
@@ -103,14 +103,13 @@ void OpenUrlJobTest::initTestCase()
KConfigGroup grp = mimeAppsCfg.group("Default Applications");
grp.writeEntry("text/plain", s_tempServiceName);
grp.writeEntry("text/html", s_tempServiceName);
- grp.writeEntry("application/x-shellscript", s_tempServiceName);
grp.sync();

- for (const char *mimeType : {"text/plain", "application/x-shellscript"}) {
- KService::Ptr preferredTextEditor = KApplicationTrader::preferredService(QString::fromLatin1(mimeType));
- QVERIFY(preferredTextEditor);
- QCOMPARE(preferredTextEditor->entryPath(), m_fakeService);
- }
+
+ // "text/plain" encompasses all scripts (shell, python, perl)
+ KService::Ptr preferredTextEditor = KApplicationTrader::preferredService(QStringLiteral("text/plain"));
+ QVERIFY(preferredTextEditor);
+ QCOMPARE(preferredTextEditor->entryPath(), m_fakeService);

// As used for preferredService
QVERIFY(KService::serviceByDesktopName("openurljobtest_service"));
@@ -230,17 +229,38 @@ void OpenUrlJobTest::invalidUrl()
QCOMPARE(job2->errorString(), QStringLiteral("Malformed URL\n/pathonly"));
}

+void OpenUrlJobTest::refuseRunningNativeExecutables_data()
+{
+ QTest::addColumn<QString>("mimeType");
+
+ // Executables under e.g. /usr/bin/ can be either of these two mimetypes
+ // see https://gitlab.freedesktop.org/xdg/shared-mime-info/-/issues/11
+ QTest::newRow("x-sharedlib") << "application/x-sharedlib";
+ QTest::newRow("x-executable") << "application/x-executable";
+}
+
void OpenUrlJobTest::refuseRunningNativeExecutables()
{
- KIO::OpenUrlJob *job = new KIO::OpenUrlJob(QUrl::fromLocalFile(QCoreApplication::applicationFilePath()), QStringLiteral("application/x-executable"), this);
+ QFETCH(QString, mimeType);
+
+ KIO::OpenUrlJob *job = new KIO::OpenUrlJob(QUrl::fromLocalFile(QCoreApplication::applicationFilePath()), mimeType, this);
QVERIFY(!job->exec());
QCOMPARE(job->error(), KJob::UserDefinedError);
QVERIFY2(job->errorString().contains("For security reasons, launching executables is not allowed in this context."), qPrintable(job->errorString()));
}

+void OpenUrlJobTest::refuseRunningRemoteNativeExecutables_data()
+{
+ QTest::addColumn<QString>("mimeType");
+ QTest::newRow("x-sharedlib") << "application/x-sharedlib";
+ QTest::newRow("x-executable") << "application/x-executable";
+}
+
void OpenUrlJobTest::refuseRunningRemoteNativeExecutables()
{
- KIO::OpenUrlJob *job = new KIO::OpenUrlJob(QUrl("protocol://host/path/exe"), QStringLiteral("application/x-executable"), this);
+ QFETCH(QString, mimeType);
+
+ KIO::OpenUrlJob *job = new KIO::OpenUrlJob(QUrl("protocol://host/path/exe"), mimeType, this);
job->setRunExecutables(true); // even with this enabled, an error will occur
QVERIFY(!job->exec());
QCOMPARE(job->error(), KJob::UserDefinedError);
@@ -273,8 +293,11 @@ void OpenUrlJobTest::runScript_data()
{
QTest::addColumn<QString>("mimeType");

+ // All text-based scripts inherit text/plain and application/x-executable, no need to test
+ // all flavours (python, perl, lua, awk ...etc), this sample should be enough
QTest::newRow("shellscript") << "application/x-shellscript";
- QTest::newRow("native") << "application/x-executable";
+ QTest::newRow("pythonscript") << "text/x-python";
+ QTest::newRow("javascript") << "application/javascript";
}

void OpenUrlJobTest::runScript()
@@ -305,16 +328,23 @@ void OpenUrlJobTest::runScript()

void OpenUrlJobTest::runNativeExecutable_data()
{
+ QTest::addColumn<QString>("mimeType");
QTest::addColumn<bool>("withHandler");
QTest::addColumn<bool>("handlerRetVal");

- QTest::newRow("no_handler") << false << false;
- QTest::newRow("handler_false") << true << false;
- QTest::newRow("handler_true") << true << true;
+ QTest::newRow("no_handler_x-sharedlib") << "application/x-sharedlib" << false << false;
+ QTest::newRow("handler_false_x-sharedlib") << "application/x-sharedlib" << true << false;
+ QTest::newRow("handler_true_x-sharedlib") << "application/x-sharedlib" << true << true;
+
+ QTest::newRow("no_handler_x-executable") << "application/x-executable" << false << false;
+ QTest::newRow("handler_false_x-executable") << "application/x-executable" << true << false;
+ QTest::newRow("handler_true_x-executable") << "application/x-executable" << true << true;
+
}

void OpenUrlJobTest::runNativeExecutable()
{
+ QFETCH(QString, mimeType);
QFETCH(bool, withHandler);
QFETCH(bool, handlerRetVal);

@@ -335,7 +365,7 @@ void OpenUrlJobTest::runNativeExecutable()
KIO::setDefaultUntrustedProgramHandler(withHandler ? &s_handler : nullptr);

// When using OpenUrlJob to run the executable
- KIO::OpenUrlJob *job = new KIO::OpenUrlJob(QUrl::fromLocalFile(scriptFile), QStringLiteral("application/x-executable"), this);
+ KIO::OpenUrlJob *job = new KIO::OpenUrlJob(QUrl::fromLocalFile(scriptFile), mimeType, this);
job->setRunExecutables(true); // startProcess tests the case where this isn't set
const bool success = job->exec();

diff --git a/autotests/openurljobtest.h b/autotests/openurljobtest.h
index e71987d9..f5b9a5be 100644
--- a/autotests/openurljobtest.h
+++ b/autotests/openurljobtest.h
@@ -26,7 +26,9 @@ private Q_SLOTS:

void noServiceNoHandler();
void invalidUrl();
+ void refuseRunningNativeExecutables_data();
void refuseRunningNativeExecutables();
+ void refuseRunningRemoteNativeExecutables_data();
void refuseRunningRemoteNativeExecutables();
void notAuthorized();
void runScript_data();
diff --git a/src/gui/openurljob.cpp b/src/gui/openurljob.cpp
index 8ac187b4..3e35c95c 100644
--- a/src/gui/openurljob.cpp
+++ b/src/gui/openurljob.cpp
@@ -73,9 +73,9 @@ public:

private:
void executeCommand();
- void handleExecutables(const QMimeType &mimeType);
+ void handleBinaries(const QMimeType &mimeType);
void handleDesktopFiles();
- void handleShellscripts();
+ void handleScripts();
void openInPreferredApp();
void runLink(const QString &filePath, const QString &urlStr, const QString &optionalServiceName);

@@ -439,14 +439,29 @@ void KIO::OpenUrlJobPrivate::emitAccessDenied()
q->emitResult();
}

-// was: KRun::isExecutable (minus application/x-desktop and application/x-shellscript mimetypes).
+// was: KRun::isExecutable (minus application/x-desktop mimetype).
// Feel free to make public if needed.
-static bool isExecutableMime(const QMimeType &mimeType)
+static bool isBinary(const QMimeType &mimeType)
{
- return (mimeType.inherits(QStringLiteral("application/x-executable")) ||
- /* e.g. /usr/bin/ls, see https://gitlab.freedesktop.org/xdg/shared-mime-info/-/issues/11 */
- mimeType.inherits(QStringLiteral("application/x-sharedlib")) ||
- mimeType.inherits(QStringLiteral("application/x-ms-dos-executable")));
+ // - Binaries could be e.g.:
+ // - application/x-executable
+ // - application/x-sharedlib e.g. /usr/bin/ls, see
+ // https://gitlab.freedesktop.org/xdg/shared-mime-info/-/issues/11
+ //
+ // - Mimetypes that inherit application/x-executable _and_ text/plain are scripts, these are
+ // handled by handleScripts()
+
+ return (mimeType.inherits(QStringLiteral("application/x-executable"))
+ || mimeType.inherits(QStringLiteral("application/x-sharedlib"))
+ || mimeType.inherits(QStringLiteral("application/x-ms-dos-executable")));
+}
+
+// Helper function that returns whether a file is a text-based script
+// e.g. ".sh", ".csh", ".py", ".js"
+static bool isTextScript(const QMimeType &mimeType)
+{
+ return (mimeType.inherits(QStringLiteral("application/x-executable"))
+ && mimeType.inherits(QStringLiteral("text/plain")));
}

// Helper function that returns whether a file has the execute bit set or not.
@@ -456,7 +471,7 @@ static bool hasExecuteBit(const QString &fileName)
}

// Handle native binaries (.e.g. /usr/bin/*); and .exe files
-void KIO::OpenUrlJobPrivate::handleExecutables(const QMimeType &mimeType)
+void KIO::OpenUrlJobPrivate::handleBinaries(const QMimeType &mimeType)
{
if (!KAuthorized::authorize(QStringLiteral("shell_access"))) {
emitAccessDenied();
@@ -475,11 +490,9 @@ void KIO::OpenUrlJobPrivate::handleExecutables(const QMimeType &mimeType)

const QString localPath = m_url.toLocalFile();

- // Check whether file is an executable script
-#ifdef Q_OS_WIN
- const bool isNativeBinary = !mimeType.inherits(QStringLiteral("text/plain"));
-#else
- const bool isNativeBinary = !mimeType.inherits(QStringLiteral("text/plain")) && !mimeType.inherits(QStringLiteral("application/x-ms-dos-executable"));
+ bool isNativeBinary = true;
+#ifndef Q_OS_WIN
+ isNativeBinary = !mimeType.inherits(QStringLiteral("application/x-ms-dos-executable"));
#endif

if (m_showOpenOrExecuteDialog) {
@@ -497,6 +510,8 @@ void KIO::OpenUrlJobPrivate::handleExecutables(const QMimeType &mimeType)
}
};

+ // Ask the user for confirmation before executing this binary (for binaries
+ // the dialog will only show Execute/Cancel)
showOpenOrExecuteFileDialog(dialogFinished);
return;
}
@@ -601,15 +616,15 @@ void KIO::OpenUrlJobPrivate::runUrlWithMimeType()
return;
}

- // Shell scripts
- if (mimeType.inherits(QStringLiteral("application/x-shellscript"))) {
- handleShellscripts();
+ // Scripts (e.g. .sh, .csh, .py, .js)
+ if (isTextScript(mimeType)) {
+ handleScripts();
return;
}

- // Binaries (e.g. /usr/bin/konsole) and .exe files
- if (isExecutableMime(mimeType)) {
- handleExecutables(mimeType);
+ // Binaries (e.g. /usr/bin/{konsole,ls}) and .exe files
+ if (isBinary(mimeType)) {
+ handleBinaries(mimeType);
return;
}

@@ -677,8 +692,9 @@ void KIO::OpenUrlJobPrivate::handleDesktopFiles()
openInPreferredApp();
}

-void KIO::OpenUrlJobPrivate::handleShellscripts()
+void KIO::OpenUrlJobPrivate::handleScripts()
{
+ // Executable scripts of any type can run arbitrary shell commands
if (!KAuthorized::authorize(QStringLiteral("shell_access"))) {
emitAccessDenied();
return;
@@ -687,8 +703,7 @@ void KIO::OpenUrlJobPrivate::handleShellscripts()
const bool isLocal = m_url.isLocalFile();
const QString localPath = m_url.toLocalFile();
if (!isLocal || !hasExecuteBit(localPath)) {
- // Open remote shell scripts or ones without the execute bit, with the
- // default application
+ // Open remote scripts or ones without the execute bit, with the default application
openInPreferredApp();
return;
}
@@ -706,7 +721,7 @@ void KIO::OpenUrlJobPrivate::handleShellscripts()
return;
}

- if (m_runExecutables) { // Local executable shell script, proceed
+ if (m_runExecutables) { // Local executable script, proceed
executeCommand();
} else { // Open in the default (text editor) app
openInPreferredApp();
@@ -767,7 +782,7 @@ void KIO::OpenUrlJobPrivate::showOpenOrExecuteFileDialog(std::function<void(bool

if (!s_openOrExecuteFileHandler) {
// No way to ask the user whether to execute or open
- if (mimeType.inherits(QStringLiteral("application/x-shellscript"))
+ if (isTextScript(mimeType)
|| mimeType.inherits(QStringLiteral("application/x-desktop"))) { // Open text-based ones in the default app
openInPreferredApp();
} else {
--
GitLab

5 changes: 4 additions & 1 deletion kde-frameworks/kio/kio-5.74.1-r1.ebuild
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,10 @@ PDEPEND="
# tests hang
RESTRICT+=" test"

PATCHES=( "${FILESDIR}"/${P}-kio_trash-too-strict-perms-check.patch )
PATCHES=(
"${FILESDIR}"/${P}-kio_trash-too-strict-perms-check.patch
"${FILESDIR}"/${P}-handle-shell-scripts-consistenty.patch
)

src_configure() {
local mycmakeargs=(
Expand Down

0 comments on commit 79bc863

Please sign in to comment.