[X2Go-Commits] [x2goclient] 01/05: sshprocess.cpp: don't use QProcess::start (QString).

git-admin at x2go.org git-admin at x2go.org
Thu Jun 11 04:23:21 CEST 2015


This is an automated email from the git hooks/post-receive script.

x2go pushed a commit to branch master
in repository x2goclient.

commit 1f7f0144459f935621716ac03c3812755a31fc08
Author: Mihai Moldovan <ionic at ionic.de>
Date:   Thu Jun 11 03:27:56 2015 +0200

    sshprocess.cpp: don't use QProcess::start (QString).
    
    Qt is trying to be too smart and causes big trouble. Instead, use
    QProcess::start (QString, QStringList) and pass the arguments as a list.
    
    On Windows, Qt will automatically double quote the arguments and
    duplicate escaped double quotes or escape non-escaped double quotes.
    
    On UNIX-like platforms, each element of the list is passed as a unique
    argv element, so there's no need for quoting them (that's only a
    shell-internal thing to group arguments.)
---
 debian/changelog   |    8 +++++++
 src/sshprocess.cpp |   66 +++++++++++++++++++++++++++++++++++++++++++---------
 2 files changed, 63 insertions(+), 11 deletions(-)

diff --git a/debian/changelog b/debian/changelog
index ec86448..21eeb1c 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -50,6 +50,14 @@ x2goclient (4.0.4.1-0x2go1) UNRELEASED; urgency=low
       valid C string representation for tr ().
     - onmainwindow.cpp: don't terminate if scdaemon exited with non-zero exit
       code.
+    - sshprocess.cpp: don't use QProcess::start (QString). Qt is trying to be
+      too smart and causes big trouble. Instead, use QProcess::start (QString,
+      QStringList) and pass the arguments as a list. On Windows, Qt will
+      automatically double quote the arguments and duplicate escaped double
+      quotes or escape non-escaped double quotes. On UNIX-like platforms, each
+      element of the list is passed as a unique argv element, so there's no
+      need for quoting them (that's only a shell-internal thing to group
+      arguments.)
 
  -- X2Go Release Manager <git-admin at x2go.org>  Tue, 26 May 2015 21:42:09 +0200
 
diff --git a/src/sshprocess.cpp b/src/sshprocess.cpp
index 8e8c4e5..03bf271 100644
--- a/src/sshprocess.cpp
+++ b/src/sshprocess.cpp
@@ -200,7 +200,7 @@ void SshProcess::startNormal(const QString& cmd)
 // #endif
     if(!masterCon->useKerberos())
     {
-        QString shcmd = "bash -c 'echo \"X2GODATABEGIN:" + uuidStr + "\"; export PATH=\"/usr/local/bin:/usr/bin:/bin\"; "+cmd+"; echo \"X2GODATAEND:" + uuidStr +"\";'";
+        QString shcmd = "bash -c 'echo \"X2GODATABEGIN:" + uuidStr + "\"; export PATH=\"/usr/local/bin:/usr/bin:/bin\"; "+cmd+"; echo \"X2GODATAEND:" + uuidStr + "\";'";
         x2goDebug << "Running masterCon->addChannelConnection(this, '" << uuidStr << "', '" << shcmd.left (200) << "');";
         masterCon->addChannelConnection(this, uuidStr, shcmd);
         connect(masterCon,SIGNAL(stdOut(SshProcess*,QByteArray)),this,SLOT(slotStdOut(SshProcess*,QByteArray)));
@@ -210,30 +210,74 @@ void SshProcess::startNormal(const QString& cmd)
     {
         QString host=masterCon->getHost();
 
-        QString shcmd = "bash -c 'echo \\\"X2GODATABEGIN:" + uuidStr + "\\\"; export PATH=\\\"/usr/local/bin:/usr/bin:/bin\\\"; "+cmd+"; echo \\\"X2GODATAEND:" + uuidStr +"\\\";'";
+        QString shcmd = "";
+
+        /* On Windows, arguments are automatically wrapped in double quotes.
+         * Additionally, QProcess automatically doubles escape characters before
+         * double quotes and inserts an escape character before any non-escaped
+         * double quotes.
+         * Thus, we don't escape double quotes here and let Qt handle this stuff.
+         *
+         * On UNIX-like platforms, likewise, we MUST NOT escape double quotes,
+         * as there is no preceding "outer double quote" the whole argument
+         * is wrapped in.
+         */
+        shcmd = "bash -c 'echo \"X2GODATABEGIN:" + uuidStr + "\"; export PATH=\"/usr/local/bin:/usr/bin:/bin\"; "+cmd+"; echo \"X2GODATAEND:" + uuidStr + "\";'";
 
         proc=new QProcess(this);
+        QString local_cmd = "";
+        QStringList local_args;
 #ifdef Q_OS_WIN
         if(masterCon->get_kerberosDelegation())
         {
             addPuttyReg(host, uuidStr);
             host = uuidStr;
         }
-        QString sshString="plink -batch -P "+
+        local_cmd = "plink";
+
+        /* General options. */
+        local_args << "-batch";
+
+        /* Port option. Must be the last one added! */
+        local_args << "-P";
 #else
-        QString krbDelegOption=" -k ";
+        local_cmd = "ssh";
+
+        /* General options. */
+        local_args << QString (KEEPALIVE_OPTION).trimmed ().split (" ");
+
+        /* Kerberos options. */
+        local_args << "-k";
         if(masterCon->get_kerberosDelegation())
         {
-            krbDelegOption=" -K ";
+            local_args << "-K";
         }
-        QString sshString=QString::null+"ssh"+ KEEPALIVE_OPTION +krbDelegOption+" -o GSSApiAuthentication=yes -o PasswordAuthentication=no -o PubkeyAuthentication=no -p "+
-#endif
-                          QString::number(masterCon->getPort())+" -l "+
-                          masterCon->getUser()+" "+ host +  " \""+shcmd+"\"";
 
-        x2goDebug<<"Evoking SSH command via SshProcess object "<<pid<<": "<<sshString;
+        /* Authentication options. */
+        local_args << "-o" << "GSSApiAuthentication=yes";
+        local_args << "-o" << "PasswordAuthentication=no";
+        local_args << "-o" << "PubkeyAuthentication=no";
+
+        /* Port option. Must be the last one added! */
+        local_args << "-p";
+#endif
+        local_args << QString::number (masterCon->getPort ());
+        local_args << "-l" << masterCon->getUser ();
+        local_args << host;
+
+        /* On Windows, arguments are automatically wrapped in double quotes.
+         * This means we do not have to wrap shcmd ourselves.
+         *
+         * On UNIX-like platforms, we likewise MUST NOT wrap the command in
+         * double quotes, as each entry in the arguments list is passed as
+         * one entry in argv.
+         */
+        local_args << shcmd;
+
+        x2goDebug << "Invoking SSH command via SshProcess object " << pid<< ": "
+                  << local_cmd << local_args.join (" ");
         procUuid=uuidStr;
-        proc->start(sshString);
+        proc->start (local_cmd, local_args);
 
         if (!proc->waitForStarted(15000))
         {

--
Alioth's /srv/git/code.x2go.org/x2goclient.git//..//_hooks_/post-receive-email on /srv/git/code.x2go.org/x2goclient.git


More information about the x2go-commits mailing list