diff --git a/acme/acme/standalone.py b/acme/acme/standalone.py index 52ac07915..7a61ba868 100644 --- a/acme/acme/standalone.py +++ b/acme/acme/standalone.py @@ -176,10 +176,10 @@ class HTTPServer(BaseHTTPServer.HTTPServer): class HTTP01Server(HTTPServer, ACMEServerMixin): """HTTP01 Server.""" - def __init__(self, server_address, resources, ipv6=False): + def __init__(self, server_address, resources, ipv6=False, timeout=30): HTTPServer.__init__( self, server_address, HTTP01RequestHandler.partial_init( - simple_http_resources=resources), ipv6=ipv6) + simple_http_resources=resources, timeout=timeout), ipv6=ipv6) class HTTP01DualNetworkedServers(BaseDualNetworkedServers): @@ -204,6 +204,7 @@ class HTTP01RequestHandler(BaseHTTPServer.BaseHTTPRequestHandler): def __init__(self, *args, **kwargs): self.simple_http_resources = kwargs.pop("simple_http_resources", set()) + self.timeout = kwargs.pop('timeout', 30) BaseHTTPServer.BaseHTTPRequestHandler.__init__(self, *args, **kwargs) def log_message(self, format, *args): # pylint: disable=redefined-builtin @@ -253,7 +254,7 @@ class HTTP01RequestHandler(BaseHTTPServer.BaseHTTPRequestHandler): self.path) @classmethod - def partial_init(cls, simple_http_resources): + def partial_init(cls, simple_http_resources, timeout): """Partially initialize this handler. This is useful because `socketserver.BaseServer` takes @@ -262,7 +263,8 @@ class HTTP01RequestHandler(BaseHTTPServer.BaseHTTPRequestHandler): """ return functools.partial( - cls, simple_http_resources=simple_http_resources) + cls, simple_http_resources=simple_http_resources, + timeout=timeout) class _BaseRequestHandlerWithLogging(socketserver.BaseRequestHandler): diff --git a/acme/tests/standalone_test.py b/acme/tests/standalone_test.py index 8c08ab89b..8face7c7b 100644 --- a/acme/tests/standalone_test.py +++ b/acme/tests/standalone_test.py @@ -85,6 +85,28 @@ class HTTP01ServerTest(unittest.TestCase): def test_http01_not_found(self): self.assertFalse(self._test_http01(add=False)) + def test_timely_shutdown(self): + from acme.standalone import HTTP01Server + server = HTTP01Server(('', 0), resources=set(), timeout=0.05) + server_thread = threading.Thread(target=server.serve_forever) + server_thread.start() + + client = socket.socket() + client.connect(('localhost', server.socket.getsockname()[1])) + + stop_thread = threading.Thread(target=server.shutdown) + stop_thread.start() + server_thread.join(5.) + + is_hung = server_thread.is_alive() + try: + client.shutdown(socket.SHUT_RDWR) + except: # pragma: no cover, pylint: disable=bare-except + # may raise error because socket could already be closed + pass + + self.assertFalse(is_hung, msg='Server shutdown should not be hung') + @unittest.skipIf(not challenges.TLSALPN01.is_supported(), "pyOpenSSL too old") class TLSALPN01ServerTest(unittest.TestCase): diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md index 88a4ab190..99c1de63b 100644 --- a/certbot/CHANGELOG.md +++ b/certbot/CHANGELOG.md @@ -26,6 +26,8 @@ Certbot adheres to [Semantic Versioning](https://semver.org/). * Fix nginx plugin crash when non-ASCII configuration file is being read (instead, the user will be warned that UTF-8 must be used). * Fix hanging OCSP queries during revocation checking - added a 10 second timeout. +* Standalone servers now have a default socket timeout of 30 seconds, fixing + cases where an idle connection can cause the standalone plugin to hang. More details about these changes can be found on our GitHub repo.