From deb35a8af28272c6548118142c39145acf328e70 Mon Sep 17 00:00:00 2001 From: MobyNL Date: Tue, 18 Aug 2026 17:21:26 +0200 Subject: [PATCH 1/3] feat: proxy modes and proxy authentication The proxy could only ever be a forward proxy, which ruled out two things people actually need: standing in front of a service so a client needs no proxy settings at all, and running on a network where the machine cannot reach the internet without going through a proxy of its own. Start Mitm Proxy takes mode, one or several, and proxy_auth. Modes are checked when the keyword runs rather than left to the proxy: mitmproxy logs an unusable specification instead of raising it, so without this a typo surfaced as a startup timeout with no reason attached. The failure now carries mitmproxy's own explanation and an example of what a mode looks like. proxyauth turned out not to be a core option. It belongs to the addon of the same name, which registers it when the master loads its addons, so it cannot be passed when the options are built and is set on the master afterwards. Passing it to Options raises KeyError, which is how this was found. Reverse mode is exercised for real against a local server: a client reaches the origin with no proxy settings, rules still apply, and a blocked request is still blocked. Upstream mode is exercised by chaining two proxies, and asserts on routing rather than on inspection: reaching the origin proves nothing on its own because the origin is reachable anyway, so the decisive test points the chain at an upstream that is not there and requires it to fail. Rules on the *upstream* proxy are deliberately not asserted - with a live upstream mitmproxy the request arrives at its port but produces no HTTP flow there, so a test claiming otherwise would be claiming something untrue. That is documented on the keyword, since a suite might otherwise expect it. Transparent and socks5 are passed through and covered only at the options level, because they need the operating system or a client set up for them. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 8 + MitmLibrary/__init__.py | 53 +++++- MitmLibrary/proxy_controller.py | 33 +++- README.md | 18 ++ tests/test_proxy_modes.py | 298 ++++++++++++++++++++++++++++++++ 5 files changed, 407 insertions(+), 3 deletions(-) create mode 100644 tests/test_proxy_modes.py diff --git a/CHANGELOG.md b/CHANGELOG.md index e34ab17..af882e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -74,6 +74,14 @@ Two behaviour changes come with it: reported in assertion failures rather than hidden, so an assertion against a shortened recording cannot look complete. Requests that failed are recorded too, so a blocked call can be asserted on. +- `Start Mitm Proxy` takes `mode`, so the proxy can be something other than a forward + proxy: `reverse:` puts it in front of one server, so a client needs no proxy settings + at all; `upstream:` sends everything on through another proxy, which is what a + corporate network needs; `transparent` and `socks5` are passed through as well. A mode + that cannot be understood fails the keyword with mitmproxy's own explanation, rather + than leaving the proxy to fail to start for an unstated reason. +- `Start Mitm Proxy` takes `proxy_auth`, requiring clients to authenticate before the + proxy serves them. - `Get Proxy Rules` returns the loaded rules in the order they are applied. - Blocking rules have an alias, like every other rule, so they are removed the same way. - Rules survive a restart of the proxy: the registry outlives it, and only the addon diff --git a/MitmLibrary/__init__.py b/MitmLibrary/__init__.py index 1199c08..82d70fd 100644 --- a/MitmLibrary/__init__.py +++ b/MitmLibrary/__init__.py @@ -12,7 +12,7 @@ applications in a more realistic and controlled environment. """ -from typing import Any, Dict, List, Optional, Sequence +from typing import Any, Dict, List, Optional, Sequence, Union from mitmproxy.tools import dump from robot.api import logger @@ -115,6 +115,43 @@ class MitmLibrary: number dropped is reported in the failure message of an assertion rather than being hidden. A body longer than the second cap is shortened, and the recorded request says so. + = Proxy modes = + By default the proxy is a normal forward proxy: a client is configured to send its + traffic through it. `mode` on `Start Mitm Proxy` changes that, and takes either one + mode or a list of them. + + - `regular`: a forward proxy. The default. + - `reverse:http://host:port`: the proxy stands in front of one server. Clients talk + to the proxy directly, as if it were that server, so nothing needs to be configured + to use a proxy at all. + - `upstream:http://host:port`: a forward proxy that passes everything on to another + proxy. This is what a corporate network needs, where the machine running the tests + may not reach the internet on its own. Rules and recording still apply here, on + this proxy; what the proxy further up the chain makes of the traffic is its own + business, and it does not necessarily see it as separate requests. + - `transparent`: traffic is routed to the proxy by the network itself. Needs the + operating system to be set up for it. + - `socks5`: a SOCKS5 proxy rather than an HTTP one. + + A mode may be followed by `@host:port` to give it its own listening address, which + overrides `listen_host` and `listen_port`. `Get Proxy Address` reports where the + proxy actually ended up listening, which is why it reads that from the proxy rather + than repeating the arguments back. + + A specification that cannot be understood fails `Start Mitm Proxy` with mitmproxy's + own explanation, rather than leaving the proxy to fail to start for an unstated + reason. + + `transparent` and `socks5` are passed through to mitmproxy but are not exercised by + this library's own tests, because they need the operating system or a client + configured for them. + + == Example == + | # Stand in front of a service, so a client needs no proxy settings at all + | Start Mitm Proxy mode=reverse:http://127.0.0.1:5000 + + | # Send everything on through the network's own proxy + | Start Mitm Proxy mode=upstream:http://corporate-proxy:3128 = Mitm Certificates = To test with SSL verification or use a browser without ignoring certificates, you need to set up @@ -188,6 +225,8 @@ def start_mitm_proxy( record: bool = False, record_limit: int = DEFAULT_LIMIT, record_body_limit: int = DEFAULT_BODY_LIMIT, + mode: Optional[Union[str, List[str]]] = None, + proxy_auth: Optional[str] = None, ) -> None: """ Starts a proxy at the given host and port. @@ -205,11 +244,19 @@ def start_mitm_proxy( had been called. Off by default; see the `Recording` section. - record_limit: How many requests to keep when recording. - record_body_limit: How many bytes of each body to keep when recording. + - mode: How the proxy handles the traffic it receives. A single mode or a list of + them. Defaults to a normal forward proxy. See the `Proxy modes` section. + - proxy_auth: Require clients to authenticate before the proxy serves them. + `username:password` for one account, `any` to accept any combination, or + `@path/to/htpasswd` for an Apache htpasswd file. - Fails if the proxy cannot be started, for example when the port is already in use. + Fails if the proxy cannot be started, for example when the port is already in use, + or if a mode cannot be understood. Example: | Start Mitm Proxy 192.168.1.100 8888 /path/to/certificates True + | Start Mitm Proxy mode=reverse:http://127.0.0.1:5000 + | Start Mitm Proxy proxy_auth=tester:secret See the 'Mitm Certificates' section in the documentation for more information. """ @@ -223,6 +270,8 @@ def start_mitm_proxy( certificates_directory, ssl_insecure, self._build_addons, + mode, + proxy_auth, ) except Exception: # The controller has already discarded the master it could not start. The diff --git a/MitmLibrary/proxy_controller.py b/MitmLibrary/proxy_controller.py index cdbb31d..b9d6d37 100644 --- a/MitmLibrary/proxy_controller.py +++ b/MitmLibrary/proxy_controller.py @@ -13,9 +13,10 @@ import logging import time from concurrent.futures import Future, TimeoutError as FutureTimeoutError -from typing import Any, Callable, Dict, List, Optional, Sequence, Tuple +from typing import Any, Callable, Dict, List, Optional, Sequence, Tuple, Union from mitmproxy import options +from mitmproxy.proxy import mode_specs from mitmproxy.tools import dump from robot.api import logger @@ -68,6 +69,8 @@ def start( certificates_directory: Optional[str], ssl_insecure: bool, addon_factory: AddonFactory, + mode: Optional[Union[str, Sequence[str]]] = None, + proxy_auth: Optional[str] = None, ) -> None: """Starts the proxy and waits until it is actually listening. @@ -84,6 +87,8 @@ def start( } if certificates_directory is not None: option_kwargs["confdir"] = certificates_directory + if mode is not None: + option_kwargs["mode"] = self._parse_modes(mode) opts = options.Options(**option_kwargs) # Bind the master to the loop it will actually run on. Without this it binds # to whatever loop happens to be running on the calling thread, which is not @@ -95,6 +100,11 @@ def start( with_dumper=False, ) self.master = master + if proxy_auth is not None: + # proxyauth belongs to the addon of the same name, which registers it when + # the master loads its addons, so it does not exist yet when the options are + # built above. + master.options.update(proxyauth=proxy_auth) self._disable_errorcheck(master) for addon in addon_factory(master): master.addons.add(addon) @@ -110,6 +120,27 @@ def start( finally: logging.getLogger().removeHandler(collector) + @staticmethod + def _parse_modes(mode: Union[str, Sequence[str]]) -> List[str]: + """Checks the mode specifications and returns them as mitmproxy wants them. + + Parsing here means an unusable specification fails the keyword that gave it, + with mitmproxy's own explanation of what is wrong. Left to the proxy it would + surface as a startup timeout with nothing useful attached, because mitmproxy + logs the problem rather than raising it. + """ + modes = [mode] if isinstance(mode, str) else list(mode) + for spec in modes: + try: + mode_specs.ProxyMode.parse(spec) + except ValueError as error: + raise ValueError( + f"'{spec}' is not a usable proxy mode: {error}. Modes look like " + f"'regular', 'reverse:http://host:port', 'upstream:http://host:port', " + f"'transparent' or 'socks5', optionally followed by '@host:port'." + ) from error + return modes + @staticmethod def _disable_errorcheck(master: dump.DumpMaster) -> None: """Removes mitmproxy's errorcheck addon, which is wrong for a library. diff --git a/README.md b/README.md index 71fcf82..7395994 100644 --- a/README.md +++ b/README.md @@ -111,6 +111,24 @@ Be aware that `0.0.0.0` exposes an intercepting proxy on every network interface who can reach the machine can route their traffic through it. +### Proxy modes + +By default the proxy is a forward proxy: a client is configured to send traffic through +it. `mode` changes that: + +```robotframework +# Stand in front of a service, so a client needs no proxy settings at all +Start Mitm Proxy mode=reverse:http://127.0.0.1:5000 + +# Send everything on through the network's own proxy +Start Mitm Proxy mode=upstream:http://corporate-proxy:3128 +``` + +`transparent` and `socks5` are passed through to mitmproxy too. A mode that cannot be +understood fails `Start Mitm Proxy` rather than leaving the proxy to fail to start for an +unstated reason. `proxy_auth` requires clients to authenticate before the proxy serves +them. + ### Why use Mitm? Mitm allows manipulation on single browser instance, by using a proxy. It does not require you to set up stubs or mocks that might influence the entire application at diff --git a/tests/test_proxy_modes.py b/tests/test_proxy_modes.py new file mode 100644 index 0000000..f9336f0 --- /dev/null +++ b/tests/test_proxy_modes.py @@ -0,0 +1,298 @@ +"""Tests for the proxy modes and for proxy authentication. + +The options themselves are checked against a mocked master, because what matters there is +that the right values reach mitmproxy. Reverse and upstream mode are then exercised for +real, against a live proxy: both are modes people actually use, and both can be proven +without any network beyond localhost. +""" + +import http.server +import socket +import threading +import unittest +import urllib.error +import urllib.request +from types import SimpleNamespace +from unittest.mock import patch + +from MitmLibrary import MitmLibrary + + +def free_port() -> int: + """Returns a port that is free right now, to keep parallel runs from colliding.""" + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: + sock.bind(("127.0.0.1", 0)) + return sock.getsockname()[1] + + +def _addons_by_name(proxyserver): + """Fakes mitmproxy's addon lookup, which answers per name.""" + + def get(name): + return proxyserver if name == "proxyserver" else None + + return get + + +async def _noop_update(_modes): + return True + + +async def _runs_until_stopped(stop): + import asyncio + + while not stop.is_set(): + await asyncio.sleep(0.01) + + +class _Handler(http.server.BaseHTTPRequestHandler): + """Answers everything with a fixed body, so a test can tell it apart.""" + + def do_GET(self): # noqa: N802 - the name is fixed by http.server + body = b"hello from the origin" + self.send_response(200) + self.send_header("Content-Length", str(len(body))) + self.end_headers() + self.wfile.write(body) + + def log_message(self, *args): + """Silences the default logging to stderr, which is noise in a test run.""" + + +class TestModeOptions(unittest.TestCase): + """What reaches mitmproxy's options, against a mocked master.""" + + def setUp(self): + self.library = MitmLibrary() + self.stop = threading.Event() + patcher = patch("MitmLibrary.dump.DumpMaster") + mock_master = patcher.start() + self.addCleanup(patcher.stop) + mock_master.return_value.addons.get.side_effect = _addons_by_name( + SimpleNamespace( + listen_addrs=lambda: [("127.0.0.1", 8099)], + servers=SimpleNamespace(update=_noop_update), + ) + ) + mock_master.return_value.shutdown.side_effect = self.stop.set + mock_master.return_value.run = lambda: _runs_until_stopped(self.stop) + options_patcher = patch("MitmLibrary.proxy_controller.options.Options") + self.mock_options = options_patcher.start() + self.addCleanup(options_patcher.stop) + + def tearDown(self): + self.library.controller.shutdown() + + def kwargs(self): + return self.mock_options.call_args.kwargs + + def test_no_mode_is_passed_when_none_is_given(self): + """mitmproxy has its own default, and an unasked-for value would override it.""" + self.library.start_mitm_proxy() + self.assertNotIn("mode", self.kwargs()) + self.assertNotIn("proxyauth", self.kwargs()) + + def test_a_single_mode_is_passed_as_a_list(self): + """The option is a sequence, but a suite naturally passes one string.""" + self.library.start_mitm_proxy(mode="reverse:http://127.0.0.1:5000") + self.assertEqual(self.kwargs()["mode"], ["reverse:http://127.0.0.1:5000"]) + + def test_several_modes_are_passed_through(self): + self.library.start_mitm_proxy(mode=["regular", "socks5@127.0.0.1:9050"]) + self.assertEqual(self.kwargs()["mode"], ["regular", "socks5@127.0.0.1:9050"]) + + def test_proxy_authentication_is_set_on_the_master(self): + """proxyauth belongs to an addon, so it is not a core option and cannot be + passed when the options are built; it is set once the master has loaded them. + """ + self.library.start_mitm_proxy(proxy_auth="tester:secret") + self.assertNotIn("proxyauth", self.kwargs()) + self.library.proxy_master.options.update.assert_called_once_with( + proxyauth="tester:secret" + ) + + def test_transparent_and_socks_are_accepted_even_though_untested(self): + """Passed through to mitmproxy; the library does not second-guess them.""" + self.library.start_mitm_proxy(mode="transparent") + self.assertEqual(self.kwargs()["mode"], ["transparent"]) + + +class TestModeValidation(unittest.TestCase): + def setUp(self): + self.library = MitmLibrary() + + def tearDown(self): + self.library.controller.shutdown() + + def test_an_unknown_mode_fails_the_keyword(self): + """Left to the proxy this would be a startup timeout with no reason attached.""" + with self.assertRaises(ValueError) as context: + self.library.start_mitm_proxy(mode="nonsense:foo") + message = str(context.exception) + self.assertIn("nonsense:foo", message) + self.assertIn("not a usable proxy mode", message) + + def test_the_failure_explains_what_a_mode_looks_like(self): + with self.assertRaises(ValueError) as context: + self.library.start_mitm_proxy(mode="reverse:") + self.assertIn("reverse:http://host:port", str(context.exception)) + + def test_one_bad_mode_in_a_list_fails(self): + with self.assertRaises(ValueError): + self.library.start_mitm_proxy(mode=["regular", "nonsense:foo"]) + + def test_nothing_is_left_running_after_a_bad_mode(self): + with self.assertRaises(ValueError): + self.library.start_mitm_proxy(mode="nonsense:foo") + self.assertIsNone(self.library.proxy_master) + + +class TestReverseMode(unittest.TestCase): + """Reverse mode against a real server, which needs no network beyond localhost.""" + + def setUp(self): + self.library = MitmLibrary() + self.origin = http.server.ThreadingHTTPServer(("127.0.0.1", 0), _Handler) + self.origin_port = self.origin.server_address[1] + self.thread = threading.Thread(target=self.origin.serve_forever, daemon=True) + self.thread.start() + + def tearDown(self): + self.library.controller.shutdown() + self.origin.shutdown() + self.origin.server_close() + self.thread.join(timeout=5) + + def test_a_client_reaches_the_origin_without_proxy_settings(self): + """The point of reverse mode: the client talks to the proxy as if it were the + server, so nothing has to be configured to use a proxy at all. + """ + port = free_port() + self.library.start_mitm_proxy( + listen_port=port, mode=f"reverse:http://127.0.0.1:{self.origin_port}" + ) + with urllib.request.urlopen(f"http://127.0.0.1:{port}/", timeout=10) as answer: + self.assertEqual(answer.read(), b"hello from the origin") + + def test_rules_apply_in_reverse_mode(self): + """A mode that could not be manipulated would not be worth having.""" + port = free_port() + self.library.start_mitm_proxy( + listen_port=port, mode=f"reverse:http://127.0.0.1:{self.origin_port}" + ) + self.library.set_response_body("stub", "/", "replaced by the proxy") + with urllib.request.urlopen(f"http://127.0.0.1:{port}/", timeout=10) as answer: + self.assertEqual(answer.read(), b"replaced by the proxy") + + def test_a_blocked_request_is_blocked_in_reverse_mode(self): + """Request-phase rules have to run too, not only response-phase ones.""" + port = free_port() + self.library.start_mitm_proxy( + listen_port=port, mode=f"reverse:http://127.0.0.1:{self.origin_port}" + ) + self.library.block_requests("blocked", "/", status_code=503) + with self.assertRaises(urllib.error.HTTPError) as context: + urllib.request.urlopen(f"http://127.0.0.1:{port}/", timeout=10) + self.assertEqual(context.exception.code, 503) + + +class TestUpstreamMode(unittest.TestCase): + """Two proxies chained, which is what a corporate network needs. + + These assert that traffic is *routed* through the upstream proxy, by contrasting a + live upstream with a dead one. They deliberately do not assert that the upstream + proxy can read or change the traffic: with a live upstream mitmproxy the request + arrives at its port but produces no HTTP flow there, so rules on the upstream proxy + do not fire. Routing is what this library configures and what a suite depends on; + what an arbitrary upstream proxy then does with the traffic is its own business. + """ + + def setUp(self): + self.origin = http.server.ThreadingHTTPServer(("127.0.0.1", 0), _Handler) + self.origin_port = self.origin.server_address[1] + self.thread = threading.Thread(target=self.origin.serve_forever, daemon=True) + self.thread.start() + self.upstream = MitmLibrary() + self.downstream = MitmLibrary() + + def tearDown(self): + self.downstream.controller.shutdown() + self.upstream.controller.shutdown() + self.origin.shutdown() + self.origin.server_close() + self.thread.join(timeout=5) + + def _open_through(self, proxy_port): + opener = urllib.request.build_opener( + urllib.request.ProxyHandler({"http": f"http://127.0.0.1:{proxy_port}"}) + ) + return opener.open(f"http://127.0.0.1:{self.origin_port}/", timeout=10) + + def test_a_request_reaches_the_origin_through_the_chain(self): + upstream_port = free_port() + downstream_port = free_port() + self.upstream.start_mitm_proxy(listen_port=upstream_port) + self.downstream.start_mitm_proxy( + listen_port=downstream_port, + mode=f"upstream:http://127.0.0.1:{upstream_port}", + ) + with self._open_through(downstream_port) as answer: + self.assertEqual(answer.read(), b"hello from the origin") + + def test_the_request_really_goes_through_the_upstream_proxy(self): + """The other half of the test above, and the half that proves anything. + + Reaching the origin does not on its own show the upstream proxy was involved, + because the origin is reachable either way. Pointing the chain at an upstream + that is not there has to break it. + """ + downstream_port = free_port() + nothing_listening = free_port() + self.downstream.start_mitm_proxy( + listen_port=downstream_port, + mode=f"upstream:http://127.0.0.1:{nothing_listening}", + ) + with self.assertRaises(urllib.error.HTTPError) as context: + self._open_through(downstream_port) + self.assertEqual(context.exception.code, 502) + + def test_the_downstream_proxy_still_applies_its_own_rules(self): + upstream_port = free_port() + downstream_port = free_port() + self.upstream.start_mitm_proxy(listen_port=upstream_port) + self.downstream.start_mitm_proxy( + listen_port=downstream_port, + mode=f"upstream:http://127.0.0.1:{upstream_port}", + ) + self.downstream.set_response_body("stub", "/", "answered downstream") + with self._open_through(downstream_port) as answer: + self.assertEqual(answer.read(), b"answered downstream") + + +class TestProxyAuthentication(unittest.TestCase): + def setUp(self): + self.library = MitmLibrary() + self.origin = http.server.ThreadingHTTPServer(("127.0.0.1", 0), _Handler) + self.origin_port = self.origin.server_address[1] + self.thread = threading.Thread(target=self.origin.serve_forever, daemon=True) + self.thread.start() + + def tearDown(self): + self.library.controller.shutdown() + self.origin.shutdown() + self.origin.server_close() + self.thread.join(timeout=5) + + def test_a_client_without_credentials_is_refused(self): + port = free_port() + self.library.start_mitm_proxy(listen_port=port, proxy_auth="tester:secret") + opener = urllib.request.build_opener( + urllib.request.ProxyHandler({"http": f"http://127.0.0.1:{port}"}) + ) + with self.assertRaises(urllib.error.HTTPError) as context: + opener.open(f"http://127.0.0.1:{self.origin_port}/", timeout=10) + self.assertEqual(context.exception.code, 407) + + +if __name__ == "__main__": + unittest.main() From 4bcfe2c2482d8dcd2a676e1a4614b69aa97dbc9d Mon Sep 17 00:00:00 2001 From: MobyNL Date: Tue, 18 Aug 2026 17:26:12 +0200 Subject: [PATCH 2/3] fix: remove the mitmproxy log handler when the proxy stops The Robot Framework version legs caught this: a pre-existing test failed with "multiple unraisable exception warnings", triggered by the new tests in this branch rather than by anything they assert. mitmproxy attaches a handler to the root logger that forwards every log record to its master's event loop, and never removes it. Once the proxy has stopped and its loop is closed, any later log record - from anywhere in the test run, not just from this library - raises "Event loop is closed" inside logging. Those surface as unraisable exceptions attributed to whichever test happens to be running, which is why the failure landed on an unrelated one. It is also a leak: every proxy started leaves another handler on the root logger, all of them holding a dead master. Discarding a proxy now uninstalls its handler. Two tests cover it: one that a stopped proxy leaves none behind and that logging afterwards is harmless, and one that starting three proxies does not pile them up. Co-Authored-By: Claude Opus 5 --- MitmLibrary/proxy_controller.py | 20 +++++++++++++++++ tests/test_proxy_integration.py | 40 +++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+) diff --git a/MitmLibrary/proxy_controller.py b/MitmLibrary/proxy_controller.py index b9d6d37..ec1ec80 100644 --- a/MitmLibrary/proxy_controller.py +++ b/MitmLibrary/proxy_controller.py @@ -259,9 +259,29 @@ def discard(self, wait: bool = True) -> None: ) except Exception as error: # pylint: disable=broad-exception-caught logger.info(f"The proxy stopped with an error: {error}") + self._uninstall_log_handler() self.master = None self.future = None + def _uninstall_log_handler(self) -> None: + """Removes the root logger handler mitmproxy installed for this master. + + mitmproxy attaches a handler to the root logger that forwards every log record + to the master's event loop. It never removes it, so once the proxy has stopped + and its loop is closed, any later log record - from anywhere in the test run - + raises "Event loop is closed" inside logging. Starting several proxies in one run + leaves one such handler behind each time. + """ + if self.master is None: + return + handler = getattr(self.master, "_legacy_log_events", None) + if handler is None: # pragma: no cover - present in every version we support + return + try: + handler.uninstall() + except Exception as error: # pylint: disable=broad-exception-caught + logger.info(f"Could not remove the mitmproxy log handler: {error}") + def _close_servers(self) -> None: """Closes the listening sockets held by the proxyserver addon. diff --git a/tests/test_proxy_integration.py b/tests/test_proxy_integration.py index 652b860..5792b2d 100644 --- a/tests/test_proxy_integration.py +++ b/tests/test_proxy_integration.py @@ -168,6 +168,46 @@ def _request_through_proxy(proxy_port, url): opener.open(url, timeout=5).close() except Exception: # noqa: BLE001 - the answer does not matter, only the record pass + def test_stopping_removes_the_mitmproxy_log_handler(self): + """mitmproxy leaves a root logger handler behind that outlives its own loop. + + Every record logged afterwards is forwarded to a closed event loop, which raises + inside logging, and a run that starts several proxies accumulates one handler + per proxy. Nothing in the library logs enough to notice; a long suite does. + """ + from mitmproxy import log as mitmproxy_log + + def installed(): + return [ + handler + for handler in logging.getLogger().handlers + if isinstance(handler, mitmproxy_log.MitmLogHandler) + ] + + before = len(installed()) + self.library.start_mitm_proxy(listen_port=self.port) + self.assertGreater(len(installed()), before) + self.library.stop_mitm_proxy() + self.assertEqual(len(installed()), before) + + # Logging after the proxy is gone must not raise into the logging machinery. + logging.getLogger("some.other.component").warning("after the proxy stopped") + + def test_starting_several_proxies_does_not_pile_up_log_handlers(self): + from mitmproxy import log as mitmproxy_log + + def installed(): + return [ + handler + for handler in logging.getLogger().handlers + if isinstance(handler, mitmproxy_log.MitmLogHandler) + ] + + before = len(installed()) + for _ in range(3): + self.library.start_mitm_proxy(listen_port=free_port()) + self.library.stop_mitm_proxy() + self.assertEqual(len(installed()), before) if __name__ == "__main__": From 45913fb8050c196560defe8389735c4da0687851 Mon Sep 17 00:00:00 2001 From: MobyNL Date: Tue, 18 Aug 2026 17:30:44 +0200 Subject: [PATCH 3/3] fix: only fail a startup on a failure to bind, not on unrelated noise The Robot Framework version legs caught this too: starting a proxy failed with "Task was destroyed but it is pending!" and "Addon error: Event loop is closed", neither of which has anything to do with the proxy being started. StartupErrorCollector listens on the root logger, because that is where mitmproxy reports a bind failure. It therefore also hears everything else: a proxy stopped moments ago still logs from its own teardown, asyncio complains about tasks it destroyed, and the rest of the test run logs as well. Any of those arriving during the startup window ended the wait and failed the keyword, which is the same mistake mitmproxy's own errorcheck addon makes and which this library removes that addon for. The collector now keeps only records from mitmproxy, and only a message reporting a failure to listen ends the wait. Anything else is still remembered and still reported if the startup times out for its own reasons, so nothing diagnostic is lost. Tests: one that logs exactly that noise from another thread while a proxy starts and requires the proxy to come up anyway, and one that a real bind failure is still reported, so the filter cannot swallow the failure it exists to report. Co-Authored-By: Claude Opus 5 --- MitmLibrary/proxy_controller.py | 24 ++++++++++++++++++++--- tests/test_proxy_integration.py | 34 +++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 3 deletions(-) diff --git a/MitmLibrary/proxy_controller.py b/MitmLibrary/proxy_controller.py index ec1ec80..596c18f 100644 --- a/MitmLibrary/proxy_controller.py +++ b/MitmLibrary/proxy_controller.py @@ -34,14 +34,30 @@ class StartupErrorCollector(logging.Handler): mitmproxy reports bind failures through the logging module rather than by raising, so this is the only way to tell the user *why* the proxy did not start. + + It listens on the root logger, which means it also hears errors that have nothing to + do with this proxy - a proxy stopped moments ago still logs from its own teardown, + and so does the rest of the test run. Only records from mitmproxy are kept, and only + those that report a failure to listen are treated as a reason to give up. Everything + else is remembered for the failure message but does not, by itself, fail a startup + that would otherwise have succeeded. """ + #: What mitmproxy says when it cannot bind, which is the failure worth acting on. + BIND_FAILURE = "failed to listen" + def __init__(self) -> None: super().__init__(level=logging.ERROR) self.messages: List[str] = [] + self.bind_failures: List[str] = [] def emit(self, record: logging.LogRecord) -> None: - self.messages.append(record.getMessage()) + if not record.name.startswith("mitmproxy"): + return + message = record.getMessage() + self.messages.append(message) + if self.BIND_FAILURE in message: + self.bind_failures.append(message) class ProxyController: @@ -187,11 +203,13 @@ def _fail_on_startup_error( ) if self.listen_addresses(proxy_master): return - if collector.messages: + if collector.bind_failures: break time.sleep(STARTUP_POLL_INTERVAL) - reported = "; ".join(collector.messages) or "no error reported" + reported = "; ".join( + collector.bind_failures or collector.messages + ) or "no error reported" self.discard(wait=False) raise RuntimeError( f"Could not start the proxy on {listen_host}:{listen_port}: {reported}" diff --git a/tests/test_proxy_integration.py b/tests/test_proxy_integration.py index 5792b2d..9f720df 100644 --- a/tests/test_proxy_integration.py +++ b/tests/test_proxy_integration.py @@ -100,6 +100,40 @@ def test_port_zero_reports_the_port_the_system_picked(self): with self.assertRaises(OSError): sock.bind(("127.0.0.1", address.port)) + def test_an_unrelated_error_during_startup_does_not_fail_the_keyword(self): + """A proxy stopped moments ago still logs from its own teardown, and the rest of + the test run logs too. Those must not be read as this proxy failing to bind: the + collector listens on the root logger, so it hears all of them. + """ + noisy = threading.Thread(target=self._log_errors_briefly) + noisy.start() + self.addCleanup(noisy.join, 5) + self.library.start_mitm_proxy(listen_port=self.port) + self.assertEqual( + self.library.controller.listen_addresses(), [("127.0.0.1", self.port)] + ) + + @staticmethod + def _log_errors_briefly(): + """Logs the kind of noise a stopping proxy leaves behind.""" + deadline = time.monotonic() + 1 + mitm_logger = logging.getLogger("mitmproxy.addons.something") + while time.monotonic() < deadline: + mitm_logger.error("Addon error: Event loop is closed") + logging.getLogger("asyncio").error("Task was destroyed but it is pending!") + time.sleep(0.02) + + def test_a_real_bind_failure_is_still_reported(self): + """The noise filter must not swallow the failure it exists to report.""" + blocker = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + blocker.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) + blocker.bind(("127.0.0.1", self.port)) + blocker.listen(1) + self.addCleanup(blocker.close) + with self.assertRaises(RuntimeError) as context: + self.library.start_mitm_proxy(listen_port=self.port) + self.assertIn("failed to listen", str(context.exception)) + def test_an_unrelated_logged_error_does_not_kill_the_proxy(self): """mitmproxy's errorcheck addon exits the process when anything logged an error while a master starts. It watches the root logger, so the error can come from a