From 984b300fd4f87c4a8d33a4250556636bc2e7beb4 Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Sun, 24 Oct 2021 17:33:57 +0900 Subject: [PATCH] fix(5925): suppress ValueError due to invalid datetime header (#6012) Co-authored-by: Andrew Svetlov --- CHANGES/5925.bugfix | 1 + CONTRIBUTORS.txt | 1 + aiohttp/helpers.py | 11 +++++++++++ aiohttp/web_request.py | 17 ++++------------- aiohttp/web_response.py | 9 ++------- tests/test_helpers.py | 27 +++++++++++++++++++++++++++ tests/test_web_request.py | 26 ++++++++++++++++++++++++++ tests/test_web_response.py | 13 +++++++++++++ 8 files changed, 85 insertions(+), 20 deletions(-) create mode 100644 CHANGES/5925.bugfix diff --git a/CHANGES/5925.bugfix b/CHANGES/5925.bugfix new file mode 100644 index 00000000000..297033b0f44 --- /dev/null +++ b/CHANGES/5925.bugfix @@ -0,0 +1 @@ +Return ``None`` from ``request.if_modified_since``, ``request.if_unmodified_since``, ``request.if_range`` and ``response.last_modified`` when corresponding http date headers are invalid. diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index b1c849b84c2..afd226fbcce 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -127,6 +127,7 @@ Gustavo Carneiro Günther Jena Hans Adema Harmon Y. +Hiroshi Ogawa Hrishikesh Paranjape Hu Bo Hugh Young diff --git a/aiohttp/helpers.py b/aiohttp/helpers.py index ac50c4481e3..42e3ab3712d 100644 --- a/aiohttp/helpers.py +++ b/aiohttp/helpers.py @@ -17,6 +17,7 @@ import weakref from collections import namedtuple from contextlib import suppress +from email.utils import parsedate from math import ceil from pathlib import Path from types import TracebackType @@ -854,3 +855,13 @@ def validate_etag_value(value: str) -> None: raise ValueError( f"Value {value!r} is not a valid etag. Maybe it contains '\"'?" ) + + +def parse_http_date(date_str: Optional[str]) -> Optional[datetime.datetime]: + """Process a date string, return a datetime object""" + if date_str is not None: + timetuple = parsedate(date_str) + if timetuple is not None: + with suppress(ValueError): + return datetime.datetime(*timetuple[:6], tzinfo=datetime.timezone.utc) + return None diff --git a/aiohttp/web_request.py b/aiohttp/web_request.py index ed5aae1adf4..dc810f5de23 100644 --- a/aiohttp/web_request.py +++ b/aiohttp/web_request.py @@ -7,7 +7,6 @@ import tempfile import types import warnings -from email.utils import parsedate from http.cookies import SimpleCookie from types import MappingProxyType from typing import ( @@ -38,6 +37,7 @@ ChainMapProxy, ETag, HeadersMixin, + parse_http_date, reify, sentinel, ) @@ -479,22 +479,13 @@ def raw_headers(self) -> RawHeaders: """A sequence of pairs for all headers.""" return self._message.raw_headers - @staticmethod - def _http_date(_date_str: Optional[str]) -> Optional[datetime.datetime]: - """Process a date string, return a datetime object""" - if _date_str is not None: - timetuple = parsedate(_date_str) - if timetuple is not None: - return datetime.datetime(*timetuple[:6], tzinfo=datetime.timezone.utc) - return None - @reify def if_modified_since(self) -> Optional[datetime.datetime]: """The value of If-Modified-Since HTTP header, or None. This header is represented as a `datetime` object. """ - return self._http_date(self.headers.get(hdrs.IF_MODIFIED_SINCE)) + return parse_http_date(self.headers.get(hdrs.IF_MODIFIED_SINCE)) @reify def if_unmodified_since(self) -> Optional[datetime.datetime]: @@ -502,7 +493,7 @@ def if_unmodified_since(self) -> Optional[datetime.datetime]: This header is represented as a `datetime` object. """ - return self._http_date(self.headers.get(hdrs.IF_UNMODIFIED_SINCE)) + return parse_http_date(self.headers.get(hdrs.IF_UNMODIFIED_SINCE)) @staticmethod def _etag_values(etag_header: str) -> Iterator[ETag]: @@ -556,7 +547,7 @@ def if_range(self) -> Optional[datetime.datetime]: This header is represented as a `datetime` object. """ - return self._http_date(self.headers.get(hdrs.IF_RANGE)) + return parse_http_date(self.headers.get(hdrs.IF_RANGE)) @reify def keep_alive(self) -> bool: diff --git a/aiohttp/web_response.py b/aiohttp/web_response.py index 291b61569a9..9477526c462 100644 --- a/aiohttp/web_response.py +++ b/aiohttp/web_response.py @@ -8,7 +8,6 @@ import warnings import zlib from concurrent.futures import Executor -from email.utils import parsedate from http.cookies import Morsel, SimpleCookie from typing import ( TYPE_CHECKING, @@ -33,6 +32,7 @@ QUOTED_ETAG_RE, ETag, HeadersMixin, + parse_http_date, rfc822_formatted_time, sentinel, validate_etag_value, @@ -326,12 +326,7 @@ def last_modified(self) -> Optional[datetime.datetime]: This header is represented as a `datetime` object. """ - httpdate = self._headers.get(hdrs.LAST_MODIFIED) - if httpdate is not None: - timetuple = parsedate(httpdate) - if timetuple is not None: - return datetime.datetime(*timetuple[:6], tzinfo=datetime.timezone.utc) - return None + return parse_http_date(self._headers.get(hdrs.LAST_MODIFIED)) @last_modified.setter def last_modified( diff --git a/tests/test_helpers.py b/tests/test_helpers.py index 582d03166b6..7038fe9be62 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -1,5 +1,6 @@ import asyncio import base64 +import datetime import gc import os import platform @@ -13,6 +14,7 @@ from yarl import URL from aiohttp import helpers +from aiohttp.helpers import parse_http_date IS_PYPY = platform.python_implementation() == "PyPy" @@ -709,3 +711,28 @@ def test_repr(self) -> None: cp = helpers.ChainMapProxy([d1, d2]) expected = f"ChainMapProxy({d1!r}, {d2!r})" assert expected == repr(cp) + + +@pytest.mark.parametrize( + ["value", "expected"], + [ + # email.utils.parsedate returns None + pytest.param("xxyyzz", None), + # datetime.datetime fails with ValueError("year 4446413 is out of range") + pytest.param("Tue, 08 Oct 4446413 00:56:40 GMT", None), + # datetime.datetime fails with ValueError("second must be in 0..59") + pytest.param("Tue, 08 Oct 2000 00:56:80 GMT", None), + # OK + pytest.param( + "Tue, 08 Oct 2000 00:56:40 GMT", + datetime.datetime(2000, 10, 8, 0, 56, 40, tzinfo=datetime.timezone.utc), + ), + # OK (ignore timezone and overwrite to UTC) + pytest.param( + "Tue, 08 Oct 2000 00:56:40 +0900", + datetime.datetime(2000, 10, 8, 0, 56, 40, tzinfo=datetime.timezone.utc), + ), + ], +) +def test_parse_http_date(value, expected): + assert parse_http_date(value) == expected diff --git a/tests/test_web_request.py b/tests/test_web_request.py index 584e7c44533..ab17da6ca23 100644 --- a/tests/test_web_request.py +++ b/tests/test_web_request.py @@ -1,4 +1,5 @@ import asyncio +import datetime import socket from collections.abc import MutableMapping from typing import Any @@ -787,3 +788,28 @@ async def test_loop_prop() -> None: def test_etag_headers(header, header_attr, header_val, expected) -> None: req = make_mocked_request("GET", "/", headers={header: header_val}) assert getattr(req, header_attr) == expected + + +@pytest.mark.parametrize( + ["header", "header_attr"], + [ + pytest.param("If-Modified-Since", "if_modified_since"), + pytest.param("If-Unmodified-Since", "if_unmodified_since"), + pytest.param("If-Range", "if_range"), + ], +) +@pytest.mark.parametrize( + ["header_val", "expected"], + [ + pytest.param("xxyyzz", None), + pytest.param("Tue, 08 Oct 4446413 00:56:40 GMT", None), + pytest.param("Tue, 08 Oct 2000 00:56:80 GMT", None), + pytest.param( + "Tue, 08 Oct 2000 00:56:40 GMT", + datetime.datetime(2000, 10, 8, 0, 56, 40, tzinfo=datetime.timezone.utc), + ), + ], +) +def test_datetime_headers(header, header_attr, header_val, expected) -> None: + req = make_mocked_request("GET", "/", headers={header: header_val}) + assert getattr(req, header_attr) == expected diff --git a/tests/test_web_response.py b/tests/test_web_response.py index 583f076dc90..c514cbad5e8 100644 --- a/tests/test_web_response.py +++ b/tests/test_web_response.py @@ -251,6 +251,19 @@ def test_last_modified_reset() -> None: assert resp.last_modified is None +@pytest.mark.parametrize( + ["header_val", "expected"], + [ + pytest.param("xxyyzz", None), + pytest.param("Tue, 08 Oct 4446413 00:56:40 GMT", None), + pytest.param("Tue, 08 Oct 2000 00:56:80 GMT", None), + ], +) +def test_last_modified_string_invalid(header_val, expected) -> None: + resp = StreamResponse(headers={"Last-Modified": header_val}) + assert resp.last_modified == expected + + def test_etag_initial() -> None: resp = StreamResponse() assert resp.etag is None