[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    [kcm-grub2] src: Properly handle the $PATH environment variable in the helper.
From:       Konstantinos Smanis <konstantinos.smanis () gmail ! com>
Date:       2013-02-04 18:43:31
Message-ID: 20130204184331.98D80A6091 () git ! kde ! org
[Download RAW message or body]

Git commit 7c5beb979fdf9dd14abfffb0e24d4f69b11ca985 by Konstantinos Smanis.
Committed on 04/02/2013 at 19:27.
Pushed by ksmanis into branch 'master'.

Properly handle the $PATH environment variable in the helper.

D-Bus activation erases most environment variables, but we need at least
$PATH set for the scripts we execute, or else they malfuntion.

In the past, we used to pass the $PATH from the main application to the
helper but this was just a crude workaround.

What we do now is launch a login shell (1) and 'echo $PATH' in it.
This way:
1. We don't miss paths which are not available to the user
   (e.g. /sbin/, /usr/sbin/, /usr/local/sbin/).
2. We honor the precedence of paths as set in $PATH by the user.
3. We only use the user's $PATH (D-Bus activation works for non-root
   users too).

There are still certain shortcomings (such as some shells not being
compliant) but this is the best we can do. Ideas on how best handle
this issue are most welcome.

(1) The reason why we need a login shell is documented in the code.

M  +36   -1    src/helper/helper.cpp
M  +1    -0    src/helper/helper.h
M  +0    -1    src/kcm_grub2.cpp

http://commits.kde.org/kcm-grub2/7c5beb979fdf9dd14abfffb0e24d4f69b11ca985

diff --git a/src/helper/helper.cpp b/src/helper/helper.cpp
index 2f8bf43..19c5903 100644
--- a/src/helper/helper.cpp
+++ b/src/helper/helper.cpp
@@ -32,6 +32,7 @@
 #include <KGlobal>
 #include <KLocale>
 #include <KProcess>
+#include <KStandardDirs>
 #include <KAuth/HelperSupport>
 
 //Project
@@ -41,9 +42,24 @@
 #include <hd.h>
 #endif
 
+//The $PATH environment variable is emptied by D-Bus activation,
+//so let's provide a sane default. To be used as a fallback.
+static const QString path = QLatin1String("/usr/sbin:/usr/bin:/sbin:/bin");
+
 Helper::Helper()
 {
     KGlobal::locale()->insertCatalog("kcm-grub2");
+    //-l stands for --login. A login shell is needed in order to properly
+    //source /etc/profile, ~/.profile and/or other shell-specific login
+    //scripts (such as ~/.bash_profile for Bash).
+    //Not all shells implement this option, in which case we have the fallback.
+    //Bash, DASH, ksh and Zsh seem to work properly.
+    ActionReply pathReply = executeCommand(QStringList() << findShell() << "-l" << \
"-c" << "echo $PATH"); +    if (pathReply.succeeded()) {
+        qputenv("PATH", pathReply.data().value("output").toByteArray().trimmed());
+    } else {
+        qputenv("PATH", path.toLatin1());
+    }
     //TODO: system encoding should be sent from the core application
     QTextCodec::setCodecForLocale(QTextCodec::codecForName("UTF-8"));
 }
@@ -91,6 +107,26 @@ ActionReply Helper::executeCommand(const QStringList &command)
 #endif
     return reply;
 }
+QString Helper::findShell()
+{
+    QString shell = QFile::symLinkTarget(QLatin1String("/bin/sh"));
+    if (shell.isEmpty()) {
+        shell = KStandardDirs::findExe(QLatin1String("bash"), path);
+        if (shell.isEmpty()) {
+            shell = KStandardDirs::findExe(QLatin1String("dash"), path);
+            if (shell.isEmpty()) {
+                shell = KStandardDirs::findExe(QLatin1String("ksh"), path);
+                if (shell.isEmpty()) {
+                    shell = KStandardDirs::findExe(QLatin1String("zsh"), path);
+                    if (shell.isEmpty()) {
+                        shell = QLatin1String("/bin/sh");
+                    }
+                }
+            }
+        }
+    }
+    return shell;
+}
 
 ActionReply Helper::defaults(QVariantMap args)
 {
@@ -232,7 +268,6 @@ ActionReply Helper::probevbe(QVariantMap args)
 ActionReply Helper::save(QVariantMap args)
 {
     ActionReply reply;
-    qputenv("PATH", args.value("PATH").toByteArray());
     QString mkconfigExePath = args.value("mkconfigExePath").toString();
     QString set_defaultExePath = args.value("set_defaultExePath").toString();
     QString configFileName = args.value("configFileName").toString();
diff --git a/src/helper/helper.h b/src/helper/helper.h
index 782d382..e84af67 100644
--- a/src/helper/helper.h
+++ b/src/helper/helper.h
@@ -29,6 +29,7 @@ public:
     Helper();
 private:
     ActionReply executeCommand(const QStringList &command);
+    QString findShell();
 public Q_SLOTS:
     ActionReply defaults(QVariantMap args);
     ActionReply install(QVariantMap args);
diff --git a/src/kcm_grub2.cpp b/src/kcm_grub2.cpp
index c51b09b..8dd14d6 100644
--- a/src/kcm_grub2.cpp
+++ b/src/kcm_grub2.cpp
@@ -411,7 +411,6 @@ void KCMGRUB2::save()
 
     Action saveAction("org.kde.kcontrol.kcmgrub2.save");
     saveAction.setHelperID("org.kde.kcontrol.kcmgrub2");
-    saveAction.addArgument("PATH", qgetenv("PATH"));
     saveAction.addArgument("mkconfigExePath", mkconfigExePath);
     saveAction.addArgument("set_defaultExePath", set_defaultExePath);
     saveAction.addArgument("configFileName", configPath);


[prev in list] [next in list] [prev in thread] [next in thread]