Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open pywinrm shell sessions with UTF-8 codepage #6038

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
~~~~~

Expand Down
8 changes: 6 additions & 2 deletions contrib/runners/winrm_runner/tests/unit/test_winrm_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down Expand Up @@ -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"]
Expand Down
34 changes: 19 additions & 15 deletions contrib/runners/winrm_runner/winrm_runner/winrm_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import base64
import os
import re
import six
import time

from base64 import b64encode
Expand Down Expand Up @@ -194,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:
Expand Down Expand Up @@ -257,10 +263,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
Expand Down Expand Up @@ -316,7 +322,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}"
Expand Down Expand Up @@ -446,7 +452,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"
Expand All @@ -459,7 +465,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 += "}"
Expand All @@ -473,7 +479,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
Expand All @@ -486,9 +492,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)
Expand Down
Loading