From b66aad1d1b0416f282853a47d307466347d7f55f Mon Sep 17 00:00:00 2001 From: Viacheslav Greshilov Date: Sat, 5 Dec 2020 18:42:36 +0200 Subject: [PATCH] Update ETag functionality --- aiohttp/helpers.py | 11 +++++++++-- aiohttp/web_request.py | 19 ++++++++++++------- aiohttp/web_response.py | 19 +++++++++++-------- tests/test_web_request.py | 7 +++++++ tests/test_web_response.py | 15 ++++++++++++--- 5 files changed, 51 insertions(+), 20 deletions(-) diff --git a/aiohttp/helpers.py b/aiohttp/helpers.py index 99ed94ad050..7e51c4f5b2e 100644 --- a/aiohttp/helpers.py +++ b/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 '\"'?" + ) diff --git a/aiohttp/web_request.py b/aiohttp/web_request.py index 60a1e198779..2193cf9e117 100644 --- a/aiohttp/web_request.py +++ b/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( diff --git a/aiohttp/web_response.py b/aiohttp/web_response.py index 23eb6042767..652763e9d57 100644 --- a/aiohttp/web_response.py +++ b/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" ) diff --git a/tests/test_web_request.py b/tests/test_web_request.py index 77337b1bd75..dea6ec14f17 100644 --- a/tests/test_web_request.py +++ b/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="*"),), diff --git a/tests/test_web_response.py b/tests/test_web_response.py index 14a0ddc6888..d3a164f81e6 100644 --- a/tests/test_web_response.py +++ b/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: