From 7c7493feb53429577efca2c4b0380af03ddc149b Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Thu, 11 Dec 2014 23:55:50 +0100 Subject: [PATCH] Port processutils to Python 3 Add encoding and errors parameters to execute() and ssh_execute(). By default, use the locale encoding in strict mode on Python 2, or the locale encoding with the 'surrogateescape' error handler on Python 3. Fix also unit tests to use bytes strings for stdin, stdout and stderr. Without this change, tests are failing with Python 3 when run with: PYTHON="python -bb" testr run Using -bb, Python 3 raises a TypeError when a bytes string is casted to a text string, which occurred in many places. Change-Id: I655d5abf932c9a104e3ab487e23c372377f7096a --- oslo_concurrency/processutils.py | 21 ++++++++++++++++++- .../tests/unit/test_processutils.py | 16 +++++++------- tests/test_processutils.py | 16 +++++++------- 3 files changed, 36 insertions(+), 17 deletions(-) diff --git a/oslo_concurrency/processutils.py b/oslo_concurrency/processutils.py index 051acec..78f8f14 100644 --- a/oslo_concurrency/processutils.py +++ b/oslo_concurrency/processutils.py @@ -23,6 +23,7 @@ import os import random import shlex import signal +import sys import time from oslo.utils import importutils @@ -142,6 +143,13 @@ def execute(*cmd, **kwargs): last attempt, and LOG_ALL_ERRORS requires logging on each occurence of an error. :type log_errors: integer. + :param encoding: encoding used to decode stdout and stderr, + sys.getfilesystemencoding() by default. + :type encoding: str + :param errors: error handler used to decode stdout and stderr, + default: 'surrogateescape' on Python 3, + 'strict' on Python 2. + :type errors: str :returns: (stdout, stderr) from process execution :raises: :class:`UnknownArgumentError` on receiving unknown arguments @@ -160,6 +168,8 @@ def execute(*cmd, **kwargs): shell = kwargs.pop('shell', False) loglevel = kwargs.pop('loglevel', logging.DEBUG) log_errors = kwargs.pop('log_errors', None) + encoding = kwargs.pop('encoding', sys.getfilesystemencoding()) + errors = kwargs.pop('errors', 'surrogateescape' if six.PY3 else 'strict') if isinstance(check_exit_code, bool): ignore_exit_code = not check_exit_code @@ -214,6 +224,11 @@ def execute(*cmd, **kwargs): end_time = time.time() - start_time LOG.log(loglevel, 'CMD "%s" returned: %s in %ss' % (sanitized_cmd, _returncode, end_time)) + if result is not None: + (stdout, stderr) = result + stdout = stdout.decode(encoding, errors) + stderr = stderr.decode(encoding, errors) + result = (stdout, stderr) if not ignore_exit_code and _returncode not in check_exit_code: (stdout, stderr) = result sanitized_stdout = strutils.mask_password(stdout) @@ -292,7 +307,9 @@ def trycmd(*args, **kwargs): def ssh_execute(ssh, cmd, process_input=None, - addl_env=None, check_exit_code=True): + addl_env=None, check_exit_code=True, + encoding=sys.getfilesystemencoding(), + errors='surrogateescape' if six.PY3 else 'strict'): sanitized_cmd = strutils.mask_password(cmd) LOG.debug('Running cmd (SSH): %s', sanitized_cmd) if addl_env: @@ -308,8 +325,10 @@ def ssh_execute(ssh, cmd, process_input=None, # NOTE(justinsb): This seems suspicious... # ...other SSH clients have buffering issues with this approach stdout = stdout_stream.read() + stdout = stdout.decode(encoding, errors) sanitized_stdout = strutils.mask_password(stdout) stderr = stderr_stream.read() + stderr = stderr.decode(encoding, errors) sanitized_stderr = strutils.mask_password(stderr) stdin_stream.close() diff --git a/oslo_concurrency/tests/unit/test_processutils.py b/oslo_concurrency/tests/unit/test_processutils.py index 50ea611..65b4e2d 100644 --- a/oslo_concurrency/tests/unit/test_processutils.py +++ b/oslo_concurrency/tests/unit/test_processutils.py @@ -295,7 +295,7 @@ grep foo out, err = processutils.execute('/usr/bin/env', env_variables=env_vars) - self.assertIn(b'SUPER_UNIQUE_VAR=The answer is 42', out) + self.assertIn('SUPER_UNIQUE_VAR=The answer is 42', out) def test_exception_and_masking(self): tmpfilename = self.create_tempfiles( @@ -426,7 +426,7 @@ class FakeSshChannel(object): return self.rc -class FakeSshStream(six.StringIO): +class FakeSshStream(six.BytesIO): def setup_channel(self, rc): self.channel = FakeSshChannel(rc) @@ -436,11 +436,11 @@ class FakeSshConnection(object): self.rc = rc def exec_command(self, cmd): - stdout = FakeSshStream('stdout') + stdout = FakeSshStream(b'stdout') stdout.setup_channel(self.rc) - return (six.StringIO(), + return (six.BytesIO(), stdout, - six.StringIO('stderr')) + six.BytesIO(b'stderr')) class SshExecuteTestCase(test_base.BaseTestCase): @@ -465,13 +465,13 @@ class SshExecuteTestCase(test_base.BaseTestCase): def _test_compromising_ssh(self, rc, check): fixture = self.useFixture(fixtures.FakeLogger(level=logging.DEBUG)) - fake_stdin = six.StringIO() + fake_stdin = six.BytesIO() fake_stdout = mock.Mock() fake_stdout.channel.recv_exit_status.return_value = rc - fake_stdout.read.return_value = 'password="secret"' + fake_stdout.read.return_value = b'password="secret"' - fake_stderr = six.StringIO('password="foobar"') + fake_stderr = six.BytesIO(b'password="foobar"') command = 'ls --password="bar"' diff --git a/tests/test_processutils.py b/tests/test_processutils.py index 179a5e3..ca15385 100644 --- a/tests/test_processutils.py +++ b/tests/test_processutils.py @@ -296,7 +296,7 @@ grep foo out, err = processutils.execute('/usr/bin/env', env_variables=env_vars) - self.assertIn(b'SUPER_UNIQUE_VAR=The answer is 42', out) + self.assertIn('SUPER_UNIQUE_VAR=The answer is 42', out) def test_exception_and_masking(self): tmpfilename = self.create_tempfiles( @@ -427,7 +427,7 @@ class FakeSshChannel(object): return self.rc -class FakeSshStream(six.StringIO): +class FakeSshStream(six.BytesIO): def setup_channel(self, rc): self.channel = FakeSshChannel(rc) @@ -437,11 +437,11 @@ class FakeSshConnection(object): self.rc = rc def exec_command(self, cmd): - stdout = FakeSshStream('stdout') + stdout = FakeSshStream(b'stdout') stdout.setup_channel(self.rc) - return (six.StringIO(), + return (six.BytesIO(), stdout, - six.StringIO('stderr')) + six.BytesIO(b'stderr')) class SshExecuteTestCase(test_base.BaseTestCase): @@ -466,13 +466,13 @@ class SshExecuteTestCase(test_base.BaseTestCase): def _test_compromising_ssh(self, rc, check): fixture = self.useFixture(fixtures.FakeLogger(level=logging.DEBUG)) - fake_stdin = six.StringIO() + fake_stdin = six.BytesIO() fake_stdout = mock.Mock() fake_stdout.channel.recv_exit_status.return_value = rc - fake_stdout.read.return_value = 'password="secret"' + fake_stdout.read.return_value = b'password="secret"' - fake_stderr = six.StringIO('password="foobar"') + fake_stderr = six.BytesIO(b'password="foobar"') command = 'ls --password="bar"'