Merge pull request #763 from requests-cache/string-expiration

Raise an error for invalid expiration string values 

4 files changed, 27 insertions, 4 deletions

diff --git a/HISTORY.md b/HISTORY.md
index 5f88393..88a715a 100644
--- a/HISTORY.md
+++ b/HISTORY.md
@@ -110,6 +110,8 @@
 * Add `CachedRequest.path_url` property for compatibility with `RequestEncodingMixin`
 * Fix potential `AttributeError` due to undetected imports when requests-cache is bundled in a PyInstaller package
 * Fix `AttributeError` when attempting to unpickle a `CachedSession` object, and instead disable pickling by raising a `NotImplementedError`
+* Raise an error for invalid expiration string values (except for headers containing httpdates)
+  * Previously, this would be quietly ignored, and the response would be cached indefinitely
 
 📦 **Dependencies:**
 * Replace `appdirs` with `platformdirs`
diff --git a/requests_cache/policy/expiration.py b/requests_cache/policy/expiration.py
index 18a2593..1041e36 100644
--- a/requests_cache/policy/expiration.py
+++ b/requests_cache/policy/expiration.py
@@ -21,6 +21,7 @@ def get_expiration_datetime(
     expire_after: ExpirationTime,
     start_time: Optional[datetime] = None,
     negative_delta: bool = False,
+    ignore_invalid_httpdate: bool = False,
 ) -> Optional[datetime]:
     """Convert an expiration value in any supported format to an absolute datetime"""
     # Never expire (or do not cache, in which case expiration won't be used)
@@ -29,9 +30,12 @@ def get_expiration_datetime(
     # Expire immediately
     elif try_int(expire_after) == EXPIRE_IMMEDIATELY:
         return start_time or datetime.utcnow()
-    # Already a datetime or datetime str
+    # Already a datetime or httpdate str (allowed for headers only)
     if isinstance(expire_after, str):
-        return _parse_http_date(expire_after)
+        expire_after_dt = _parse_http_date(expire_after)
+        if not expire_after_dt and not ignore_invalid_httpdate:
+            raise ValueError(f'Invalid HTTP date: {expire_after}')
+        return expire_after_dt
     elif isinstance(expire_after, datetime):
         return _to_utc(expire_after)
 
@@ -47,7 +51,7 @@ def get_expiration_seconds(expire_after: ExpirationTime) -> int:
     """Convert an expiration value in any supported format to an expiration time in seconds"""
     if expire_after == DO_NOT_CACHE:
         return DO_NOT_CACHE
-    expires = get_expiration_datetime(expire_after)
+    expires = get_expiration_datetime(expire_after, ignore_invalid_httpdate=True)
     return ceil((expires - datetime.utcnow()).total_seconds()) if expires else NEVER_EXPIRE
 
 
diff --git a/tests/unit/policy/test_expiration.py b/tests/unit/policy/test_expiration.py
index d248fa7..54eb2a2 100644
--- a/tests/unit/policy/test_expiration.py
+++ b/tests/unit/policy/test_expiration.py
@@ -41,7 +41,9 @@ def test_get_expiration_datetime__tzinfo():
 
 def test_get_expiration_datetime__httpdate():
     assert get_expiration_datetime(HTTPDATE_STR) == HTTPDATE_DATETIME
-    assert get_expiration_datetime('P12Y34M56DT78H90M12.345S') is None
+    assert get_expiration_datetime('P12Y34M56DT78H90M12.345S', ignore_invalid_httpdate=True) is None
+    with pytest.raises(ValueError):
+        get_expiration_datetime('P12Y34M56DT78H90M12.345S')
 
 
 @pytest.mark.parametrize(
diff --git a/tests/unit/test_session.py b/tests/unit/test_session.py
index e8ec72d..72c6977 100644
--- a/tests/unit/test_session.py
+++ b/tests/unit/test_session.py
@@ -660,6 +660,21 @@ def test_url_allowlist(mock_session):
     assert not mock_session.cache.contains(url=MOCKED_URL)
 
 
+def test_invalid_expiration(mock_session):
+    mock_session.settings.expire_after = 'tomorrow'
+    with pytest.raises(ValueError):
+        mock_session.get(MOCKED_URL)
+
+    mock_session.settings.expire_after = object()
+    with pytest.raises(TypeError):
+        mock_session.get(MOCKED_URL)
+
+    mock_session.settings.expire_after = None
+    mock_session.settings.urls_expire_after = {'*': 'tomorrow'}
+    with pytest.raises(ValueError):
+        mock_session.get(MOCKED_URL)
+
+
 def test_stale_while_revalidate(mock_session):
     # Start with expired responses
     mocked_url_2 = f'{MOCKED_URL_ETAG}?k=v'