From 559ba467c09b112ecd7dc8681888b6631fcacba3 Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Tue, 22 Dec 2015 11:11:50 -0800 Subject: [PATCH 1/4] Revert "Convert to bytes later so that make_become_command can jsut operate on text type." This reverts commit c4da5840b5e38aea1740e68f7100256c93dfbb17. Going to do this in the connection plugins --- lib/ansible/plugins/action/__init__.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py index 5383f8afd4..e54898b6db 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/lib/ansible/plugins/action/__init__.py @@ -488,6 +488,8 @@ 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) ''' + # We may need to revisit this later. + cmd = to_bytes(cmd, errors='strict') if executable is not None: cmd = executable + ' -c ' + cmd @@ -504,7 +506,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 From 1ed3a018eb27dd06b08dbad57a162c2865abb635 Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Tue, 22 Dec 2015 11:12:14 -0800 Subject: [PATCH 2/4] Revert "Fix make tests-py3 on devel. Fix for https://github.com/ansible/ansible/issues/13638." This reverts commit e70061334aa99bee466295980f4cd4146096dc29. Going to do this in the connection plugins --- test/units/plugins/action/test_action.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/units/plugins/action/test_action.py b/test/units/plugins/action/test_action.py index dcd0437595..0e47b6a538 100644 --- a/test/units/plugins/action/test_action.py +++ b/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) - play_context.make_become_cmd.assert_called_once_with(b'ECHO', executable=None) + action_base._low_level_execute_command('ECHO', sudoable=True) + 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) - play_context.make_become_cmd.assert_called_once_with(b'ECHO SAME', executable=None) + action_base._low_level_execute_command('ECHO SAME', sudoable=True) + play_context.make_become_cmd.assert_called_once_with('ECHO SAME', executable=None) finally: C.BECOME_ALLOW_SAME_USER = become_allow_same_user From 8d57ffd16bd1025f7b04127fec760c13aca6d6dd Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Tue, 22 Dec 2015 11:12:41 -0800 Subject: [PATCH 3/4] Revert "Transform the command we pass to subprocess into a byte string in _low_level-exec_command" This reverts commit 0c013f592a31c06baac7aadf27d23598f6abe931. Going to do this in the connection plugin --- lib/ansible/plugins/action/__init__.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py index e54898b6db..3f4fff588e 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/lib/ansible/plugins/action/__init__.py @@ -488,8 +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) ''' - # We may need to revisit this later. - cmd = to_bytes(cmd, errors='strict') + if executable is not None: cmd = executable + ' -c ' + cmd From c0a8cd950b909983cdc763f80495595d68597089 Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Mon, 4 Jan 2016 19:23:12 -0800 Subject: [PATCH 4/4] Fix problems with non-ascii values passed as part of the command to connection plugins @drybjed discovered this with non-ascii environment variables and command line arguments to script and raw module. --- lib/ansible/plugins/connection/__init__.py | 1 + lib/ansible/plugins/connection/chroot.py | 2 + lib/ansible/plugins/connection/docker.py | 7 ++- lib/ansible/plugins/connection/jail.py | 6 ++- lib/ansible/plugins/connection/libvirt_lxc.py | 6 ++- lib/ansible/plugins/connection/local.py | 11 ++++- lib/ansible/plugins/connection/ssh.py | 17 +++++-- lib/ansible/plugins/connection/zone.py | 8 ++-- test/integration/unicode-test-script | 7 +++ test/integration/unicode.yml | 45 +++++++++++++++++++ 10 files changed, 97 insertions(+), 13 deletions(-) create mode 100755 test/integration/unicode-test-script diff --git a/lib/ansible/plugins/connection/__init__.py b/lib/ansible/plugins/connection/__init__.py index 06616bac4c..ff00bc0238 100644 --- a/lib/ansible/plugins/connection/__init__.py +++ b/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): diff --git a/lib/ansible/plugins/connection/chroot.py b/lib/ansible/plugins/connection/chroot.py index c86ea1fc35..ba41ffb5d8 100644 --- a/lib/ansible/plugins/connection/chroot.py +++ b/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) diff --git a/lib/ansible/plugins/connection/docker.py b/lib/ansible/plugins/connection/docker.py index 4e08f56a09..ce556a1431 100644 --- a/lib/ansible/plugins/connection/docker.py +++ b/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) diff --git a/lib/ansible/plugins/connection/jail.py b/lib/ansible/plugins/connection/jail.py index e665692543..8f88b6ad28 100644 --- a/lib/ansible/plugins/connection/jail.py +++ b/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) diff --git a/lib/ansible/plugins/connection/libvirt_lxc.py b/lib/ansible/plugins/connection/libvirt_lxc.py index dc82d98404..3bfff8b1c3 100644 --- a/lib/ansible/plugins/connection/libvirt_lxc.py +++ b/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) diff --git a/lib/ansible/plugins/connection/local.py b/lib/ansible/plugins/connection/local.py index e69281d0f3..29b1e9a5ca 100644 --- a/lib/ansible/plugins/connection/local.py +++ b/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), diff --git a/lib/ansible/plugins/connection/ssh.py b/lib/ansible/plugins/connection/ssh.py index a2abcf20ae..074f6aaa8a 100644 --- a/lib/ansible/plugins/connection/ssh.py +++ b/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() diff --git a/lib/ansible/plugins/connection/zone.py b/lib/ansible/plugins/connection/zone.py index 75d7db545d..b65c80b73f 100644 --- a/lib/ansible/plugins/connection/zone.py +++ b/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.zlogin_cmd = self._search_executable('zlogin') + self.zoneadm_cmd = to_bytes(self._search_executable('zoneadm')) + 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, diff --git a/test/integration/unicode-test-script b/test/integration/unicode-test-script new file mode 100755 index 0000000000..340f2a9f5b --- /dev/null +++ b/test/integration/unicode-test-script @@ -0,0 +1,7 @@ +#!/bin/sh + +echo "Non-ascii arguments:" +echo $@ + +echo "Non-ascii Env var:" +echo $option diff --git a/test/integration/unicode.yml b/test/integration/unicode.yml index 6e8e073a79..f38bf8f5e8 100644 --- a/test/integration/unicode.yml +++ b/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