From fe9aaf9f19877e0f7b8fb23dcff421a9efa82c72 Mon Sep 17 00:00:00 2001 From: Daniel Porter Date: Wed, 18 Oct 2023 20:02:58 +0100 Subject: [PATCH 1/2] Drop six use for binary/string type identification With Python 3.6 being the minimum supported version, dropping six here and utilising 3.x type identifiers. --- .../winrm_runner/winrm_runner/winrm_base.py | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/contrib/runners/winrm_runner/winrm_runner/winrm_base.py b/contrib/runners/winrm_runner/winrm_runner/winrm_base.py index f63d9c4be9..b0939ac13e 100644 --- a/contrib/runners/winrm_runner/winrm_runner/winrm_base.py +++ b/contrib/runners/winrm_runner/winrm_runner/winrm_base.py @@ -18,7 +18,6 @@ import base64 import os import re -import six import time from base64 import b64encode @@ -257,10 +256,10 @@ def _translate_response(self, response): } # Ensure stdout and stderr is always a string - if isinstance(result["stdout"], six.binary_type): + if isinstance(result["stdout"], bytes): result["stdout"] = result["stdout"].decode("utf-8") - if isinstance(result["stderr"], six.binary_type): + if isinstance(result["stderr"], bytes): result["stderr"] = result["stderr"].decode("utf-8") # automatically convert result stdout/stderr from JSON strings to @@ -316,7 +315,7 @@ def _upload(self, src_path_or_data, dst_path): def _upload_chunk(self, dst_path, src_data): # adapted from https://github.com/diyan/pywinrm/issues/18 - if not isinstance(src_data, six.binary_type): + if not isinstance(src_data, bytes): src_data = src_data.encode("utf-8") ps = """$filePath = "{dst_path}" @@ -446,7 +445,7 @@ def _param_to_ps(self, param): ps_str = "" if param is None: ps_str = "$null" - elif isinstance(param, six.string_types): + elif isinstance(param, str): ps_str = '"' + self._multireplace(param, PS_ESCAPE_SEQUENCES) + '"' elif isinstance(param, bool): ps_str = "$true" if param else "$false" @@ -459,7 +458,7 @@ def _param_to_ps(self, param): ps_str += "; ".join( [ (self._param_to_ps(k) + " = " + self._param_to_ps(v)) - for k, v in six.iteritems(param) + for k, v in param.items() ] ) ps_str += "}" @@ -473,7 +472,7 @@ def _transform_params_to_ps(self, positional_args, named_args): positional_args[i] = self._param_to_ps(arg) if named_args: - for key, value in six.iteritems(named_args): + for key, value in named_args.items(): named_args[key] = self._param_to_ps(value) return positional_args, named_args @@ -486,9 +485,7 @@ def create_ps_params_string(self, positional_args, named_args): # concatenate them into a long string ps_params_str = "" if named_args: - ps_params_str += " ".join( - [(k + " " + v) for k, v in six.iteritems(named_args)] - ) + ps_params_str += " ".join([(k + " " + v) for k, v in named_args.items()]) ps_params_str += " " if positional_args: ps_params_str += " ".join(positional_args) From 5cdc0e71f0c3f43e36a8990f09a36ede91835697 Mon Sep 17 00:00:00 2001 From: Daniel Porter Date: Wed, 18 Oct 2023 20:12:51 +0100 Subject: [PATCH 2/2] Use UTF-8 codepage in pywinrm shells This change opens a shell with the 65001 codepage, ensuring raw string responses are UTF-8 encoded. Fixes #6034. --- CHANGELOG.rst | 3 +++ .../winrm_runner/tests/unit/test_winrm_base.py | 8 ++++++-- .../winrm_runner/winrm_runner/winrm_base.py | 17 ++++++++++++----- 3 files changed, 21 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index e3d6e797e7..50ac1efec7 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -15,6 +15,9 @@ Fixed * Avoid logging sensitive information in debug (fix #5977) +* Shells via `pywinrm` are initialized with the 65001 codepage to ensure raw string responses are UTF-8 (fix #6034) + Contributed by @stealthii + Added ~~~~~ diff --git a/contrib/runners/winrm_runner/tests/unit/test_winrm_base.py b/contrib/runners/winrm_runner/tests/unit/test_winrm_base.py index 1fb5f8ac2e..c2fd1c56cc 100644 --- a/contrib/runners/winrm_runner/tests/unit/test_winrm_base.py +++ b/contrib/runners/winrm_runner/tests/unit/test_winrm_base.py @@ -304,7 +304,9 @@ def test_winrm_run_cmd(self): self.assertEqual(result.__dict__, expected_response.__dict__) mock_protocol.open_shell.assert_called_with( - env_vars={"PATH": "C:\\st2\\bin"}, working_directory="C:\\st2" + working_directory="C:\\st2", + env_vars={"PATH": "C:\\st2\\bin"}, + codepage=65001, ) mock_protocol.run_command.assert_called_with( 123, "fake-command", ["arg1", "arg2"] @@ -336,7 +338,9 @@ def test_winrm_run_cmd_timeout(self, mock_get_command_output): self.assertEqual(result.__dict__, expected_response.__dict__) mock_protocol.open_shell.assert_called_with( - env_vars={"PATH": "C:\\st2\\bin"}, working_directory="C:\\st2" + working_directory="C:\\st2", + env_vars={"PATH": "C:\\st2\\bin"}, + codepage=65001, ) mock_protocol.run_command.assert_called_with( 123, "fake-command", ["arg1", "arg2"] diff --git a/contrib/runners/winrm_runner/winrm_runner/winrm_base.py b/contrib/runners/winrm_runner/winrm_runner/winrm_base.py index b0939ac13e..06131386dd 100644 --- a/contrib/runners/winrm_runner/winrm_runner/winrm_base.py +++ b/contrib/runners/winrm_runner/winrm_runner/winrm_base.py @@ -193,11 +193,18 @@ def _winrm_get_command_output(self, protocol, shell_id, command_id): return b"".join(stdout_buffer), b"".join(stderr_buffer), return_code def _winrm_run_cmd(self, session, command, args=(), env=None, cwd=None): - # NOTE: this is copied from pywinrm because it doesn't support - # passing env and working_directory from the Session.run_cmd. - # It also doesn't support timeouts. All of these things have been - # added - shell_id = session.protocol.open_shell(env_vars=env, working_directory=cwd) + """Run a command on the remote host and return the standard output and + error as a tuple. + + Extended from pywinrm to support passing env and cwd to open_shell, + as well as enforcing the UTF-8 codepage. Also supports timeouts. + + """ + shell_id = session.protocol.open_shell( + working_directory=cwd, + env_vars=env, + codepage=65001, + ) command_id = session.protocol.run_command(shell_id, command, args) # try/catch is for custom timeout handing (StackStorm custom) try: