diff --git a/CHANGES/13480.bugfix.rst b/CHANGES/13480.bugfix.rst new file mode 100644 index 00000000000..6eff8ca4045 --- /dev/null +++ b/CHANGES/13480.bugfix.rst @@ -0,0 +1,5 @@ +Fixed :class:`~aiohttp.web.FileResponse` ignoring the entity-tag form of the +``If-Range`` request header, which caused a ``Range`` request with a stale +``If-Range: ""`` to be served a ``206`` partial from a changed +file instead of the full ``200`` representation, per :rfc:`9110#section-13.1.5` +-- by :user:`arshsmith1`. diff --git a/CHANGES/13553.bugfix.rst b/CHANGES/13553.bugfix.rst new file mode 100644 index 00000000000..116b52bd28e --- /dev/null +++ b/CHANGES/13553.bugfix.rst @@ -0,0 +1,4 @@ +Fixed the WebSocket reader accepting a new data frame injected between the +fragments of an in-progress message; per :rfc:`6455#section-5.4` every frame +after the first fragment and before the ``FIN`` must be a continuation, and +such a stream is now rejected as a protocol error -- by :user:`arshsmith1`. diff --git a/aiohttp/_websocket/reader_py.py b/aiohttp/_websocket/reader_py.py index 00588632dc2..659e97f610d 100644 --- a/aiohttp/_websocket/reader_py.py +++ b/aiohttp/_websocket/reader_py.py @@ -268,6 +268,13 @@ def _handle_frame( if not fin: # got partial frame payload if opcode != OP_CODE_CONTINUATION: + # https://datatracker.ietf.org/doc/html/rfc6455#section-5.4 + if self._opcode != OP_CODE_NOT_SET: + raise WebSocketError( + WSCloseCode.PROTOCOL_ERROR, + "The opcode in non-fin frame is expected " + f"to be zero, got {opcode!r}", + ) self._opcode = opcode self._partial += payload return diff --git a/aiohttp/web_fileresponse.py b/aiohttp/web_fileresponse.py index a6d764d9d2a..e30bff091e6 100644 --- a/aiohttp/web_fileresponse.py +++ b/aiohttp/web_fileresponse.py @@ -1,5 +1,6 @@ import asyncio import io +import math import os import pathlib import sys @@ -309,10 +310,26 @@ async def _prepare_open_file( file_mtime: float = st.st_mtime count: int = file_size start: int | None = None + etag_value = f"{st.st_mtime_ns:x}-{st.st_size:x}" - if (ifrange := request.if_range) is None or file_mtime <= ifrange.timestamp(): + # https://www.rfc-editor.org/info/rfc9110/#name-if-range + if_range = request.if_range + if isinstance(if_range, ETag): + # https://www.rfc-editor.org/info/rfc9110/#section-13.1.5-12.1 + range_applies = not if_range.is_weak and if_range.value == etag_value + elif if_range is not None: + # https://www.rfc-editor.org/info/rfc9110/#section-13.1.5-10.2 + # Last-Modified is emitted as math.ceil(mtime), so the strong + # comparison is against that same rounded value. + range_applies = math.ceil(file_mtime) == if_range.timestamp() + else: + # A malformed validator can't match the current representation, + # so only an absent header lets the Range through. + range_applies = hdrs.IF_RANGE not in request.headers + + if range_applies: # If-Range header check: - # condition = cached date >= last modification date + # condition = cached validator matches current representation. # return 206 if True else 200. # if False: # Range header would not be processed, return 200 @@ -398,7 +415,7 @@ async def _prepare_open_file( # compress. self._compression = False - self.etag = f"{st.st_mtime_ns:x}-{st.st_size:x}" + self.etag = etag_value self.last_modified = file_mtime self.content_length = count diff --git a/aiohttp/web_request.py b/aiohttp/web_request.py index 08c53b21e99..d0e3f0e1531 100644 --- a/aiohttp/web_request.py +++ b/aiohttp/web_request.py @@ -32,6 +32,7 @@ DEFAULT_CHUNK_SIZE, ETAG_ANY, LIST_QUOTED_ETAG_RE, + QUOTED_ETAG_RE, ChainMapProxy, ETag, HeadersDictProxy, @@ -605,12 +606,21 @@ def if_none_match(self) -> tuple[ETag, ...] | None: return self._if_match_or_none_impl(self.headers.get(hdrs.IF_NONE_MATCH)) @reify - def if_range(self) -> datetime.datetime | None: + def if_range(self) -> datetime.datetime | ETag | None: """The value of If-Range HTTP header, or None. - This header is represented as a `datetime` object. + This header is represented as a `datetime` (HTTP-date form) or an + `ETag` (entity-tag form) object. """ - return parse_http_date(self.headers.get(hdrs.IF_RANGE)) + if_range = self.headers.get(hdrs.IF_RANGE) + if if_range is None: + return None + if (date := parse_http_date(if_range)) is not None: + return date + match = QUOTED_ETAG_RE.fullmatch(if_range.strip()) + if match is None: + return None + return ETag(is_weak=bool(match.group(1)), value=match.group(2)) @reify def keep_alive(self) -> bool: diff --git a/docs/web_reference.rst b/docs/web_reference.rst index 07dd06d8b38..41525f96e40 100644 --- a/docs/web_reference.rst +++ b/docs/web_reference.rst @@ -374,15 +374,20 @@ and :ref:`aiohttp-web-signals` handlers. .. attribute:: if_range - Read-only property that returns the date specified in the + Read-only property that returns the value specified in the *If-Range* header. - Returns :class:`datetime.datetime` or ``None`` if - *If-Range* header is absent or is not a valid - HTTP date. + Returns :class:`datetime.datetime` for the HTTP-date form, an + :class:`~aiohttp.ETag` for the entity-tag form, or ``None`` if the + *If-Range* header is absent or malformed. .. versionadded:: 3.1 + .. versionchanged:: 4.0 + + The entity-tag form is now parsed and returned as an + :class:`~aiohttp.ETag`. + .. method:: clone(*, method=..., rel_url=..., headers=...) Clone itself with replacement some attributes. diff --git a/tests/test_web_sendfile_functional.py b/tests/test_web_sendfile_functional.py index ec325b3b55d..2ab0153509a 100644 --- a/tests/test_web_sendfile_functional.py +++ b/tests/test_web_sendfile_functional.py @@ -1085,21 +1085,73 @@ async def test_static_file_if_range_past_with_range( await client.close() -async def test_static_file_if_range_future_with_range( +async def test_static_file_if_range_matching_date_with_range( aiohttp_client: AiohttpClient, app_with_static_route: web.Application ) -> None: client = await aiohttp_client(app_with_static_route) - lastmod = "Fri, 31 Dec 9999 23:59:59 GMT" + async with client.get("/") as resp: + assert 200 == resp.status + last_modified = resp.headers["Last-Modified"] - resp = await client.get("/", headers={"If-Range": lastmod, "Range": "bytes=2-"}) - assert 206 == resp.status - assert resp.headers["Content-Range"] == "bytes 2-12/13" - assert resp.headers["Content-Length"] == "11" - resp.close() + async with client.get( + "/", headers={"If-Range": last_modified, "Range": "bytes=2-"} + ) as resp: + assert 206 == resp.status + assert resp.headers["Content-Range"] == "bytes 2-12/13" + assert resp.headers["Content-Length"] == "11" - resp.release() - await client.close() + +async def test_static_file_if_range_etag_match_with_range( + aiohttp_client: AiohttpClient, app_with_static_route: web.Application +) -> None: + client = await aiohttp_client(app_with_static_route) + + async with client.get("/") as resp: + assert 200 == resp.status + etag = resp.headers["ETag"] + + async with client.get("/", headers={"If-Range": etag, "Range": "bytes=2-"}) as resp: + assert 206 == resp.status + assert resp.headers["Content-Range"] == "bytes 2-12/13" + assert resp.headers["Content-Length"] == "11" + + +async def test_static_file_if_range_stale_etag_with_range( + aiohttp_client: AiohttpClient, app_with_static_route: web.Application +) -> None: + client = await aiohttp_client(app_with_static_route) + + async with client.get( + "/", headers={"If-Range": '"stale-etag"', "Range": "bytes=2-"} + ) as resp: + assert 200 == resp.status + assert resp.headers["Content-Length"] == "13" + assert "Content-Range" not in resp.headers + + +@pytest.mark.parametrize( + "if_range", + ("{etag}", '"{etag}', '"{etag}", "{etag}"', "not a valid HTTP-date"), + ids=("unquoted", "unterminated", "list", "date"), +) +async def test_static_file_if_range_malformed_with_range( + aiohttp_client: AiohttpClient, + app_with_static_route: web.Application, + if_range: str, +) -> None: + client = await aiohttp_client(app_with_static_route) + + async with client.get("/") as resp: + assert 200 == resp.status + etag = resp.headers["ETag"].strip('"') + + async with client.get( + "/", headers={"If-Range": if_range.format(etag=etag), "Range": "bytes=2-"} + ) as resp: + assert 200 == resp.status + assert resp.headers["Content-Length"] == "13" + assert "Content-Range" not in resp.headers async def test_static_file_if_unmodified_since_past_without_range( diff --git a/tests/test_websocket_parser.py b/tests/test_websocket_parser.py index 4bd1568194a..ffa23681e46 100644 --- a/tests/test_websocket_parser.py +++ b/tests/test_websocket_parser.py @@ -495,6 +495,16 @@ def test_continuation_err( parser._handle_frame(True, WSMsgType.TEXT, b"line2", 0) +def test_continuation_non_fin_err( + out: WebSocketDataQueue, parser: PatchableWebSocketReader +) -> None: + # https://datatracker.ietf.org/doc/html/rfc6455#section-5.4 + parser._handle_frame(False, WSMsgType.TEXT, b"line1", 0) + with pytest.raises(WebSocketError) as ctx: + parser._handle_frame(False, WSMsgType.TEXT, b"line2", 0) + assert ctx.value.code == WSCloseCode.PROTOCOL_ERROR + + def test_continuation_with_close( out: WebSocketDataQueue, parser: WebSocketReader ) -> None: