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(