[pull] master from aio-libs:master

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: "<etag>"`` 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`.

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`.

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

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
 

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:

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.

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(

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: