From d6e95b4617e0946141e0665d9a2c1ea2ed8ddd22 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sun, 6 Sep 2015 12:47:30 +0000 Subject: [PATCH 1/3] Manual plugin test mode busy wait (fixes #755). --- letsencrypt/plugins/manual.py | 22 ++++++++++++++++++---- letsencrypt/plugins/manual_test.py | 8 ++++++-- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/letsencrypt/plugins/manual.py b/letsencrypt/plugins/manual.py index d13f35f99..29f8ccc88 100644 --- a/letsencrypt/plugins/manual.py +++ b/letsencrypt/plugins/manual.py @@ -4,6 +4,7 @@ import logging import pipes import shutil import signal +import socket import subprocess import sys import tempfile @@ -122,6 +123,19 @@ binary for temporary key/certificate generation.""".replace("\n", "") responses.append(self._perform_single(achall)) return responses + def _test_mode_busy_wait(self, port): + while True: + time.sleep(1) + sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + try: + sock.connect(("localhost", port)) + except socket.error: # pragma: no cover + pass + else: + break + finally: + sock.close() + def _perform_single(self, achall): # same path for each challenge response would be easier for # users, but will not work if multiple domains point at the @@ -129,13 +143,13 @@ binary for temporary key/certificate generation.""".replace("\n", "") response, validation = achall.gen_response_and_validation( tls=(not self.config.no_simple_http_tls)) + port = (response.port if self.config.simple_http_port is None + else self.config.simple_http_port) command = self.template.format( root=self._root, achall=achall, response=response, validation=pipes.quote(validation.json_dumps()), encoded_token=achall.chall.encode("token"), - ct=response.CONTENT_TYPE, port=( - response.port if self.config.simple_http_port is None - else self.config.simple_http_port)) + ct=response.CONTENT_TYPE, port=port) if self.conf("test-mode"): logger.debug("Test mode. Executing the manual command: %s", command) try: @@ -153,7 +167,7 @@ binary for temporary key/certificate generation.""".replace("\n", "") logger.debug("Manual command running as PID %s.", self._httpd.pid) # give it some time to bootstrap, before we try to verify # (cert generation in case of simpleHttpS might take time) - time.sleep(4) # XXX + self._test_mode_busy_wait(port) if self._httpd.poll() is not None: raise errors.Error("Couldn't execute manual command") else: diff --git a/letsencrypt/plugins/manual_test.py b/letsencrypt/plugins/manual_test.py index caf7fb3c4..c1ca9f70e 100644 --- a/letsencrypt/plugins/manual_test.py +++ b/letsencrypt/plugins/manual_test.py @@ -71,25 +71,29 @@ class ManualAuthenticatorTest(unittest.TestCase): mock_popen.side_effect = OSError self.assertEqual([False], self.auth_test_mode.perform(self.achalls)) + @mock.patch("letsencrypt.plugins.manual.socket.socket", autospec=True) @mock.patch("letsencrypt.plugins.manual.time.sleep", autospec=True) @mock.patch("letsencrypt.plugins.manual.subprocess.Popen", autospec=True) def test_perform_test_command_run_failure( - self, mock_popen, unused_mock_sleep): + self, mock_popen, unused_mock_sleep, unused_mock_socket): mock_popen.poll.return_value = 10 mock_popen.return_value.pid = 1234 self.assertRaises( errors.Error, self.auth_test_mode.perform, self.achalls) + @mock.patch("letsencrypt.plugins.manual.socket.socket", autospec=True) @mock.patch("letsencrypt.plugins.manual.time.sleep", autospec=True) @mock.patch("acme.challenges.SimpleHTTPResponse.simple_verify", autospec=True) @mock.patch("letsencrypt.plugins.manual.subprocess.Popen", autospec=True) - def test_perform_test_mode(self, mock_popen, mock_verify, mock_sleep): + def test_perform_test_mode(self, mock_popen, mock_verify, mock_sleep, + mock_socket): mock_popen.return_value.poll.side_effect = [None, 10] mock_popen.return_value.pid = 1234 mock_verify.return_value = False self.assertEqual([False], self.auth_test_mode.perform(self.achalls)) self.assertEqual(1, mock_sleep.call_count) + self.assertEqual(1, mock_socket.call_count) def test_cleanup_test_mode_already_terminated(self): # pylint: disable=protected-access From 41c08416cd994a1d3280c41ff541946032d47c87 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sun, 6 Sep 2015 12:54:13 +0000 Subject: [PATCH 2/3] Cast port to int --- letsencrypt/plugins/manual.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/plugins/manual.py b/letsencrypt/plugins/manual.py index 29f8ccc88..24d3a5a77 100644 --- a/letsencrypt/plugins/manual.py +++ b/letsencrypt/plugins/manual.py @@ -144,7 +144,7 @@ binary for temporary key/certificate generation.""".replace("\n", "") tls=(not self.config.no_simple_http_tls)) port = (response.port if self.config.simple_http_port is None - else self.config.simple_http_port) + else int(self.config.simple_http_port)) command = self.template.format( root=self._root, achall=achall, response=response, validation=pipes.quote(validation.json_dumps()), From 1c27c7ed546849d5a59b9c600fd683ceb570f07c Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sun, 6 Sep 2015 13:00:53 +0000 Subject: [PATCH 3/3] lint: fix no-self-use --- letsencrypt/plugins/manual.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/letsencrypt/plugins/manual.py b/letsencrypt/plugins/manual.py index 24d3a5a77..e16fc152f 100644 --- a/letsencrypt/plugins/manual.py +++ b/letsencrypt/plugins/manual.py @@ -123,7 +123,8 @@ binary for temporary key/certificate generation.""".replace("\n", "") responses.append(self._perform_single(achall)) return responses - def _test_mode_busy_wait(self, port): + @classmethod + def _test_mode_busy_wait(cls, port): while True: time.sleep(1) sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)