diff --git a/AGENTS.md b/AGENTS.md index ec687201121..e75662a948a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -64,7 +64,7 @@ This file provides guidance to AI coding agents working with this repository. - 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.). +- Any default referenced in the document changes (`client_max_size`, `client_max_fields`, `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`. diff --git a/CHANGES/13738.feature.rst b/CHANGES/13738.feature.rst new file mode 100644 index 00000000000..2ac4b9952c6 --- /dev/null +++ b/CHANGES/13738.feature.rst @@ -0,0 +1,7 @@ +Switched ``application/x-www-form-urlencoded`` parsing in +:meth:`~aiohttp.web.BaseRequest.post` to the faster :func:`yarl.query_to_pairs` +parser and added the ``client_max_fields`` argument to +:class:`~aiohttp.web.Application` (default ``1000``) to cap the number of form +fields accepted by :meth:`~aiohttp.web.BaseRequest.post`. Forms with more +than 1000 fields now receive a ``413`` response unless the cap is raised; +``0`` disables it -- by :user:`bdraco`. diff --git a/THREAT_MODEL.md b/THREAT_MODEL.md index 11529668f82..d86f0744fe0 100644 --- a/THREAT_MODEL.md +++ b/THREAT_MODEL.md @@ -654,7 +654,7 @@ boundary at which user-supplied strings can become wire bytes. | # | Component / Vector | STRIDE | Threat | Risk | | :--- | :--- | :--- | :--- | :--- | | 4.1 | Boundary parameter parsing | T | Malformed boundary parameter (oversized, missing, or containing bytes outside the RFC 2046 §5.1.1 safe set — digits, letters, and a small punctuation set) could enable multipart parser confusion or smuggling. | Low | -| 4.2 | Number of parts per body | D | A peer submits a body packed with many tiny parts (e.g. ten thousand 100-byte parts inside a 1 MiB body). Each part allocates a `BodyPartReader` plus header dict, so the live-Python-object footprint is far larger than the on-wire byte count. `client_max_size` caps the wire bytes but not the per-part allocation amplification. | Low | +| 4.2 | Number of parts per body | D | A peer submits a body packed with many tiny parts (e.g. ten thousand 100-byte parts inside a 1 MiB body). Each part allocates a `BodyPartReader` plus header dict, so the live-Python-object footprint is far larger than the on-wire byte count. `client_max_size` caps the wire bytes but not the per-part allocation amplification. The same amplification applies to `application/x-www-form-urlencoded` bodies, where every `&`-separated field becomes a decoded pair in the `MultiDict`. | Low | | 4.3 | Nested multipart recursion | D | `MultipartReader.next()` recurses into nested multiparts without a depth cap; deeply nested input can hit `RecursionError`. `Request.post()` short-circuits this by rejecting any nested multipart it sees, but the bare API does not. | Medium | | 4.4 | Per-part header block size | D | A peer submits a part with an oversized header block (very long field values, or hundreds of headers per part) to drive memory growth at parse time, multiplied across many parts. | Low | | 4.5 | Per-part body size | D | A peer submits a single part with a body that grows arbitrarily large before any framing boundary — if size checking happens only after buffering the whole part, memory blows up before the cap fires. | Low | @@ -673,7 +673,7 @@ boundary at which user-supplied strings can become wire bytes. | # | Threat | Existing | Recommended | | :--- | :--- | :--- | :--- | | 4.1 | Boundary parameter | 70-char cap; missing-boundary raises; HTTP header layer ([§5.1](#51-http1-parser)) catches CR/LF/NUL. | None. | -| 4.2 | Many small parts | `client_max_size` caps total bytes. | Documented design decision: rely on `client_max_size` rather than introducing a `max_parts` knob. **User**: operators sensitive to live-object count should reduce `client_max_size`. | +| 4.2 | Many small parts | `client_max_size` caps total bytes. `Request.post()` additionally caps the number of form fields at `client_max_fields` (default `1000`, `0` disables) since PR #13738: multipart parts are counted before each part is read, and urlencoded bodies are rejected by `yarl.query_to_pairs` before any pair is materialised. Both paths raise `HTTPRequestEntityTooLarge`. | The cap only covers `Request.post()`. Direct `MultipartReader` / `Request.multipart()` users still get an unbounded part count; a `max_parts` parameter on `MultipartReader` would close that path. **User**: operators sensitive to live-object count should reduce `client_max_fields` and `client_max_size`. | | 4.3 | Nested-multipart recursion | `Request.post()` rejects any nested multipart with `ValueError` ("To decode nested multipart you need to use custom reader") (`web_request.py:BaseRequest.post`). | **Direct `MultipartReader` users get unlimited recursion. Add a `max_nesting_depth` parameter (default e.g. 10) to fail cleanly before `RecursionError`.** | | 4.4 | Per-part headers bounded | `max_field_size` / `max_headers` plumbed since 5fe9dfb64 (Mar 2026). | None. | | 4.5 | Per-part body bounded | Per-iteration size check since 9cc4b917c (Mar 2026). | None. | @@ -724,5 +724,8 @@ boundary at which user-supplied strings can become wire bytes. multipart body parts whose `Content-Length` header is not a plain decimal sequence (e.g. `+5`, `-1`, `1_0`) are now rejected, matching the main request parser's strictness per RFC 9110 §8.6. +- **PR #13738** (3.14.4) — `Request.post()` caps the number of form fields + at `client_max_fields` (default `1000`) for both multipart and + urlencoded bodies (threat 4.2). These are all currently in place; this section assumes no regression. diff --git a/aiohttp/test_utils.py b/aiohttp/test_utils.py index b2686b24eab..aa836c0d15e 100644 --- a/aiohttp/test_utils.py +++ b/aiohttp/test_utils.py @@ -575,6 +575,7 @@ def make_mocked_request( payload: StreamReader = EMPTY_PAYLOAD, sslcontext: SSLContext | None = None, client_max_size: int = 1024**2, + client_max_fields: int = 1000, loop: Any = ..., ) -> Request: """Creates mocked web.Request testing purposes. @@ -654,7 +655,14 @@ def make_mocked_request( protocol.transport = transport req = Request( - message, payload, protocol, writer, task, loop, client_max_size=client_max_size + message, + payload, + protocol, + writer, + task, + loop, + client_max_size=client_max_size, + client_max_fields=client_max_fields, ) match_info = UrlMappingMatchInfo( diff --git a/aiohttp/web_app.py b/aiohttp/web_app.py index 5a10bc25d96..c48c31b947a 100644 --- a/aiohttp/web_app.py +++ b/aiohttp/web_app.py @@ -88,6 +88,7 @@ class Application(MutableMapping[str | AppKey[Any], Any]): "_on_shutdown", "_on_cleanup", "_client_max_size", + "_client_max_fields", "_cleanup_ctx", ) @@ -98,6 +99,7 @@ def __init__( middlewares: Iterable[Middleware] = (), handler_args: Mapping[str, Any] | None = None, client_max_size: int = 1024**2, + client_max_fields: int = 1000, debug: Any = ..., # mypy doesn't support ellipsis ) -> None: if debug is not ...: @@ -130,6 +132,7 @@ def __init__( self._on_startup.append(self._cleanup_ctx._on_startup) self._on_cleanup.append(self._cleanup_ctx._on_cleanup) self._client_max_size = client_max_size + self._client_max_fields = client_max_fields def __init_subclass__(cls: type["Application"]) -> None: raise TypeError( diff --git a/aiohttp/web_request.py b/aiohttp/web_request.py index d0e3f0e1531..4685a9607d1 100644 --- a/aiohttp/web_request.py +++ b/aiohttp/web_request.py @@ -19,10 +19,9 @@ cast, overload, ) -from urllib.parse import parse_qsl from multidict import CIMultiDict, MultiDict, MultiDictProxy -from yarl import URL +from yarl import URL, query_to_pairs from . import hdrs from ._cookie_helpers import parse_cookie_header @@ -146,6 +145,12 @@ class FileField: ############################################################ +def _too_many_fields(max_fields: int) -> HTTPRequestEntityTooLarge: + return HTTPRequestEntityTooLarge( + max_fields, text=f"Maximum number of form fields {max_fields} exceeded." + ) + + class BaseRequest(MutableMapping[str | RequestKey[Any], Any], HeadersMixin): POST_METHODS = { hdrs.METH_PATCH, @@ -169,6 +174,7 @@ def __init__( loop: asyncio.AbstractEventLoop, *, client_max_size: int = 1024**2, + client_max_fields: int = 1000, state: dict[RequestKey[Any] | str, Any] | None = None, scheme: str | None = None, host: str | None = None, @@ -209,6 +215,7 @@ def __init__( self._state = {} if state is None else state self._task = task self._client_max_size = client_max_size + self._client_max_fields = client_max_fields self._loop = loop self._transport_sslcontext = protocol.ssl_context @@ -228,6 +235,7 @@ def clone( host: str | _SENTINEL = sentinel, remote: str | _SENTINEL = sentinel, client_max_size: int | _SENTINEL = sentinel, + client_max_fields: int | _SENTINEL = sentinel, ) -> "BaseRequest": """Clone itself with replacement some attributes. @@ -265,6 +273,8 @@ def clone( kwargs["remote"] = remote if client_max_size is sentinel: client_max_size = self._client_max_size + if client_max_fields is sentinel: + client_max_fields = self._client_max_fields return self.__class__( message, @@ -274,6 +284,7 @@ def clone( self._task, self._loop, client_max_size=client_max_size, + client_max_fields=client_max_fields, state=self._state.copy(), pre_handler_error=self._pre_handler_error, **kwargs, @@ -299,6 +310,10 @@ def writer(self) -> AbstractStreamWriter: def client_max_size(self) -> int: return self._client_max_size + @property + def client_max_fields(self) -> int: + return self._client_max_fields + @property def pre_handler_error(self) -> HTTPBadRequest | None: return self._pre_handler_error @@ -778,11 +793,13 @@ async def post(self) -> "MultiDictProxy[str | bytes | FileField]": self._post = MultiDictProxy(MultiDict()) return self._post - out: MultiDict[str | bytes | FileField] = MultiDict() + out: MultiDict[str | bytes | FileField] if content_type == "multipart/form-data": + out = MultiDict() multipart = await self.multipart() max_size = self._client_max_size + max_fields = self._client_max_fields payload = self._payload while (field := await multipart.next()) is not None: @@ -790,6 +807,8 @@ async def post(self) -> "MultiDictProxy[str | bytes | FileField]": # overhead without entering the loop and the check below. if 0 < max_size < payload.total_bytes: raise HTTPRequestEntityTooLarge(max_size) + if 0 < max_fields <= len(out): + raise _too_many_fields(max_fields) field_ct = field.headers.get(hdrs.CONTENT_TYPE) @@ -869,18 +888,26 @@ async def post(self) -> "MultiDictProxy[str | bytes | FileField]": raise ValueError( "To decode nested multipart you need to use custom reader", ) + elif not (data := await self.read()): + out = MultiDict() else: - data = await self.read() - if data: - charset = self.charset or "utf-8" - bytes_query = data.rstrip() - try: - query = bytes_query.decode(charset) - except (LookupError, UnicodeDecodeError): - raise HTTPUnsupportedMediaType() - out.extend( - parse_qsl(qs=query, keep_blank_values=True, encoding=charset) + charset = self.charset or "utf-8" + bytes_query = data.rstrip() + try: + query = bytes_query.decode(charset) + except (LookupError, UnicodeDecodeError): + raise HTTPUnsupportedMediaType() + max_fields = self._client_max_fields + try: + out = MultiDict( + query_to_pairs( + query, + max_fields=max_fields if max_fields > 0 else None, + encoding=charset, + ) ) + except ValueError: + raise _too_many_fields(max_fields) from None self._post = MultiDictProxy(out) return self._post @@ -935,6 +962,7 @@ def clone( host: str | _SENTINEL = sentinel, remote: str | _SENTINEL = sentinel, client_max_size: int | _SENTINEL = sentinel, + client_max_fields: int | _SENTINEL = sentinel, ) -> "Request": ret = super().clone( method=method, @@ -944,6 +972,7 @@ def clone( host=host, remote=remote, client_max_size=client_max_size, + client_max_fields=client_max_fields, ) new_ret = cast(Request, ret) new_ret._match_info = self._match_info diff --git a/aiohttp/web_runner.py b/aiohttp/web_runner.py index 413dafbc10e..8dbe629ab60 100644 --- a/aiohttp/web_runner.py +++ b/aiohttp/web_runner.py @@ -494,6 +494,7 @@ def _make_request( task, loop, client_max_size=self.app._client_max_size, + client_max_fields=self.app._client_max_fields, pre_handler_error=pre_handler_error, ) diff --git a/docs/web_advanced.rst b/docs/web_advanced.rst index 0d7e82b3215..152fe44f74b 100644 --- a/docs/web_advanced.rst +++ b/docs/web_advanced.rst @@ -1312,8 +1312,9 @@ That's why *aiohttp server* should setup *forwarded* headers in custom middleware in tight conjunction with *reverse proxy configuration*. For changing :attr:`BaseRequest.scheme` :attr:`BaseRequest.host` -:attr:`BaseRequest.remote` and :attr:`BaseRequest.client_max_size` -the middleware might use :meth:`BaseRequest.clone`. +:attr:`BaseRequest.remote`, :attr:`BaseRequest.client_max_size` and +:attr:`BaseRequest.client_max_fields` the middleware might use +:meth:`BaseRequest.clone`. .. seealso:: diff --git a/docs/web_quickstart.rst b/docs/web_quickstart.rst index 5c565dfc5ae..4ab92c890a1 100644 --- a/docs/web_quickstart.rst +++ b/docs/web_quickstart.rst @@ -489,8 +489,10 @@ To access form data with ``"POST"`` method use :meth:`aiohttp.web.BaseRequest.post` accepts both ``'application/x-www-form-urlencoded'`` and ``'multipart/form-data'`` form's data encoding (e.g. ``