From 0ed235df306defafad395622a97c3e9bd7210044 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 27 Oct 2021 10:17:25 -0500 Subject: [PATCH] Fix sending zero byte files after sendfile changes (#5380) Changing the sendfile implementation in #5157 caused a regression with sending zero byte files, and the test had too much mocking to expose the issue. (cherry picked from commit 11b46df4215d4802a0de9aa4f5b3d25a874431d0) --- CHANGES/5380.bugfix | 1 + aiohttp/web_fileresponse.py | 3 +- tests/test_web_sendfile_functional.py | 56 +++++++++++++++++++++++---- 3 files changed, 52 insertions(+), 8 deletions(-) create mode 100644 CHANGES/5380.bugfix diff --git a/CHANGES/5380.bugfix b/CHANGES/5380.bugfix new file mode 100644 index 00000000000..5ff003a3fac --- /dev/null +++ b/CHANGES/5380.bugfix @@ -0,0 +1 @@ +Ensure sending a zero byte file does not throw an exception (round 2) diff --git a/aiohttp/web_fileresponse.py b/aiohttp/web_fileresponse.py index df61bc74e4a..f41ed3fd0a9 100644 --- a/aiohttp/web_fileresponse.py +++ b/aiohttp/web_fileresponse.py @@ -272,7 +272,8 @@ async def prepare(self, request: "BaseRequest") -> Optional[AbstractStreamWriter real_start, real_start + count - 1, file_size ) - if request.method == hdrs.METH_HEAD or self.status in [204, 304]: + # If we are sending 0 bytes calling sendfile() will throw a ValueError + if count == 0 or request.method == hdrs.METH_HEAD or self.status in [204, 304]: return await super().prepare(request) fobj = await loop.run_in_executor(None, filepath.open, "rb") diff --git a/tests/test_web_sendfile_functional.py b/tests/test_web_sendfile_functional.py index 41d7ac85504..17c77be1bfd 100644 --- a/tests/test_web_sendfile_functional.py +++ b/tests/test_web_sendfile_functional.py @@ -25,6 +25,17 @@ def sendfile(*args, **kwargs): return loop +@pytest.fixture +def loop_with_mocked_native_sendfile(loop: Any): + def sendfile(transport, fobj, offset, count): + if count == 0: + raise ValueError("count must be a positive integer (got 0)") + raise NotImplementedError + + loop.sendfile = sendfile + return loop + + @pytest.fixture(params=["sendfile", "no_sendfile"], ids=["sendfile", "no_sendfile"]) def sender(request, loop_without_sendfile): def maker(*args, **kwargs): @@ -74,13 +85,44 @@ async def handler(request): app.router.add_get("/", handler) client = await aiohttp_client(app) - resp = await client.get("/") - assert resp.status == 200 - txt = await resp.text() - assert "" == txt.rstrip() - assert "application/octet-stream" == resp.headers["Content-Type"] - assert resp.headers.get("Content-Encoding") is None - await resp.release() + # Run the request multiple times to ensure + # that an untrapped exception is not hidden + # because there is no read of the zero bytes + for i in range(2): + resp = await client.get("/") + assert resp.status == 200 + txt = await resp.text() + assert "" == txt.rstrip() + assert "application/octet-stream" == resp.headers["Content-Type"] + assert resp.headers.get("Content-Encoding") is None + await resp.release() + + +async def test_zero_bytes_file_mocked_native_sendfile( + aiohttp_client: Any, loop_with_mocked_native_sendfile: Any +) -> None: + filepath = pathlib.Path(__file__).parent / "data.zero_bytes" + + async def handler(request): + asyncio.set_event_loop(loop_with_mocked_native_sendfile) + return web.FileResponse(filepath) + + app = web.Application() + app.router.add_get("/", handler) + client = await aiohttp_client(app) + + # Run the request multiple times to ensure + # that an untrapped exception is not hidden + # because there is no read of the zero bytes + for i in range(2): + resp = await client.get("/") + assert resp.status == 200 + txt = await resp.text() + assert "" == txt.rstrip() + assert "application/octet-stream" == resp.headers["Content-Type"] + assert resp.headers.get("Content-Encoding") is None + assert resp.headers.get("Content-Length") == "0" + await resp.release() async def test_static_file_ok_string_path(