From 28d1214f76e59e84d5905c0b7dedfac73f785b18 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 17 May 2026 16:30:41 -0700 Subject: [PATCH 1/5] Stop calling socket.getfqdn() in BaseRequest.host fallback (#12597) --- CHANGES/12597.breaking.rst | 1 + CHANGES/9308.breaking.rst | 12 +++++++ aiohttp/test_utils.py | 8 +++-- aiohttp/web_protocol.py | 11 ++++++ aiohttp/web_request.py | 18 ++++++++-- docs/web_reference.rst | 11 +++++- tests/test_web_request.py | 69 ++++++++++++++++++++++++++++++++------ 7 files changed, 113 insertions(+), 17 deletions(-) create mode 120000 CHANGES/12597.breaking.rst create mode 100644 CHANGES/9308.breaking.rst diff --git a/CHANGES/12597.breaking.rst b/CHANGES/12597.breaking.rst new file mode 120000 index 00000000000..651891d5065 --- /dev/null +++ b/CHANGES/12597.breaking.rst @@ -0,0 +1 @@ +9308.breaking.rst \ No newline at end of file diff --git a/CHANGES/9308.breaking.rst b/CHANGES/9308.breaking.rst new file mode 100644 index 00000000000..afb3965ab8f --- /dev/null +++ b/CHANGES/9308.breaking.rst @@ -0,0 +1,12 @@ +Stopped calling :func:`socket.getfqdn` as the fallback for +:attr:`aiohttp.web.BaseRequest.host`. :func:`socket.getfqdn` +performs blocking reverse DNS resolution on the event loop +thread and can stall a worker for many seconds when the system +resolver is slow, and could be triggered remotely by an HTTP/1.0 +request that omits the ``Host`` header. The fallback when no +``Host`` header is present is now the local socket address the +request arrived on (transport ``sockname``), or an empty string +if no transport information is available. Code that relied on +the FQDN being returned must now read it from +:func:`socket.getfqdn` directly, off the event loop +-- by :user:`bdraco`. diff --git a/aiohttp/test_utils.py b/aiohttp/test_utils.py index 6bbe590b18c..2fd3fef78de 100644 --- a/aiohttp/test_utils.py +++ b/aiohttp/test_utils.py @@ -550,11 +550,10 @@ def set_dict(app: Any, key: str, value: Any) -> None: def _create_transport(sslcontext: SSLContext | None = None) -> mock.Mock: transport = mock.Mock() - def get_extra_info(key: str) -> SSLContext | None: + def get_extra_info(key: str) -> SSLContext | tuple[str, int] | None: if key == "sslcontext": return sslcontext - else: - return None + return ("127.0.0.1", 80) if key == "sockname" else None transport.get_extra_info.side_effect = get_extra_info return transport @@ -635,6 +634,9 @@ def make_mocked_request( type(protocol).peername = mock.PropertyMock( return_value=transport.get_extra_info("peername") ) + type(protocol).sockname = mock.PropertyMock( + return_value=transport.get_extra_info("sockname") + ) type(protocol).ssl_context = mock.PropertyMock(return_value=sslcontext) if writer is None: diff --git a/aiohttp/web_protocol.py b/aiohttp/web_protocol.py index 2728eecf953..479ad931a3c 100644 --- a/aiohttp/web_protocol.py +++ b/aiohttp/web_protocol.py @@ -305,6 +305,17 @@ def peername( else self.transport.get_extra_info("peername") ) + @under_cached_property + def sockname( + self, + ) -> str | tuple[str, int, int, int] | tuple[str, int] | None: + """Return sockname if available.""" + return ( + None + if self.transport is None + else self.transport.get_extra_info("sockname") + ) + @property def keepalive_timeout(self) -> float: return self._keepalive_timeout diff --git a/aiohttp/web_request.py b/aiohttp/web_request.py index ee85c2af6f2..745681e699d 100644 --- a/aiohttp/web_request.py +++ b/aiohttp/web_request.py @@ -2,7 +2,6 @@ import datetime import io import re -import socket import string import sys import tempfile @@ -165,6 +164,7 @@ def __init__( self._transport_sslcontext = protocol.ssl_context self._transport_peername = protocol.peername + self._transport_sockname = protocol.sockname if remote is not None: self._cache["remote"] = remote @@ -372,7 +372,9 @@ def host(self) -> str: - overridden value by .clone(host=new_host) call. - HOST HTTP header - - socket.getfqdn() value + - local socket address the request arrived on + (transport ``sockname``) + - empty string if no transport information is available For example, 'example.com' or 'localhost:8080'. @@ -381,7 +383,17 @@ def host(self) -> str: host = self._message.headers.get(hdrs.HOST) if host is not None: return host - return socket.getfqdn() + sockname = self._transport_sockname + if sockname is None: + return "" + if isinstance(sockname, tuple): + # AF_INET6 returns a 4-tuple (host, port, flowinfo, scopeid); + # bracket the bare address so it matches the Host-header shape + # and is a valid URL authority component. + if len(sockname) == 4: + return f"[{sockname[0]}]" + return str(sockname[0]) + return str(sockname) @reify def remote(self) -> str | None: diff --git a/docs/web_reference.rst b/docs/web_reference.rst index 99318ec90ee..efb1d6e1608 100644 --- a/docs/web_reference.rst +++ b/docs/web_reference.rst @@ -119,7 +119,9 @@ and :ref:`aiohttp-web-signals` handlers. - Overridden value by :meth:`~BaseRequest.clone` call. - *Host* HTTP header - - :func:`socket.getfqdn` + - local socket address the request arrived on + (transport ``sockname``) + - empty string if no transport information is available Read-only :class:`str` property. @@ -130,6 +132,13 @@ and :ref:`aiohttp-web-signals` handlers. Call ``.clone(host=new_host)`` for setting up the value explicitly. + .. versionchanged:: 3.13 + + The fallback when no ``Host`` header is present no longer + calls :func:`socket.getfqdn`, which performed blocking + reverse-DNS resolution on the event loop. The local socket + address (transport ``sockname``) is used instead. + .. seealso:: :ref:`aiohttp-web-forwarded-support` .. attribute:: remote diff --git a/tests/test_web_request.py b/tests/test_web_request.py index 9c43f9d609c..3ad652916d9 100644 --- a/tests/test_web_request.py +++ b/tests/test_web_request.py @@ -1,7 +1,6 @@ import asyncio import datetime import logging -import socket import ssl import sys import time @@ -44,16 +43,19 @@ def test_base_ctor() -> None: URL("/path/to?a=1&b=2"), ) + protocol = mock.Mock() + protocol.ssl_context = None + protocol.peername = None + protocol.sockname = ("127.0.0.1", 80) req = web.BaseRequest( - message, mock.Mock(), mock.Mock(), mock.Mock(), mock.Mock(), mock.Mock() + message, mock.Mock(), protocol, mock.Mock(), mock.Mock(), mock.Mock() ) assert "GET" == req.method assert HttpVersion(1, 1) == req.version - # MacOS may return CamelCased host name, need .lower() - # FQDN can be wider than host, e.g. - # 'fv-az397-495' in 'fv-az397-495.internal.cloudapp.net' - assert req.host.lower() in socket.getfqdn().lower() + # No Host header in this request, so host falls back to the + # local socket address the request arrived on. + assert req.host == "127.0.0.1" assert "/path/to?a=1&b=2" == req.path_qs assert "/path/to" == req.path assert "a=1&b=2" == req.query_string @@ -75,10 +77,10 @@ def test_ctor() -> None: assert "GET" == req.method assert HttpVersion(1, 1) == req.version - # MacOS may return CamelCased host name, need .lower() - # FQDN can be wider than host, e.g. - # 'fv-az397-495' in 'fv-az397-495.internal.cloudapp.net' - assert req.host.lower() in socket.getfqdn().lower() + # No Host header in this request, so host falls back to the + # local socket address the request arrived on (the default + # sockname configured by make_mocked_request). + assert req.host == "127.0.0.1" assert "/path/to?a=1&b=2" == req.path_qs assert "/path/to" == req.path assert "a=1&b=2" == req.query_string @@ -114,6 +116,53 @@ def test_ctor() -> None: assert req.task is req._task +def test_host_falls_back_to_sockname_not_dns() -> None: + """Regression: request.host must not call socket.getfqdn(). + + socket.getfqdn() does blocking reverse DNS resolution on the + event loop thread and can stall a worker for many seconds when + the system resolver is slow. The fallback for a request with no + Host header is the local socket address the request arrived on, + not the system FQDN. + """ + req = make_mocked_request("GET", "/") + assert req.host == "127.0.0.1" + assert str(req.url).startswith("http://127.0.0.1") + + +def test_host_with_ipv6_sockname() -> None: + """AF_INET6 sockname is bracketed to form a valid URL authority. + + A bare IPv6 string would cause ``URL.build(authority=...)`` to + raise ``ValueError``. + """ + transport = mock.Mock() + transport.get_extra_info.side_effect = lambda key: ( + ("::1", 80, 0, 0) if key == "sockname" else None + ) + req = make_mocked_request("GET", "/", transport=transport) + assert req.host == "[::1]" + assert str(req.url) == "http://[::1]/" + + +def test_host_with_unix_socket_sockname() -> None: + """Unix-socket transports expose sockname as a str path.""" + transport = mock.Mock() + transport.get_extra_info.side_effect = lambda key: ( + "/tmp/aiohttp.sock" if key == "sockname" else None + ) + req = make_mocked_request("GET", "/", transport=transport) + assert req.host == "/tmp/aiohttp.sock" + + +def test_host_with_no_transport_sockname() -> None: + """An empty string is returned when no sockname is available.""" + transport = mock.Mock() + transport.get_extra_info.return_value = None + req = make_mocked_request("GET", "/", transport=transport) + assert req.host == "" + + def test_doubleslashes() -> None: # NB: //foo/bar is an absolute URL with foo netloc and /bar path req = make_mocked_request("GET", "/bar//foo/") From 7e3ddbd5192e6628b077409134c1d32962144176 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 18 May 2026 00:19:34 +0000 Subject: [PATCH 2/5] Bump uvloop from 0.21.0 to 0.22.1 (#11682) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bumps [uvloop](https://github.com/MagicStack/uvloop) from 0.21.0 to 0.22.1.
Release notes

Sourced from uvloop's releases.

v0.22.1

This is identical to 0.22.0, re-ran with CI fixes

v0.22.0

Changes

Fixes

Commits

Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: J. Nick Koston --- requirements/base.txt | 2 +- requirements/constraints.txt | 2 +- requirements/dev.txt | 2 +- requirements/lint.txt | 2 +- requirements/test.txt | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/requirements/base.txt b/requirements/base.txt index c19ff93a897..04f4bd98e6b 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -45,7 +45,7 @@ typing-extensions==4.15.0 ; python_version < "3.13" # -r requirements/runtime-deps.in # aiosignal # multidict -uvloop==0.21.0 ; platform_system != "Windows" and implementation_name == "cpython" +uvloop==0.22.1 ; platform_system != "Windows" and implementation_name == "cpython" # via -r requirements/base.in yarl==1.22.0 # via -r requirements/runtime-deps.in diff --git a/requirements/constraints.txt b/requirements/constraints.txt index 8ebe46e1298..10c1d61adf3 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -299,7 +299,7 @@ typing-inspection==0.4.2 # via pydantic urllib3==2.7.0 # via requests -uvloop==0.21.0 ; platform_system != "Windows" +uvloop==0.22.1 ; platform_system != "Windows" # via # -r requirements/base.in # -r requirements/lint.in diff --git a/requirements/dev.txt b/requirements/dev.txt index fbb0de86979..e7f10a52253 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -289,7 +289,7 @@ typing-inspection==0.4.2 # via pydantic urllib3==2.7.0 # via requests -uvloop==0.21.0 ; platform_system != "Windows" and implementation_name == "cpython" +uvloop==0.22.1 ; platform_system != "Windows" and implementation_name == "cpython" # via # -r requirements/base.in # -r requirements/lint.in diff --git a/requirements/lint.txt b/requirements/lint.txt index 6ec743cb89e..4c70287b346 100644 --- a/requirements/lint.txt +++ b/requirements/lint.txt @@ -161,7 +161,7 @@ typing-extensions==4.15.0 # virtualenv typing-inspection==0.4.2 # via pydantic -uvloop==0.21.0 ; platform_system != "Windows" +uvloop==0.22.1 ; platform_system != "Windows" # via -r requirements/lint.in valkey==6.1.1 # via -r requirements/lint.in diff --git a/requirements/test.txt b/requirements/test.txt index 602e45e91d1..16cb56e094f 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -168,7 +168,7 @@ typing-extensions==4.15.0 ; python_version < "3.13" # typing-inspection typing-inspection==0.4.2 # via pydantic -uvloop==0.21.0 ; platform_system != "Windows" and implementation_name == "cpython" +uvloop==0.22.1 ; platform_system != "Windows" and implementation_name == "cpython" # via -r requirements/base.in wait-for-it==2.3.0 # via -r requirements/test-common.in From dbe83fd0776fd91b94543c354f5b8172b49a1d15 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 17 May 2026 17:25:14 -0700 Subject: [PATCH 3/5] Fix flaky test_handler_returns_not_response on PyPy via debug-mode fixture (#12603) --- CHANGES/12603.contrib.rst | 8 ++++++++ tests/conftest.py | 18 ++++++++++++++++++ tests/test_web_functional.py | 10 ++++++---- 3 files changed, 32 insertions(+), 4 deletions(-) create mode 100644 CHANGES/12603.contrib.rst diff --git a/CHANGES/12603.contrib.rst b/CHANGES/12603.contrib.rst new file mode 100644 index 00000000000..51574d02868 --- /dev/null +++ b/CHANGES/12603.contrib.rst @@ -0,0 +1,8 @@ +Fixed flaky ``test_handler_returns_not_response`` and +``test_handler_returns_none`` by routing ``loop.set_debug(True)`` +through a new ``loop_debug_mode`` fixture that disables debug +mode before the ``aiohttp_client`` fixture finalizes. Leaving +debug on through teardown let PyPy 3.11's asyncio slow-callback +logger walk into ``Task.__repr__`` during connector close, +surfacing a spurious ``RuntimeWarning: coroutine was never +awaited`` -- by :user:`bdraco`. diff --git a/tests/conftest.py b/tests/conftest.py index f7300c49560..a37b13ca309 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -179,6 +179,24 @@ def _proto_factory() -> Any: yield _proto_factory +@pytest.fixture +async def loop_debug_mode() -> AsyncIterator[None]: + """Enable asyncio debug mode for the duration of the test. + + Disables debug before teardown so PyPy 3.11's recursive + ``Task.__repr__``, triggered by the asyncio slow-callback + logger during connector close, does not surface a spurious + ``RuntimeWarning: coroutine ... was never awaited`` while + the ``aiohttp_client`` fixture finalizes. + """ + loop = asyncio.get_running_loop() + loop.set_debug(True) + try: + yield + finally: + loop.set_debug(False) + + @pytest.fixture def unix_sockname( tmp_path: Path, tmp_path_factory: pytest.TempPathFactory diff --git a/tests/test_web_functional.py b/tests/test_web_functional.py index bd338723f46..2b9ac6734b6 100644 --- a/tests/test_web_functional.py +++ b/tests/test_web_functional.py @@ -95,9 +95,10 @@ async def handler(request: web.Request) -> web.Response: async def test_handler_returns_not_response( - aiohttp_server: AiohttpServer, aiohttp_client: AiohttpClient + aiohttp_server: AiohttpServer, + aiohttp_client: AiohttpClient, + loop_debug_mode: None, ) -> None: - asyncio.get_running_loop().set_debug(True) logger = mock.Mock() async def handler(request: web.Request) -> str: @@ -113,9 +114,10 @@ async def handler(request: web.Request) -> str: async def test_handler_returns_none( - aiohttp_server: AiohttpServer, aiohttp_client: AiohttpClient + aiohttp_server: AiohttpServer, + aiohttp_client: AiohttpClient, + loop_debug_mode: None, ) -> None: - asyncio.get_running_loop().set_debug(True) logger = mock.Mock() async def handler(request: web.Request) -> None: From b26cfbff865fcb7c3f89f37d0c351fb25193fb1d Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 17 May 2026 19:19:44 -0700 Subject: [PATCH 4/5] Fix flaky test_shutdown_handler_cancellation_suppressed (#12609) --- tests/test_run_app.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/test_run_app.py b/tests/test_run_app.py index 68131a49eb1..a677e3fcaa5 100644 --- a/tests/test_run_app.py +++ b/tests/test_run_app.py @@ -1340,6 +1340,7 @@ def test_shutdown_handler_cancellation_suppressed( port = sock.getsockname()[1] actions = [] t = None + suppressed = asyncio.Event() async def test() -> None: async def test_resp(sess: ClientSession) -> None: @@ -1351,7 +1352,8 @@ async def test_resp(sess: ClientSession) -> None: async with ClientSession() as sess: t = asyncio.create_task(test_resp(sess)) - await asyncio.sleep(0.5) + # Wait until the handler has observed its cancellation. + await asyncio.wait_for(suppressed.wait(), timeout=3) # Handler is in-progress while we trigger server shutdown. actions.append("PRESTOP") async with sess.get(f"http://127.0.0.1:{port}/stop"): @@ -1372,6 +1374,7 @@ async def handler(request: web.Request) -> web.Response: await asyncio.sleep(5) except asyncio.CancelledError: actions.append("SUPPRESSED") + suppressed.set() await asyncio.sleep(2) actions.append("DONE") return web.Response(text="FOO") From 131c82138f5f40649e362bf4caeb016a746f618e Mon Sep 17 00:00:00 2001 From: Sam Bull Date: Mon, 18 May 2026 04:00:44 +0100 Subject: [PATCH 5/5] Alternative AGENTS.md (#12610) --- AGENTS.md | 582 +++++------------------------------------------------- 1 file changed, 51 insertions(+), 531 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 2eec53c47f1..ec687201121 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,551 +1,71 @@ -# Notes for LLM contributors +# AGENTS.md -## Rule zero: prove it works before opening the PR +This file provides guidance to AI coding agents working with this repository. -**Your job is to deliver code that is proven to work.** If you -have not proven the change works, it is not time to open the PR -yet. "It compiles", "type checks pass", and "the diff looks -right" are not proof. Proof is: the relevant tests run locally -and pass, the new behaviour is exercised by a test you added or -extended, and any user-visible path you touched has been -executed end-to-end. For aiohttp specifically, that means the -suite passes under **both** the default C-extension build **and** -the pure-Python build (`AIOHTTP_NO_EXTENSIONS=1`); see the -[Tests](#tests) section for the exact commands. If you cannot -run the suite in your environment, say so explicitly in the PR -body rather than implying coverage you did not actually achieve. -Opening a PR that turns out not to work wastes the reviewer's -time and is the single fastest way to lose trust on this repo. +## Branching -The rest of this document covers how to dress up that proven -change for review. None of it matters if rule zero is not met. +- All new features and bug fixes target `master`. A bot creates backports to the `x.y` release branches automatically. ---- +## Build -Read this before opening a pull request against `aio-libs/aiohttp`. -Agents keep getting the same things wrong in this repo, so the rules -below are not optional. If you are about to skip a section because it -sounds like boilerplate, that is the section to re-read. +- Full dev setup: `make install-dev` (installs deps, cythonizes, builds extensions) +- Vendored llhttp: `git submodule update --init` + `make generate-llhttp` (requires Node.js) +- Cython extensions: `make cythonize` (.pyx → .c), then `pip install -e .` to compile +- Pure Python mode: `AIOHTTP_NO_EXTENSIONS=1 pip install -e .` +- `AIOHTTP_CYTHON_TRACE=1` enables Cython trace macros (only useful with linetrace-enabled .c files) -Human-facing contributor docs live under -[docs/contributing.rst](docs/contributing.rst), -[CONTRIBUTING.rst](CONTRIBUTING.rst), and -[CHANGES/README.rst](CHANGES/README.rst); this file is the short -orientation for agents. +## Test -## What this project is +- Run all: `PYTHONPATH='.' pytest --numprocesses=auto` +- Single test: `PYTHONPATH='.' pytest tests/test_foo.py::test_name` +- Pure Python only: `PYTHONPATH='.' AIOHTTP_NO_EXTENSIONS=1 pytest` -`aiohttp` is the async HTTP client and server stack for the -`aio-libs` ecosystem. It is large, widely deployed, performance -sensitive, and ships **two parallel implementations of the hot -paths that must stay behaviourally identical**: a pure-Python -fallback and a set of C / Cython extensions. The C parser is -built on top of `llhttp`, which lives under `vendor/llhttp/` as -a **git submodule** pinned to a specific upstream commit; see -the [Submodules and llhttp](#submodules-and-llhttp) section -below. +## Lint & Format -Useful entry points: +- `pre-commit run --all-files` runs all checks (black, isort, flake8, pyupgrade, codespell) +- `black` for formatting only, `mypy` for type checking +- black with 88-col line length, isort with trailing commas -| Path | What | -| ------------------------------------------ | -------------------------------------------------------------------- | -| `aiohttp/__init__.py` | public package surface and version | -| `aiohttp/client.py` | `ClientSession`, top-level client API | -| `aiohttp/client_reqrep.py` | `ClientRequest`, `ClientResponse` | -| `aiohttp/connector.py` | `TCPConnector`, connection pooling, DNS | -| `aiohttp/web.py` and `aiohttp/web_*.py` | server framework (app, request, response, runners, middlewares) | -| `aiohttp/http_parser.py` | pure-Python HTTP parser; dispatches to C parser when available | -| `aiohttp/_http_parser.pyx` | Cython HTTP parser bound to the `llhttp` submodule | -| `aiohttp/http_writer.py` | pure-Python HTTP writer; falls back from the Cython one | -| `aiohttp/_http_writer.pyx` | Cython HTTP writer | -| `aiohttp/_websocket/reader.py` | dispatcher that selects the Cython or pure-Python WS reader | -| `aiohttp/_websocket/reader_py.py` | pure-Python WebSocket reader | -| `aiohttp/_websocket/reader_c.py` | sibling source compiled by Cython; must match `reader_py.py` | -| `aiohttp/_websocket/mask.pyx` | Cython masking primitive (`websocket_mask`) | -| `aiohttp/_websocket/helpers.py` | shared WS helpers and the `websocket_mask` Python fallback | -| `aiohttp/hdrs.py` and `aiohttp/_headers.pxi` | known header constants; the `.pxi` is generated by `tools/gen.py` | -| `vendor/llhttp/` | git submodule pinned to a `nodejs/llhttp` commit (not a copy) | -| `tests/` | pytest suite, parametrised across both backends | -| `CHANGES/` | towncrier news fragments, one per PR | -| `docs/` | Sphinx docs source | +## Documentation & code style -`AIOHTTP_NO_EXTENSIONS=1` forces the pure-Python build at install -time; the default is the C extension. The dispatch flag at runtime is -`aiohttp.helpers.NO_EXTENSIONS`. +- User-visible API changes need a docs update under `docs/` (`docs/client_reference.rst` / `docs/web_reference.rst` plus any narrative pages). +- Docstrings in code, prose in Sphinx; `make doc` builds locally. +- No docstrings or comments that just restate the code. -## Pull request rules +## Changelog -These are the rules agents most often violate. Treat them as mandatory. +- Fragments in `CHANGES/{pr_or_issue}.{type}.rst` (valid types are defined in `pyproject.toml` under `[tool.towncrier]`). +- Sign with `` -- by :user:`github-handle` ``. +- Both issue and PR number wanted: keep the issue-numbered file and symlink: `ln -s 1234.bugfix.rst CHANGES/1240.bugfix.rst`. +- Multiple fragments same category: `1234.feature.rst`, `1234.feature.1.rst`. -### 1. Use the shipped pull request template +## PRs -`aiohttp` ships its own template at -[`.github/PULL_REQUEST_TEMPLATE.md`](.github/PULL_REQUEST_TEMPLATE.md), -and `gh pr create` will load it automatically. **Use it as shipped.** -Do not invent your own `## What / ## Why / ## How / ## Testing` -layout; that is the marker that the PR was written by an agent -without reading the conventions. +- **Prove it works before opening the PR**. This means: + - Relevant tests pass locally. + - New behaviour is covered by a test. + - Any parser/websocket related changes have been tested with Cython extensions installed. +- Use the shipped template at [`.github/PULL_REQUEST_TEMPLATE.md`](.github/PULL_REQUEST_TEMPLATE.md). + - A couple of sentences per section is plenty. + - Tick checklist boxes that apply; write `N/A` next to ones that do not. + - First-time contributors add themselves to `CONTRIBUTORS.txt` (alphabetical by first name). +- **Draft.** Use `gh pr create --draft`. + - Every submission must be reviewed by a human before going out of draft; that review is the operator's job, not the maintainers'. + - Do not mark ready or request reviewers yourself. +- **Disclosure.** One plain line at the bottom: `Drafted with ; reviewed by .` +- **No `Co-Authored-By:`** LLM trailers in commits or PR body. +- Agent run output (test logs) goes in a collapsed `
` block **below** the template summary. -The template looks like this; fill it out verbatim: +**Commits**: Don't use conventional commits; match recent imperative subjects. -```markdown - +## Threat model -## What do these changes do? +`THREAT_MODEL.md` is a living document and should be revised when: - +- A CVE / GHSA is filed against aiohttp. +- The parser configuration changes (llhttp lenient flags, size limits, version regex). +- Any default referenced in the document changes (`client_max_size`, `keepalive_timeout`, `max_redirects`, `limit`, `limit_per_host`, etc.). +- The vendored llhttp version is bumped. +- A public API surface is added or removed in `client.py` / `web_*.py` / `multipart.py`. -## Are there changes in behavior for the user? - - - -## Is it a substantial burden for the maintainers to support this? - - - -## Related issue number - -Fixes #NNNN - - -## Checklist - -- [x] I think the code is well written -- [x] Unit tests for the changes exist -- [x] Documentation reflects the changes -- [x] If you provide code modification, please add yourself to `CONTRIBUTORS.txt` -- [x] Add a new news fragment into the `CHANGES/` folder -``` - -Tick the boxes that actually apply. If a row does not apply (e.g. a -CI-only change with no tests), write `N/A` next to it rather than -silently leaving it blank. `CONTRIBUTORS.txt` is sorted alphabetically -by first name; insert your ` ` line in the right place -the first time you contribute, then leave it alone on follow-ups. - -### 2. Add a CHANGES fragment - -Every user-visible or contributor-visible PR needs a towncrier news -fragment in `CHANGES/`, named `..rst`. Categories -(defined in [CHANGES/README.rst](CHANGES/README.rst) and the -`[tool.towncrier]` section of [pyproject.toml](pyproject.toml)): - -| Category | When to use | -| -------------- | --------------------------------------------------------------- | -| `bugfix` | corrects undesired behaviour | -| `feature` | new public API or behaviour | -| `deprecation` | announces a future removal | -| `breaking` | removes or changes something public in a breaking way | -| `doc` | documentation structure or build process | -| `packaging` | downstream-visible packaging or build changes | -| `contrib` | contributor experience (CI, dev env, test invocation) | -| `misc` | does not fit any of the above | - -Conventions for the fragment body: - -- Use the past tense (`Fixed`, `Added`, `Bumped`), since it is read - as a "what changed since the previous release" digest. -- Use reStructuredText, not Markdown. -- Do not include the issue or PR number in the body; towncrier adds - it automatically from the filename. -- Sign with `` -- by :user:`github-handle` `` at the end. -- For multiple fragments in the same category, append a sequence - number: `1234.feature.rst`, `1234.feature.1.rst`. - -Example (`CHANGES/8074.bugfix.rst` style): - -```rst -Fixed an unhandled exception in the Python HTTP parser on header -lines starting with a colon -- by :user:`pajod`. -``` - -Pick the number for the fragment filename as follows: - -- **If the change has a linked issue, name the fragment after - the issue number** (e.g. `CHANGES/1234.bugfix.rst` for a fix - that closes `#1234`). The issue number is stable and known - before the PR is opened. -- **If there is no linked issue,** you have two options: - - Open the PR first, then add the fragment as a follow-up - commit on the same branch using the assigned PR number; or - - Guess the next PR number (scan - `gh pr list --repo aio-libs/aiohttp --state all --limit 5` - for the current top of the range), include the fragment in - your initial push, and rename in a follow-up commit if the - guess was off by the time the PR opened. -- **If both an issue and a PR number are in play and you want - both to resolve,** keep the issue-numbered file as the real - fragment and add a symlink at - `CHANGES/..rst` pointing to it, so - towncrier and the GitHub cross-reference both find the entry: - - ```bash - ln -s 1234.bugfix.rst CHANGES/1240.bugfix.rst - ``` - -### 3. Open the PR as a draft, and leave it that way - -Use `gh pr create --draft`. **Every LLM-authored submission -must be fully reviewed by a human before it is marked ready -out of draft, with no exceptions.** That review is the -responsibility of the person running the agent, not of the -project maintainers; do not shift the burden of reviewing LLM -work onto them. Maintainers do not look at drafts, so the -draft state is the agent's hand-off to the operator's review, -not a request for the project to review the code on the -operator's behalf. Do not mark the PR ready yourself, and do -not request reviewers from the agent session; the human who -reviewed the change and flipped it out of draft is the one who -routes it. - -### 4. Disclose the agent, do not advertise it - -Disclosure is required, advertising is not welcome. Put one -plain line at the bottom of the PR body naming the agent that -drafted the change, for example: - -``` -Drafted with ; reviewed by . -``` - -That single line is enough. Beyond that: - -- **No `Co-Authored-By:` trailers** for an LLM or any AI tool, in - commits or in the PR body. Attribution goes to the human who - reviewed the change. -- **Agent output goes in a footer below the PR summary, ideally - in a collapsed `
` block.** The aio-libs template - sections (What / Are there changes in behavior / etc.) come - first and read like a human wrote them. Anything the agent - wants to surface for reviewers (scan results, test logs, - branch hygiene notes, pipeline output) goes underneath that. - A collapsed `
` block at the very bottom is the - recommended shape; it keeps the summary readable while still - letting a curious reviewer expand the agent's work: - - ```markdown -
- Agent run details (optional, for reviewers) - - Tests: - Lint: -
- ``` - - What is not OK is mixing this content into the template - sections themselves, or pushing it above the human-readable - summary so reviewers have to scroll past it. The shape and - content of the footer is otherwise up to the agent. -- No emoji decoration (`🤖`, `✨`, `🚀`) in commit messages, PR - titles, PR bodies, or news fragments. Project style is plain prose. -- Commit messages and PR prose should read as if a human contributor - wrote them. Specifically: - - **No em-dashes (`—`)** and no dashes used as sentence separators - (`foo - bar`). Use a semicolon or a comma. This is the strongest - tell for AI-generated prose in this project, and reviewers do - read for it. - - No "Let me", "I'll", or first-person narration of what the agent - did. Describe the change, not the author. - - No filler sections ("Overview", "Summary of changes", "Key - takeaways") on top of the template. The template already has - the right sections. - -### 5. Keep the PR body short - -A couple of sentences per template section is plenty. If the change -is non-obvious, a short reproducer or a paragraph on root cause is -welcome. Long, multi-section essays with bolded sub-headings are -not the style here. - -### 6. Run the docs spell check before pushing - -The `lint` CI job builds the docs with `sphinxcontrib.spelling` -under `make doc-spelling`, which is invoked with `-W ---keep-going` so any unknown word is a hard failure. The spell -checker reads every `CHANGES/*.rst` fragment as part of the -build, so a technical word in your news fragment (`codecov`, -`monkeypatch`, `parametrize`, `repr`, and so on) that is -not in -[`docs/spelling_wordlist.txt`](docs/spelling_wordlist.txt) -will fail `make doc-spelling` and burn a CI run before a human -even sees the PR. - -Before pushing: - -```bash -make doc-spelling -``` - -If it flags a word you actually meant to use, add it to -`docs/spelling_wordlist.txt` (one word per line, roughly -alphabetical) in the same commit as the fragment. If it flags -a typo, fix the typo. Do not paper over real misspellings by -adding them to the wordlist. - -### 7. Commit hygiene - -- One logical change per PR. If a refactor and a bugfix are bundled - together, split them. -- Pre-commit hooks (the changelog filename check, isort, black, - pyupgrade, flake8, yesqa, codespell, end-of-file-fixer, an - rst-linter, and others under - [`.pre-commit-config.yaml`](.pre-commit-config.yaml)) rewrite - files in place and may abort the commit; when that happens, - re-stage and commit again. Do not pass `--no-verify`. Mypy is - not in pre-commit; it runs via `make mypy` (and is included in - `make lint`). -- The repo does **not** use Conventional Commits as a CI gate. Recent - landed subjects are short imperative or descriptive prose (e.g. - `Fix digest authentication for URLs with reserved characters`, - `ci: report slowest benchmarks via --durations=30`, - `Bump pytest-codspeed from 5.0.1 to 5.0.2`). Match that style; do - not force `feat:` / `fix:` prefixes onto every commit. -- The default branch is `master`, not `main`. Open PRs against - `master`; backports to the active release branches (`3.13`, - `3.14`, etc.) are handled automatically by the `Patchback` - GitHub App once the merged PR carries a `backport-3.X` label. - Use `backport:skip` to opt a PR out of backporting. - -## Dual-backend discipline - -This is the single biggest source of broken aiohttp PRs from agents. - -aiohttp's hot paths live in both a Cython / C implementation and a -pure-Python fallback. When you change behaviour in one, check whether -the other needs the same change: - -- A bug fix in `aiohttp/http_parser.py` (pure-Python HTTP parser) - usually needs a matching fix in `aiohttp/_http_parser.pyx`. If - the bug is in the underlying C parser it lives upstream in - `nodejs/llhttp` (see [Submodules and llhttp](#submodules-and-llhttp)); - fix it there and bump the submodule pin in a separate PR. If - the C path is genuinely unaffected (different code path, - different invariant), say so explicitly in the "What do these - changes do?" section. -- A bug fix in `aiohttp/http_writer.py` usually needs a matching - fix in `aiohttp/_http_writer.pyx`. -- The WebSocket reader is split across - `aiohttp/_websocket/reader_py.py` (pure Python) and - `aiohttp/_websocket/reader_c.py` (the same source compiled by - Cython). **They must stay byte-for-byte equivalent in - behaviour**: any change to one must land in the other in the - same PR. The dispatcher is `aiohttp/_websocket/reader.py`. -- The WebSocket masking primitive has a Python fallback in - `aiohttp/_websocket/helpers.py` and a Cython implementation in - `aiohttp/_websocket/mask.pyx`; the same parity rule applies. -- A new public API must land in both backends with identical - signatures, identical behaviour, identical type hints, and - matching docstrings. -- Tests in `tests/` are exercised against both builds in CI (one - job runs with `AIOHTTP_NO_EXTENSIONS=1`), so a divergence will - surface as a failure on one of the two legs. Reproduce both - locally before opening the PR. - -If you can only fix one backend in scope, file a follow-up issue -and call it out in the PR body. Do not silently leave the -implementations divergent. - -## Tests - -Install dev deps, build the extensions, and run the suite: - -```bash -make .develop # installs deps, regenerates llhttp parser tables, builds Cython in place -make test # pytest -q against the built extension -``` - -`make vtest` runs verbose plus a `-X dev` pass, `make cov-dev` adds -coverage with an HTML report. - -To exercise both backends explicitly: - -```bash -# C extension (default) -pip install -e . -pytest -q - -# Pure Python -AIOHTTP_NO_EXTENSIONS=1 pip install -e . --force-reinstall --no-deps -pytest -q -``` - -`make lint` runs `pre-commit` across the tree plus `mypy`. - -CI runs the full matrix across the supported CPython versions -(3.10+), plus an `AIOHTTP_NO_EXTENSIONS=1` leg, a docs build, a -CodSpeed benchmark leg, and wheel builds for manylinux, musllinux, -macOS, and Windows. Do not regress the benchmarks without flagging -the trade-off in the PR body. - -### Every line in a test must be covered - -Coverage in this repo tracks both `aiohttp/` and `tests/`; the -CI test jobs run pytest with `--cov=aiohttp/ --cov=tests/` and -the cython-coverage job does the same, so uncovered lines in -`tests/` show up in the codecov patch report on every PR. -Reviewers do look at that report. This catches a class of -mistake agents make all the time: a defensive -`raise RuntimeError("must not be called")` inside a -monkeypatched stub the happy path never invokes, a cleanup -branch behind `if had_own_attr:` that the test never enters, an -`else` arm guarding a condition that is always true under the -fixture. From the perspective of the unit suite all of those -lines are dead code, and the codecov report flags them the same -as dead code in `aiohttp/`. - -Design tests so every line runs: - -- Drive the fixture deterministically so both arms of any - conditional are hit, or drop the conditional entirely and - assert the single shape you actually set up. -- Do not add `raise TypeError("must not be invoked")` guards - inside stubs the test installs; if the stub is never meant to - fire, either omit it or assert at the call site that it did - not. An unreachable `raise` is the most common form of this - failure. -- Cleanup branches that only run when setup took a particular - shape (`if had_own_attr: ...` style restores) need a second - test, or a parametrize, that exercises the other shape. If - you cannot justify the second case, unconditionally restore - instead. -- Prefer `monkeypatch` (which auto-reverts) over hand-rolled - save/restore blocks; the auto-revert path has no untaken - branch for coverage to flag. - -See [aio-libs/yarl#1687](https://github.com/aio-libs/yarl/pull/1687) -for the canonical example: a test added an unreachable `raise` -inside a patched `__getstate__` and a conditional restore of the -original attribute, both of which the codecov report rejected as -uncovered. The same pattern recurs in aiohttp test PRs and the -same review feedback applies. - -## Cython and generated files - -`aiohttp/_http_parser.pyx`, `aiohttp/_http_writer.pyx`, and -`aiohttp/_websocket/mask.pyx` are compiled by Cython; the resulting -`*.c` and `*.so` files are build outputs and must not be committed. -`make cythonize` regenerates the `.c` siblings during development. - -`aiohttp/_headers.pxi` and `aiohttp/_find_header.c` are generated -from `aiohttp/hdrs.py` by `tools/gen.py`; regenerate by adding a -header in `hdrs.py` and running `make cythonize` (the rule runs -`tools/gen.py` for you). - -## Submodules and llhttp - -`vendor/llhttp/` is a **git submodule** that points at -[`nodejs/llhttp`](https://github.com/nodejs/llhttp); aiohttp's -checked-in tree only tracks the submodule sha (the current pin is -visible in `git submodule status`). Cloning aiohttp without -submodules will not work; `setup.py` exits with a hint to run -`git submodule update --init` if the checkout is missing. - -The C parser extension is built from three files inside the -submodule checkout, in this order: - -- `vendor/llhttp/build/c/llhttp.c`: the parser tables, - **generated from the TypeScript sources** by - `make generate-llhttp` (which runs `make -C vendor/llhttp generate` - after `npm ci`). This file is a build output; it is regenerated - locally and is not committed upstream. -- `vendor/llhttp/src/native/api.c` and `vendor/llhttp/src/native/http.c`: - tracked C sources inside the submodule, owned by the upstream - llhttp repo. - -What this means in practice: - -- **Do not edit anything under `vendor/llhttp/` by hand.** That is - upstream `nodejs/llhttp` code; fixes belong there, not in the - aiohttp tree. -- **Bumping the llhttp version is a submodule-pointer bump, not an - edit.** From inside `vendor/llhttp` run `git fetch && git checkout `, - then in the aiohttp root `git add vendor/llhttp` to record the - new pointer. Keep the bump in its own PR and call out which - upstream tag / sha you are moving to. -- `make generate-llhttp` only regenerates the build-time parser - tables for the currently pinned commit; it does **not** change - the pin, and there is nothing to commit afterwards in the - aiohttp tree. - -Files you should not edit by hand or commit alongside source -changes: - -- `aiohttp/*.c`, `aiohttp/*.html`, `aiohttp/*.so` -- `aiohttp/_websocket/*.c`, `aiohttp/_websocket/*.html`, - `aiohttp/_websocket/*.so` -- `*.py,cover` coverage artefacts -- `__pycache__/`, `.hash/`, build trees under `build/` and `dist/` - -## Documentation - -User-visible API changes need a docs update under `docs/` (the -relevant section of `docs/client_reference.rst` or -`docs/web_reference.rst` plus any narrative pages). The docstring -goes in the code; the prose context goes in the Sphinx sources. -`make doc` builds the docs locally; `make doc-spelling` runs the -spell-check leg that CI also runs. - -## Code style - -- `pyproject.toml` pins `requires-python = ">= 3.10"`. Match the - surrounding file's import and typing conventions; do not - introduce `from __future__ import annotations` to files that - do not already use it. -- Pre-commit runs isort, black, pyupgrade, flake8, yesqa, - codespell, end-of-file-fixer, the rst-linter, and the - changelog filename check, among others; mypy runs separately - via `make mypy`. Let the hooks rewrite files in place rather - than reformatting by hand. -- Do not add docstrings or comments that just restate the code. - Match the existing terse style in the surrounding module. - -## Things not to do - -- Do not open a PR for code you have not proven works (see - _Rule zero_ at the top of this file). Run the relevant tests - on **both** backends, cover the new behaviour with a test, - exercise the user-visible path end-to-end, and say so honestly - in the PR body if any of that was not possible in your - environment. -- Do not invent a `## What / ## Why / ## How / ## Testing` PR - body; use the shipped template at - `.github/PULL_REQUEST_TEMPLATE.md`. -- Do not push without running `make doc-spelling` first if you - edited any `.rst` file (including a `CHANGES/` fragment). The - docs build fails on unknown words and burns a CI run; see - _Run the docs spell check before pushing_ above. -- Do not skip the `CHANGES/` fragment "because the change is - small". Even a one-line bugfix needs one. -- Do not add `Co-Authored-By` trailers for LLM tools, in either - commits or the PR body. -- Do not mix agent-generated scan output, test summaries, or - pipeline reports into the template sections. Put them in a - collapsed `
` footer below the PR summary instead. -- Do not use em-dashes or sentence-separating dashes in PR prose - or commit messages. -- Do not edit files under `vendor/llhttp/` by hand; they belong - to the upstream `nodejs/llhttp` submodule. To bump the pinned - version, move the submodule pointer (see - [Submodules and llhttp](#submodules-and-llhttp)) and split that - bump into its own PR. -- Do not commit build artefacts (`*.c`, `*.so`, `*.html`, - `*.py,cover`, `__pycache__/`, `build/`, `dist/`) alongside - source changes. -- Do not change one backend without checking the other; see - "Dual-backend discipline" above. -- Do not leave unreachable lines in tests (defensive `raise` - inside a stub the suite never invokes, cleanup branches that - only run for a setup shape the test does not exercise). Tests - are part of the coverage report; see _Every line in a test - must be covered_ above. -- Do not open PRs against the release branches (`3.12`, `3.13`, - etc.); target `master` and let `patchback` handle the - backport after merge. -- Do not mark the PR ready for review yourself; that is the - call of the human running the agent, not the agent itself. - Maintainers do not look at drafts, but that does not mean - they should be doing your review; the operator is responsible - for reviewing the LLM-authored change before flipping the PR - out of draft. -- Do not request reviewers from the agent session; the human - who flips the PR out of draft will route it. +When a chunk's content is materially affected, update both the chunk and the relevant entries in §6.1–§6.4. The "Past advisories / hardening (recap)" subsection of each chunk is the audit trail for what has been verified-in-place.