From d1763e997db4a3e0d81c6730631ddf41546e5b74 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 2 Jan 2021 10:01:02 -1000 Subject: [PATCH] Fix sending zero byte files after sendfile changes 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. --- 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 6e475010fc2..06e5cc1a4be 100644 --- a/aiohttp/web_fileresponse.py +++ b/aiohttp/web_fileresponse.py @@ -230,7 +230,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 31e364b73c1..41034791f76 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: Any, loop_without_sendfile: Any): def maker(*args, **kwargs): @@ -65,13 +76,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(aiohttp_client: Any, sender: Any) -> None: