From a5c0af7f506b841b492408109a763024d32995f4 Mon Sep 17 00:00:00 2001 From: Charlie Leitheiser Date: Tue, 28 Apr 2026 21:23:17 +0100 Subject: [PATCH] Use a private duplicate of stdout/stderr fd in CliRunner Since 8.3.3, ``_NamedTextIOWrapper.fileno()`` returns the file descriptor of the stream being replaced. Code under test that calls ``os.dup2`` over that value (logging tees, build tools, subprocess wrappers, ...) ends up redirecting the host process's real stdout/stderr fd, breaking outer harnesses such as pytest's fd-based capture. Reproduce with the snippet from issue #3384: pytest exits with ``OSError: Illegal seek`` from ``_pytest.capture`` while tearing down its own capture. Mirror pytest's own ``capfd`` model: ``os.dup`` the original fd inside ``CliRunner`` and expose the duplicate via ``fileno()``. ``os.dup2`` against the duplicate now only redirects this private copy, leaving the host's stdout/stderr intact. Close the duplicates when isolation exits. --- CHANGES.rst | 14 ++++++++++++++ src/click/testing.py | 38 ++++++++++++++++++++++++++++++++------ tests/test_testing.py | 34 ++++++++++++++++++++++++++++++++++ 3 files changed, 80 insertions(+), 6 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 506877590..03b4ad062 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -1,5 +1,19 @@ .. currentmodule:: click +Version 8.3.4 +------------- + +Unreleased + +- ``CliRunner`` now exposes a private duplicate of the original ``stdout`` + and ``stderr`` file descriptors via ``fileno()`` instead of the + descriptors themselves. Code under test that calls ``os.dup2`` over the + fd from ``sys.stdout.fileno()`` (e.g. logging tees, build tools, + subprocess wrappers) no longer overwrites the host process's real + ``stdout``/``stderr``, so outer harnesses such as ``pytest`` capture + keep working. :issue:`3384` + + Version 8.3.3 ------------- diff --git a/src/click/testing.py b/src/click/testing.py index 04e7f1d92..35b3d8a31 100644 --- a/src/click/testing.py +++ b/src/click/testing.py @@ -107,11 +107,20 @@ class _NamedTextIOWrapper(io.TextIOWrapper): An optional ``original_fd`` preserves the file descriptor of the stream being replaced, so that C-level consumers that call :meth:`fileno` (``faulthandler``, ``subprocess``, ...) still work. - Inspired by pytest's ``capsys``/``capfd`` split: see :doc:`/testing` - for details. + Callers should pass a private duplicate (``os.dup``) of the + underlying fd: that way, code that performs ``os.dup2`` over the + value returned by :meth:`fileno` redirects only the duplicate and + does not clobber the surrounding process's stdout/stderr (e.g. + pytest's fd-based capture). Inspired by pytest's ``capsys`` / + ``capfd`` split: see :doc:`/testing` for details. .. versionchanged:: 8.3.3 Added ``original_fd`` parameter and :meth:`fileno` override. + + .. versionchanged:: 8.3.4 + ``CliRunner`` now passes a private duplicate of the original + fd, so user code performing ``os.dup2`` on the returned value + no longer clobbers the host process's stdout/stderr. """ def __init__( @@ -360,14 +369,25 @@ def isolation( # valid fd from the redirected streams. The original streams # may themselves lack a fileno() (e.g. when CliRunner is used # inside pytest's capsys), so we fall back to -1. - def _safe_fileno(stream: t.IO[t.Any]) -> int: + # + # We expose a duplicate (``os.dup``) rather than the underlying + # fd directly. This is the same isolation pytest's ``capfd`` + # uses: if the invoked command performs ``os.dup2`` over the fd + # returned by ``fileno()`` it only redirects our private copy, + # not the original (which an outer harness like pytest may be + # capturing). The duplicate is closed when isolation exits. + def _safe_dup_fileno(stream: t.IO[t.Any]) -> int: try: - return stream.fileno() + fd = stream.fileno() except (AttributeError, io.UnsupportedOperation): return -1 + try: + return os.dup(fd) + except OSError: + return -1 - old_stdout_fd = _safe_fileno(old_stdout) - old_stderr_fd = _safe_fileno(old_stderr) + old_stdout_fd = _safe_dup_fileno(old_stdout) + old_stderr_fd = _safe_dup_fileno(old_stderr) if self.echo_stdin: bytes_input = echo_input = t.cast( @@ -514,6 +534,12 @@ def _patched_pdb_init( sys.stdout = old_stdout sys.stderr = old_stderr sys.stdin = old_stdin + for dup_fd in (old_stdout_fd, old_stderr_fd): + if dup_fd >= 0: + try: + os.close(dup_fd) + except OSError: + pass termui.visible_prompt_func = old_visible_prompt_func termui.hidden_prompt_func = old_hidden_prompt_func termui._getchar = old__getchar_func diff --git a/tests/test_testing.py b/tests/test_testing.py index c8fca4d4f..7d3b90690 100644 --- a/tests/test_testing.py +++ b/tests/test_testing.py @@ -545,3 +545,37 @@ def cli(flag): result = runner.invoke(cli, ["--flag", True]) assert result.exit_code == 0, result.output assert "Finished executing main function." in result.output + + +@pytest.mark.skipif(sys.platform == "win32", reason="POSIX fd semantics only") +def test_dup2_over_stdout_fd_does_not_leak(runner): + """``os.dup2`` over ``sys.stdout.fileno()`` inside ``CliRunner`` must not + overwrite the host process's stdout fd. The runner exposes a private + duplicate of the original fd, so user code (logging tees, subprocess + wrappers, etc.) only redirects its own copy. This isolation matches + pytest's ``capfd`` model and prevents breaking outer fd-based capture. + + Reproduce: https://github.com/pallets/click/issues/3384 + """ + + @click.command() + def cli(): + stdout_fd = sys.stdout.fileno() + # The fd CliRunner returns is a duplicate, not the host's + # stdout. Confirm by redirecting it to an os.pipe -- the host + # stdout (fd captured by the test runner) must be unaffected. + r, w = os.pipe() + try: + os.dup2(w, stdout_fd) + finally: + os.close(w) + os.close(r) + click.echo("hello") + + host_stdout_fd = os.dup(1) + try: + result = runner.invoke(cli) + finally: + os.close(host_stdout_fd) + assert result.exit_code == 0, result.output + assert "hello" in result.output