Update ETag functionality

aiohttp/helpers.py

@@ -883,9 +883,9 @@ def populate_with_cookies(
 
 # https://tools.ietf.org/html/rfc7232#section-2.3
 _ETAGC = r"[!#-}\x80-\xff]+"
-_QUOTED_ETAG = fr'(W/)?("{_ETAGC}")'
+_ETAGC_RE = re.compile(_ETAGC)
+_QUOTED_ETAG = fr'(W/)?"({_ETAGC})"'
 QUOTED_ETAG_RE = re.compile(_QUOTED_ETAG)
-LIST_QUOTED_ETAG_RE = re.compile(fr"({_QUOTED_ETAG})(?:\s*,\s*|$)")
 
 ETAG_ANY = "*"
 
@@ -894,3 +894,10 @@ def populate_with_cookies(
 class ETag:
     value: str
     is_weak: bool = False
+
+
+def validate_etag_value(value: str) -> None:
+    if value != ETAG_ANY and not _ETAGC_RE.fullmatch(value):
+        raise ValueError(
+            f"Value {value!r} is not a valid etag. Maybe it contains '\"'?"
+        )

aiohttp/web_request.py

@@ -33,7 +33,7 @@
 from .helpers import (
     _SENTINEL,
     ETAG_ANY,
-    LIST_QUOTED_ETAG_RE,
+    QUOTED_ETAG_RE,
     ChainMapProxy,
     ETag,
     HeadersMixin,
@@ -506,12 +506,17 @@ def _etag_values(etag_header: str) -> Iterator[ETag]:
                 value=ETAG_ANY,
             )
         else:
-            for match in LIST_QUOTED_ETAG_RE.finditer(etag_header):
-                is_weak, quoted_value = match.group(2), match.group(3)
-                yield ETag(
-                    is_weak=bool(is_weak),
-                    value=quoted_value[1:-1],
-                )
+            for raw_etag in etag_header.split(","):
+                # TODO: use walrus operator after dropping 3.7
+                match = QUOTED_ETAG_RE.fullmatch(raw_etag.strip())
+                if match:
+                    is_weak, value = match.group(1, 2)
+                    yield ETag(
+                        is_weak=bool(is_weak),
+                        value=value,
+                    )
+                else:
+                    break
 
     @classmethod
     def _if_match_or_none_impl(

aiohttp/web_response.py

@@ -37,6 +37,7 @@
     populate_with_cookies,
     rfc822_formatted_time,
     sentinel,
+    validate_etag_value,
 )
 from .http import RESPONSES, SERVER_SOFTWARE, HttpVersion10, HttpVersion11
 from .payload import Payload
@@ -278,31 +279,33 @@ def etag(self) -> Optional[ETag]:
             return None
         elif quoted_value == ETAG_ANY:
             return ETag(value=ETAG_ANY)
-        match = QUOTED_ETAG_RE.match(quoted_value)
+        match = QUOTED_ETAG_RE.fullmatch(quoted_value)
         if not match:
             return None
-        is_weak, value = match.group(1), match.group(2)
+        is_weak, value = match.group(1, 2)
         return ETag(
             is_weak=bool(is_weak),
-            value=value[1:-1],
+            value=value,
         )
 
     @etag.setter
     def etag(self, value: Optional[Union[ETag, str]]) -> None:
-        if (isinstance(value, str) and value == ETAG_ANY) or (
+        if value is None:
+            self._headers.pop(hdrs.ETAG, None)
+        elif (isinstance(value, str) and value == ETAG_ANY) or (
             isinstance(value, ETag) and value.value == ETAG_ANY
         ):
             self._headers[hdrs.ETAG] = ETAG_ANY
         elif isinstance(value, str):
+            validate_etag_value(value)
             self._headers[hdrs.ETAG] = f'"{value}"'
         elif isinstance(value, ETag) and isinstance(value.value, str):
-            hdr_value = f'W/"{value.value}"' if value.is_weak else f'"{value}"'
+            validate_etag_value(value.value)
+            hdr_value = f'W/"{value.value}"' if value.is_weak else f'"{value.value}"'
             self._headers[hdrs.ETAG] = hdr_value
-        elif value is None:
-            self._headers.pop(hdrs.ETAG, None)
         else:
             raise ValueError(
-                f"Unsupported etag type: {type(value)!r}. "
+                f"Unsupported etag type: {type(value)}. "
                 f"etag must be str, ETag or None"
             )
 

tests/test_web_request.py

@@ -845,6 +845,13 @@ async def invalid_handler_1(request):
             '"bfc1ef-5b2c2730249c88ca92d82d"',
             (ETag(is_weak=False, value="bfc1ef-5b2c2730249c88ca92d82d"),),
         ),
+        pytest.param(
+            '"valid-tag", "also-valid-tag",somegarbage"last-tag"',
+            (
+                ETag(is_weak=False, value="valid-tag"),
+                ETag(is_weak=False, value="also-valid-tag"),
+            ),
+        ),
         pytest.param(
             "*",
             (ETag(is_weak=False, value="*"),),

tests/test_web_response.py

@@ -286,11 +286,20 @@ def test_etag_any() -> None:
     assert resp.headers[hdrs.ETAG] == "*"
 
 
-@pytest.mark.parametrize("wrong_value", (123, ETag(value=123, is_weak=True)))
-def test_etag_wrong_class(wrong_value) -> None:
+@pytest.mark.parametrize(
+    "invalid", ('"invalid"', ETag(value='"invalid"', is_weak=True))
+)
+def test_etag_invalid_value(invalid) -> None:
     resp = StreamResponse()
     with pytest.raises(ValueError):
-        resp.etag = wrong_value
+        resp.etag = invalid
+
+
+@pytest.mark.parametrize("invalid", (123, ETag(value=123, is_weak=True)))
+def test_etag_invalid_value_class(invalid) -> None:
+    resp = StreamResponse()
+    with pytest.raises(ValueError):
+        resp.etag = invalid
 
 
 def test_etag_reset() -> None: