Skip to content

Commit

Permalink
Fix sending zero byte files after sendfile changes
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bdraco committed Jan 2, 2021
1 parent 3edc43c commit 7749c02
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 10 deletions.
1 change: 1 addition & 0 deletions CHANGES/5380.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Ensure sending a zero byte file does not throw an exception (round 2)
3 changes: 2 additions & 1 deletion aiohttp/web_fileresponse.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
37 changes: 28 additions & 9 deletions tests/test_web_sendfile_functional.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -55,23 +66,31 @@ async def handler(request):
await resp.release()


async def test_zero_bytes_file_ok(aiohttp_client: Any, sender: Any) -> None:
async def test_zero_bytes_file_ok(
aiohttp_client: Any, loop_with_mocked_native_sendfile: Any
) -> None:
filepath = pathlib.Path(__file__).parent / "data.zero_bytes"

async def handler(request):
return sender(filepath)
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)

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
assert resp.headers.get("Content-Length") == "0"
await resp.release()


async def test_static_file_ok_string_path(aiohttp_client: Any, sender: Any) -> None:
Expand Down

0 comments on commit 7749c02

Please sign in to comment.