Revert encoding changes

certbot/certbot/_internal/hooks.py

@@ -2,14 +2,13 @@
 from __future__ import print_function
 
 import logging
-from subprocess import PIPE
-from subprocess import Popen
 
 from acme.magic_typing import List
 from acme.magic_typing import Set
 from certbot import errors
 from certbot import util
 from certbot.compat import filesystem
+from certbot.compat import misc
 from certbot.compat import os
 from certbot.plugins import util as plug_util
 
@@ -229,36 +228,10 @@ def _run_hook(cmd_name, shell_cmd):
     :type shell_cmd: `list` of `str` or `str`
 
     :returns: stderr if there was any"""
-    err, _ = execute(cmd_name, shell_cmd)
+    err, _ = misc.execute_command(cmd_name, shell_cmd)
     return err
 
 
-def execute(cmd_name, shell_cmd):
-    """Run a command.
-
-    :param str cmd_name: the user facing name of the hook being run
-    :param shell_cmd: shell command to execute
-    :type shell_cmd: `list` of `str` or `str`
-
-    :returns: `tuple` (`str` stderr, `str` stdout)"""
-    logger.info("Running %s command: %s", cmd_name, shell_cmd)
-
-    # universal_newlines causes Popen.communicate()
-    # to return str objects instead of bytes in Python 3
-    cmd = Popen(shell_cmd, shell=True, stdout=PIPE,
-                stderr=PIPE, universal_newlines=True)
-    out, err = cmd.communicate()
-    base_cmd = os.path.basename(shell_cmd.split(None, 1)[0])
-    if out:
-        logger.info('Output from %s command %s:\n%s', cmd_name, base_cmd, out)
-    if cmd.returncode != 0:
-        logger.error('%s command "%s" returned error code %d',
-                     cmd_name, shell_cmd, cmd.returncode)
-    if err:
-        logger.error('Error output from %s command %s:\n%s', cmd_name, base_cmd, err)
-    return err, out
-
-
 def list_hooks(dir_path):
     """List paths to all hooks found in dir_path in sorted order.
 

certbot/certbot/_internal/plugins/manual.py

@@ -9,6 +9,7 @@ from certbot import errors
 from certbot import interfaces
 from certbot import reverter
 from certbot._internal import hooks
+from certbot.compat import misc
 from certbot.compat import os
 from certbot.plugins import common
 
@@ -186,4 +187,4 @@ permitted by DNS standards.)
         self.reverter.recovery_routine()
 
     def _execute_hook(self, hook_name):
-        return hooks.execute(self.option_name(hook_name), self.conf(hook_name))
+        return misc.execute_command(self.option_name(hook_name), self.conf(hook_name))

certbot/certbot/compat/misc.py

@@ -4,7 +4,9 @@ particular category.
 """
 from __future__ import absolute_import
 
+import logging
 import select
+import subprocess
 import sys
 
 from certbot import errors
@@ -17,6 +19,7 @@ except ImportError:  # pragma: no cover
     POSIX_MODE = True
 
 
+logger = logging.getLogger(__name__)
 
 # For Linux: define OS specific standard binary directories
 STANDARD_BINARY_DIRS = ["/usr/sbin", "/usr/local/bin", "/usr/local/sbin"] if POSIX_MODE else []
@@ -109,3 +112,39 @@ def underscores_for_unsupported_characters_in_path(path):
     # Windows specific
     drive, tail = os.path.splitdrive(path)
     return drive + tail.replace(':', '_')
+
+
+def execute_command(cmd_name, shell_cmd):
+    """
+    Run a command:
+        - on Linux command will be run by the standard shell selected with Popen(shell=True)
+        - on Windows command will be run in a Powershell shell
+
+    :param str cmd_name: the user facing name of the hook being run
+    :param str shell_cmd: shell command to execute
+    :type shell_cmd: `list` of `str` or `str`
+
+    :returns: `tuple` (`str` stderr, `str` stdout)
+    """
+    logger.info("Running %s command: %s", cmd_name, shell_cmd)
+
+    if POSIX_MODE:
+        cmd = subprocess.Popen(shell_cmd, shell=True, stdout=subprocess.PIPE,
+                               stderr=subprocess.PIPE, universal_newlines=True)
+    else:
+        line = ['powershell.exe', '-Command', shell_cmd]
+        cmd = subprocess.Popen(line, stdout=subprocess.PIPE, stderr=subprocess.PIPE,
+                               universal_newlines=True)
+
+    # universal_newlines causes Popen.communicate()
+    # to return str objects instead of bytes in Python 3
+    out, err = cmd.communicate()
+    base_cmd = os.path.basename(shell_cmd.split(None, 1)[0])
+    if out:
+        logger.info('Output from %s command %s:\n%s', cmd_name, base_cmd, out)
+    if cmd.returncode != 0:
+        logger.error('%s command "%s" returned error code %d',
+                     cmd_name, shell_cmd, cmd.returncode)
+    if err:
+        logger.error('Error output from %s command %s:\n%s', cmd_name, base_cmd, err)
+    return err, out

certbot/tests/compat/misc_test.py

@@ -0,0 +1,44 @@
+"""Tests for certbot.compat.misc"""
+import mock
+import unittest
+
+from certbot.compat import os
+
+class ExecuteTest(unittest.TestCase):
+    """Tests for certbot.compat.misc.execute_command."""
+
+    @classmethod
+    def _call(cls, *args, **kwargs):
+        from certbot.compat.misc import execute_command
+        return execute_command(*args, **kwargs)
+
+    def test_it(self):
+        for returncode in range(0, 2):
+            for stdout in ("", "Hello World!",):
+                for stderr in ("", "Goodbye Cruel World!"):
+                    self._test_common(returncode, stdout, stderr)
+
+    def _test_common(self, returncode, stdout, stderr):
+        given_command = "foo"
+        given_name = "foo-hook"
+        with mock.patch("certbot.compat.misc.subprocess.Popen") as mock_popen:
+            mock_popen.return_value.communicate.return_value = (stdout, stderr)
+            mock_popen.return_value.returncode = returncode
+            with mock.patch("certbot.compat.misc.logger") as mock_logger:
+                self.assertEqual(self._call(given_name, given_command), (stderr, stdout))
+
+        executed_command = mock_popen.call_args[1].get(
+            "args", mock_popen.call_args[0][0])
+        if os.name == 'nt':
+            expected_command = ['powershell.exe', '-Command', given_command]
+        else:
+            expected_command = given_command
+        self.assertEqual(executed_command, expected_command)
+
+        mock_logger.info.assert_any_call("Running %s command: %s",
+                                         given_name, given_command)
+        if stdout:
+            mock_logger.info.assert_any_call(mock.ANY, mock.ANY,
+                                             mock.ANY, stdout)
+        if stderr or returncode:
+            self.assertTrue(mock_logger.error.called)

certbot/tests/hook_test.py

@@ -72,13 +72,13 @@ class HookTest(test_util.ConfigTestCase):
 
     @classmethod
     def _call_with_mock_execute(cls, *args, **kwargs):
-        """Calls self._call after mocking out certbot._internal.hooks.execute.
+        """Calls self._call after mocking out certbot.compat.misc.execute_command.
 
         The mock execute object is returned rather than the return value
         of self._call.
 
         """
-        with mock.patch("certbot._internal.hooks.execute") as mock_execute:
+        with mock.patch("certbot.compat.misc.execute_command") as mock_execute:
             mock_execute.return_value = ("", "")
             cls._call(*args, **kwargs)
         return mock_execute
@@ -292,7 +292,7 @@ class RenewalHookTest(HookTest):
     # pylint: disable=abstract-method
 
     def _call_with_mock_execute(self, *args, **kwargs):
-        """Calls self._call after mocking out certbot._internal.hooks.execute.
+        """Calls self._call after mocking out certbot.compat.misc.execute_command.
 
         The mock execute object is returned rather than the return value
         of self._call. The mock execute object asserts that environment
@@ -313,7 +313,7 @@ class RenewalHookTest(HookTest):
             self.assertEqual(os.environ["RENEWED_LINEAGE"], lineage)
             return ("", "")
 
-        with mock.patch("certbot._internal.hooks.execute") as mock_execute:
+        with mock.patch("certbot.compat.misc.execute_command") as mock_execute:
             mock_execute.side_effect = execute_side_effect
             self._call(*args, **kwargs)
         return mock_execute
@@ -418,42 +418,6 @@ class RenewHookTest(RenewalHookTest):
         mock_execute.assert_called_with("deploy-hook", self.config.renew_hook)
 
 
-class ExecuteTest(unittest.TestCase):
-    """Tests for certbot._internal.hooks.execute."""
-
-    @classmethod
-    def _call(cls, *args, **kwargs):
-        from certbot._internal.hooks import execute
-        return execute(*args, **kwargs)
-
-    def test_it(self):
-        for returncode in range(0, 2):
-            for stdout in ("", "Hello World!",):
-                for stderr in ("", "Goodbye Cruel World!"):
-                    self._test_common(returncode, stdout, stderr)
-
-    def _test_common(self, returncode, stdout, stderr):
-        given_command = "foo"
-        given_name = "foo-hook"
-        with mock.patch("certbot._internal.hooks.Popen") as mock_popen:
-            mock_popen.return_value.communicate.return_value = (stdout, stderr)
-            mock_popen.return_value.returncode = returncode
-            with mock.patch("certbot._internal.hooks.logger") as mock_logger:
-                self.assertEqual(self._call(given_name, given_command), (stderr, stdout))
-
-        executed_command = mock_popen.call_args[1].get(
-            "args", mock_popen.call_args[0][0])
-        self.assertEqual(executed_command, given_command)
-
-        mock_logger.info.assert_any_call("Running %s command: %s",
-                                         given_name, given_command)
-        if stdout:
-            mock_logger.info.assert_any_call(mock.ANY, mock.ANY,
-                                             mock.ANY, stdout)
-        if stderr or returncode:
-            self.assertTrue(mock_logger.error.called)
-
-
 class ListHooksTest(test_util.TempDirTestCase):
     """Tests for certbot._internal.hooks.list_hooks."""