ensure bytes in run_command (#58411)

* ensure we pass bytes to run_command

* changed tests for new behaviour

* dont b the pytest

* fixes by sivel

changelogs/fragments/ensure_bytes_run_command.yml

@@ -0,0 +1,2 @@
+bugfixes:
+    - ensure run_command passes bytes to Popen, which is what it expects.

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)

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'])