diff --git a/changelogs/fragments/ensure_bytes_run_command.yml b/changelogs/fragments/ensure_bytes_run_command.yml new file mode 100644 index 00000000000..ad620c60580 --- /dev/null +++ b/changelogs/fragments/ensure_bytes_run_command.yml @@ -0,0 +1,2 @@ +bugfixes: + - ensure run_command passes bytes to Popen, which is what it expects. diff --git a/lib/ansible/module_utils/basic.py b/lib/ansible/module_utils/basic.py index 6ff8a389760..291ccadf967 100644 --- a/lib/ansible/module_utils/basic.py +++ b/lib/ansible/module_utils/basic.py @@ -2452,13 +2452,16 @@ class AnsibleModule(object): # stringify args for unsafe/direct shell usage if isinstance(args, list): - args = " ".join([shlex_quote(x) for x in args]) + args = b" ".join([to_bytes(shlex_quote(x), errors='surrogate_or_strict') for x in args]) + else: + args = to_bytes(args, errors='surrogate_or_strict') # not set explicitly, check if set by controller if executable: - args = [executable, '-c', args] + executable = to_bytes(executable, errors='surrogate_or_strict') + args = [executable, b'-c', args] elif self._shell not in (None, '/bin/sh'): - args = [self._shell, '-c', args] + args = [to_bytes(self._shell, errors='surrogate_or_strict'), b'-c', args] else: shell = True else: @@ -2474,9 +2477,9 @@ class AnsibleModule(object): # expand ``~`` in paths, and all environment vars if expand_user_and_vars: - args = [os.path.expanduser(os.path.expandvars(x)) for x in args if x is not None] + args = [to_bytes(os.path.expanduser(os.path.expandvars(x)), errors='surrogate_or_strict') for x in args if x is not None] else: - args = [x for x in args if x is not None] + args = [to_bytes(x, errors='surrogate_or_strict') for x in args if x is not None] prompt_re = None if prompt_regex: @@ -2543,7 +2546,7 @@ class AnsibleModule(object): # make sure we're in the right working directory if cwd and os.path.isdir(cwd): - cwd = os.path.abspath(os.path.expanduser(cwd)) + cwd = to_bytes(os.path.abspath(os.path.expanduser(cwd)), errors='surrogate_or_strict') kwargs['cwd'] = cwd try: os.chdir(cwd) diff --git a/test/units/module_utils/basic/test_run_command.py b/test/units/module_utils/basic/test_run_command.py index 607f3f47dad..e41a4960b45 100644 --- a/test/units/module_utils/basic/test_run_command.py +++ b/test/units/module_utils/basic/test_run_command.py @@ -90,8 +90,8 @@ def rc_am(mocker, am, mock_os, mock_subprocess): class TestRunCommandArgs: # Format is command as passed to run_command, command to Popen as list, command to Popen as string ARGS_DATA = ( - (['/bin/ls', 'a', 'b', 'c'], ['/bin/ls', 'a', 'b', 'c'], '/bin/ls a b c'), - ('/bin/ls a " b" "c "', ['/bin/ls', 'a', ' b', 'c '], '/bin/ls a " b" "c "'), + (['/bin/ls', 'a', 'b', 'c'], [b'/bin/ls', b'a', b'b', b'c'], b'/bin/ls a b c'), + ('/bin/ls a " b" "c "', [b'/bin/ls', b'a', b' b', b'c '], b'/bin/ls a " b" "c "'), ) # pylint bug: https://github.com/PyCQA/pylint/issues/511 @@ -119,13 +119,13 @@ class TestRunCommandCwd: def test_cwd(self, mocker, rc_am): rc_am._os.getcwd.return_value = '/old' rc_am.run_command('/bin/ls', cwd='/new') - assert rc_am._os.chdir.mock_calls == [mocker.call('/new'), mocker.call('/old'), ] + assert rc_am._os.chdir.mock_calls == [mocker.call(b'/new'), mocker.call('/old'), ] @pytest.mark.parametrize('stdin', [{}], indirect=['stdin']) def test_cwd_relative_path(self, mocker, rc_am): rc_am._os.getcwd.return_value = '/old' rc_am.run_command('/bin/ls', cwd='sub-dir') - assert rc_am._os.chdir.mock_calls == [mocker.call('/old/sub-dir'), mocker.call('/old'), ] + assert rc_am._os.chdir.mock_calls == [mocker.call(b'/old/sub-dir'), mocker.call('/old'), ] @pytest.mark.parametrize('stdin', [{}], indirect=['stdin']) def test_cwd_not_a_dir(self, mocker, rc_am): @@ -134,14 +134,6 @@ class TestRunCommandCwd: rc_am.run_command('/bin/ls', cwd='/not-a-dir') assert rc_am._os.chdir.mock_calls == [mocker.call('/old'), ] - @pytest.mark.parametrize('stdin', [{}], indirect=['stdin']) - def test_cwd_inaccessible(self, rc_am): - with pytest.raises(SystemExit): - rc_am.run_command('/bin/ls', cwd='/inaccessible') - assert rc_am.fail_json.called - args, kwargs = rc_am.fail_json.call_args - assert kwargs['rc'] == errno.EPERM - class TestRunCommandPrompt: @pytest.mark.parametrize('stdin', [{}], indirect=['stdin'])