Merge pull request #13729 from ansible/connection-plugins-unicode-fix

Connection plugins unicode fix

lib/ansible/plugins/action/__init__.py

@@ -488,6 +488,7 @@ class ActionBase(with_metaclass(ABCMeta, object)):
             verbatim, then this won't work.  May have to use some sort of
             replacement strategy (python3 could use surrogateescape)
         '''
+
         if executable is not None:
             cmd = executable + ' -c ' + cmd
 
@@ -504,7 +505,7 @@ class ActionBase(with_metaclass(ABCMeta, object)):
             cmd = self._play_context.make_become_cmd(cmd, executable=executable)
 
         display.debug("_low_level_execute_command(): executing: %s" % (cmd,))
-        rc, stdout, stderr = self._connection.exec_command(to_bytes(cmd, errors='strict'), in_data=in_data, sudoable=sudoable)
+        rc, stdout, stderr = self._connection.exec_command(cmd, in_data=in_data, sudoable=sudoable)
 
         # stdout and stderr may be either a file-like or a bytes object.
         # Convert either one to a text type

lib/ansible/plugins/connection/__init__.py

@@ -91,6 +91,7 @@ class ConnectionBase(with_metaclass(ABCMeta, object)):
 
     @property
     def connected(self):
+        '''Read-only property holding whether the connection to the remote host is active or closed.'''
         return self._connected
 
     def _become_method_supported(self):

lib/ansible/plugins/connection/chroot.py

@@ -30,6 +30,7 @@ from ansible import constants as C
 from ansible.errors import AnsibleError
 from ansible.plugins.connection import ConnectionBase
 from ansible.module_utils.basic import is_executable
+from ansible.utils.unicode import to_bytes
 
 try:
     from __main__ import display
@@ -90,6 +91,7 @@ class Connection(ConnectionBase):
         local_cmd = [self.chroot_cmd, self.chroot, executable, '-c', cmd]
 
         display.vvv("EXEC %s" % (local_cmd), host=self.chroot)
+        local_cmd = map(to_bytes, local_cmd)
         p = subprocess.Popen(local_cmd, shell=False, stdin=stdin,
                 stdout=subprocess.PIPE, stderr=subprocess.PIPE)
 

lib/ansible/plugins/connection/docker.py

@@ -36,6 +36,7 @@ from distutils.version import LooseVersion
 import ansible.constants as C
 from ansible.errors import AnsibleError, AnsibleFileNotFound
 from ansible.plugins.connection import ConnectionBase
+from ansible.utils.unicode import to_bytes
 
 try:
     from __main__ import display
@@ -125,7 +126,8 @@ class Connection(ConnectionBase):
         # -i is needed to keep stdin open which allows pipelining to work
         local_cmd = [self.docker_cmd, "exec", '-i', self._play_context.remote_addr, executable, '-c', cmd]
 
-        display.vvv("EXEC %s" % (local_cmd), host=self._play_context.remote_addr)
+        display.vvv("EXEC %s" % (local_cmd,), host=self._play_context.remote_addr)
+        local_cmd = map(to_bytes, local_cmd)
         p = subprocess.Popen(local_cmd, shell=False, stdin=subprocess.PIPE,
                              stdout=subprocess.PIPE, stderr=subprocess.PIPE)
 
@@ -159,6 +161,7 @@ class Connection(ConnectionBase):
         if self.can_copy_bothways:
             # only docker >= 1.8.1 can do this natively
             args = [ self.docker_cmd, "cp", in_path, "%s:%s" % (self._play_context.remote_addr, out_path) ]
+            args = map(to_bytes, args)
             p = subprocess.Popen(args, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
             stdout, stderr = p.communicate()
             if p.returncode != 0:
@@ -170,6 +173,7 @@ class Connection(ConnectionBase):
             executable = C.DEFAULT_EXECUTABLE.split()[0] if C.DEFAULT_EXECUTABLE else '/bin/sh'
             args = [self.docker_cmd, "exec", "-i", self._play_context.remote_addr, executable, "-c",
                     "dd of={0} bs={1}".format(out_path, BUFSIZE)]
+            args = map(to_bytes, args)
             with open(in_path, 'rb') as in_file:
                 try:
                     p = subprocess.Popen(args, stdin=in_file,
@@ -192,6 +196,7 @@ class Connection(ConnectionBase):
         out_dir = os.path.dirname(out_path)
 
         args = [self.docker_cmd, "cp", "%s:%s" % (self._play_context.remote_addr, in_path), out_dir]
+        args = map(to_bytes, args)
 
         p = subprocess.Popen(args, stdin=subprocess.PIPE,
                 stdout=subprocess.PIPE, stderr=subprocess.PIPE)

lib/ansible/plugins/connection/jail.py

@@ -30,6 +30,7 @@ import traceback
 from ansible import constants as C
 from ansible.errors import AnsibleError
 from ansible.plugins.connection import ConnectionBase
+from ansible.utils.unicode import to_bytes
 
 try:
     from __main__ import display
@@ -83,7 +84,7 @@ class Connection(ConnectionBase):
         return stdout.split()
 
     def get_jail_path(self):
-        p = subprocess.Popen([self.jls_cmd, '-j', self.jail, '-q', 'path'],
+        p = subprocess.Popen([self.jls_cmd, '-j', to_bytes(self.jail), '-q', 'path'],
                              stdin=subprocess.PIPE,
                              stdout=subprocess.PIPE, stderr=subprocess.PIPE)
 
@@ -109,7 +110,8 @@ class Connection(ConnectionBase):
         executable = C.DEFAULT_EXECUTABLE.split()[0] if C.DEFAULT_EXECUTABLE else '/bin/sh'
         local_cmd = [self.jexec_cmd, self.jail, executable, '-c', cmd]
 
-        display.vvv("EXEC %s" % (local_cmd), host=self.jail)
+        display.vvv("EXEC %s" % (local_cmd,), host=self.jail)
+        local_cmd = map(to_bytes, local_cmd)
         p = subprocess.Popen(local_cmd, shell=False, stdin=stdin,
                 stdout=subprocess.PIPE, stderr=subprocess.PIPE)
 

lib/ansible/plugins/connection/libvirt_lxc.py

@@ -30,6 +30,7 @@ import traceback
 from ansible import constants as C
 from ansible.errors import AnsibleError
 from ansible.plugins.connection import ConnectionBase
+from ansible.utils.unicode import to_bytes
 
 try:
     from __main__ import display
@@ -65,7 +66,7 @@ class Connection(ConnectionBase):
         return cmd
 
     def _check_domain(self, domain):
-        p = subprocess.Popen([self.virsh, '-q', '-c', 'lxc:///', 'dominfo', domain],
+        p = subprocess.Popen([self.virsh, '-q', '-c', 'lxc:///', 'dominfo', to_bytes(domain)],
                              stdout=subprocess.PIPE, stderr=subprocess.PIPE)
         p.communicate()
         if p.returncode:
@@ -89,7 +90,8 @@ class Connection(ConnectionBase):
         executable = C.DEFAULT_EXECUTABLE.split()[0] if C.DEFAULT_EXECUTABLE else '/bin/sh'
         local_cmd = [self.virsh, '-q', '-c', 'lxc:///', 'lxc-enter-namespace', self.lxc, '--', executable , '-c', cmd]
 
-        display.vvv("EXEC %s" % (local_cmd), host=self.lxc)
+        display.vvv("EXEC %s" % (local_cmd,), host=self.lxc)
+        local_cmd = map(to_bytes, local_cmd)
         p = subprocess.Popen(local_cmd, shell=False, stdin=stdin,
                 stdout=subprocess.PIPE, stderr=subprocess.PIPE)
 

lib/ansible/plugins/connection/local.py

@@ -25,10 +25,13 @@ import select
 import fcntl
 import getpass
 
+from ansible.compat.six import text_type, binary_type
+
 import ansible.constants as C
 
 from ansible.errors import AnsibleError, AnsibleFileNotFound
 from ansible.plugins.connection import ConnectionBase
+from ansible.utils.unicode import to_bytes
 
 try:
     from __main__ import display
@@ -69,9 +72,15 @@ class Connection(ConnectionBase):
             raise AnsibleError("Internal Error: this module does not support optimized module pipelining")
         executable = C.DEFAULT_EXECUTABLE.split()[0] if C.DEFAULT_EXECUTABLE else None
 
-        display.vvv("{0} EXEC {1}".format(self._play_context.remote_addr, cmd))
+        display.vvv(u"{0} EXEC {1}".format(self._play_context.remote_addr, cmd))
         # FIXME: cwd= needs to be set to the basedir of the playbook
         display.debug("opening command with Popen()")
+
+        if isinstance(cmd, (text_type, binary_type)):
+            cmd = to_bytes(cmd)
+        else:
+            cmd = map(to_bytes, cmd)
+
         p = subprocess.Popen(
             cmd,
             shell=isinstance(cmd, basestring),

lib/ansible/plugins/connection/ssh.py

@@ -33,6 +33,7 @@ from ansible.errors import AnsibleError, AnsibleConnectionFailure, AnsibleFileNo
 from ansible.plugins.connection import ConnectionBase
 from ansible.utils.path import unfrackpath, makedirs_safe
 from ansible.utils.unicode import to_bytes, to_unicode
+from ansible.compat.six import text_type, binary_type
 
 try:
     from __main__ import display
@@ -320,7 +321,7 @@ class Connection(ConnectionBase):
         '''
 
         display_cmd = map(pipes.quote, cmd)
-        display.vvv('SSH: EXEC {0}'.format(' '.join(display_cmd)), host=self.host)
+        display.vvv(u'SSH: EXEC {0}'.format(u' '.join(display_cmd)), host=self.host)
 
         # Start the given command. If we don't need to pipeline data, we can try
         # to use a pseudo-tty (ssh will have been invoked with -tt). If we are
@@ -328,6 +329,12 @@ class Connection(ConnectionBase):
         # old pipes.
 
         p = None
+
+        if isinstance(cmd, (text_type, binary_type)):
+            cmd = to_bytes(cmd)
+        else:
+            cmd = map(to_bytes, cmd)
+
         if not in_data:
             try:
                 # Make sure stdin is a proper pty to avoid tcgetattr errors
@@ -365,7 +372,7 @@ class Connection(ConnectionBase):
         # only when using ssh. Otherwise we can send initial data straightaway.
 
         state = states.index('ready_to_send')
-        if 'ssh' in cmd:
+        if b'ssh' in cmd:
             if self._play_context.prompt:
                 # We're requesting escalation with a password, so we have to
                 # wait for a password prompt.
@@ -538,7 +545,7 @@ class Connection(ConnectionBase):
         stdin.close()
 
         if C.HOST_KEY_CHECKING:
-            if cmd[0] == "sshpass" and p.returncode == 6:
+            if cmd[0] == b"sshpass" and p.returncode == 6:
                 raise AnsibleError('Using a SSH password instead of a key is not possible because Host Key checking is enabled and sshpass does not support this.  Please add this host\'s fingerprint to your known_hosts file to manage this host.')
 
         controlpersisterror = 'Bad configuration option: ControlPersist' in stderr or 'unknown configuration option: ControlPersist' in stderr
@@ -600,7 +607,7 @@ class Connection(ConnectionBase):
                     raise AnsibleConnectionFailure("Failed to connect to the host via ssh.")
             except (AnsibleConnectionFailure, Exception) as e:
                 if attempt == remaining_tries - 1:
-                    raise e
+                    raise
                 else:
                     pause = 2 ** attempt - 1
                     if pause > 30:
@@ -674,6 +681,8 @@ class Connection(ConnectionBase):
         # temporarily disabled as we are forced to currently close connections after every task because of winrm
         # if self._connected and self._persistent:
         #     cmd = self._build_command('ssh', '-O', 'stop', self.host)
+        #
+        #     cmd = map(to_bytes, cmd)
         #     p = subprocess.Popen(cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
         #     stdout, stderr = p.communicate()
 

lib/ansible/plugins/connection/zone.py

@@ -31,6 +31,7 @@ import traceback
 from ansible import constants as C
 from ansible.errors import AnsibleError
 from ansible.plugins.connection import ConnectionBase
+from ansible.utils import to_bytes
 
 try:
     from __main__ import display
@@ -56,8 +57,8 @@ class Connection(ConnectionBase):
         if os.geteuid() != 0:
             raise AnsibleError("zone connection requires running as root")
 
-        self.zoneadm_cmd = self._search_executable('zoneadm')
+        self.zoneadm_cmd = to_bytes(self._search_executable('zoneadm'))
-        self.zlogin_cmd = self._search_executable('zlogin')
+        self.zlogin_cmd = to_bytes(self._search_executable('zlogin'))
 
         if self.zone not in self.list_zones():
             raise AnsibleError("incorrect zone name %s" % self.zone)
@@ -86,7 +87,7 @@ class Connection(ConnectionBase):
     def get_zone_path(self):
         #solaris10vm# zoneadm -z cswbuild list -p
         #-:cswbuild:installed:/zones/cswbuild:479f3c4b-d0c6-e97b-cd04-fd58f2c0238e:native:shared
-        process = subprocess.Popen([self.zoneadm_cmd, '-z', self.zone, 'list', '-p'],
+        process = subprocess.Popen([self.zoneadm_cmd, '-z', to_bytes(self.zone), 'list', '-p'],
                              stdin=subprocess.PIPE,
                              stdout=subprocess.PIPE, stderr=subprocess.PIPE)
 
@@ -113,6 +114,7 @@ class Connection(ConnectionBase):
         # this through /bin/sh -c here.  Instead it goes through the shell
         # that zlogin selects.
         local_cmd = [self.zlogin_cmd, self.zone, cmd]
+        local_cmd = map(to_bytes, local_cmd)
 
         display.vvv("EXEC %s" % (local_cmd), host=self.zone)
         p = subprocess.Popen(local_cmd, shell=False, stdin=stdin,

test/integration/unicode-test-script

@@ -0,0 +1,7 @@
+#!/bin/sh
+
+echo "Non-ascii arguments:"
+echo $@
+
+echo "Non-ascii Env var:"
+echo $option

test/integration/unicode.yml

@@ -49,6 +49,51 @@
         that:
           - "'¯ ° ± ² ³ ´ µ ¶ · ¸ ¹ º » ¼ ½ ¾ ¿ À Á Â Ã Ä Å Æ Ç È É Ê Ë Ì Í Î Ï Ð Ñ Ò Ó Ô Õ Ö ×' in output.stdout_lines"
 
+    - name: Run raw with non-ascii options
+      raw: "/bin/echo Zażółć gęślą jaźń"
+      register: results
+
+    - name: Check that raw output the right thing
+      assert:
+        that:
+          - "'Zażółć gęślą jaźń' in results.stdout_lines"
+
+    - name: Run a script with non-ascii options and environment
+      script: unicode-test-script --option "Zażółć gęślą jaźń"
+      environment:
+        option: Zażółć
+      register: results
+
+    - name: Check that script output includes the nonascii arguments and environment values
+      assert:
+        that:
+          - "'--option Zażółć gęślą jaźń' in results.stdout_lines"
+          - "'Zażółć' in results.stdout_lines"
+
+    - name: Ping with non-ascii environment variable and option
+      ping:
+        data: "Zażółć gęślą jaźń"
+      environment:
+        option: Zażółć
+      register: results
+
+    - name: Check that ping with non-ascii data was correct
+      assert:
+        that:
+          - "'Zażółć gęślą jaźń' == results.ping"
+
+    - name: Command that echos a non-ascii env var
+      command: "echo $option"
+      environment:
+        option: Zażółć
+      register: results
+
+    - name: Check that a non-ascii env var was passed to the command module
+      assert:
+        that:
+          - "'Zażółć' in results.stdout_lines"
+
+
 - name: 'A play for hosts in group: ĪīĬĭ'
   hosts: 'ĪīĬĭ'
   gather_facts: true

test/units/plugins/action/test_action.py

@@ -42,14 +42,14 @@ class TestActionBase(unittest.TestCase):
 
         play_context.become = True
         play_context.become_user = play_context.remote_user = 'root'
-        play_context.make_become_cmd = Mock(return_value=b'CMD')
+        play_context.make_become_cmd = Mock(return_value='CMD')
 
-        action_base._low_level_execute_command(b'ECHO', sudoable=True)
+        action_base._low_level_execute_command('ECHO', sudoable=True)
         play_context.make_become_cmd.assert_not_called()
 
         play_context.remote_user = 'apo'
-        action_base._low_level_execute_command(b'ECHO', sudoable=True)
+        action_base._low_level_execute_command('ECHO', sudoable=True)
-        play_context.make_become_cmd.assert_called_once_with(b'ECHO', executable=None)
+        play_context.make_become_cmd.assert_called_once_with('ECHO', executable=None)
 
         play_context.make_become_cmd.reset_mock()
 
@@ -57,7 +57,7 @@ class TestActionBase(unittest.TestCase):
         C.BECOME_ALLOW_SAME_USER = True
         try:
             play_context.remote_user = 'root'
-            action_base._low_level_execute_command(b'ECHO SAME', sudoable=True)
+            action_base._low_level_execute_command('ECHO SAME', sudoable=True)
-            play_context.make_become_cmd.assert_called_once_with(b'ECHO SAME', executable=None)
+            play_context.make_become_cmd.assert_called_once_with('ECHO SAME', executable=None)
         finally:
             C.BECOME_ALLOW_SAME_USER = become_allow_same_user