diff --git a/.github/workflows/tox.yml b/.github/workflows/tox.yml index 759800eb1..e96319a2b 100644 --- a/.github/workflows/tox.yml +++ b/.github/workflows/tox.yml @@ -21,8 +21,7 @@ jobs: - macos-13 # Not testing Windows, because tests need Unix-only fcntl, grp, pwd, etc. python-version: - # CPython <= 3.7 is EoL since 2023-06-27 - - "3.7" + # CPython <= 3.8 is EoL since 2024-10-07 https://peps.python.org/pep-0569/ - "3.8" - "3.9" - "3.10" diff --git a/docs/gunicorn_ext.py b/docs/gunicorn_ext.py index 4310162eb..4e78919e4 100755 --- a/docs/gunicorn_ext.py +++ b/docs/gunicorn_ext.py @@ -52,7 +52,10 @@ def fmt_setting(s): val = s.default_doc elif callable(s.default): val = inspect.getsource(s.default) - val = "\n".join(" %s" % line for line in val.splitlines()) + # defaults are def'd inside class; strip the @decorator + val = "\n".join(" %s" % line + for line in val.splitlines() + if not line.strip() == "@staticmethod") val = "\n\n.. code-block:: python\n\n" + val elif s.default == '': val = "``''``" diff --git a/docs/source/settings.rst b/docs/source/settings.rst index e1e91fa76..d75d6999a 100644 --- a/docs/source/settings.rst +++ b/docs/source/settings.rst @@ -85,6 +85,10 @@ because it consumes less system resources. .. note:: In order to use the inotify reloader, you must have the ``inotify`` package installed. +.. warning:: + By default, enabling this will modify the handling of application errors + such that sensitive information is shared in response to any request; + see :ref:`on-fatal` for details. .. _reload-engine: @@ -114,10 +118,13 @@ Valid engines are: **Default:** ``[]`` -Extends :ref:`reload` option to also watch and reload on additional files -(e.g., templates, configurations, specifications, etc.). +Reload when these files appear modified. Can be used either on its own or to extend + the :ref:`reload` option to also watch and reload on additional files + (e.g., templates, configurations, specifications, etc.). .. versionadded:: 19.8 +.. versionchanged:: 23.1.0 + Now effective also when :ref:`reload` is not enabled. .. _spew: @@ -1136,6 +1143,9 @@ A valid user id (as an integer) or the name of a user that can be retrieved with a call to ``pwd.getpwnam(value)`` or ``None`` to not change the worker process user. +.. note:: + Leaving this option unspecified does not skip username lookup. + .. _group: ``group`` @@ -1148,9 +1158,14 @@ change the worker process user. Switch worker process to run as this group. A valid group id (as an integer) or the name of a user that can be -retrieved with a call to ``pwd.getgrnam(value)`` or ``None`` to not +retrieved with a call to ``grp.getgrnam(value)`` or ``None`` to not change the worker processes group. +.. note:: + Leaving this option unspecified does not skip username lookup. +.. warning:: + This sets effective group ID - beware of supplemental groups! + .. _umask: ``umask`` @@ -1183,6 +1198,8 @@ groups of which the specified username is a member, plus the specified group id. .. versionadded:: 19.7 +.. note:: + Silently ignored when username lookup fails. .. _tmp-upload-dir: @@ -1560,6 +1577,30 @@ on a proxy in front of Gunicorn. .. versionadded:: 22.0.0 +.. _on-fatal: + +``on_fatal`` +~~~~~~~~~~~~ + +**Command line:** ``--on-fatal`` + +**Default:** ``'world-readable-with-reload'`` + +Configure what to do if loading the application fails + +If set to ``world-readable``, send the traceback to the client. +If set to ``brief``, repond with a simple error status. +If set to ``refuse``, stop processing requests. +The default behavior is ``world-readable-with-reload``, which is equivalent +to ``world-readable`` when :ref:`reload` is enabled, or ``refuse`` otherwise. + +The behaviour of ``world-readable`` (or, the default in conjunction with +``reload``) risks exposing sensitive code and data and is not suitable +for production use. + +.. versionadded:: 23.1.0 + The new *default* matches the previous behavior. + Server Socket ------------- diff --git a/gunicorn/app/base.py b/gunicorn/app/base.py index 9bf7a4f0f..333085065 100644 --- a/gunicorn/app/base.py +++ b/gunicorn/app/base.py @@ -101,8 +101,12 @@ def get_config_from_filename(self, filename): if ext in [".py", ".pyc"]: spec = importlib.util.spec_from_file_location(module_name, filename) else: - msg = "configuration file should have a valid Python extension.\n" - util.warn(msg) + if filename == getattr(os, "devnull", "/dev/null"): + # unambiguous and generally deliberate. no need to warn in this case. + pass + else: + msg = "configuration file should have a valid Python extension.\n" + util.warn(msg) loader_ = importlib.machinery.SourceFileLoader(module_name, filename) spec = importlib.util.spec_from_file_location(module_name, filename, loader=loader_) mod = importlib.util.module_from_spec(spec) diff --git a/gunicorn/config.py b/gunicorn/config.py index 07c5aab34..de1d5e030 100644 --- a/gunicorn/config.py +++ b/gunicorn/config.py @@ -924,6 +924,10 @@ class Reload(Setting): .. note:: In order to use the inotify reloader, you must have the ``inotify`` package installed. + .. warning:: + By default, enabling this will modify the handling of application errors + such that sensitive information is shared in response to any request; + see :ref:`on-fatal` for details. ''' @@ -956,10 +960,13 @@ class ReloadExtraFiles(Setting): validator = validate_list_of_existing_files default = [] desc = """\ - Extends :ref:`reload` option to also watch and reload on additional files - (e.g., templates, configurations, specifications, etc.). + Reload when these files appear modified. Can be used either on its own or to extend + the :ref:`reload` option to also watch and reload on additional files + (e.g., templates, configurations, specifications, etc.). .. versionadded:: 19.8 + .. versionchanged:: 23.1.0 + Now effective also when :ref:`reload` is not enabled. """ @@ -1162,6 +1169,9 @@ class User(Setting): A valid user id (as an integer) or the name of a user that can be retrieved with a call to ``pwd.getpwnam(value)`` or ``None`` to not change the worker process user. + + .. note:: + Leaving this option unspecified does not skip username lookup. """ @@ -1179,6 +1189,11 @@ class Group(Setting): A valid group id (as an integer) or the name of a user that can be retrieved with a call to ``grp.getgrnam(value)`` or ``None`` to not change the worker processes group. + + .. note:: + Leaving this option unspecified does not skip username lookup. + .. warning:: + This sets effective group ID - beware of supplemental groups! """ @@ -1216,6 +1231,8 @@ class Initgroups(Setting): group id. .. versionadded:: 19.7 + .. note:: + Silently ignored when username lookup fails. """ @@ -1760,9 +1777,10 @@ class OnStarting(Setting): validator = validate_callable(1) type = callable + @staticmethod def on_starting(server): pass - default = staticmethod(on_starting) + default = on_starting desc = """\ Called just before the master process is initialized. @@ -1776,9 +1794,10 @@ class OnReload(Setting): validator = validate_callable(1) type = callable + @staticmethod def on_reload(server): pass - default = staticmethod(on_reload) + default = on_reload desc = """\ Called to recycle workers during a reload via SIGHUP. @@ -1792,9 +1811,10 @@ class WhenReady(Setting): validator = validate_callable(1) type = callable + @staticmethod def when_ready(server): pass - default = staticmethod(when_ready) + default = when_ready desc = """\ Called just after the server is started. @@ -1808,9 +1828,10 @@ class Prefork(Setting): validator = validate_callable(2) type = callable + @staticmethod def pre_fork(server, worker): pass - default = staticmethod(pre_fork) + default = pre_fork desc = """\ Called just before a worker is forked. @@ -1825,9 +1846,10 @@ class Postfork(Setting): validator = validate_callable(2) type = callable + @staticmethod def post_fork(server, worker): pass - default = staticmethod(post_fork) + default = post_fork desc = """\ Called just after a worker has been forked. @@ -1842,10 +1864,11 @@ class PostWorkerInit(Setting): validator = validate_callable(1) type = callable + @staticmethod def post_worker_init(worker): pass - default = staticmethod(post_worker_init) + default = post_worker_init desc = """\ Called just after a worker has initialized the application. @@ -1860,10 +1883,11 @@ class WorkerInt(Setting): validator = validate_callable(1) type = callable + @staticmethod def worker_int(worker): pass - default = staticmethod(worker_int) + default = worker_int desc = """\ Called just after a worker exited on SIGINT or SIGQUIT. @@ -1878,10 +1902,11 @@ class WorkerAbort(Setting): validator = validate_callable(1) type = callable + @staticmethod def worker_abort(worker): pass - default = staticmethod(worker_abort) + default = worker_abort desc = """\ Called when a worker received the SIGABRT signal. @@ -1898,9 +1923,10 @@ class PreExec(Setting): validator = validate_callable(1) type = callable + @staticmethod def pre_exec(server): pass - default = staticmethod(pre_exec) + default = pre_exec desc = """\ Called just before a new master process is forked. @@ -1914,9 +1940,10 @@ class PreRequest(Setting): validator = validate_callable(2) type = callable + @staticmethod def pre_request(worker, req): worker.log.debug("%s %s", req.method, req.path) - default = staticmethod(pre_request) + default = pre_request desc = """\ Called just before a worker processes the request. @@ -1931,9 +1958,10 @@ class PostRequest(Setting): validator = validate_post_request type = callable + @staticmethod def post_request(worker, req, environ, resp): pass - default = staticmethod(post_request) + default = post_request desc = """\ Called after a worker processes the request. @@ -1948,9 +1976,10 @@ class ChildExit(Setting): validator = validate_callable(2) type = callable + @staticmethod def child_exit(server, worker): pass - default = staticmethod(child_exit) + default = child_exit desc = """\ Called just after a worker has been exited, in the master process. @@ -1967,9 +1996,10 @@ class WorkerExit(Setting): validator = validate_callable(2) type = callable + @staticmethod def worker_exit(server, worker): pass - default = staticmethod(worker_exit) + default = worker_exit desc = """\ Called just after a worker has been exited, in the worker process. @@ -1984,9 +2014,10 @@ class NumWorkersChanged(Setting): validator = validate_callable(3) type = callable + @staticmethod def nworkers_changed(server, new_value, old_value): pass - default = staticmethod(nworkers_changed) + default = nworkers_changed desc = """\ Called just after *num_workers* has been changed. @@ -2003,10 +2034,11 @@ class OnExit(Setting): section = "Server Hooks" validator = validate_callable(1) + @staticmethod def on_exit(server): pass - default = staticmethod(on_exit) + default = on_exit desc = """\ Called just before exiting Gunicorn. @@ -2020,10 +2052,11 @@ class NewSSLContext(Setting): validator = validate_callable(2) type = callable + @staticmethod def ssl_context(config, default_ssl_context_factory): return default_ssl_context_factory() - default = staticmethod(ssl_context) + default = ssl_context desc = """\ Called when SSLContext is needed. @@ -2440,3 +2473,47 @@ class HeaderMap(Setting): .. versionadded:: 22.0.0 """ + + +def validate_fatal_behaviour(val): + # FIXME: refactor all of this subclassing stdlib argparse + + if val is None: + return + + if not isinstance(val, str): + raise TypeError("Invalid type for casting: %s" % val) + if val.lower().strip() == "world-readable": + return "world-readable" + elif val.lower().strip() == "refuse": + return "refuse" + elif val.lower().strip() == "brief": + return "brief" + elif val.lower().strip() == "world-readable-with-reload": + return "world-readable-with-reload" + else: + raise ValueError("Invalid header map behaviour: %s" % val) + + +class OnFatal(Setting): + name = "on_fatal" + section = "Server Mechanics" + cli = ["--on-fatal"] + validator = validate_fatal_behaviour + default = "world-readable-with-reload" + desc = """\ + Configure what to do if loading the application fails + + If set to ``world-readable``, send the traceback to the client. + If set to ``brief``, repond with a simple error status. + If set to ``refuse``, stop processing requests. + The default behavior is ``world-readable-with-reload``, which is equivalent + to ``world-readable`` when :ref:`reload` is enabled, or ``refuse`` otherwise. + + The behaviour of ``world-readable`` (or, the default in conjunction with + ``reload``) risks exposing sensitive code and data and is not suitable + for production use. + + .. versionadded:: 23.1.0 + The new *default* matches the previous behavior. + """ diff --git a/gunicorn/reloader.py b/gunicorn/reloader.py index 1c67f2a7d..55777a2c3 100644 --- a/gunicorn/reloader.py +++ b/gunicorn/reloader.py @@ -13,23 +13,31 @@ COMPILED_EXT_RE = re.compile(r'py[co]$') +def _detect_loaded_files(): + fnames = [] + for module in tuple(sys.modules.values()): + if getattr(module, '__file__', None): + fnames.append(COMPILED_EXT_RE.sub('py', module.__file__)) + return fnames + + class Reloader(threading.Thread): - def __init__(self, extra_files=None, interval=1, callback=None): + def __init__(self, extra_files=None, interval=1, callback=None, auto_detect=False): super().__init__() self.daemon = True self._extra_files = set(extra_files or ()) self._interval = interval self._callback = callback + self._auto_detect = auto_detect def add_extra_file(self, filename): self._extra_files.add(filename) def get_files(self): - fnames = [ - COMPILED_EXT_RE.sub('py', module.__file__) - for module in tuple(sys.modules.values()) - if getattr(module, '__file__', None) - ] + fnames = [] + + if self._auto_detect: + fnames.extend(self._detect_loaded_files()) fnames.extend(self._extra_files) @@ -71,12 +79,13 @@ class InotifyReloader(threading.Thread): | inotify.constants.IN_MOVE_SELF | inotify.constants.IN_MOVED_FROM | inotify.constants.IN_MOVED_TO) - def __init__(self, extra_files=None, callback=None): + def __init__(self, extra_files=None, callback=None, auto_detect=False): super().__init__() self.daemon = True self._callback = callback self._dirs = set() self._watcher = Inotify() + self._auto_detect = auto_detect for extra_file in extra_files: self.add_extra_file(extra_file) @@ -91,11 +100,12 @@ def add_extra_file(self, filename): self._dirs.add(dirname) def get_dirs(self): - fnames = [ - os.path.dirname(os.path.abspath(COMPILED_EXT_RE.sub('py', module.__file__))) - for module in tuple(sys.modules.values()) - if getattr(module, '__file__', None) - ] + fnames = [] + + if self._auto_detect: + fnames.extend( + [os.path.dirname(os.path.abspath(fname)) for fname in _detect_loaded_files()] + ) return set(fnames) @@ -117,7 +127,7 @@ def run(self): else: class InotifyReloader: - def __init__(self, extra_files=None, callback=None): + def __init__(self, extra_files=None, callback=None, auto_detect=False): raise ImportError('You must have the inotify module installed to ' 'use the inotify reloader') diff --git a/gunicorn/util.py b/gunicorn/util.py index e66dbebf3..cc31014f1 100644 --- a/gunicorn/util.py +++ b/gunicorn/util.py @@ -140,6 +140,10 @@ def get_username(uid): def set_owner_process(uid, gid, initgroups=False): """ set user and group of workers processes """ + # note: uid/gid can be larger than 2**32 + # note: setgid() does not empty supplemental group list + # note: will never act on uid=0 / gid=0 + if gid: if uid: try: @@ -157,6 +161,7 @@ def set_owner_process(uid, gid, initgroups=False): def chown(path, uid, gid): + # N.B. os.chown semantics are -1 for unchanged os.chown(path, uid, gid) diff --git a/gunicorn/workers/base.py b/gunicorn/workers/base.py index 93c465c98..e78f7825a 100644 --- a/gunicorn/workers/base.py +++ b/gunicorn/workers/base.py @@ -119,7 +119,7 @@ def init_process(self): self.init_signals() # start the reloader - if self.cfg.reload: + if self.cfg.reload or self.cfg.reload_extra_files: def changed(fname): self.log.info("Worker reloading: %s modified", fname) self.alive = False @@ -130,6 +130,7 @@ def changed(fname): reloader_cls = reloader_engines[self.cfg.reload_engine] self.reloader = reloader_cls(extra_files=self.cfg.reload_extra_files, + auto_detect=self.cfg.reload, callback=changed) self.load_wsgi() @@ -146,24 +147,26 @@ def load_wsgi(self): try: self.wsgi = self.app.wsgi() except SyntaxError as e: - if not self.cfg.reload: + if self.cfg.on_fatal == "world-readable": + pass + elif self.cfg.on_fatal == "brief": + self.log.exception(e) + self.wsgi = util.make_fail_app("Internal Server Error") + return + elif self.cfg.on_fatal == "world-readable-with-reload" and self.cfg.reload: + pass + else: # self.cfg.on_fatal == "refuse" + # secure fallthrough raise self.log.exception(e) - # fix from PR #1228 - # storing the traceback into exc_tb will create a circular reference. - # per https://docs.python.org/2/library/sys.html#sys.exc_info warning, - # delete the traceback after use. - try: - _, exc_val, exc_tb = sys.exc_info() - self.reloader.add_extra_file(exc_val.filename) + if self.reloader is not None and e.filename is not None: + self.reloader.add_extra_file(e.filename) - tb_string = io.StringIO() - traceback.print_tb(exc_tb, file=tb_string) + with io.StringIO() as tb_string: + traceback.print_tb(e.__traceback__, file=tb_string) self.wsgi = util.make_fail_app(tb_string.getvalue()) - finally: - del exc_tb def init_signals(self): # reset signaling diff --git a/pyproject.toml b/pyproject.toml index eaca1eac0..99070bdf9 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -59,10 +59,12 @@ testing = [ "gevent", "eventlet", "coverage", - "pytest", + "pytest>=7.2.0", "pytest-cov", + "tornado>=6.0", ] + [project.scripts] # duplicates "python -m gunicorn" handling in __main__.py gunicorn = "gunicorn.app.wsgiapp:run" @@ -72,10 +74,11 @@ gunicorn = "gunicorn.app.wsgiapp:run" main = "gunicorn.app.pasterapp:serve" [tool.pytest.ini_options] -# # can override these: python -m pytest --override-ini="addopts=" +# can override these: python -m pytest --override-ini="addopts=" norecursedirs = ["examples", "lib", "local", "src"] testpaths = ["tests/"] -addopts = "--assert=plain --cov=gunicorn --cov-report=xml" +addopts = ["--strict-markers", "--assert=plain", "--cov=gunicorn", "--cov-report=xml"] +xfail_strict = true [tool.setuptools] zip-safe = false diff --git a/tests/test_e2e.py b/tests/test_e2e.py new file mode 100644 index 000000000..44fb2a962 --- /dev/null +++ b/tests/test_e2e.py @@ -0,0 +1,463 @@ +import os +import platform +import secrets +import signal +import subprocess +import logging +import sys +import time +from pathlib import Path +from tempfile import TemporaryDirectory + +import pytest + +# pytest does not like exceptions from threads +# - so use subprocess.Popen for now +# from threading import Thread, Event + +logger = logging.getLogger(__name__) + +GRACEFUL_TIMEOUT = 0 +TIMEOUT_SEC_PER_SUBTEST = 3.0 + +# test flaky for WORKER_COUNT != 1, awaiting *last* worker not implemented +WORKER_COUNT = 1 +APP_BASENAME = "testsyntax" +APP_APPNAME = "wsgiapp" + +TEST_TOLERATES_BAD_BOOT = [ + "sync", + "eventlet", + "gevent", + "gevent_wsgi", + "gevent_pywsgi", + "gthread", + # pytest.param("expected_failure", marks=pytest.mark.xfail), +] + +TEST_TOLERATES_BAD_RELOAD = [ + "sync", + "eventlet", + "gevent", + "gevent_wsgi", + "gevent_pywsgi", + "gthread", + # pytest.param("expected_failure", marks=pytest.mark.xfail), +] + + +try: + from tornado import options as installed_check_ # pylint: disable=unused-import + + for T in (TEST_TOLERATES_BAD_BOOT, TEST_TOLERATES_BAD_RELOAD): + T.append(pytest.param("tornado", marks=pytest.mark.xfail)) +except ImportError: + for T in (TEST_TOLERATES_BAD_BOOT, TEST_TOLERATES_BAD_RELOAD): + T.append( + pytest.param("tornado", marks=pytest.mark.skip("tornado not installed")) + ) + + +PY_OK = """ +import sys +import logging + +if sys.version_info >= (3, 8): + logging.basicConfig(force=True) + logger = logging.getLogger(__name__) + logger.info("logger has been reset") +else: + logging.basicConfig() + logger = logging.getLogger(__name__) + +def wsgiapp(environ_, start_response): + # print("stdout from app", file=sys.stdout) + print("Application called - continue test!", file=sys.stderr) + # needed for Python <= 3.8 + sys.stderr.flush() + body = b"response body from app" + response_head = [ + ("Content-Type", "text/plain"), + ("Content-Length", "%d" % len(body)), + ] + start_response("200 OK", response_head) + return iter([body]) +""" + +PY_LOG_CONFIG = """ +def post_fork(a_, b_): + pass # import syntax_error +def post_worker_init(worker): + worker.log.debug("Worker booted - continue test!") +""" + +PY_BAD_IMPORT = """ +def bad_method(): + syntax_error: +""" + +PY_BAD = """ +import sys +import logging + +import signal +import os + +if sys.version_info >= (3, 8): + logging.basicConfig(force=True) + logger = logging.getLogger(__name__) + logger.info("logger has been reset") +else: + logger = logging.getLogger(__name__) + logging.basicConfig() + +# os.kill(os.getppid(), signal.SIGTERM) +# sys.exit(3) +import syntax_error + +def wsgiapp(environ_, start_response_): + raise RuntimeError("The SyntaxError should raise") +""" + + +class Server(subprocess.Popen): + def __init__( + self, + *, + temp_path, + server_bind, + worker_class, + start_valid=True, + use_config=False, + public_traceback=True, + ): + # super().__init__(*args, **kwargs) + # self.launched = Event() + assert isinstance(temp_path, Path) + self.temp_path = temp_path + self.py_path = (temp_path / ("%s.py" % APP_BASENAME)).absolute() + self.conf_path = ( + (temp_path / "gunicorn.conf.py").absolute() + if use_config + else Path(os.devnull) + ) + self._write_initial = self.write_ok if start_valid else self.write_bad + with open(self.conf_path, "w+") as f: + f.write(PY_LOG_CONFIG) + self._argv = [ + sys.executable, + # "-B", # PYTHONDONTWRITEBYTECODE - avoid inotify reporting __pycache__ + "-m", + "gunicorn", + "--config=%s" % self.conf_path, + "--log-level=debug", + "--worker-class=%s" % worker_class, + "--workers=%d" % WORKER_COUNT, + "--enable-stdio-inheritance", + "--access-logfile=-", + "--disable-redirect-access-to-syslog", + "--graceful-timeout=%d" % (GRACEFUL_TIMEOUT,), + "--on-fatal=%s" % ("world-readable" if public_traceback else "brief",), + # "--reload", + "--reload-engine=poll", + "--reload-extra=%s" % self.py_path, + "--bind=%s" % server_bind, + "--reuse-port", + "%s:%s" % (APP_BASENAME, APP_APPNAME), + ] + self.last_timestamp = time.monotonic() + self._write_initial() + super().__init__( + self._argv, + bufsize=0, # allow read to return short + cwd=self.temp_path, + shell=False, + stdin=subprocess.DEVNULL, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + # creationflags=subprocess.CREATE_NEW_PROCESS_GROUP, + ) + + os.set_blocking(self.stdout.fileno(), False) + os.set_blocking(self.stderr.fileno(), False) + self.assert_fast() + # self.launched.set() + + def assert_fast(self, limit=TIMEOUT_SEC_PER_SUBTEST): + now = time.monotonic() + elapsed = now - self.last_timestamp + if elapsed > limit: + try: + stdout, stderr = self.communicate(timeout=2 + GRACEFUL_TIMEOUT) + except subprocess.TimeoutExpired: + stdout, stderr = b"", b"" + assert False, (elapsed, stdout, stderr) + assert 0 <= elapsed <= limit, elapsed + self.last_timestamp = now + + def write_bad(self): + with open(self.conf_path, "w+") as f: + f.write(PY_LOG_CONFIG) + with open(self.temp_path / "syntax_error.py", "w+") as f: + f.write(PY_BAD_IMPORT) + with open(self.py_path, "w+") as f: + f.write(PY_BAD) + self.assert_fast() + + def write_ok(self): + with open(self.py_path, "w+") as f: + f.write(PY_OK) + self.assert_fast() + + def __exit__(self, *exc): + if self.returncode is None: + self.send_signal(signal.SIGKILL) + try: + stdout, _ = self.communicate(timeout=1) + if stdout: + logger.debug( + "stdout not empty on shutdown, sample: %r", stdout[-512:] + ) + except subprocess.TimeoutExpired: + pass + # still alive + if self.returncode is None: + self.kill() # no need to wait, Popen.__exit__ does that + super().__exit__(*exc) + + def fast_shutdown(self, expect=()): + stdout = self.stdout.read(64 * 1024) or b"" + stderr = self.stderr.read(64 * 1024) or b"" + assert self.stdin is None + if self.returncode is None: + self.send_signal(signal.SIGQUIT) + try: + o, e = self.communicate(timeout=2 + GRACEFUL_TIMEOUT) + stdout += o + stderr += e + except subprocess.TimeoutExpired: + pass + assert stdout == b"" + exitcode = self.poll() # will return None if running + assert exitcode == 0, (exitcode, stdout, stderr) + errors = stderr.decode("utf-8", "surrogateescape") + for keyword in expect: + assert keyword in errors, (keyword, errors) + self.assert_fast() + return stdout, stderr + + def read_stdio(self, *, timeout_sec=5, wait_for=()): + # try: + # stdout, stderr = self.communicate(timeout=timeout) + # except subprocess.TimeoutExpired: + buf = ["", ""] + wanted_strings = set(wait_for) + poll_per_second = 100 + for _ in range(timeout_sec * poll_per_second): + for fd, file in enumerate([self.stdout, self.stderr]): + read = file.read(64 * 1024) + if read is not None: + buf[fd] += read.decode("utf-8", "surrogateescape") + for either_buf in buf: + for wanted_str in tuple(wanted_strings): + if wanted_str in either_buf: + wanted_strings.remove(wanted_str) + # gathered all the context we wanted + if not wanted_strings: + break + time.sleep(1.0 / poll_per_second) + for wanted_str in wanted_strings: + assert any(wanted_str in either_buf for either_buf in buf), ( + wanted_str, + *buf, + ) + self.assert_fast(timeout_sec + TIMEOUT_SEC_PER_SUBTEST) + return buf + + +class Client: + def __init__(self, host_port): + self._host_port = host_port + + def run(self): + import http.client + + conn = http.client.HTTPConnection(self._host_port, timeout=2) + conn.request("GET", "/", headers={"Host": "localhost"}, body="GETBODY!") + return conn.getresponse() + + +@pytest.mark.skipif( + platform.python_implementation() == "PyPy", reason="slow on Github CI" +) +@pytest.mark.parametrize("worker_class", TEST_TOLERATES_BAD_BOOT) +def test_process_request_after_fixing_syntax_error(worker_class): + # 1. start up the server with invalid app + # 2. fixup the app by writing to file + # 3. await reload: the app should begin working soon + + fixed_port = 2048 + secrets.randbelow(1024 * 14) + # FIXME: should also test inherited socket (LISTEN_FDS) + server_bind = "[::1]:%d" % fixed_port + + client = Client(server_bind) + + with TemporaryDirectory(suffix="_temp_py") as tempdir_name: + with Server( + worker_class=worker_class, + server_bind=server_bind, + temp_path=Path(tempdir_name), + start_valid=False, + use_config=True, + public_traceback=False, + ) as server: + _boot_log = server.read_stdio( + wait_for={ + "Arbiter booted", + "SyntaxError: invalid syntax", + '%s.py", line ' % (APP_BASENAME,), + # yes, --on-fatal=brief still calls post_worker_init hook! + "Worker booted - continue test!", # see gunicorn.conf.py + }, + ) + + # raise RuntimeError(boot_log) + + # worker could not load, request will fail + response = client.run() + assert response.status == 500, (response.status, response.reason) + assert response.reason == "Internal Server Error", response.reason + body = response.read(64 * 1024).decode("utf-8", "surrogateescape") + # --on-fatal=brief responds, but does NOT share traceback + assert "error" in body.lower() + assert "load_wsgi" not in body.lower() + + _access_log = server.read_stdio( + wait_for={'"GET / HTTP/1.1" 500 '}, + ) + + # trigger reloader + server.write_ok() + # os.utime(editable_file) + + _reload_log = server.read_stdio( + wait_for={ + "reloading", + "%s.py modified" % (APP_BASENAME,), + "Booting worker", + "Worker exiting", # safeguard against hitting the old worker + "Worker booted - continue test!", # see gunicorn.conf.py + }, + ) + + # worker did boot now, request should work + response = client.run() + assert response.status == 200, (response.status, response.reason) + assert response.reason == "OK", response.reason + body = response.read(64 * 1024).decode("utf-8", "surrogateescape") + assert "response body from app" == body + + server.assert_fast() + + _debug_log = server.read_stdio( + wait_for={ + "Application called - continue test!", + # read access log + '"GET / HTTP/1.1"', + }, + ) + + _shutdown_log = server.fast_shutdown( + expect={ + # "Handling signal: term", + "Handling signal: quit", + # "Worker exiting ", # need graceful-timouet >= 1 to log + "Shutting down: Master", + } + ) + + +@pytest.mark.skipif( + platform.python_implementation() == "PyPy", reason="slow on Github CI" +) +@pytest.mark.parametrize("worker_class", TEST_TOLERATES_BAD_RELOAD) +def test_process_shutdown_cleanly_after_inserting_syntax_error(worker_class): + # 1. start with valid application + # 2. now insert fatal error by writing to app + # 3. await reload, the shutdown gracefully + + fixed_port = 2048 + secrets.randbelow(1024 * 14) + # FIXME: should also test inherited socket (LISTEN_FDS) + server_bind = "[::1]:%d" % fixed_port + + client = Client(server_bind) + + with TemporaryDirectory(suffix="_temp_py") as tempdir_name: + with Server( + server_bind=server_bind, + worker_class=worker_class, + temp_path=Path(tempdir_name), + use_config=True, + start_valid=True, + ) as server: + _boot_log = server.read_stdio( + wait_for={ + "Arbiter booted", + "Booting worker", + "Worker booted - continue test!", # see gunicorn.conf.py + }, + ) + + # worker did boot now, request should work + response = client.run() + assert response.status == 200, (response.status, response.reason) + assert response.reason == "OK", response.reason + body = response.read(64 * 1024).decode("utf-8", "surrogateescape") + assert "response body from app" == body + + server.assert_fast() + + _debug_log = server.read_stdio( + wait_for={"Application called - continue test!"}, + ) + + # trigger reloader + server.write_bad() + # os.utime(editable_file) + + # this test can fail flaky, when the keyword is not last line logged + # .. but the worker count is only logged when changed + _reload_log = server.read_stdio( + timeout_sec=7, + wait_for={ + # "%d workers" % WORKER_COUNT, + "SyntaxError: ", + "reloading", + "%s.py modified" % (APP_BASENAME,), + "SyntaxError: invalid syntax", + '%s.py", line ' % (APP_BASENAME,), + }, + ) + + # worker could not load, request will fail + response = client.run() + assert response.status == 500, (response.status, response.reason) + assert response.reason == "Internal Server Error", response.reason + body = response.read(64 * 1024).decode("utf-8", "surrogateescape") + # its a traceback + assert "load_wsgi" in body.lower() + + server.assert_fast() + + _access_log = server.read_stdio( + wait_for={'"GET / HTTP/1.1" 500 '}, + ) + + _shutdown_log = server.fast_shutdown( + expect={ + # "Handling signal: term", + "Handling signal: quit", + # "Worker exiting ", # need graceful-timouet >= 1 to log + "Shutting down: Master", + }, + ) diff --git a/tox.ini b/tox.ini index 9bf99e1be..d03f36bcb 100644 --- a/tox.ini +++ b/tox.ini @@ -31,6 +31,7 @@ commands = pylint -j0 \ --max-line-length=120 \ gunicorn \ + tests/test_e2e.py \ tests/test_arbiter.py \ tests/test_config.py \ tests/test_http.py \