From e7ac04bd50dff29b61c1c03ab9cbfefa7a47979a Mon Sep 17 00:00:00 2001 From: Viacheslav Greshilov Date: Sat, 28 Nov 2020 18:09:05 +0200 Subject: [PATCH] FileResponse now supports ETag (#4594) --- CHANGES/4594.feature | 1 + aiohttp/web_fileresponse.py | 67 +++++- tests/test_web_sendfile.py | 15 +- tests/test_web_sendfile_functional.py | 316 +++++++++++++------------- 4 files changed, 229 insertions(+), 170 deletions(-) create mode 100644 CHANGES/4594.feature diff --git a/CHANGES/4594.feature b/CHANGES/4594.feature new file mode 100644 index 00000000000..f00e14a5e93 --- /dev/null +++ b/CHANGES/4594.feature @@ -0,0 +1 @@ +FileResponse now supports ETag. diff --git a/aiohttp/web_fileresponse.py b/aiohttp/web_fileresponse.py index ff904dd57b0..d11a72e838a 100644 --- a/aiohttp/web_fileresponse.py +++ b/aiohttp/web_fileresponse.py @@ -9,8 +9,10 @@ Any, Awaitable, Callable, + Iterator, List, Optional, + Tuple, Union, cast, ) @@ -19,6 +21,7 @@ from . import hdrs from .abc import AbstractStreamWriter +from .helpers import ETAG_ANY, ETag from .typedefs import LooseHeaders from .web_exceptions import ( HTTPNotModified, @@ -102,6 +105,31 @@ async def _sendfile( await super().write_eof() return writer + @staticmethod + def _strong_etag_match(etag_value: str, etags: Tuple[ETag, ...]) -> bool: + if len(etags) == 1 and etags[0].value == ETAG_ANY: + return True + else: + return any(etag.value == etag_value for etag in etags if not etag.is_weak) + + async def _not_modified( + self, request: "BaseRequest", etag_value: str, last_modified: float + ) -> Optional[AbstractStreamWriter]: + self.set_status(HTTPNotModified.status_code) + self._length_check = False + self.etag = etag_value # type: ignore + self.last_modified = last_modified # type: ignore + # Delete any Content-Length headers provided by user. HTTP 304 + # should always have empty response body + return await super().prepare(request) + + async def _precondition_failed( + self, request: "BaseRequest" + ) -> Optional[AbstractStreamWriter]: + self.set_status(HTTPPreconditionFailed.status_code) + self.content_length = 0 + return await super().prepare(request) + async def prepare(self, request: "BaseRequest") -> Optional[AbstractStreamWriter]: filepath = self._path @@ -114,20 +142,35 @@ async def prepare(self, request: "BaseRequest") -> Optional[AbstractStreamWriter gzip = True loop = asyncio.get_event_loop() - st = await loop.run_in_executor(None, filepath.stat) + st: os.stat_result = await loop.run_in_executor(None, filepath.stat) - modsince = request.if_modified_since - if modsince is not None and st.st_mtime <= modsince.timestamp(): - self.set_status(HTTPNotModified.status_code) - self._length_check = False - # Delete any Content-Length headers provided by user. HTTP 304 - # should always have empty response body - return await super().prepare(request) + etag_value = f"{st.st_mtime_ns:x}-{st.st_size:x}" + last_modified = st.st_mtime + + # https://tools.ietf.org/html/rfc7232#section-6 + ifmatch = request.if_match + if ifmatch is not None and not self._strong_etag_match(etag_value, ifmatch): + return await self._precondition_failed(request) unmodsince = request.if_unmodified_since - if unmodsince is not None and st.st_mtime > unmodsince.timestamp(): - self.set_status(HTTPPreconditionFailed.status_code) - return await super().prepare(request) + if ( + unmodsince is not None + and ifmatch is None + and st.st_mtime > unmodsince.timestamp() + ): + return await self._precondition_failed(request) + + ifnonematch = request.if_none_match + if ifnonematch is not None and self._strong_etag_match(etag_value, ifnonematch): + return await self._not_modified(request, etag_value, last_modified) + + modsince = request.if_modified_since + if ( + modsince is not None + and ifnonematch is None + and st.st_mtime <= modsince.timestamp() + ): + return await self._not_modified(request, etag_value, last_modified) if hdrs.CONTENT_TYPE not in self.headers: ct, encoding = mimetypes.guess_type(str(filepath)) @@ -218,6 +261,8 @@ async def prepare(self, request: "BaseRequest") -> Optional[AbstractStreamWriter self.headers[hdrs.CONTENT_ENCODING] = encoding if gzip: self.headers[hdrs.VARY] = hdrs.ACCEPT_ENCODING + + self.etag = etag_value # type: ignore[assignment] self.last_modified = st.st_mtime # type: ignore[assignment] self.content_length = count diff --git a/tests/test_web_sendfile.py b/tests/test_web_sendfile.py index aa366c22891..5462fadea87 100644 --- a/tests/test_web_sendfile.py +++ b/tests/test_web_sendfile.py @@ -15,7 +15,8 @@ def test_using_gzip_if_header_present_and_file_available(loop: Any) -> None: gz_filepath.open = mock.mock_open() gz_filepath.is_file.return_value = True gz_filepath.stat.return_value = mock.MagicMock() - gz_filepath.stat.st_size = 1024 + gz_filepath.stat.return_value.st_size = 1024 + gz_filepath.stat.return_value.st_mtime_ns = 1603733507222449291 filepath = mock.Mock() filepath.name = "logo.png" @@ -43,7 +44,8 @@ def test_gzip_if_header_not_present_and_file_available(loop: Any) -> None: filepath.open = mock.mock_open() filepath.with_name.return_value = gz_filepath filepath.stat.return_value = mock.MagicMock() - filepath.stat.st_size = 1024 + filepath.stat.return_value.st_size = 1024 + filepath.stat.return_value.st_mtime_ns = 1603733507222449291 file_sender = FileResponse(filepath) file_sender._sendfile = make_mocked_coro(None) # type: ignore[assignment] @@ -66,7 +68,8 @@ def test_gzip_if_header_not_present_and_file_not_available(loop: Any) -> None: filepath.open = mock.mock_open() filepath.with_name.return_value = gz_filepath filepath.stat.return_value = mock.MagicMock() - filepath.stat.st_size = 1024 + filepath.stat.return_value.st_size = 1024 + filepath.stat.return_value.st_mtime_ns = 1603733507222449291 file_sender = FileResponse(filepath) file_sender._sendfile = make_mocked_coro(None) # type: ignore[assignment] @@ -91,7 +94,8 @@ def test_gzip_if_header_present_and_file_not_available(loop: Any) -> None: filepath.open = mock.mock_open() filepath.with_name.return_value = gz_filepath filepath.stat.return_value = mock.MagicMock() - filepath.stat.st_size = 1024 + filepath.stat.return_value.st_size = 1024 + filepath.stat.return_value.st_mtime_ns = 1603733507222449291 file_sender = FileResponse(filepath) file_sender._sendfile = make_mocked_coro(None) # type: ignore[assignment] @@ -109,7 +113,8 @@ def test_status_controlled_by_user(loop: Any) -> None: filepath.name = "logo.png" filepath.open = mock.mock_open() filepath.stat.return_value = mock.MagicMock() - filepath.stat.st_size = 1024 + filepath.stat.return_value.st_size = 1024 + filepath.stat.return_value.st_mtime_ns = 1603733507222449291 file_sender = FileResponse(filepath, status=203) file_sender._sendfile = make_mocked_coro(None) # type: ignore[assignment] diff --git a/tests/test_web_sendfile_functional.py b/tests/test_web_sendfile_functional.py index 31e364b73c1..3351ead3754 100644 --- a/tests/test_web_sendfile_functional.py +++ b/tests/test_web_sendfile_functional.py @@ -3,7 +3,7 @@ import pathlib import socket import zlib -from typing import Any +from typing import Any, Iterable import pytest @@ -36,15 +36,23 @@ def maker(*args, **kwargs): return maker -async def test_static_file_ok(aiohttp_client: Any, sender: Any) -> None: - filepath = pathlib.Path(__file__).parent / "data.unknown_mime_type" +@pytest.fixture +def app_with_static_route(sender: Any) -> web.Application: + filename = "data.unknown_mime_type" + filepath = pathlib.Path(__file__).parent / filename async def handler(request): return sender(filepath) app = web.Application() app.router.add_get("/", handler) - client = await aiohttp_client(app) + return app + + +async def test_static_file_ok( + aiohttp_client: Any, app_with_static_route: web.Application +) -> None: + client = await aiohttp_client(app_with_static_route) resp = await client.get("/") assert resp.status == 200 @@ -74,15 +82,10 @@ async def handler(request): await resp.release() -async def test_static_file_ok_string_path(aiohttp_client: Any, sender: Any) -> None: - filepath = pathlib.Path(__file__).parent / "data.unknown_mime_type" - - async def handler(request): - return sender(str(filepath)) - - app = web.Application() - app.router.add_get("/", handler) - client = await aiohttp_client(app) +async def test_static_file_ok_string_path( + aiohttp_client: Any, app_with_static_route: web.Application +) -> None: + client = await aiohttp_client(app_with_static_route) resp = await client.get("/") assert resp.status == 200 @@ -215,16 +218,10 @@ async def handler(request): resp.close() -async def test_static_file_if_modified_since(aiohttp_client: Any, sender: Any) -> None: - filename = "data.unknown_mime_type" - filepath = pathlib.Path(__file__).parent / filename - - async def handler(request): - return sender(filepath) - - app = web.Application() - app.router.add_get("/", handler) - client = await aiohttp_client(app) +async def test_static_file_if_modified_since( + aiohttp_client: Any, app_with_static_route: web.Application +) -> None: + client = await aiohttp_client(app_with_static_route) resp = await client.get("/") assert 200 == resp.status @@ -236,22 +233,15 @@ async def handler(request): body = await resp.read() assert 304 == resp.status assert resp.headers.get("Content-Length") is None + assert resp.headers.get("Last-Modified") == lastmod assert b"" == body resp.close() async def test_static_file_if_modified_since_past_date( - aiohttp_client: Any, sender: Any + aiohttp_client: Any, app_with_static_route: web.Application ) -> None: - filename = "data.unknown_mime_type" - filepath = pathlib.Path(__file__).parent / filename - - async def handler(request): - return sender(filepath) - - app = web.Application() - app.router.add_get("/", handler) - client = await aiohttp_client(app) + client = await aiohttp_client(app_with_static_route) lastmod = "Mon, 1 Jan 1990 01:01:01 GMT" @@ -261,17 +251,9 @@ async def handler(request): async def test_static_file_if_modified_since_invalid_date( - aiohttp_client: Any, sender: Any + aiohttp_client: Any, app_with_static_route: web.Application ): - filename = "data.unknown_mime_type" - filepath = pathlib.Path(__file__).parent / filename - - async def handler(request): - return sender(filepath) - - app = web.Application() - app.router.add_get("/", handler) - client = await aiohttp_client(app) + client = await aiohttp_client(app_with_static_route) lastmod = "not a valid HTTP-date" @@ -281,17 +263,9 @@ async def handler(request): async def test_static_file_if_modified_since_future_date( - aiohttp_client: Any, sender: Any + aiohttp_client: Any, app_with_static_route: web.Application ): - filename = "data.unknown_mime_type" - filepath = pathlib.Path(__file__).parent / filename - - async def handler(request): - return sender(filepath) - - app = web.Application() - app.router.add_get("/", handler) - client = await aiohttp_client(app) + client = await aiohttp_client(app_with_static_route) lastmod = "Fri, 31 Dec 9999 23:59:59 GMT" @@ -299,13 +273,121 @@ async def handler(request): body = await resp.read() assert 304 == resp.status assert resp.headers.get("Content-Length") is None + assert resp.headers.get("Last-Modified") + assert b"" == body + resp.close() + + +@pytest.mark.parametrize("if_unmodified_since", ("", "Fri, 31 Dec 0000 23:59:59 GMT")) +async def test_static_file_if_match( + aiohttp_client: Any, + app_with_static_route: web.Application, + if_unmodified_since: str, +) -> None: + client = await aiohttp_client(app_with_static_route) + + resp = await client.get("/") + assert 200 == resp.status + original_etag = resp.headers.get("ETag") + + assert original_etag is not None + resp.close() + + headers = {"If-Match": original_etag, "If-Unmodified-Since": if_unmodified_since} + resp = await client.head("/", headers=headers) + body = await resp.read() + assert 200 == resp.status + assert resp.headers.get("ETag") + assert resp.headers.get("Last-Modified") + assert b"" == body + resp.close() + + +@pytest.mark.parametrize("if_unmodified_since", ("", "Fri, 31 Dec 0000 23:59:59 GMT")) +@pytest.mark.parametrize( + "etags,expected_status", + [ + (("*",), 200), + (('"example-tag"', 'W/"weak-tag"'), 412), + ], +) +async def test_static_file_if_match_custom_tags( + aiohttp_client: Any, + app_with_static_route: web.Application, + if_unmodified_since: str, + etags: Iterable[str], + expected_status: Iterable[int], +) -> None: + client = await aiohttp_client(app_with_static_route) + + if_match = ", ".join(etags) + headers = {"If-Match": if_match, "If-Unmodified-Since": if_unmodified_since} + resp = await client.head("/", headers=headers) + body = await resp.read() + assert expected_status == resp.status + assert b"" == body + resp.close() + + +@pytest.mark.parametrize("if_modified_since", ("", "Fri, 31 Dec 9999 23:59:59 GMT")) +@pytest.mark.parametrize( + "additional_etags", + ( + (), + ('"some-other-strong-etag"', 'W/"weak-tag"', "invalid-tag"), + ), +) +async def test_static_file_if_none_match( + aiohttp_client: Any, + app_with_static_route: web.Application, + if_modified_since: str, + additional_etags: Iterable[str], +) -> None: + client = await aiohttp_client(app_with_static_route) + + resp = await client.get("/") + assert 200 == resp.status + original_etag = resp.headers.get("ETag") + + assert resp.headers.get("Last-Modified") is not None + assert original_etag is not None + resp.close() + + etag = ",".join((original_etag, *additional_etags)) + + resp = await client.get( + "/", headers={"If-None-Match": etag, "If-Modified-Since": if_modified_since} + ) + body = await resp.read() + assert 304 == resp.status + assert resp.headers.get("Content-Length") is None + assert resp.headers.get("ETag") == original_etag + assert b"" == body + resp.close() + + +async def test_static_file_if_none_match_star( + aiohttp_client: Any, + app_with_static_route: web.Application, +) -> None: + client = await aiohttp_client(app_with_static_route) + + resp = await client.head("/", headers={"If-None-Match": "*"}) + body = await resp.read() + assert 304 == resp.status + assert resp.headers.get("Content-Length") is None + assert resp.headers.get("ETag") + assert resp.headers.get("Last-Modified") assert b"" == body resp.close() @pytest.mark.skipif(not ssl, reason="ssl not supported") async def test_static_file_ssl( - aiohttp_server: Any, ssl_ctx: Any, aiohttp_client: Any, client_ssl_ctx: Any + aiohttp_server: Any, + ssl_ctx: Any, + aiohttp_client: Any, + client_ssl_ctx: Any, ) -> None: dirname = pathlib.Path(__file__).parent filename = "data.unknown_mime_type" @@ -564,17 +646,9 @@ async def handler(request): async def test_static_file_if_unmodified_since_past_with_range( - aiohttp_client: Any, sender: Any + aiohttp_client: Any, app_with_static_route: web.Application ): - filename = "data.unknown_mime_type" - filepath = pathlib.Path(__file__).parent / filename - - async def handler(request): - return sender(filepath) - - app = web.Application() - app.router.add_get("/", handler) - client = await aiohttp_client(app) + client = await aiohttp_client(app_with_static_route) lastmod = "Mon, 1 Jan 1990 01:01:01 GMT" @@ -586,17 +660,9 @@ async def handler(request): async def test_static_file_if_unmodified_since_future_with_range( - aiohttp_client: Any, sender: Any + aiohttp_client: Any, app_with_static_route: web.Application ): - filename = "data.unknown_mime_type" - filepath = pathlib.Path(__file__).parent / filename - - async def handler(request): - return sender(filepath) - - app = web.Application() - app.router.add_get("/", handler) - client = await aiohttp_client(app) + client = await aiohttp_client(app_with_static_route) lastmod = "Fri, 31 Dec 9999 23:59:59 GMT" @@ -609,16 +675,10 @@ async def handler(request): resp.close() -async def test_static_file_if_range_past_with_range(aiohttp_client: Any, sender: Any): - filename = "data.unknown_mime_type" - filepath = pathlib.Path(__file__).parent / filename - - async def handler(request): - return sender(filepath) - - app = web.Application() - app.router.add_get("/", handler) - client = await aiohttp_client(app) +async def test_static_file_if_range_past_with_range( + aiohttp_client: Any, app_with_static_route: web.Application +): + client = await aiohttp_client(app_with_static_route) lastmod = "Mon, 1 Jan 1990 01:01:01 GMT" @@ -628,16 +688,10 @@ async def handler(request): resp.close() -async def test_static_file_if_range_future_with_range(aiohttp_client: Any, sender: Any): - filename = "data.unknown_mime_type" - filepath = pathlib.Path(__file__).parent / filename - - async def handler(request): - return sender(filepath) - - app = web.Application() - app.router.add_get("/", handler) - client = await aiohttp_client(app) +async def test_static_file_if_range_future_with_range( + aiohttp_client: Any, app_with_static_route: web.Application +): + client = await aiohttp_client(app_with_static_route) lastmod = "Fri, 31 Dec 9999 23:59:59 GMT" @@ -649,17 +703,9 @@ async def handler(request): async def test_static_file_if_unmodified_since_past_without_range( - aiohttp_client: Any, sender: Any + aiohttp_client: Any, app_with_static_route: web.Application ): - filename = "data.unknown_mime_type" - filepath = pathlib.Path(__file__).parent / filename - - async def handler(request): - return sender(filepath) - - app = web.Application() - app.router.add_get("/", handler) - client = await aiohttp_client(app) + client = await aiohttp_client(app_with_static_route) lastmod = "Mon, 1 Jan 1990 01:01:01 GMT" @@ -669,17 +715,9 @@ async def handler(request): async def test_static_file_if_unmodified_since_future_without_range( - aiohttp_client: Any, sender: Any + aiohttp_client: Any, app_with_static_route: web.Application ): - filename = "data.unknown_mime_type" - filepath = pathlib.Path(__file__).parent / filename - - async def handler(request): - return sender(filepath) - - app = web.Application() - app.router.add_get("/", handler) - client = await aiohttp_client(app) + client = await aiohttp_client(app_with_static_route) lastmod = "Fri, 31 Dec 9999 23:59:59 GMT" @@ -690,17 +728,9 @@ async def handler(request): async def test_static_file_if_range_past_without_range( - aiohttp_client: Any, sender: Any + aiohttp_client: Any, app_with_static_route: web.Application ): - filename = "data.unknown_mime_type" - filepath = pathlib.Path(__file__).parent / filename - - async def handler(request): - return sender(filepath) - - app = web.Application() - app.router.add_get("/", handler) - client = await aiohttp_client(app) + client = await aiohttp_client(app_with_static_route) lastmod = "Mon, 1 Jan 1990 01:01:01 GMT" @@ -711,17 +741,9 @@ async def handler(request): async def test_static_file_if_range_future_without_range( - aiohttp_client: Any, sender: Any + aiohttp_client: Any, app_with_static_route: web.Application ): - filename = "data.unknown_mime_type" - filepath = pathlib.Path(__file__).parent / filename - - async def handler(request): - return sender(filepath) - - app = web.Application() - app.router.add_get("/", handler) - client = await aiohttp_client(app) + client = await aiohttp_client(app_with_static_route) lastmod = "Fri, 31 Dec 9999 23:59:59 GMT" @@ -732,17 +754,9 @@ async def handler(request): async def test_static_file_if_unmodified_since_invalid_date( - aiohttp_client: Any, sender: Any + aiohttp_client: Any, app_with_static_route: web.Application ): - filename = "data.unknown_mime_type" - filepath = pathlib.Path(__file__).parent / filename - - async def handler(request): - return sender(filepath) - - app = web.Application() - app.router.add_get("/", handler) - client = await aiohttp_client(app) + client = await aiohttp_client(app_with_static_route) lastmod = "not a valid HTTP-date" @@ -751,16 +765,10 @@ async def handler(request): resp.close() -async def test_static_file_if_range_invalid_date(aiohttp_client: Any, sender: Any): - filename = "data.unknown_mime_type" - filepath = pathlib.Path(__file__).parent / filename - - async def handler(request): - return sender(filepath) - - app = web.Application() - app.router.add_get("/", handler) - client = await aiohttp_client(app) +async def test_static_file_if_range_invalid_date( + aiohttp_client: Any, app_with_static_route: web.Application +): + client = await aiohttp_client(app_with_static_route) lastmod = "not a valid HTTP-date"