From ae2b050dffda8e82bcab78aa0fb455494166b718 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 1 Dec 2024 17:44:22 -0600 Subject: [PATCH 01/22] Increment version to 3.11.10.dev0 (#10087) --- aiohttp/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aiohttp/__init__.py b/aiohttp/__init__.py index 5615e5349ae..0024853acaf 100644 --- a/aiohttp/__init__.py +++ b/aiohttp/__init__.py @@ -1,4 +1,4 @@ -__version__ = "3.11.9" +__version__ = "3.11.10.dev0" from typing import TYPE_CHECKING, Tuple From 86bb6ad30c9411a3dc4320b356959a3dc0189eca Mon Sep 17 00:00:00 2001 From: "patchback[bot]" <45432694+patchback[bot]@users.noreply.github.com> Date: Mon, 2 Dec 2024 01:22:11 +0000 Subject: [PATCH 02/22] [PR #10088/29c3ca93 backport][3.11] Avoid calling len on the same data in the stream reader twice (#10090) Co-authored-by: J. Nick Koston --- aiohttp/streams.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/aiohttp/streams.py b/aiohttp/streams.py index b97846171b1..029d577b88c 100644 --- a/aiohttp/streams.py +++ b/aiohttp/streams.py @@ -517,8 +517,9 @@ def _read_nowait_chunk(self, n: int) -> bytes: else: data = self._buffer.popleft() - self._size -= len(data) - self._cursor += len(data) + data_len = len(data) + self._size -= data_len + self._cursor += data_len chunk_splits = self._http_chunk_splits # Prevent memory leak: drop useless chunk splits From 6865d6b93a6a7a1da3202b22cdc65a6a2096dc8e Mon Sep 17 00:00:00 2001 From: "patchback[bot]" <45432694+patchback[bot]@users.noreply.github.com> Date: Mon, 2 Dec 2024 01:26:35 +0000 Subject: [PATCH 03/22] [PR #10088/29c3ca93 backport][3.12] Avoid calling len on the same data in the stream reader twice (#10091) Co-authored-by: J. Nick Koston --- aiohttp/streams.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/aiohttp/streams.py b/aiohttp/streams.py index b97846171b1..029d577b88c 100644 --- a/aiohttp/streams.py +++ b/aiohttp/streams.py @@ -517,8 +517,9 @@ def _read_nowait_chunk(self, n: int) -> bytes: else: data = self._buffer.popleft() - self._size -= len(data) - self._cursor += len(data) + data_len = len(data) + self._size -= data_len + self._cursor += data_len chunk_splits = self._http_chunk_splits # Prevent memory leak: drop useless chunk splits From d872e3429f6f0ddd9aba5e847f58a57b04ca8dd5 Mon Sep 17 00:00:00 2001 From: "patchback[bot]" <45432694+patchback[bot]@users.noreply.github.com> Date: Mon, 2 Dec 2024 14:48:32 +0000 Subject: [PATCH 04/22] [PR #10074/f733258e backport][3.12] Support absolute url to override base url (#10094) **This is a backport of PR #10074 as merged into master (f733258ed9e5e71b7b97511f5654efd6799cac46).** Co-authored-by: vivodi <103735539+vivodi@users.noreply.github.com> --- CHANGES/10074.feature.rst | 2 ++ CONTRIBUTORS.txt | 1 + aiohttp/client.py | 6 ++---- docs/client_reference.rst | 4 ++++ tests/test_client_session.py | 18 ++++++++++++++++++ 5 files changed, 27 insertions(+), 4 deletions(-) create mode 100644 CHANGES/10074.feature.rst diff --git a/CHANGES/10074.feature.rst b/CHANGES/10074.feature.rst new file mode 100644 index 00000000000..d956c38af57 --- /dev/null +++ b/CHANGES/10074.feature.rst @@ -0,0 +1,2 @@ +Added support for overriding the base URL with an absolute one in client sessions +-- by :user:`vivodi`. diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index 6adb3b97fb1..c3abc66bebf 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -52,6 +52,7 @@ Arseny Timoniq Artem Yushkovskiy Arthur Darcet Austin Scola +Bai Haoran Ben Bader Ben Greiner Ben Kallus diff --git a/aiohttp/client.py b/aiohttp/client.py index e04a6ff989a..7539310aa8a 100644 --- a/aiohttp/client.py +++ b/aiohttp/client.py @@ -461,11 +461,9 @@ def request( def _build_url(self, str_or_url: StrOrURL) -> URL: url = URL(str_or_url) - if self._base_url is None: - return url - else: - assert not url.absolute + if self._base_url and not url.absolute: return self._base_url.join(url) + return url async def _request( self, diff --git a/docs/client_reference.rst b/docs/client_reference.rst index c9031de5383..7e7cdf12184 100644 --- a/docs/client_reference.rst +++ b/docs/client_reference.rst @@ -79,6 +79,10 @@ The client session supports the context manager protocol for self closing. .. versionadded:: 3.8 + .. versionchanged:: 3.12 + + Added support for overriding the base URL with an absolute one in client sessions. + :param aiohttp.BaseConnector connector: BaseConnector sub-class instance to support connection pooling. diff --git a/tests/test_client_session.py b/tests/test_client_session.py index 65f80b6abe9..a2c4833b83e 100644 --- a/tests/test_client_session.py +++ b/tests/test_client_session.py @@ -1086,6 +1086,24 @@ async def test_requote_redirect_setter() -> None: URL("http://example.com/test1/test2?q=foo#bar"), id="base_url=URL('http://example.com/test1/') url='test2?q=foo#bar'", ), + pytest.param( + URL("http://example.com/test1/"), + "http://foo.com/bar", + URL("http://foo.com/bar"), + id="base_url=URL('http://example.com/test1/') url='http://foo.com/bar'", + ), + pytest.param( + URL("http://example.com"), + "http://foo.com/bar", + URL("http://foo.com/bar"), + id="base_url=URL('http://example.com') url='http://foo.com/bar'", + ), + pytest.param( + URL("http://example.com/test1/"), + "http://foo.com", + URL("http://foo.com"), + id="base_url=URL('http://example.com/test1/') url='http://foo.com'", + ), ], ) async def test_build_url_returns_expected_url( From 094ffb6e06feaea3d26b3d9084a40aafcb1a92ef Mon Sep 17 00:00:00 2001 From: "patchback[bot]" <45432694+patchback[bot]@users.noreply.github.com> Date: Tue, 3 Dec 2024 20:14:37 -0600 Subject: [PATCH 05/22] [PR #10095/fcce1bf6 backport][3.12] Add a benchmark for web.FileResponse (#10098) **This is a backport of PR #10095 as merged into master (fcce1bf6a5c339a3d63ab1678dcb6658ffc7d570).** We didn't have any benchmarks for file responses. From the benchmarks it turns out most of the time is creating and processing the executor jobs. If we combine the stat into a job that returns the open fileobj it will likely be faster and solve https://github.com/aio-libs/aiohttp/issues/8013 Co-authored-by: J. Nick Koston --- tests/test_benchmarks_web_fileresponse.py | 64 +++++++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 tests/test_benchmarks_web_fileresponse.py diff --git a/tests/test_benchmarks_web_fileresponse.py b/tests/test_benchmarks_web_fileresponse.py new file mode 100644 index 00000000000..7cdbda2efbb --- /dev/null +++ b/tests/test_benchmarks_web_fileresponse.py @@ -0,0 +1,64 @@ +"""codspeed benchmarks for the web file responses.""" + +import asyncio +import pathlib + +from pytest_codspeed import BenchmarkFixture + +from aiohttp import web +from aiohttp.pytest_plugin import AiohttpClient + + +def test_simple_web_file_response( + loop: asyncio.AbstractEventLoop, + aiohttp_client: AiohttpClient, + benchmark: BenchmarkFixture, +) -> None: + """Benchmark creating 100 simple web.FileResponse.""" + response_count = 100 + filepath = pathlib.Path(__file__).parent / "sample.txt" + + async def handler(request: web.Request) -> web.FileResponse: + return web.FileResponse(path=filepath) + + app = web.Application() + app.router.add_route("GET", "/", handler) + + async def run_file_resonse_benchmark() -> None: + client = await aiohttp_client(app) + for _ in range(response_count): + await client.get("/") + await client.close() + + @benchmark + def _run() -> None: + loop.run_until_complete(run_file_resonse_benchmark()) + + +def test_simple_web_file_sendfile_fallback_response( + loop: asyncio.AbstractEventLoop, + aiohttp_client: AiohttpClient, + benchmark: BenchmarkFixture, +) -> None: + """Benchmark creating 100 simple web.FileResponse without sendfile.""" + response_count = 100 + filepath = pathlib.Path(__file__).parent / "sample.txt" + + async def handler(request: web.Request) -> web.FileResponse: + transport = request.transport + assert transport is not None + transport._sendfile_compatible = False # type: ignore[attr-defined] + return web.FileResponse(path=filepath) + + app = web.Application() + app.router.add_route("GET", "/", handler) + + async def run_file_resonse_benchmark() -> None: + client = await aiohttp_client(app) + for _ in range(response_count): + await client.get("/") + await client.close() + + @benchmark + def _run() -> None: + loop.run_until_complete(run_file_resonse_benchmark()) From c41ffc7fea9d93378f933035e33240d278197b52 Mon Sep 17 00:00:00 2001 From: "patchback[bot]" <45432694+patchback[bot]@users.noreply.github.com> Date: Tue, 3 Dec 2024 20:14:47 -0600 Subject: [PATCH 06/22] [PR #10095/fcce1bf6 backport][3.11] Add a benchmark for web.FileResponse (#10097) **This is a backport of PR #10095 as merged into master (fcce1bf6a5c339a3d63ab1678dcb6658ffc7d570).** We didn't have any benchmarks for file responses. From the benchmarks it turns out most of the time is creating and processing the executor jobs. If we combine the stat into a job that returns the open fileobj it will likely be faster and solve https://github.com/aio-libs/aiohttp/issues/8013 Co-authored-by: J. Nick Koston --- tests/test_benchmarks_web_fileresponse.py | 64 +++++++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 tests/test_benchmarks_web_fileresponse.py diff --git a/tests/test_benchmarks_web_fileresponse.py b/tests/test_benchmarks_web_fileresponse.py new file mode 100644 index 00000000000..7cdbda2efbb --- /dev/null +++ b/tests/test_benchmarks_web_fileresponse.py @@ -0,0 +1,64 @@ +"""codspeed benchmarks for the web file responses.""" + +import asyncio +import pathlib + +from pytest_codspeed import BenchmarkFixture + +from aiohttp import web +from aiohttp.pytest_plugin import AiohttpClient + + +def test_simple_web_file_response( + loop: asyncio.AbstractEventLoop, + aiohttp_client: AiohttpClient, + benchmark: BenchmarkFixture, +) -> None: + """Benchmark creating 100 simple web.FileResponse.""" + response_count = 100 + filepath = pathlib.Path(__file__).parent / "sample.txt" + + async def handler(request: web.Request) -> web.FileResponse: + return web.FileResponse(path=filepath) + + app = web.Application() + app.router.add_route("GET", "/", handler) + + async def run_file_resonse_benchmark() -> None: + client = await aiohttp_client(app) + for _ in range(response_count): + await client.get("/") + await client.close() + + @benchmark + def _run() -> None: + loop.run_until_complete(run_file_resonse_benchmark()) + + +def test_simple_web_file_sendfile_fallback_response( + loop: asyncio.AbstractEventLoop, + aiohttp_client: AiohttpClient, + benchmark: BenchmarkFixture, +) -> None: + """Benchmark creating 100 simple web.FileResponse without sendfile.""" + response_count = 100 + filepath = pathlib.Path(__file__).parent / "sample.txt" + + async def handler(request: web.Request) -> web.FileResponse: + transport = request.transport + assert transport is not None + transport._sendfile_compatible = False # type: ignore[attr-defined] + return web.FileResponse(path=filepath) + + app = web.Application() + app.router.add_route("GET", "/", handler) + + async def run_file_resonse_benchmark() -> None: + client = await aiohttp_client(app) + for _ in range(response_count): + await client.get("/") + await client.close() + + @benchmark + def _run() -> None: + loop.run_until_complete(run_file_resonse_benchmark()) From 23a4b31ce98522b526a435bc339429f38f5a6295 Mon Sep 17 00:00:00 2001 From: "patchback[bot]" <45432694+patchback[bot]@users.noreply.github.com> Date: Wed, 4 Dec 2024 19:37:47 +0000 Subject: [PATCH 07/22] [PR #10102/7557b03d backport][3.11] Fix deprecated calls to `guess_type` for paths with Python 3.13 (#10103) Co-authored-by: J. Nick Koston --- CHANGES/10102.bugfix.rst | 1 + aiohttp/payload.py | 7 ++++++- aiohttp/web_fileresponse.py | 9 ++++++--- 3 files changed, 13 insertions(+), 4 deletions(-) create mode 100644 CHANGES/10102.bugfix.rst diff --git a/CHANGES/10102.bugfix.rst b/CHANGES/10102.bugfix.rst new file mode 100644 index 00000000000..86dda8684dd --- /dev/null +++ b/CHANGES/10102.bugfix.rst @@ -0,0 +1 @@ +Replaced deprecated call to :func:`mimetypes.guess_type` with :func:`mimetypes.guess_file_type` when using Python 3.13+ -- by :user:`bdraco`. diff --git a/aiohttp/payload.py b/aiohttp/payload.py index c8c01814698..3f6d3672db2 100644 --- a/aiohttp/payload.py +++ b/aiohttp/payload.py @@ -4,6 +4,7 @@ import json import mimetypes import os +import sys import warnings from abc import ABC, abstractmethod from itertools import chain @@ -169,7 +170,11 @@ def __init__( if content_type is not sentinel and content_type is not None: self._headers[hdrs.CONTENT_TYPE] = content_type elif self._filename is not None: - content_type = mimetypes.guess_type(self._filename)[0] + if sys.version_info >= (3, 13): + guesser = mimetypes.guess_file_type + else: + guesser = mimetypes.guess_type + content_type = guesser(self._filename)[0] if content_type is None: content_type = self._default_content_type self._headers[hdrs.CONTENT_TYPE] = content_type diff --git a/aiohttp/web_fileresponse.py b/aiohttp/web_fileresponse.py index 3b2bc2caf12..ff191415ed5 100644 --- a/aiohttp/web_fileresponse.py +++ b/aiohttp/web_fileresponse.py @@ -1,6 +1,7 @@ import asyncio import os import pathlib +import sys from contextlib import suppress from mimetypes import MimeTypes from stat import S_ISREG @@ -317,9 +318,11 @@ async def prepare(self, request: "BaseRequest") -> Optional[AbstractStreamWriter # extension of the request path. The encoding returned by guess_type # can be ignored since the map was cleared above. if hdrs.CONTENT_TYPE not in self.headers: - self.content_type = ( - CONTENT_TYPES.guess_type(self._path)[0] or FALLBACK_CONTENT_TYPE - ) + if sys.version_info >= (3, 13): + guesser = CONTENT_TYPES.guess_file_type + else: + guesser = CONTENT_TYPES.guess_type + self.content_type = guesser(self._path)[0] or FALLBACK_CONTENT_TYPE if file_encoding: self.headers[hdrs.CONTENT_ENCODING] = file_encoding From f180fc1987721523c41425d102277c778a6ad762 Mon Sep 17 00:00:00 2001 From: "patchback[bot]" <45432694+patchback[bot]@users.noreply.github.com> Date: Wed, 4 Dec 2024 19:46:24 +0000 Subject: [PATCH 08/22] [PR #10102/7557b03d backport][3.12] Fix deprecated calls to `guess_type` for paths with Python 3.13 (#10104) Co-authored-by: J. Nick Koston --- CHANGES/10102.bugfix.rst | 1 + aiohttp/payload.py | 7 ++++++- aiohttp/web_fileresponse.py | 9 ++++++--- 3 files changed, 13 insertions(+), 4 deletions(-) create mode 100644 CHANGES/10102.bugfix.rst diff --git a/CHANGES/10102.bugfix.rst b/CHANGES/10102.bugfix.rst new file mode 100644 index 00000000000..86dda8684dd --- /dev/null +++ b/CHANGES/10102.bugfix.rst @@ -0,0 +1 @@ +Replaced deprecated call to :func:`mimetypes.guess_type` with :func:`mimetypes.guess_file_type` when using Python 3.13+ -- by :user:`bdraco`. diff --git a/aiohttp/payload.py b/aiohttp/payload.py index c8c01814698..3f6d3672db2 100644 --- a/aiohttp/payload.py +++ b/aiohttp/payload.py @@ -4,6 +4,7 @@ import json import mimetypes import os +import sys import warnings from abc import ABC, abstractmethod from itertools import chain @@ -169,7 +170,11 @@ def __init__( if content_type is not sentinel and content_type is not None: self._headers[hdrs.CONTENT_TYPE] = content_type elif self._filename is not None: - content_type = mimetypes.guess_type(self._filename)[0] + if sys.version_info >= (3, 13): + guesser = mimetypes.guess_file_type + else: + guesser = mimetypes.guess_type + content_type = guesser(self._filename)[0] if content_type is None: content_type = self._default_content_type self._headers[hdrs.CONTENT_TYPE] = content_type diff --git a/aiohttp/web_fileresponse.py b/aiohttp/web_fileresponse.py index 3b2bc2caf12..ff191415ed5 100644 --- a/aiohttp/web_fileresponse.py +++ b/aiohttp/web_fileresponse.py @@ -1,6 +1,7 @@ import asyncio import os import pathlib +import sys from contextlib import suppress from mimetypes import MimeTypes from stat import S_ISREG @@ -317,9 +318,11 @@ async def prepare(self, request: "BaseRequest") -> Optional[AbstractStreamWriter # extension of the request path. The encoding returned by guess_type # can be ignored since the map was cleared above. if hdrs.CONTENT_TYPE not in self.headers: - self.content_type = ( - CONTENT_TYPES.guess_type(self._path)[0] or FALLBACK_CONTENT_TYPE - ) + if sys.version_info >= (3, 13): + guesser = CONTENT_TYPES.guess_file_type + else: + guesser = CONTENT_TYPES.guess_type + self.content_type = guesser(self._path)[0] or FALLBACK_CONTENT_TYPE if file_encoding: self.headers[hdrs.CONTENT_ENCODING] = file_encoding From 07d17590833b9d3785e95f6ec991a4f996fdaf9e Mon Sep 17 00:00:00 2001 From: "patchback[bot]" <45432694+patchback[bot]@users.noreply.github.com> Date: Wed, 4 Dec 2024 20:28:53 +0000 Subject: [PATCH 09/22] [PR #10101/678993a4 backport][3.11] Fix race in `FileResponse` if file is replaced during `prepare` (#10105) Co-authored-by: J. Nick Koston fixes #8013 --- CHANGES/10101.bugfix.rst | 1 + aiohttp/web_fileresponse.py | 79 ++++++++++++++++++++++++--------- tests/test_web_urldispatcher.py | 7 +-- 3 files changed, 63 insertions(+), 24 deletions(-) create mode 100644 CHANGES/10101.bugfix.rst diff --git a/CHANGES/10101.bugfix.rst b/CHANGES/10101.bugfix.rst new file mode 100644 index 00000000000..e06195ac028 --- /dev/null +++ b/CHANGES/10101.bugfix.rst @@ -0,0 +1 @@ +Fixed race condition in :class:`aiohttp.web.FileResponse` that could have resulted in an incorrect response if the file was replaced on the file system during ``prepare`` -- by :user:`bdraco`. diff --git a/aiohttp/web_fileresponse.py b/aiohttp/web_fileresponse.py index ff191415ed5..7c5149bb62a 100644 --- a/aiohttp/web_fileresponse.py +++ b/aiohttp/web_fileresponse.py @@ -1,4 +1,5 @@ import asyncio +import io import os import pathlib import sys @@ -16,6 +17,7 @@ Iterator, List, Optional, + Set, Tuple, Union, cast, @@ -73,6 +75,9 @@ CONTENT_TYPES.add_type(content_type, extension) # type: ignore[attr-defined] +_CLOSE_FUTURES: Set[asyncio.Future[None]] = set() + + class FileResponse(StreamResponse): """A response object can be used to send files.""" @@ -161,10 +166,10 @@ async def _precondition_failed( self.content_length = 0 return await super().prepare(request) - def _get_file_path_stat_encoding( + def _open_file_path_stat_encoding( self, accept_encoding: str - ) -> Tuple[pathlib.Path, os.stat_result, Optional[str]]: - """Return the file path, stat result, and encoding. + ) -> Tuple[Optional[io.BufferedReader], os.stat_result, Optional[str]]: + """Return the io object, stat result, and encoding. If an uncompressed file is returned, the encoding is set to :py:data:`None`. @@ -182,10 +187,27 @@ def _get_file_path_stat_encoding( # Do not follow symlinks and ignore any non-regular files. st = compressed_path.lstat() if S_ISREG(st.st_mode): - return compressed_path, st, file_encoding + fobj = compressed_path.open("rb") + with suppress(OSError): + # fstat() may not be available on all platforms + # Once we open the file, we want the fstat() to ensure + # the file has not changed between the first stat() + # and the open(). + st = os.stat(fobj.fileno()) + return fobj, st, file_encoding # Fallback to the uncompressed file - return file_path, file_path.stat(), None + st = file_path.stat() + if not S_ISREG(st.st_mode): + return None, st, None + fobj = file_path.open("rb") + with suppress(OSError): + # fstat() may not be available on all platforms + # Once we open the file, we want the fstat() to ensure + # the file has not changed between the first stat() + # and the open(). + st = os.stat(fobj.fileno()) + return fobj, st, None async def prepare(self, request: "BaseRequest") -> Optional[AbstractStreamWriter]: loop = asyncio.get_running_loop() @@ -193,20 +215,44 @@ async def prepare(self, request: "BaseRequest") -> Optional[AbstractStreamWriter # https://www.rfc-editor.org/rfc/rfc9110#section-8.4.1 accept_encoding = request.headers.get(hdrs.ACCEPT_ENCODING, "").lower() try: - file_path, st, file_encoding = await loop.run_in_executor( - None, self._get_file_path_stat_encoding, accept_encoding + fobj, st, file_encoding = await loop.run_in_executor( + None, self._open_file_path_stat_encoding, accept_encoding ) + except PermissionError: + self.set_status(HTTPForbidden.status_code) + return await super().prepare(request) except OSError: # Most likely to be FileNotFoundError or OSError for circular # symlinks in python >= 3.13, so respond with 404. self.set_status(HTTPNotFound.status_code) return await super().prepare(request) - # Forbid special files like sockets, pipes, devices, etc. - if not S_ISREG(st.st_mode): - self.set_status(HTTPForbidden.status_code) - return await super().prepare(request) + try: + # Forbid special files like sockets, pipes, devices, etc. + if not fobj or not S_ISREG(st.st_mode): + self.set_status(HTTPForbidden.status_code) + return await super().prepare(request) + return await self._prepare_open_file(request, fobj, st, file_encoding) + finally: + if fobj: + # We do not await here because we do not want to wait + # for the executor to finish before returning the response + # so the connection can begin servicing another request + # as soon as possible. + close_future = loop.run_in_executor(None, fobj.close) + # Hold a strong reference to the future to prevent it from being + # garbage collected before it completes. + _CLOSE_FUTURES.add(close_future) + close_future.add_done_callback(_CLOSE_FUTURES.remove) + + async def _prepare_open_file( + self, + request: "BaseRequest", + fobj: io.BufferedReader, + st: os.stat_result, + file_encoding: Optional[str], + ) -> Optional[AbstractStreamWriter]: etag_value = f"{st.st_mtime_ns:x}-{st.st_size:x}" last_modified = st.st_mtime @@ -349,18 +395,9 @@ async def prepare(self, request: "BaseRequest") -> Optional[AbstractStreamWriter if count == 0 or must_be_empty_body(request.method, self.status): return await super().prepare(request) - try: - fobj = await loop.run_in_executor(None, file_path.open, "rb") - except PermissionError: - self.set_status(HTTPForbidden.status_code) - return await super().prepare(request) - if start: # be aware that start could be None or int=0 here. offset = start else: offset = 0 - try: - return await self._sendfile(request, fobj, offset, count) - finally: - await asyncio.shield(loop.run_in_executor(None, fobj.close)) + return await self._sendfile(request, fobj, offset, count) diff --git a/tests/test_web_urldispatcher.py b/tests/test_web_urldispatcher.py index 92066f09b7d..ee60b6917c5 100644 --- a/tests/test_web_urldispatcher.py +++ b/tests/test_web_urldispatcher.py @@ -585,16 +585,17 @@ async def test_access_mock_special_resource( my_special.touch() real_result = my_special.stat() - real_stat = pathlib.Path.stat + real_stat = os.stat - def mock_stat(self: pathlib.Path, **kwargs: Any) -> os.stat_result: - s = real_stat(self, **kwargs) + def mock_stat(path: Any, **kwargs: Any) -> os.stat_result: + s = real_stat(path, **kwargs) if os.path.samestat(s, real_result): mock_mode = S_IFIFO | S_IMODE(s.st_mode) s = os.stat_result([mock_mode] + list(s)[1:]) return s monkeypatch.setattr("pathlib.Path.stat", mock_stat) + monkeypatch.setattr("os.stat", mock_stat) app = web.Application() app.router.add_static("/", str(tmp_path)) From fa6804c189805ed0179693b12e1ca2eca99a7863 Mon Sep 17 00:00:00 2001 From: "patchback[bot]" <45432694+patchback[bot]@users.noreply.github.com> Date: Wed, 4 Dec 2024 20:38:09 +0000 Subject: [PATCH 10/22] [PR #10101/678993a4 backport][3.12] Fix race in `FileResponse` if file is replaced during `prepare` (#10106) Co-authored-by: J. Nick Koston fixes #8013 --- CHANGES/10101.bugfix.rst | 1 + aiohttp/web_fileresponse.py | 79 ++++++++++++++++++++++++--------- tests/test_web_urldispatcher.py | 7 +-- 3 files changed, 63 insertions(+), 24 deletions(-) create mode 100644 CHANGES/10101.bugfix.rst diff --git a/CHANGES/10101.bugfix.rst b/CHANGES/10101.bugfix.rst new file mode 100644 index 00000000000..e06195ac028 --- /dev/null +++ b/CHANGES/10101.bugfix.rst @@ -0,0 +1 @@ +Fixed race condition in :class:`aiohttp.web.FileResponse` that could have resulted in an incorrect response if the file was replaced on the file system during ``prepare`` -- by :user:`bdraco`. diff --git a/aiohttp/web_fileresponse.py b/aiohttp/web_fileresponse.py index ff191415ed5..7c5149bb62a 100644 --- a/aiohttp/web_fileresponse.py +++ b/aiohttp/web_fileresponse.py @@ -1,4 +1,5 @@ import asyncio +import io import os import pathlib import sys @@ -16,6 +17,7 @@ Iterator, List, Optional, + Set, Tuple, Union, cast, @@ -73,6 +75,9 @@ CONTENT_TYPES.add_type(content_type, extension) # type: ignore[attr-defined] +_CLOSE_FUTURES: Set[asyncio.Future[None]] = set() + + class FileResponse(StreamResponse): """A response object can be used to send files.""" @@ -161,10 +166,10 @@ async def _precondition_failed( self.content_length = 0 return await super().prepare(request) - def _get_file_path_stat_encoding( + def _open_file_path_stat_encoding( self, accept_encoding: str - ) -> Tuple[pathlib.Path, os.stat_result, Optional[str]]: - """Return the file path, stat result, and encoding. + ) -> Tuple[Optional[io.BufferedReader], os.stat_result, Optional[str]]: + """Return the io object, stat result, and encoding. If an uncompressed file is returned, the encoding is set to :py:data:`None`. @@ -182,10 +187,27 @@ def _get_file_path_stat_encoding( # Do not follow symlinks and ignore any non-regular files. st = compressed_path.lstat() if S_ISREG(st.st_mode): - return compressed_path, st, file_encoding + fobj = compressed_path.open("rb") + with suppress(OSError): + # fstat() may not be available on all platforms + # Once we open the file, we want the fstat() to ensure + # the file has not changed between the first stat() + # and the open(). + st = os.stat(fobj.fileno()) + return fobj, st, file_encoding # Fallback to the uncompressed file - return file_path, file_path.stat(), None + st = file_path.stat() + if not S_ISREG(st.st_mode): + return None, st, None + fobj = file_path.open("rb") + with suppress(OSError): + # fstat() may not be available on all platforms + # Once we open the file, we want the fstat() to ensure + # the file has not changed between the first stat() + # and the open(). + st = os.stat(fobj.fileno()) + return fobj, st, None async def prepare(self, request: "BaseRequest") -> Optional[AbstractStreamWriter]: loop = asyncio.get_running_loop() @@ -193,20 +215,44 @@ async def prepare(self, request: "BaseRequest") -> Optional[AbstractStreamWriter # https://www.rfc-editor.org/rfc/rfc9110#section-8.4.1 accept_encoding = request.headers.get(hdrs.ACCEPT_ENCODING, "").lower() try: - file_path, st, file_encoding = await loop.run_in_executor( - None, self._get_file_path_stat_encoding, accept_encoding + fobj, st, file_encoding = await loop.run_in_executor( + None, self._open_file_path_stat_encoding, accept_encoding ) + except PermissionError: + self.set_status(HTTPForbidden.status_code) + return await super().prepare(request) except OSError: # Most likely to be FileNotFoundError or OSError for circular # symlinks in python >= 3.13, so respond with 404. self.set_status(HTTPNotFound.status_code) return await super().prepare(request) - # Forbid special files like sockets, pipes, devices, etc. - if not S_ISREG(st.st_mode): - self.set_status(HTTPForbidden.status_code) - return await super().prepare(request) + try: + # Forbid special files like sockets, pipes, devices, etc. + if not fobj or not S_ISREG(st.st_mode): + self.set_status(HTTPForbidden.status_code) + return await super().prepare(request) + return await self._prepare_open_file(request, fobj, st, file_encoding) + finally: + if fobj: + # We do not await here because we do not want to wait + # for the executor to finish before returning the response + # so the connection can begin servicing another request + # as soon as possible. + close_future = loop.run_in_executor(None, fobj.close) + # Hold a strong reference to the future to prevent it from being + # garbage collected before it completes. + _CLOSE_FUTURES.add(close_future) + close_future.add_done_callback(_CLOSE_FUTURES.remove) + + async def _prepare_open_file( + self, + request: "BaseRequest", + fobj: io.BufferedReader, + st: os.stat_result, + file_encoding: Optional[str], + ) -> Optional[AbstractStreamWriter]: etag_value = f"{st.st_mtime_ns:x}-{st.st_size:x}" last_modified = st.st_mtime @@ -349,18 +395,9 @@ async def prepare(self, request: "BaseRequest") -> Optional[AbstractStreamWriter if count == 0 or must_be_empty_body(request.method, self.status): return await super().prepare(request) - try: - fobj = await loop.run_in_executor(None, file_path.open, "rb") - except PermissionError: - self.set_status(HTTPForbidden.status_code) - return await super().prepare(request) - if start: # be aware that start could be None or int=0 here. offset = start else: offset = 0 - try: - return await self._sendfile(request, fobj, offset, count) - finally: - await asyncio.shield(loop.run_in_executor(None, fobj.close)) + return await self._sendfile(request, fobj, offset, count) diff --git a/tests/test_web_urldispatcher.py b/tests/test_web_urldispatcher.py index 92066f09b7d..ee60b6917c5 100644 --- a/tests/test_web_urldispatcher.py +++ b/tests/test_web_urldispatcher.py @@ -585,16 +585,17 @@ async def test_access_mock_special_resource( my_special.touch() real_result = my_special.stat() - real_stat = pathlib.Path.stat + real_stat = os.stat - def mock_stat(self: pathlib.Path, **kwargs: Any) -> os.stat_result: - s = real_stat(self, **kwargs) + def mock_stat(path: Any, **kwargs: Any) -> os.stat_result: + s = real_stat(path, **kwargs) if os.path.samestat(s, real_result): mock_mode = S_IFIFO | S_IMODE(s.st_mode) s = os.stat_result([mock_mode] + list(s)[1:]) return s monkeypatch.setattr("pathlib.Path.stat", mock_stat) + monkeypatch.setattr("os.stat", mock_stat) app = web.Application() app.router.add_static("/", str(tmp_path)) From ae153abfcc95d06a5a8c8189f47fea25b198f340 Mon Sep 17 00:00:00 2001 From: "patchback[bot]" <45432694+patchback[bot]@users.noreply.github.com> Date: Wed, 4 Dec 2024 21:07:46 +0000 Subject: [PATCH 11/22] [PR #10107/84bb77d1 backport][3.11] Use internal `self._headers` var in `FileResponse` (#10108) Co-authored-by: J. Nick Koston --- aiohttp/web_fileresponse.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/aiohttp/web_fileresponse.py b/aiohttp/web_fileresponse.py index 7c5149bb62a..53a27feb098 100644 --- a/aiohttp/web_fileresponse.py +++ b/aiohttp/web_fileresponse.py @@ -315,7 +315,7 @@ async def _prepare_open_file( # # Will do the same below. Many servers ignore this and do not # send a Content-Range header with HTTP 416 - self.headers[hdrs.CONTENT_RANGE] = f"bytes */{file_size}" + self._headers[hdrs.CONTENT_RANGE] = f"bytes */{file_size}" self.set_status(HTTPRequestRangeNotSatisfiable.status_code) return await super().prepare(request) @@ -351,7 +351,7 @@ async def _prepare_open_file( # suffix-byte-range-spec with a non-zero suffix-length, # then the byte-range-set is satisfiable. Otherwise, the # byte-range-set is unsatisfiable. - self.headers[hdrs.CONTENT_RANGE] = f"bytes */{file_size}" + self._headers[hdrs.CONTENT_RANGE] = f"bytes */{file_size}" self.set_status(HTTPRequestRangeNotSatisfiable.status_code) return await super().prepare(request) @@ -363,7 +363,7 @@ async def _prepare_open_file( # If the Content-Type header is not already set, guess it based on the # extension of the request path. The encoding returned by guess_type # can be ignored since the map was cleared above. - if hdrs.CONTENT_TYPE not in self.headers: + if hdrs.CONTENT_TYPE not in self._headers: if sys.version_info >= (3, 13): guesser = CONTENT_TYPES.guess_file_type else: @@ -371,8 +371,8 @@ async def _prepare_open_file( self.content_type = guesser(self._path)[0] or FALLBACK_CONTENT_TYPE if file_encoding: - self.headers[hdrs.CONTENT_ENCODING] = file_encoding - self.headers[hdrs.VARY] = hdrs.ACCEPT_ENCODING + self._headers[hdrs.CONTENT_ENCODING] = file_encoding + self._headers[hdrs.VARY] = hdrs.ACCEPT_ENCODING # Disable compression if we are already sending # a compressed file since we don't want to double # compress. @@ -382,12 +382,12 @@ async def _prepare_open_file( self.last_modified = st.st_mtime # type: ignore[assignment] self.content_length = count - self.headers[hdrs.ACCEPT_RANGES] = "bytes" + self._headers[hdrs.ACCEPT_RANGES] = "bytes" real_start = cast(int, start) if status == HTTPPartialContent.status_code: - self.headers[hdrs.CONTENT_RANGE] = "bytes {}-{}/{}".format( + self._headers[hdrs.CONTENT_RANGE] = "bytes {}-{}/{}".format( real_start, real_start + count - 1, file_size ) From 8745cf4fbf6024d133891a29ab451686f26f505a Mon Sep 17 00:00:00 2001 From: "patchback[bot]" <45432694+patchback[bot]@users.noreply.github.com> Date: Wed, 4 Dec 2024 21:12:41 +0000 Subject: [PATCH 12/22] [PR #10107/84bb77d1 backport][3.12] Use internal `self._headers` var in `FileResponse` (#10109) Co-authored-by: J. Nick Koston --- aiohttp/web_fileresponse.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/aiohttp/web_fileresponse.py b/aiohttp/web_fileresponse.py index 7c5149bb62a..53a27feb098 100644 --- a/aiohttp/web_fileresponse.py +++ b/aiohttp/web_fileresponse.py @@ -315,7 +315,7 @@ async def _prepare_open_file( # # Will do the same below. Many servers ignore this and do not # send a Content-Range header with HTTP 416 - self.headers[hdrs.CONTENT_RANGE] = f"bytes */{file_size}" + self._headers[hdrs.CONTENT_RANGE] = f"bytes */{file_size}" self.set_status(HTTPRequestRangeNotSatisfiable.status_code) return await super().prepare(request) @@ -351,7 +351,7 @@ async def _prepare_open_file( # suffix-byte-range-spec with a non-zero suffix-length, # then the byte-range-set is satisfiable. Otherwise, the # byte-range-set is unsatisfiable. - self.headers[hdrs.CONTENT_RANGE] = f"bytes */{file_size}" + self._headers[hdrs.CONTENT_RANGE] = f"bytes */{file_size}" self.set_status(HTTPRequestRangeNotSatisfiable.status_code) return await super().prepare(request) @@ -363,7 +363,7 @@ async def _prepare_open_file( # If the Content-Type header is not already set, guess it based on the # extension of the request path. The encoding returned by guess_type # can be ignored since the map was cleared above. - if hdrs.CONTENT_TYPE not in self.headers: + if hdrs.CONTENT_TYPE not in self._headers: if sys.version_info >= (3, 13): guesser = CONTENT_TYPES.guess_file_type else: @@ -371,8 +371,8 @@ async def _prepare_open_file( self.content_type = guesser(self._path)[0] or FALLBACK_CONTENT_TYPE if file_encoding: - self.headers[hdrs.CONTENT_ENCODING] = file_encoding - self.headers[hdrs.VARY] = hdrs.ACCEPT_ENCODING + self._headers[hdrs.CONTENT_ENCODING] = file_encoding + self._headers[hdrs.VARY] = hdrs.ACCEPT_ENCODING # Disable compression if we are already sending # a compressed file since we don't want to double # compress. @@ -382,12 +382,12 @@ async def _prepare_open_file( self.last_modified = st.st_mtime # type: ignore[assignment] self.content_length = count - self.headers[hdrs.ACCEPT_RANGES] = "bytes" + self._headers[hdrs.ACCEPT_RANGES] = "bytes" real_start = cast(int, start) if status == HTTPPartialContent.status_code: - self.headers[hdrs.CONTENT_RANGE] = "bytes {}-{}/{}".format( + self._headers[hdrs.CONTENT_RANGE] = "bytes {}-{}/{}".format( real_start, real_start + count - 1, file_size ) From 78473b9c695903fe4021688b74cf7506b95e6463 Mon Sep 17 00:00:00 2001 From: "patchback[bot]" <45432694+patchback[bot]@users.noreply.github.com> Date: Thu, 5 Dec 2024 15:55:34 +0000 Subject: [PATCH 13/22] [PR #10114/94569554 backport][3.11] Add 304 benchmark for FileResponse (#10115) Co-authored-by: J. Nick Koston --- tests/test_benchmarks_web_fileresponse.py | 51 ++++++++++++++++++++--- 1 file changed, 46 insertions(+), 5 deletions(-) diff --git a/tests/test_benchmarks_web_fileresponse.py b/tests/test_benchmarks_web_fileresponse.py index 7cdbda2efbb..01aa7448c86 100644 --- a/tests/test_benchmarks_web_fileresponse.py +++ b/tests/test_benchmarks_web_fileresponse.py @@ -3,9 +3,10 @@ import asyncio import pathlib +from multidict import CIMultiDict from pytest_codspeed import BenchmarkFixture -from aiohttp import web +from aiohttp import ClientResponse, web from aiohttp.pytest_plugin import AiohttpClient @@ -24,7 +25,7 @@ async def handler(request: web.Request) -> web.FileResponse: app = web.Application() app.router.add_route("GET", "/", handler) - async def run_file_resonse_benchmark() -> None: + async def run_file_response_benchmark() -> None: client = await aiohttp_client(app) for _ in range(response_count): await client.get("/") @@ -32,7 +33,7 @@ async def run_file_resonse_benchmark() -> None: @benchmark def _run() -> None: - loop.run_until_complete(run_file_resonse_benchmark()) + loop.run_until_complete(run_file_response_benchmark()) def test_simple_web_file_sendfile_fallback_response( @@ -53,7 +54,7 @@ async def handler(request: web.Request) -> web.FileResponse: app = web.Application() app.router.add_route("GET", "/", handler) - async def run_file_resonse_benchmark() -> None: + async def run_file_response_benchmark() -> None: client = await aiohttp_client(app) for _ in range(response_count): await client.get("/") @@ -61,4 +62,44 @@ async def run_file_resonse_benchmark() -> None: @benchmark def _run() -> None: - loop.run_until_complete(run_file_resonse_benchmark()) + loop.run_until_complete(run_file_response_benchmark()) + + +def test_simple_web_file_response_not_modified( + loop: asyncio.AbstractEventLoop, + aiohttp_client: AiohttpClient, + benchmark: BenchmarkFixture, +) -> None: + """Benchmark web.FileResponse that return a 304.""" + response_count = 100 + filepath = pathlib.Path(__file__).parent / "sample.txt" + + async def handler(request: web.Request) -> web.FileResponse: + return web.FileResponse(path=filepath) + + app = web.Application() + app.router.add_route("GET", "/", handler) + + async def make_last_modified_header() -> CIMultiDict[str]: + client = await aiohttp_client(app) + resp = await client.get("/") + last_modified = resp.headers["Last-Modified"] + headers = CIMultiDict({"If-Modified-Since": last_modified}) + return headers + + async def run_file_response_benchmark( + headers: CIMultiDict[str], + ) -> ClientResponse: + client = await aiohttp_client(app) + for _ in range(response_count): + resp = await client.get("/", headers=headers) + + await client.close() + return resp # type: ignore[possibly-undefined] + + headers = loop.run_until_complete(make_last_modified_header()) + + @benchmark + def _run() -> None: + resp = loop.run_until_complete(run_file_response_benchmark(headers)) + assert resp.status == 304 From cd0c2c8b410665056966606f0d085736cc2cabad Mon Sep 17 00:00:00 2001 From: "patchback[bot]" <45432694+patchback[bot]@users.noreply.github.com> Date: Thu, 5 Dec 2024 16:01:05 +0000 Subject: [PATCH 14/22] [PR #10114/94569554 backport][3.12] Add 304 benchmark for FileResponse (#10116) Co-authored-by: J. Nick Koston --- tests/test_benchmarks_web_fileresponse.py | 51 ++++++++++++++++++++--- 1 file changed, 46 insertions(+), 5 deletions(-) diff --git a/tests/test_benchmarks_web_fileresponse.py b/tests/test_benchmarks_web_fileresponse.py index 7cdbda2efbb..01aa7448c86 100644 --- a/tests/test_benchmarks_web_fileresponse.py +++ b/tests/test_benchmarks_web_fileresponse.py @@ -3,9 +3,10 @@ import asyncio import pathlib +from multidict import CIMultiDict from pytest_codspeed import BenchmarkFixture -from aiohttp import web +from aiohttp import ClientResponse, web from aiohttp.pytest_plugin import AiohttpClient @@ -24,7 +25,7 @@ async def handler(request: web.Request) -> web.FileResponse: app = web.Application() app.router.add_route("GET", "/", handler) - async def run_file_resonse_benchmark() -> None: + async def run_file_response_benchmark() -> None: client = await aiohttp_client(app) for _ in range(response_count): await client.get("/") @@ -32,7 +33,7 @@ async def run_file_resonse_benchmark() -> None: @benchmark def _run() -> None: - loop.run_until_complete(run_file_resonse_benchmark()) + loop.run_until_complete(run_file_response_benchmark()) def test_simple_web_file_sendfile_fallback_response( @@ -53,7 +54,7 @@ async def handler(request: web.Request) -> web.FileResponse: app = web.Application() app.router.add_route("GET", "/", handler) - async def run_file_resonse_benchmark() -> None: + async def run_file_response_benchmark() -> None: client = await aiohttp_client(app) for _ in range(response_count): await client.get("/") @@ -61,4 +62,44 @@ async def run_file_resonse_benchmark() -> None: @benchmark def _run() -> None: - loop.run_until_complete(run_file_resonse_benchmark()) + loop.run_until_complete(run_file_response_benchmark()) + + +def test_simple_web_file_response_not_modified( + loop: asyncio.AbstractEventLoop, + aiohttp_client: AiohttpClient, + benchmark: BenchmarkFixture, +) -> None: + """Benchmark web.FileResponse that return a 304.""" + response_count = 100 + filepath = pathlib.Path(__file__).parent / "sample.txt" + + async def handler(request: web.Request) -> web.FileResponse: + return web.FileResponse(path=filepath) + + app = web.Application() + app.router.add_route("GET", "/", handler) + + async def make_last_modified_header() -> CIMultiDict[str]: + client = await aiohttp_client(app) + resp = await client.get("/") + last_modified = resp.headers["Last-Modified"] + headers = CIMultiDict({"If-Modified-Since": last_modified}) + return headers + + async def run_file_response_benchmark( + headers: CIMultiDict[str], + ) -> ClientResponse: + client = await aiohttp_client(app) + for _ in range(response_count): + resp = await client.get("/", headers=headers) + + await client.close() + return resp # type: ignore[possibly-undefined] + + headers = loop.run_until_complete(make_last_modified_header()) + + @benchmark + def _run() -> None: + resp = loop.run_until_complete(run_file_response_benchmark(headers)) + assert resp.status == 304 From 6207a6d84fdfc2aef0c8bd50958d65e8b5ae3fe7 Mon Sep 17 00:00:00 2001 From: "patchback[bot]" <45432694+patchback[bot]@users.noreply.github.com> Date: Thu, 5 Dec 2024 11:53:06 -0600 Subject: [PATCH 15/22] [PR #10113/01302134 backport][3.12] Restore 304 performance after fixing `FileResponse` replace race (#10118) --- CHANGES/10113.bugfix.rst | 1 + aiohttp/web_fileresponse.py | 163 ++++++++++++++++++++---------------- 2 files changed, 92 insertions(+), 72 deletions(-) create mode 120000 CHANGES/10113.bugfix.rst diff --git a/CHANGES/10113.bugfix.rst b/CHANGES/10113.bugfix.rst new file mode 120000 index 00000000000..89cef58729f --- /dev/null +++ b/CHANGES/10113.bugfix.rst @@ -0,0 +1 @@ +10101.bugfix.rst \ No newline at end of file diff --git a/aiohttp/web_fileresponse.py b/aiohttp/web_fileresponse.py index 53a27feb098..2c54c988920 100644 --- a/aiohttp/web_fileresponse.py +++ b/aiohttp/web_fileresponse.py @@ -4,6 +4,7 @@ import pathlib import sys from contextlib import suppress +from enum import Enum, auto from mimetypes import MimeTypes from stat import S_ISREG from types import MappingProxyType @@ -69,6 +70,16 @@ } ) + +class _FileResponseResult(Enum): + """The result of the file response.""" + + SEND_FILE = auto() # Ie a regular file to send + NOT_ACCEPTABLE = auto() # Ie a socket, or non-regular file + PRE_CONDITION_FAILED = auto() # Ie If-Match or If-None-Match failed + NOT_MODIFIED = auto() # 304 Not Modified + + # Add custom pairs and clear the encodings map so guess_type ignores them. CONTENT_TYPES.encodings_map.clear() for content_type, extension in ADDITIONAL_CONTENT_TYPES.items(): @@ -166,10 +177,12 @@ async def _precondition_failed( self.content_length = 0 return await super().prepare(request) - def _open_file_path_stat_encoding( - self, accept_encoding: str - ) -> Tuple[Optional[io.BufferedReader], os.stat_result, Optional[str]]: - """Return the io object, stat result, and encoding. + def _make_response( + self, request: "BaseRequest", accept_encoding: str + ) -> Tuple[ + _FileResponseResult, Optional[io.BufferedReader], os.stat_result, Optional[str] + ]: + """Return the response result, io object, stat result, and encoding. If an uncompressed file is returned, the encoding is set to :py:data:`None`. @@ -177,6 +190,52 @@ def _open_file_path_stat_encoding( This method should be called from a thread executor since it calls os.stat which may block. """ + file_path, st, file_encoding = self._get_file_path_stat_encoding( + accept_encoding + ) + if not file_path: + return _FileResponseResult.NOT_ACCEPTABLE, None, st, None + + etag_value = f"{st.st_mtime_ns:x}-{st.st_size:x}" + + # https://www.rfc-editor.org/rfc/rfc9110#section-13.1.1-2 + if (ifmatch := request.if_match) is not None and not self._etag_match( + etag_value, ifmatch, weak=False + ): + return _FileResponseResult.PRE_CONDITION_FAILED, None, st, file_encoding + + if ( + (unmodsince := request.if_unmodified_since) is not None + and ifmatch is None + and st.st_mtime > unmodsince.timestamp() + ): + return _FileResponseResult.PRE_CONDITION_FAILED, None, st, file_encoding + + # https://www.rfc-editor.org/rfc/rfc9110#section-13.1.2-2 + if (ifnonematch := request.if_none_match) is not None and self._etag_match( + etag_value, ifnonematch, weak=True + ): + return _FileResponseResult.NOT_MODIFIED, None, st, file_encoding + + if ( + (modsince := request.if_modified_since) is not None + and ifnonematch is None + and st.st_mtime <= modsince.timestamp() + ): + return _FileResponseResult.NOT_MODIFIED, None, st, file_encoding + + fobj = file_path.open("rb") + with suppress(OSError): + # fstat() may not be available on all platforms + # Once we open the file, we want the fstat() to ensure + # the file has not changed between the first stat() + # and the open(). + st = os.stat(fobj.fileno()) + return _FileResponseResult.SEND_FILE, fobj, st, file_encoding + + def _get_file_path_stat_encoding( + self, accept_encoding: str + ) -> Tuple[Optional[pathlib.Path], os.stat_result, Optional[str]]: file_path = self._path for file_extension, file_encoding in ENCODING_EXTENSIONS.items(): if file_encoding not in accept_encoding: @@ -187,27 +246,13 @@ def _open_file_path_stat_encoding( # Do not follow symlinks and ignore any non-regular files. st = compressed_path.lstat() if S_ISREG(st.st_mode): - fobj = compressed_path.open("rb") - with suppress(OSError): - # fstat() may not be available on all platforms - # Once we open the file, we want the fstat() to ensure - # the file has not changed between the first stat() - # and the open(). - st = os.stat(fobj.fileno()) - return fobj, st, file_encoding + return compressed_path, st, file_encoding # Fallback to the uncompressed file st = file_path.stat() if not S_ISREG(st.st_mode): return None, st, None - fobj = file_path.open("rb") - with suppress(OSError): - # fstat() may not be available on all platforms - # Once we open the file, we want the fstat() to ensure - # the file has not changed between the first stat() - # and the open(). - st = os.stat(fobj.fileno()) - return fobj, st, None + return file_path, st, None async def prepare(self, request: "BaseRequest") -> Optional[AbstractStreamWriter]: loop = asyncio.get_running_loop() @@ -215,8 +260,8 @@ async def prepare(self, request: "BaseRequest") -> Optional[AbstractStreamWriter # https://www.rfc-editor.org/rfc/rfc9110#section-8.4.1 accept_encoding = request.headers.get(hdrs.ACCEPT_ENCODING, "").lower() try: - fobj, st, file_encoding = await loop.run_in_executor( - None, self._open_file_path_stat_encoding, accept_encoding + response_result, fobj, st, file_encoding = await loop.run_in_executor( + None, self._make_response, request, accept_encoding ) except PermissionError: self.set_status(HTTPForbidden.status_code) @@ -227,24 +272,32 @@ async def prepare(self, request: "BaseRequest") -> Optional[AbstractStreamWriter self.set_status(HTTPNotFound.status_code) return await super().prepare(request) - try: - # Forbid special files like sockets, pipes, devices, etc. - if not fobj or not S_ISREG(st.st_mode): - self.set_status(HTTPForbidden.status_code) - return await super().prepare(request) + # Forbid special files like sockets, pipes, devices, etc. + if response_result is _FileResponseResult.NOT_ACCEPTABLE: + self.set_status(HTTPForbidden.status_code) + return await super().prepare(request) + + if response_result is _FileResponseResult.PRE_CONDITION_FAILED: + return await self._precondition_failed(request) + if response_result is _FileResponseResult.NOT_MODIFIED: + etag_value = f"{st.st_mtime_ns:x}-{st.st_size:x}" + last_modified = st.st_mtime + return await self._not_modified(request, etag_value, last_modified) + + assert fobj is not None + try: return await self._prepare_open_file(request, fobj, st, file_encoding) finally: - if fobj: - # We do not await here because we do not want to wait - # for the executor to finish before returning the response - # so the connection can begin servicing another request - # as soon as possible. - close_future = loop.run_in_executor(None, fobj.close) - # Hold a strong reference to the future to prevent it from being - # garbage collected before it completes. - _CLOSE_FUTURES.add(close_future) - close_future.add_done_callback(_CLOSE_FUTURES.remove) + # We do not await here because we do not want to wait + # for the executor to finish before returning the response + # so the connection can begin servicing another request + # as soon as possible. + close_future = loop.run_in_executor(None, fobj.close) + # Hold a strong reference to the future to prevent it from being + # garbage collected before it completes. + _CLOSE_FUTURES.add(close_future) + close_future.add_done_callback(_CLOSE_FUTURES.remove) async def _prepare_open_file( self, @@ -253,43 +306,9 @@ async def _prepare_open_file( st: os.stat_result, file_encoding: Optional[str], ) -> Optional[AbstractStreamWriter]: - etag_value = f"{st.st_mtime_ns:x}-{st.st_size:x}" - last_modified = st.st_mtime - - # https://www.rfc-editor.org/rfc/rfc9110#section-13.1.1-2 - ifmatch = request.if_match - if ifmatch is not None and not self._etag_match( - etag_value, ifmatch, weak=False - ): - return await self._precondition_failed(request) - - unmodsince = request.if_unmodified_since - if ( - unmodsince is not None - and ifmatch is None - and st.st_mtime > unmodsince.timestamp() - ): - return await self._precondition_failed(request) - - # https://www.rfc-editor.org/rfc/rfc9110#section-13.1.2-2 - ifnonematch = request.if_none_match - if ifnonematch is not None and self._etag_match( - etag_value, ifnonematch, weak=True - ): - return await self._not_modified(request, etag_value, last_modified) - - modsince = request.if_modified_since - if ( - modsince is not None - and ifnonematch is None - and st.st_mtime <= modsince.timestamp() - ): - return await self._not_modified(request, etag_value, last_modified) - status = self._status file_size = st.st_size count = file_size - start = None ifrange = request.if_range @@ -378,7 +397,7 @@ async def _prepare_open_file( # compress. self._compression = False - self.etag = etag_value # type: ignore[assignment] + self.etag = f"{st.st_mtime_ns:x}-{st.st_size:x}" # type: ignore[assignment] self.last_modified = st.st_mtime # type: ignore[assignment] self.content_length = count From bcae5617932a779c79887a0bc6968bf738047892 Mon Sep 17 00:00:00 2001 From: "patchback[bot]" <45432694+patchback[bot]@users.noreply.github.com> Date: Thu, 5 Dec 2024 11:53:31 -0600 Subject: [PATCH 16/22] [PR #10113/01302134 backport][3.11] Restore 304 performance after fixing `FileResponse` replace race (#10117) --- CHANGES/10113.bugfix.rst | 1 + aiohttp/web_fileresponse.py | 163 ++++++++++++++++++++---------------- 2 files changed, 92 insertions(+), 72 deletions(-) create mode 120000 CHANGES/10113.bugfix.rst diff --git a/CHANGES/10113.bugfix.rst b/CHANGES/10113.bugfix.rst new file mode 120000 index 00000000000..89cef58729f --- /dev/null +++ b/CHANGES/10113.bugfix.rst @@ -0,0 +1 @@ +10101.bugfix.rst \ No newline at end of file diff --git a/aiohttp/web_fileresponse.py b/aiohttp/web_fileresponse.py index 53a27feb098..2c54c988920 100644 --- a/aiohttp/web_fileresponse.py +++ b/aiohttp/web_fileresponse.py @@ -4,6 +4,7 @@ import pathlib import sys from contextlib import suppress +from enum import Enum, auto from mimetypes import MimeTypes from stat import S_ISREG from types import MappingProxyType @@ -69,6 +70,16 @@ } ) + +class _FileResponseResult(Enum): + """The result of the file response.""" + + SEND_FILE = auto() # Ie a regular file to send + NOT_ACCEPTABLE = auto() # Ie a socket, or non-regular file + PRE_CONDITION_FAILED = auto() # Ie If-Match or If-None-Match failed + NOT_MODIFIED = auto() # 304 Not Modified + + # Add custom pairs and clear the encodings map so guess_type ignores them. CONTENT_TYPES.encodings_map.clear() for content_type, extension in ADDITIONAL_CONTENT_TYPES.items(): @@ -166,10 +177,12 @@ async def _precondition_failed( self.content_length = 0 return await super().prepare(request) - def _open_file_path_stat_encoding( - self, accept_encoding: str - ) -> Tuple[Optional[io.BufferedReader], os.stat_result, Optional[str]]: - """Return the io object, stat result, and encoding. + def _make_response( + self, request: "BaseRequest", accept_encoding: str + ) -> Tuple[ + _FileResponseResult, Optional[io.BufferedReader], os.stat_result, Optional[str] + ]: + """Return the response result, io object, stat result, and encoding. If an uncompressed file is returned, the encoding is set to :py:data:`None`. @@ -177,6 +190,52 @@ def _open_file_path_stat_encoding( This method should be called from a thread executor since it calls os.stat which may block. """ + file_path, st, file_encoding = self._get_file_path_stat_encoding( + accept_encoding + ) + if not file_path: + return _FileResponseResult.NOT_ACCEPTABLE, None, st, None + + etag_value = f"{st.st_mtime_ns:x}-{st.st_size:x}" + + # https://www.rfc-editor.org/rfc/rfc9110#section-13.1.1-2 + if (ifmatch := request.if_match) is not None and not self._etag_match( + etag_value, ifmatch, weak=False + ): + return _FileResponseResult.PRE_CONDITION_FAILED, None, st, file_encoding + + if ( + (unmodsince := request.if_unmodified_since) is not None + and ifmatch is None + and st.st_mtime > unmodsince.timestamp() + ): + return _FileResponseResult.PRE_CONDITION_FAILED, None, st, file_encoding + + # https://www.rfc-editor.org/rfc/rfc9110#section-13.1.2-2 + if (ifnonematch := request.if_none_match) is not None and self._etag_match( + etag_value, ifnonematch, weak=True + ): + return _FileResponseResult.NOT_MODIFIED, None, st, file_encoding + + if ( + (modsince := request.if_modified_since) is not None + and ifnonematch is None + and st.st_mtime <= modsince.timestamp() + ): + return _FileResponseResult.NOT_MODIFIED, None, st, file_encoding + + fobj = file_path.open("rb") + with suppress(OSError): + # fstat() may not be available on all platforms + # Once we open the file, we want the fstat() to ensure + # the file has not changed between the first stat() + # and the open(). + st = os.stat(fobj.fileno()) + return _FileResponseResult.SEND_FILE, fobj, st, file_encoding + + def _get_file_path_stat_encoding( + self, accept_encoding: str + ) -> Tuple[Optional[pathlib.Path], os.stat_result, Optional[str]]: file_path = self._path for file_extension, file_encoding in ENCODING_EXTENSIONS.items(): if file_encoding not in accept_encoding: @@ -187,27 +246,13 @@ def _open_file_path_stat_encoding( # Do not follow symlinks and ignore any non-regular files. st = compressed_path.lstat() if S_ISREG(st.st_mode): - fobj = compressed_path.open("rb") - with suppress(OSError): - # fstat() may not be available on all platforms - # Once we open the file, we want the fstat() to ensure - # the file has not changed between the first stat() - # and the open(). - st = os.stat(fobj.fileno()) - return fobj, st, file_encoding + return compressed_path, st, file_encoding # Fallback to the uncompressed file st = file_path.stat() if not S_ISREG(st.st_mode): return None, st, None - fobj = file_path.open("rb") - with suppress(OSError): - # fstat() may not be available on all platforms - # Once we open the file, we want the fstat() to ensure - # the file has not changed between the first stat() - # and the open(). - st = os.stat(fobj.fileno()) - return fobj, st, None + return file_path, st, None async def prepare(self, request: "BaseRequest") -> Optional[AbstractStreamWriter]: loop = asyncio.get_running_loop() @@ -215,8 +260,8 @@ async def prepare(self, request: "BaseRequest") -> Optional[AbstractStreamWriter # https://www.rfc-editor.org/rfc/rfc9110#section-8.4.1 accept_encoding = request.headers.get(hdrs.ACCEPT_ENCODING, "").lower() try: - fobj, st, file_encoding = await loop.run_in_executor( - None, self._open_file_path_stat_encoding, accept_encoding + response_result, fobj, st, file_encoding = await loop.run_in_executor( + None, self._make_response, request, accept_encoding ) except PermissionError: self.set_status(HTTPForbidden.status_code) @@ -227,24 +272,32 @@ async def prepare(self, request: "BaseRequest") -> Optional[AbstractStreamWriter self.set_status(HTTPNotFound.status_code) return await super().prepare(request) - try: - # Forbid special files like sockets, pipes, devices, etc. - if not fobj or not S_ISREG(st.st_mode): - self.set_status(HTTPForbidden.status_code) - return await super().prepare(request) + # Forbid special files like sockets, pipes, devices, etc. + if response_result is _FileResponseResult.NOT_ACCEPTABLE: + self.set_status(HTTPForbidden.status_code) + return await super().prepare(request) + + if response_result is _FileResponseResult.PRE_CONDITION_FAILED: + return await self._precondition_failed(request) + if response_result is _FileResponseResult.NOT_MODIFIED: + etag_value = f"{st.st_mtime_ns:x}-{st.st_size:x}" + last_modified = st.st_mtime + return await self._not_modified(request, etag_value, last_modified) + + assert fobj is not None + try: return await self._prepare_open_file(request, fobj, st, file_encoding) finally: - if fobj: - # We do not await here because we do not want to wait - # for the executor to finish before returning the response - # so the connection can begin servicing another request - # as soon as possible. - close_future = loop.run_in_executor(None, fobj.close) - # Hold a strong reference to the future to prevent it from being - # garbage collected before it completes. - _CLOSE_FUTURES.add(close_future) - close_future.add_done_callback(_CLOSE_FUTURES.remove) + # We do not await here because we do not want to wait + # for the executor to finish before returning the response + # so the connection can begin servicing another request + # as soon as possible. + close_future = loop.run_in_executor(None, fobj.close) + # Hold a strong reference to the future to prevent it from being + # garbage collected before it completes. + _CLOSE_FUTURES.add(close_future) + close_future.add_done_callback(_CLOSE_FUTURES.remove) async def _prepare_open_file( self, @@ -253,43 +306,9 @@ async def _prepare_open_file( st: os.stat_result, file_encoding: Optional[str], ) -> Optional[AbstractStreamWriter]: - etag_value = f"{st.st_mtime_ns:x}-{st.st_size:x}" - last_modified = st.st_mtime - - # https://www.rfc-editor.org/rfc/rfc9110#section-13.1.1-2 - ifmatch = request.if_match - if ifmatch is not None and not self._etag_match( - etag_value, ifmatch, weak=False - ): - return await self._precondition_failed(request) - - unmodsince = request.if_unmodified_since - if ( - unmodsince is not None - and ifmatch is None - and st.st_mtime > unmodsince.timestamp() - ): - return await self._precondition_failed(request) - - # https://www.rfc-editor.org/rfc/rfc9110#section-13.1.2-2 - ifnonematch = request.if_none_match - if ifnonematch is not None and self._etag_match( - etag_value, ifnonematch, weak=True - ): - return await self._not_modified(request, etag_value, last_modified) - - modsince = request.if_modified_since - if ( - modsince is not None - and ifnonematch is None - and st.st_mtime <= modsince.timestamp() - ): - return await self._not_modified(request, etag_value, last_modified) - status = self._status file_size = st.st_size count = file_size - start = None ifrange = request.if_range @@ -378,7 +397,7 @@ async def _prepare_open_file( # compress. self._compression = False - self.etag = etag_value # type: ignore[assignment] + self.etag = f"{st.st_mtime_ns:x}-{st.st_size:x}" # type: ignore[assignment] self.last_modified = st.st_mtime # type: ignore[assignment] self.content_length = count From db5e6bb97f51f35bfc5b44c42106fb5c55756129 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 5 Dec 2024 13:46:35 -0600 Subject: [PATCH 17/22] [PR #10122/703ce61 backport][3.11] Typing improvements for file responses (#10123) --- aiohttp/web_fileresponse.py | 32 ++++++++++++++------------------ 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/aiohttp/web_fileresponse.py b/aiohttp/web_fileresponse.py index 2c54c988920..be9cf87e069 100644 --- a/aiohttp/web_fileresponse.py +++ b/aiohttp/web_fileresponse.py @@ -250,9 +250,7 @@ def _get_file_path_stat_encoding( # Fallback to the uncompressed file st = file_path.stat() - if not S_ISREG(st.st_mode): - return None, st, None - return file_path, st, None + return file_path if S_ISREG(st.st_mode) else None, st, None async def prepare(self, request: "BaseRequest") -> Optional[AbstractStreamWriter]: loop = asyncio.get_running_loop() @@ -307,12 +305,12 @@ async def _prepare_open_file( file_encoding: Optional[str], ) -> Optional[AbstractStreamWriter]: status = self._status - file_size = st.st_size - count = file_size - start = None + file_size: int = st.st_size + file_mtime: float = st.st_mtime + count: int = file_size + start: Optional[int] = None - ifrange = request.if_range - if ifrange is None or st.st_mtime <= ifrange.timestamp(): + if (ifrange := request.if_range) is None or file_mtime <= ifrange.timestamp(): # If-Range header check: # condition = cached date >= last modification date # return 206 if True else 200. @@ -323,7 +321,7 @@ async def _prepare_open_file( try: rng = request.http_range start = rng.start - end = rng.stop + end: Optional[int] = rng.stop except ValueError: # https://tools.ietf.org/html/rfc7233: # A server generating a 416 (Range Not Satisfiable) response to @@ -340,7 +338,7 @@ async def _prepare_open_file( # If a range request has been made, convert start, end slice # notation into file pointer offset and count - if start is not None or end is not None: + if start is not None: if start < 0 and end is None: # return tail of file start += file_size if start < 0: @@ -398,25 +396,23 @@ async def _prepare_open_file( self._compression = False self.etag = f"{st.st_mtime_ns:x}-{st.st_size:x}" # type: ignore[assignment] - self.last_modified = st.st_mtime # type: ignore[assignment] + self.last_modified = file_mtime # type: ignore[assignment] self.content_length = count self._headers[hdrs.ACCEPT_RANGES] = "bytes" - real_start = cast(int, start) - if status == HTTPPartialContent.status_code: + real_start = start + assert real_start is not None self._headers[hdrs.CONTENT_RANGE] = "bytes {}-{}/{}".format( real_start, real_start + count - 1, file_size ) # If we are sending 0 bytes calling sendfile() will throw a ValueError - if count == 0 or must_be_empty_body(request.method, self.status): + if count == 0 or must_be_empty_body(request.method, status): return await super().prepare(request) - if start: # be aware that start could be None or int=0 here. - offset = start - else: - offset = 0 + # be aware that start could be None or int=0 here. + offset = start or 0 return await self._sendfile(request, fobj, offset, count) From da4e00b078c07e0eb68cfa4d6d3dd13273c543a6 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 5 Dec 2024 13:50:53 -0600 Subject: [PATCH 18/22] [PR #10122/703ce61 backport][3.12] Typing improvements for file responses (#10124) --- aiohttp/web_fileresponse.py | 32 ++++++++++++++------------------ 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/aiohttp/web_fileresponse.py b/aiohttp/web_fileresponse.py index 2c54c988920..be9cf87e069 100644 --- a/aiohttp/web_fileresponse.py +++ b/aiohttp/web_fileresponse.py @@ -250,9 +250,7 @@ def _get_file_path_stat_encoding( # Fallback to the uncompressed file st = file_path.stat() - if not S_ISREG(st.st_mode): - return None, st, None - return file_path, st, None + return file_path if S_ISREG(st.st_mode) else None, st, None async def prepare(self, request: "BaseRequest") -> Optional[AbstractStreamWriter]: loop = asyncio.get_running_loop() @@ -307,12 +305,12 @@ async def _prepare_open_file( file_encoding: Optional[str], ) -> Optional[AbstractStreamWriter]: status = self._status - file_size = st.st_size - count = file_size - start = None + file_size: int = st.st_size + file_mtime: float = st.st_mtime + count: int = file_size + start: Optional[int] = None - ifrange = request.if_range - if ifrange is None or st.st_mtime <= ifrange.timestamp(): + if (ifrange := request.if_range) is None or file_mtime <= ifrange.timestamp(): # If-Range header check: # condition = cached date >= last modification date # return 206 if True else 200. @@ -323,7 +321,7 @@ async def _prepare_open_file( try: rng = request.http_range start = rng.start - end = rng.stop + end: Optional[int] = rng.stop except ValueError: # https://tools.ietf.org/html/rfc7233: # A server generating a 416 (Range Not Satisfiable) response to @@ -340,7 +338,7 @@ async def _prepare_open_file( # If a range request has been made, convert start, end slice # notation into file pointer offset and count - if start is not None or end is not None: + if start is not None: if start < 0 and end is None: # return tail of file start += file_size if start < 0: @@ -398,25 +396,23 @@ async def _prepare_open_file( self._compression = False self.etag = f"{st.st_mtime_ns:x}-{st.st_size:x}" # type: ignore[assignment] - self.last_modified = st.st_mtime # type: ignore[assignment] + self.last_modified = file_mtime # type: ignore[assignment] self.content_length = count self._headers[hdrs.ACCEPT_RANGES] = "bytes" - real_start = cast(int, start) - if status == HTTPPartialContent.status_code: + real_start = start + assert real_start is not None self._headers[hdrs.CONTENT_RANGE] = "bytes {}-{}/{}".format( real_start, real_start + count - 1, file_size ) # If we are sending 0 bytes calling sendfile() will throw a ValueError - if count == 0 or must_be_empty_body(request.method, self.status): + if count == 0 or must_be_empty_body(request.method, status): return await super().prepare(request) - if start: # be aware that start could be None or int=0 here. - offset = start - else: - offset = 0 + # be aware that start could be None or int=0 here. + offset = start or 0 return await self._sendfile(request, fobj, offset, count) From 5ddff951a40d0e699b3037f8482b53bf1d9f3990 Mon Sep 17 00:00:00 2001 From: "patchback[bot]" <45432694+patchback[bot]@users.noreply.github.com> Date: Thu, 5 Dec 2024 22:27:46 +0000 Subject: [PATCH 19/22] [PR #10125/d58d2c3d backport][3.11] Disable zero copy writes in the ``StreamWriter`` (#10126) Co-authored-by: J. Nick Koston --- CHANGES/10125.bugfix.rst | 1 + aiohttp/http_writer.py | 2 +- tests/test_http_writer.py | 27 +++++++++++++-------------- 3 files changed, 15 insertions(+), 15 deletions(-) create mode 100644 CHANGES/10125.bugfix.rst diff --git a/CHANGES/10125.bugfix.rst b/CHANGES/10125.bugfix.rst new file mode 100644 index 00000000000..4ece1e68d96 --- /dev/null +++ b/CHANGES/10125.bugfix.rst @@ -0,0 +1 @@ +Disabled zero copy writes in the ``StreamWriter`` -- by :user:`bdraco`. diff --git a/aiohttp/http_writer.py b/aiohttp/http_writer.py index c66fda3d8d0..edd19ed65da 100644 --- a/aiohttp/http_writer.py +++ b/aiohttp/http_writer.py @@ -90,7 +90,7 @@ def _writelines(self, chunks: Iterable[bytes]) -> None: transport = self._protocol.transport if transport is None or transport.is_closing(): raise ClientConnectionResetError("Cannot write to closing transport") - transport.writelines(chunks) + transport.write(b"".join(chunks)) async def write( self, chunk: bytes, *, drain: bool = True, LIMIT: int = 0x10000 diff --git a/tests/test_http_writer.py b/tests/test_http_writer.py index 0ed0e615700..5f316fad2f7 100644 --- a/tests/test_http_writer.py +++ b/tests/test_http_writer.py @@ -104,16 +104,15 @@ async def test_write_large_payload_deflate_compression_data_in_eof( assert transport.write.called # type: ignore[attr-defined] chunks = [c[1][0] for c in list(transport.write.mock_calls)] # type: ignore[attr-defined] transport.write.reset_mock() # type: ignore[attr-defined] - assert not transport.writelines.called # type: ignore[attr-defined] # This payload compresses to 20447 bytes payload = b"".join( [bytes((*range(0, i), *range(i, 0, -1))) for i in range(255) for _ in range(64)] ) await msg.write_eof(payload) - assert not transport.write.called # type: ignore[attr-defined] - assert transport.writelines.called # type: ignore[attr-defined] - chunks.extend(transport.writelines.mock_calls[0][1][0]) # type: ignore[attr-defined] + chunks.extend([c[1][0] for c in list(transport.write.mock_calls)]) # type: ignore[attr-defined] + + assert all(chunks) content = b"".join(chunks) assert zlib.decompress(content) == (b"data" * 4096) + payload @@ -180,7 +179,7 @@ async def test_write_payload_deflate_compression_chunked( await msg.write(b"data") await msg.write_eof() - chunks = [b"".join(c[1][0]) for c in list(transport.writelines.mock_calls)] # type: ignore[attr-defined] + chunks = [c[1][0] for c in list(transport.write.mock_calls)] # type: ignore[attr-defined] assert all(chunks) content = b"".join(chunks) assert content == expected @@ -216,7 +215,7 @@ async def test_write_payload_deflate_compression_chunked_data_in_eof( await msg.write(b"data") await msg.write_eof(b"end") - chunks = [b"".join(c[1][0]) for c in list(transport.writelines.mock_calls)] # type: ignore[attr-defined] + chunks = [c[1][0] for c in list(transport.write.mock_calls)] # type: ignore[attr-defined] assert all(chunks) content = b"".join(chunks) assert content == expected @@ -235,16 +234,16 @@ async def test_write_large_payload_deflate_compression_chunked_data_in_eof( # This payload compresses to 1111 bytes payload = b"".join([bytes((*range(0, i), *range(i, 0, -1))) for i in range(255)]) await msg.write_eof(payload) - assert not transport.write.called # type: ignore[attr-defined] - chunks = [] - for write_lines_call in transport.writelines.mock_calls: # type: ignore[attr-defined] - chunked_payload = list(write_lines_call[1][0])[1:] - chunked_payload.pop() - chunks.extend(chunked_payload) + compressed = [] + chunks = [c[1][0] for c in list(transport.write.mock_calls)] # type: ignore[attr-defined] + chunked_body = b"".join(chunks) + split_body = chunked_body.split(b"\r\n") + while split_body: + if split_body.pop(0): + compressed.append(split_body.pop(0)) - assert all(chunks) - content = b"".join(chunks) + content = b"".join(compressed) assert zlib.decompress(content) == (b"data" * 4096) + payload From 6462f7553ff1e7f1778c5e1d315bd5ff890ceaa5 Mon Sep 17 00:00:00 2001 From: "patchback[bot]" <45432694+patchback[bot]@users.noreply.github.com> Date: Thu, 5 Dec 2024 22:35:01 +0000 Subject: [PATCH 20/22] [PR #10125/d58d2c3d backport][3.12] Disable zero copy writes in the ``StreamWriter`` (#10127) Co-authored-by: J. Nick Koston --- CHANGES/10125.bugfix.rst | 1 + aiohttp/http_writer.py | 2 +- tests/test_http_writer.py | 27 +++++++++++++-------------- 3 files changed, 15 insertions(+), 15 deletions(-) create mode 100644 CHANGES/10125.bugfix.rst diff --git a/CHANGES/10125.bugfix.rst b/CHANGES/10125.bugfix.rst new file mode 100644 index 00000000000..4ece1e68d96 --- /dev/null +++ b/CHANGES/10125.bugfix.rst @@ -0,0 +1 @@ +Disabled zero copy writes in the ``StreamWriter`` -- by :user:`bdraco`. diff --git a/aiohttp/http_writer.py b/aiohttp/http_writer.py index c66fda3d8d0..edd19ed65da 100644 --- a/aiohttp/http_writer.py +++ b/aiohttp/http_writer.py @@ -90,7 +90,7 @@ def _writelines(self, chunks: Iterable[bytes]) -> None: transport = self._protocol.transport if transport is None or transport.is_closing(): raise ClientConnectionResetError("Cannot write to closing transport") - transport.writelines(chunks) + transport.write(b"".join(chunks)) async def write( self, chunk: bytes, *, drain: bool = True, LIMIT: int = 0x10000 diff --git a/tests/test_http_writer.py b/tests/test_http_writer.py index 0ed0e615700..5f316fad2f7 100644 --- a/tests/test_http_writer.py +++ b/tests/test_http_writer.py @@ -104,16 +104,15 @@ async def test_write_large_payload_deflate_compression_data_in_eof( assert transport.write.called # type: ignore[attr-defined] chunks = [c[1][0] for c in list(transport.write.mock_calls)] # type: ignore[attr-defined] transport.write.reset_mock() # type: ignore[attr-defined] - assert not transport.writelines.called # type: ignore[attr-defined] # This payload compresses to 20447 bytes payload = b"".join( [bytes((*range(0, i), *range(i, 0, -1))) for i in range(255) for _ in range(64)] ) await msg.write_eof(payload) - assert not transport.write.called # type: ignore[attr-defined] - assert transport.writelines.called # type: ignore[attr-defined] - chunks.extend(transport.writelines.mock_calls[0][1][0]) # type: ignore[attr-defined] + chunks.extend([c[1][0] for c in list(transport.write.mock_calls)]) # type: ignore[attr-defined] + + assert all(chunks) content = b"".join(chunks) assert zlib.decompress(content) == (b"data" * 4096) + payload @@ -180,7 +179,7 @@ async def test_write_payload_deflate_compression_chunked( await msg.write(b"data") await msg.write_eof() - chunks = [b"".join(c[1][0]) for c in list(transport.writelines.mock_calls)] # type: ignore[attr-defined] + chunks = [c[1][0] for c in list(transport.write.mock_calls)] # type: ignore[attr-defined] assert all(chunks) content = b"".join(chunks) assert content == expected @@ -216,7 +215,7 @@ async def test_write_payload_deflate_compression_chunked_data_in_eof( await msg.write(b"data") await msg.write_eof(b"end") - chunks = [b"".join(c[1][0]) for c in list(transport.writelines.mock_calls)] # type: ignore[attr-defined] + chunks = [c[1][0] for c in list(transport.write.mock_calls)] # type: ignore[attr-defined] assert all(chunks) content = b"".join(chunks) assert content == expected @@ -235,16 +234,16 @@ async def test_write_large_payload_deflate_compression_chunked_data_in_eof( # This payload compresses to 1111 bytes payload = b"".join([bytes((*range(0, i), *range(i, 0, -1))) for i in range(255)]) await msg.write_eof(payload) - assert not transport.write.called # type: ignore[attr-defined] - chunks = [] - for write_lines_call in transport.writelines.mock_calls: # type: ignore[attr-defined] - chunked_payload = list(write_lines_call[1][0])[1:] - chunked_payload.pop() - chunks.extend(chunked_payload) + compressed = [] + chunks = [c[1][0] for c in list(transport.write.mock_calls)] # type: ignore[attr-defined] + chunked_body = b"".join(chunks) + split_body = chunked_body.split(b"\r\n") + while split_body: + if split_body.pop(0): + compressed.append(split_body.pop(0)) - assert all(chunks) - content = b"".join(chunks) + content = b"".join(compressed) assert zlib.decompress(content) == (b"data" * 4096) + payload From 0d7352aeca2ac52a4e18aa786bf2aefd47129bbe Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 5 Dec 2024 16:44:47 -0600 Subject: [PATCH 21/22] Release 3.11.10 (#10128) --- CHANGES.rst | 34 ++++++++++++++++++++++++++++++++++ CHANGES/10101.bugfix.rst | 1 - CHANGES/10102.bugfix.rst | 1 - CHANGES/10113.bugfix.rst | 1 - CHANGES/10125.bugfix.rst | 1 - aiohttp/__init__.py | 2 +- 6 files changed, 35 insertions(+), 5 deletions(-) delete mode 100644 CHANGES/10101.bugfix.rst delete mode 100644 CHANGES/10102.bugfix.rst delete mode 120000 CHANGES/10113.bugfix.rst delete mode 100644 CHANGES/10125.bugfix.rst diff --git a/CHANGES.rst b/CHANGES.rst index 8352236c320..586d70c9697 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -10,6 +10,40 @@ .. towncrier release notes start +3.11.10 (2024-12-05) +==================== + +Bug fixes +--------- + +- Fixed race condition in :class:`aiohttp.web.FileResponse` that could have resulted in an incorrect response if the file was replaced on the file system during ``prepare`` -- by :user:`bdraco`. + + + *Related issues and pull requests on GitHub:* + :issue:`10101`, :issue:`10113`. + + + +- Replaced deprecated call to :func:`mimetypes.guess_type` with :func:`mimetypes.guess_file_type` when using Python 3.13+ -- by :user:`bdraco`. + + + *Related issues and pull requests on GitHub:* + :issue:`10102`. + + + +- Disabled zero copy writes in the ``StreamWriter`` -- by :user:`bdraco`. + + + *Related issues and pull requests on GitHub:* + :issue:`10125`. + + + + +---- + + 3.11.9 (2024-12-01) =================== diff --git a/CHANGES/10101.bugfix.rst b/CHANGES/10101.bugfix.rst deleted file mode 100644 index e06195ac028..00000000000 --- a/CHANGES/10101.bugfix.rst +++ /dev/null @@ -1 +0,0 @@ -Fixed race condition in :class:`aiohttp.web.FileResponse` that could have resulted in an incorrect response if the file was replaced on the file system during ``prepare`` -- by :user:`bdraco`. diff --git a/CHANGES/10102.bugfix.rst b/CHANGES/10102.bugfix.rst deleted file mode 100644 index 86dda8684dd..00000000000 --- a/CHANGES/10102.bugfix.rst +++ /dev/null @@ -1 +0,0 @@ -Replaced deprecated call to :func:`mimetypes.guess_type` with :func:`mimetypes.guess_file_type` when using Python 3.13+ -- by :user:`bdraco`. diff --git a/CHANGES/10113.bugfix.rst b/CHANGES/10113.bugfix.rst deleted file mode 120000 index 89cef58729f..00000000000 --- a/CHANGES/10113.bugfix.rst +++ /dev/null @@ -1 +0,0 @@ -10101.bugfix.rst \ No newline at end of file diff --git a/CHANGES/10125.bugfix.rst b/CHANGES/10125.bugfix.rst deleted file mode 100644 index 4ece1e68d96..00000000000 --- a/CHANGES/10125.bugfix.rst +++ /dev/null @@ -1 +0,0 @@ -Disabled zero copy writes in the ``StreamWriter`` -- by :user:`bdraco`. diff --git a/aiohttp/__init__.py b/aiohttp/__init__.py index 0024853acaf..8c80ff3ab7d 100644 --- a/aiohttp/__init__.py +++ b/aiohttp/__init__.py @@ -1,4 +1,4 @@ -__version__ = "3.11.10.dev0" +__version__ = "3.11.10" from typing import TYPE_CHECKING, Tuple From 7f92bebb29d679fd00cae79dff642b18d9b98cda Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 5 Dec 2024 18:37:03 -0600 Subject: [PATCH 22/22] Bump Python version for benchmarks to 3.13 (#10131) --- .github/workflows/ci-cd.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci-cd.yml b/.github/workflows/ci-cd.yml index 5b227e77522..854bf275316 100644 --- a/.github/workflows/ci-cd.yml +++ b/.github/workflows/ci-cd.yml @@ -266,11 +266,11 @@ jobs: uses: actions/checkout@v4 with: submodules: true - - name: Setup Python 3.12 + - name: Setup Python 3.13 id: python-install uses: actions/setup-python@v5 with: - python-version: 3.12 + python-version: 3.13 cache: pip cache-dependency-path: requirements/*.txt - name: Update pip, wheel, setuptools, build, twine