fix(5925): suppress ValueError due to invalid datetime header (#6012)

Co-authored-by: Andrew Svetlov <[email protected]>

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.

CONTRIBUTORS.txt

@@ -127,6 +127,7 @@ Gustavo Carneiro
 Günther Jena
 Hans Adema
 Harmon Y.
+Hiroshi Ogawa
 Hrishikesh Paranjape
 Hu Bo
 Hugh Young

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

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,30 +479,21 @@ 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]:
         """The value of If-Unmodified-Since HTTP header, or None.
 
         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:

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(

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

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

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