Use external mitmproxy. (#7720)

.github/workflows/tests-ubuntu.yml

@@ -79,9 +79,6 @@ jobs:
         - python-version: "3.14"
           env:
             TOXENV: botocore
-        - python-version: "3.14"
-          env:
-            TOXENV: mitmproxy
 
     steps:
     - uses: actions/checkout@v6
@@ -97,6 +94,9 @@ jobs:
         sudo apt-get update
         sudo apt-get install libxml2-dev libxslt-dev
 
+    - name: Install mitmproxy
+      run: pipx install mitmproxy
+
     - name: Run tests
       env: ${{ matrix.env }}
       run: |

conftest.py

@@ -11,7 +11,7 @@ from scrapy.utils.reactor import set_asyncio_event_loop_policy
 from scrapy.utils.reactorless import install_reactor_import_hook
 from tests.keys import generate_keys
 from tests.mockserver.http import MockServer
-from tests.mockserver.mitm_proxy import MitmProxy
+from tests.mockserver.mitm_proxy import MitmProxy, mitmdump_cmd
 
 if TYPE_CHECKING:
     from collections.abc import Generator
@@ -127,7 +127,6 @@ def pytest_runtest_setup(item):
         "uvloop",
         "botocore",
         "boto3",
-        "mitmproxy",
     ]
 
     for module in optional_deps:
@@ -137,6 +136,9 @@ def pytest_runtest_setup(item):
             except ImportError:
                 pytest.skip(f"{module} is not installed")
 
+    if item.get_closest_marker("requires_mitmproxy") and mitmdump_cmd() is None:
+        pytest.skip("mitmdump is not available")
+
 
 # Generate localhost certificate files, needed by some tests
 generate_keys()

docs/contributing.rst

@@ -372,6 +372,21 @@ To see coverage report install :doc:`coverage <coverage:index>`
 
 see output of ``coverage --help`` for more options like html or xml report.
 
+Some tests need a ``mitmdump`` executable (from mitmproxy_) to test against a
+fully featured proxy server; they are skipped when one cannot be found
+(``mitmproxy`` is intentionally not a test dependency that would be installed
+into test venvs, as that sometimes leads to various dependency conflicts).
+To run these tests, make ``mitmdump`` available in one of these ways:
+
+* install ``mitmproxy`` so that ``mitmdump`` is on your ``PATH``, e.g. with
+  pipx_ (``pipx install mitmproxy``) or uv_ (``uv tool install mitmproxy``);
+
+* have uv_ installed, in which case the tests will run
+  ``uvx --from mitmproxy mitmdump``;
+
+* set the ``MITMDUMP`` environment variable to the path of a ``mitmdump``
+  executable.
+
 Writing tests
 -------------
 
@@ -399,3 +414,6 @@ And their unit-tests are in::
 .. _pytest-xdist: https://github.com/pytest-dev/pytest-xdist
 .. _help wanted issues: https://github.com/scrapy/scrapy/issues?q=is%3Aissue+is%3Aopen+label%3A%22help+wanted%22
 .. _test coverage: https://app.codecov.io/gh/scrapy/scrapy
+.. _mitmproxy: https://mitmproxy.org/
+.. _pipx: https://pipx.pypa.io/
+.. _uv: https://docs.astral.sh/uv/

pyproject.toml

@@ -344,7 +344,7 @@ markers = [
     "requires_uvloop: marks tests as only enabled when uvloop is known to be working",
     "requires_botocore: marks tests that need botocore (but not boto3)",
     "requires_boto3: marks tests that need botocore and boto3",
-    "requires_mitmproxy: marks tests that need mitmproxy",
+    "requires_mitmproxy: marks tests that need a mitmdump executable",
     "requires_internet: marks tests that need real Internet access",
 ]
 filterwarnings = [

tests/mockserver/mitm_proxy.py

@@ -1,11 +1,42 @@
 from __future__ import annotations
 
-import re
-import sys
+import contextlib
+import functools
+import os
+import shutil
+import signal
+import socket
+import time
 from pathlib import Path
-from subprocess import PIPE, Popen
+from subprocess import DEVNULL, Popen
 from urllib.parse import urlsplit, urlunsplit
 
+from .utils import _free_port
+
+
+@functools.cache
+def mitmdump_cmd() -> list[str] | None:
+    """Return the command prefix used to invoke ``mitmdump``, or ``None`` if it
+    cannot be resolved.
+
+    We don't want to install ``mitmproxy`` into the test env (it has a lot of
+    dependencies that can conflict with some of the Scrapy/test ones, and its
+    newer versions may not support older Python versions). So we expect it
+    installed externally. We look for the ``mitmdump`` binary in the following
+    sources:
+
+    1. the ``MITMDUMP`` environment variable;
+    2. a ``mitmdump`` binary on ``PATH``;
+    3. using ``uvx --from mitmproxy mitmdump`` if ``uvx`` is available.
+    """
+    if env := os.environ.get("MITMDUMP"):
+        return [env]
+    if path := shutil.which("mitmdump"):
+        return [path]
+    if uvx := shutil.which("uvx"):
+        return [uvx, "--from", "mitmproxy", "mitmdump"]
+    return None
+
 
 class MitmProxy:
     auth_user = "scrapy"
@@ -15,18 +46,22 @@ class MitmProxy:
         self.mode = mode
 
     def start(self) -> str:
-        script = """
-import sys
-from mitmproxy.tools.main import mitmdump
-sys.argv[0] = "mitmdump"
-sys.exit(mitmdump())
-        """
+        cmd = mitmdump_cmd()
+        if not cmd:
+            raise RuntimeError(
+                "mitmdump is not available. Please install mitmproxy or uv."
+            )
         cert_path = Path(__file__).parent.parent.resolve() / "keys"
+        # Choose a free port ourselves instead of reading the mitmdump output
+        # as there is no easy way to disable stdout buffering for all kinds of
+        # mitmdump installs that we support.
+        host = "127.0.0.1"
+        port = _free_port()
         args = [
             "--listen-host",
-            "127.0.0.1",
+            host,
             "--listen-port",
-            "0",
+            str(port),
             "--proxyauth",
             f"{self.auth_user}:{self.auth_pass}",
             "--set",
@@ -37,30 +72,39 @@ sys.exit(mitmdump())
         ]
         if self.mode:
             args += ["--mode", self.mode]
-        self.proc: Popen[str] = Popen(
-            [
-                sys.executable,
-                "-u",
-                "-c",
-                script,
-                *args,
-            ],
-            stdout=PIPE,
-            text=True,
+        self.proc: Popen[bytes] = Popen(
+            [*cmd, *args],
+            stdout=DEVNULL,
+            stderr=DEVNULL,
+            start_new_session=True,  # needed for killpg() to make sense
         )
-        assert self.proc.stdout is not None
         scheme = "socks5" if self.mode == "socks5" else "http"
-        line = ""
-        for line in self.proc.stdout:
-            m = re.search(r"listening at (?:\w+://)?([^:]+:\d+)", line)
-            if m:
-                host_port = m.group(1)
-                return f"{scheme}://{self.auth_user}:{self.auth_pass}@{host_port}"
+        deadline = time.monotonic() + 60
+        while True:
+            if self.proc.poll() is not None:
+                raise RuntimeError(
+                    f"mitmdump exited with code {self.proc.returncode} before it "
+                    f"started listening"
+                )
+            try:
+                with socket.create_connection((host, port), timeout=1):
+                    return f"{scheme}://{self.auth_user}:{self.auth_pass}@{host}:{port}"
+            except OSError:
+                if time.monotonic() >= deadline:
+                    break
+                time.sleep(0.05)
         self.stop()
-        raise RuntimeError(f"Failed to parse mitmdump output: {line}")
+        raise RuntimeError(f"mitmdump did not start listening on {host}:{port} in time")
 
     def stop(self) -> None:
-        self.proc.kill()
+        if os.name == "posix":
+            # SIGKILL doesn't propagate to the actual process (child of uvx)
+            # https://github.com/astral-sh/uv/issues/11817#issuecomment-2688830077
+            # https://stackoverflow.com/a/61980200/113586
+            with contextlib.suppress(ProcessLookupError):
+                os.killpg(os.getpgid(self.proc.pid), signal.SIGKILL)
+        else:
+            self.proc.kill()
         self.proc.communicate()
 
 

tests/mockserver/utils.py

@@ -1,5 +1,6 @@
 from __future__ import annotations
 
+import socket
 from pathlib import Path
 from typing import TYPE_CHECKING, cast
 
@@ -18,6 +19,13 @@ if TYPE_CHECKING:
     from twisted.internet.interfaces import IOpenSSLContextFactory
 
 
+def _free_port() -> int:
+    # racy but should be fine for tests
+    with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s:
+        s.bind(("127.0.0.1", 0))
+        return cast("int", s.getsockname()[1])
+
+
 def ssl_context_factory(
     keyfile: str = "keys/localhost.key",
     certfile: str = "keys/localhost.crt",

tox.ini

@@ -28,7 +28,6 @@ envlist =
     no-reactor
     no-reactor-extra-deps
     botocore
-    mitmproxy
     pypy3
     pypy3-extra-deps
 minversion = 1.7.0
@@ -55,6 +54,7 @@ deps =
 passenv =
     PYTHONTRACEMALLOC
     PYTEST_ADDOPTS
+    MITMDUMP
     S3_TEST_FILE_URI
     AWS_ACCESS_KEY_ID
     AWS_SECRET_ACCESS_KEY
@@ -319,17 +319,3 @@ setenv =
     {[min]setenv}
 commands =
     pytest {posargs:--cov-config=pyproject.toml --cov=scrapy --cov-report=xml --cov-report= tests --junitxml=min-botocore.junit.xml -o junit_family=legacy} -m requires_botocore
-
-
-# Run proxy tests that use mitmproxy in a separate env to avoid installing
-# numerous mitmproxy deps in other envs (even in extra-deps), as they can
-# conflict with other deps we want, or don't want, to have installed there.
-
-[testenv:mitmproxy]
-deps =
-    {[testenv]deps}
-    # mitmproxy does not support PyPy
-    mitmproxy; implementation_name != "pypy"
-    httpx[http2,socks]
-commands =
-    pytest {posargs:--cov-config=pyproject.toml --cov=scrapy --cov-report=xml --cov-report= tests --junitxml=mitmproxy.junit.xml -o junit_family=legacy} -m requires_mitmproxy