Improve handling of environment variables with spaces (#456) (#458)

* Refactor implementation for shell escaping

* Use Shquote from the standard library as of python3.3,
  fallback to legacy implementation in older versions.

* Improve shell-escaping of env variables (#456)

* For ParamikoMachine and SshMachine, shell escape
  environment variables in order to not run into
  ProcessExecutionErrors when using environment
  variables with spaces and special characters

* Add test for env variables with special chars

plumbum/commands/base.py

@@ -1,4 +1,6 @@
+import shlex
 import subprocess
+import sys
 import functools
 from contextlib import contextmanager
 from plumbum.commands.processes import run_proc, iter_lines
@@ -24,21 +26,12 @@ class RedirectionError(Exception):
 
 def shquote(text):
     """Quotes the given text with shell escaping (assumes as syntax similar to ``sh``)"""
-    if not text:
-        return "''"
     text = six.str(text)
-    if not text:
-        return "''"
-    for c in text:
-        if c not in _safechars:
-            break
+    if sys.version_info >= (3, 3):
+        return shlex.quote(text)
     else:
-        return text
-    if "'" not in text:
-        return "'" + text + "'"
-    res = six.str("").join(
-        (six.str('\\' + c) if c in _funnychars else c) for c in text)
-    return six.str('"') + res + six.str('"')
+        import pipes
+        return pipes.quote(text)
 
 
 def shquote_list(seq):

plumbum/machines/paramiko_machine.py

@@ -3,6 +3,7 @@
 import os
 import stat
 import socket
+from plumbum.commands.base import shquote
 from plumbum.machines.base import PopenAddons
 from plumbum.machines.remote import BaseRemoteMachine
 from plumbum.machines.session import ShellSession
@@ -319,7 +320,7 @@ def popen(self,
         argv.extend(["cd", str(cwd or self.cwd), "&&"])
         if envdelta:
             argv.append("env")
-            argv.extend("%s=%s" % (k, v) for k, v in envdelta.items())
+            argv.extend("%s=%s" % (k, shquote(v)) for k, v in envdelta.items())
         argv.extend(args.formulate())
         cmdline = " ".join(argv)
         logger.debug(cmdline)

plumbum/machines/ssh_machine.py

@@ -136,7 +136,7 @@ def popen(self, args, ssh_opts=(), **kwargs):
             cmdline.extend(["cd", str(self.cwd), "&&"])
             if envdelta:
                 cmdline.append("env")
-                cmdline.extend("%s=%s" % (k, v) for k, v in envdelta.items())
+                cmdline.extend("%s=%s" % (k, shquote(v)) for k, v in envdelta.items())
             if isinstance(args, (tuple, list)):
                 cmdline.extend(args)
             else:

tests/test_remote.py

@@ -257,6 +257,18 @@ def test_env(self):
                 p = rem.which("dummy-executable")
                 assert p == rem.cwd / "not-in-path" / "dummy-executable"
 
+    @pytest.mark.parametrize(
+        "env",
+        ["lala", "-Wl,-O2 -Wl,--sort-common", "{{}}", "''", "!@%_-+=:", "'",
+         "`", "$", "\\"])
+    def test_env_special_characters(self, env):
+        with self._connect() as rem:
+            with pytest.raises(ProcessExecutionError):
+                rem.python("-c", "import os;print(os.environ['FOOBAR72'])")
+            rem.env["FOOBAR72"] = env
+            out = rem.python("-c", "import os;print(os.environ['FOOBAR72'])")
+            assert out.strip() == env
+
     def test_read_write(self):
         with self._connect() as rem:
             with rem.tempdir() as dir: