Skip to content

Commit

Permalink
Fix HEAD requests for static content (#4813)
Browse files Browse the repository at this point in the history
* Fix HEAD requests for static content

FileResponse directly injects the response into the socket with
sendfile() (if it can), which bypasses the checks in Response that
prevent any content being sent for HEAD requests and for 204 (No
Content) and 304 (Not Modified) responses. I've duplicated that logic
into FileResponse. I'm not sure if the status checks are actually
applicable (since StaticResource already has its own handling for Not
Modified) and I don't currently have any tests for them.

Closes #4809.
  • Loading branch information
bmerry authored and asvetlov committed Oct 16, 2020
1 parent 9c00b9e commit 70d41e3
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGES/4809.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix HEAD requests for static content.
1 change: 1 addition & 0 deletions CONTRIBUTORS.txt
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ Boyi Chen
Brett Cannon
Brian C. Lane
Brian Muller
Bruce Merry
Bryan Kok
Bryce Drennan
Carl George
Expand Down
3 changes: 3 additions & 0 deletions aiohttp/web_fileresponse.py
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,9 @@ async def prepare(
self.headers[hdrs.CONTENT_RANGE] = 'bytes {0}-{1}/{2}'.format(
real_start, real_start + count - 1, file_size)

if 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')
if start: # be aware that start could be None or int=0 here.
offset = start
Expand Down
28 changes: 28 additions & 0 deletions tests/test_web_urldispatcher.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import asyncio
import pathlib
from unittest import mock
from unittest.mock import MagicMock
Expand Down Expand Up @@ -238,6 +239,33 @@ async def test_access_special_resource(tmp_path, aiohttp_client) -> None:
assert r.status == 403


async def test_static_head(tmp_path, aiohttp_client) -> None:
# Test HEAD on static route
my_file_path = tmp_path / 'test.txt'
with my_file_path.open('wb') as fw:
fw.write(b'should_not_see_this\n')

app = web.Application()
app.router.add_static('/', str(tmp_path))
client = await aiohttp_client(app)

r = await client.head('/test.txt')
assert r.status == 200

# Check that there is no content sent (see #4809). This can't easily be
# done with aiohttp_client because the buffering can consume the content.
reader, writer = await asyncio.open_connection(client.host, client.port)
writer.write(b'HEAD /test.txt HTTP/1.1\r\n')
writer.write(b'Host: localhost\r\n')
writer.write(b'Connection: close\r\n')
writer.write(b'\r\n')
while await reader.readline() != b'\r\n':
pass
content = await reader.read()
writer.close()
assert content == b''


def test_system_route() -> None:
route = SystemRoute(web.HTTPCreated(reason='test'))
with pytest.raises(RuntimeError):
Expand Down

0 comments on commit 70d41e3

Please sign in to comment.