Skip to content

Commit 0ed235d

Browse files
bdracoPatchback
authored andcommitted
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 11b46df)
1 parent 9c77083 commit 0ed235d

File tree

3 files changed

+52
-8
lines changed

3 files changed

+52
-8
lines changed

CHANGES/5380.bugfix

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Ensure sending a zero byte file does not throw an exception (round 2)

aiohttp/web_fileresponse.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,8 @@ async def prepare(self, request: "BaseRequest") -> Optional[AbstractStreamWriter
272272
real_start, real_start + count - 1, file_size
273273
)
274274

275-
if request.method == hdrs.METH_HEAD or self.status in [204, 304]:
275+
# If we are sending 0 bytes calling sendfile() will throw a ValueError
276+
if count == 0 or request.method == hdrs.METH_HEAD or self.status in [204, 304]:
276277
return await super().prepare(request)
277278

278279
fobj = await loop.run_in_executor(None, filepath.open, "rb")

tests/test_web_sendfile_functional.py

Lines changed: 49 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,17 @@ def sendfile(*args, **kwargs):
2525
return loop
2626

2727

28+
@pytest.fixture
29+
def loop_with_mocked_native_sendfile(loop: Any):
30+
def sendfile(transport, fobj, offset, count):
31+
if count == 0:
32+
raise ValueError("count must be a positive integer (got 0)")
33+
raise NotImplementedError
34+
35+
loop.sendfile = sendfile
36+
return loop
37+
38+
2839
@pytest.fixture(params=["sendfile", "no_sendfile"], ids=["sendfile", "no_sendfile"])
2940
def sender(request, loop_without_sendfile):
3041
def maker(*args, **kwargs):
@@ -74,13 +85,44 @@ async def handler(request):
7485
app.router.add_get("/", handler)
7586
client = await aiohttp_client(app)
7687

77-
resp = await client.get("/")
78-
assert resp.status == 200
79-
txt = await resp.text()
80-
assert "" == txt.rstrip()
81-
assert "application/octet-stream" == resp.headers["Content-Type"]
82-
assert resp.headers.get("Content-Encoding") is None
83-
await resp.release()
88+
# Run the request multiple times to ensure
89+
# that an untrapped exception is not hidden
90+
# because there is no read of the zero bytes
91+
for i in range(2):
92+
resp = await client.get("/")
93+
assert resp.status == 200
94+
txt = await resp.text()
95+
assert "" == txt.rstrip()
96+
assert "application/octet-stream" == resp.headers["Content-Type"]
97+
assert resp.headers.get("Content-Encoding") is None
98+
await resp.release()
99+
100+
101+
async def test_zero_bytes_file_mocked_native_sendfile(
102+
aiohttp_client: Any, loop_with_mocked_native_sendfile: Any
103+
) -> None:
104+
filepath = pathlib.Path(__file__).parent / "data.zero_bytes"
105+
106+
async def handler(request):
107+
asyncio.set_event_loop(loop_with_mocked_native_sendfile)
108+
return web.FileResponse(filepath)
109+
110+
app = web.Application()
111+
app.router.add_get("/", handler)
112+
client = await aiohttp_client(app)
113+
114+
# Run the request multiple times to ensure
115+
# that an untrapped exception is not hidden
116+
# because there is no read of the zero bytes
117+
for i in range(2):
118+
resp = await client.get("/")
119+
assert resp.status == 200
120+
txt = await resp.text()
121+
assert "" == txt.rstrip()
122+
assert "application/octet-stream" == resp.headers["Content-Type"]
123+
assert resp.headers.get("Content-Encoding") is None
124+
assert resp.headers.get("Content-Length") == "0"
125+
await resp.release()
84126

85127

86128
async def test_static_file_ok_string_path(

0 commit comments

Comments
 (0)