From ce0d5495af8ee10e0afb681f3a67e096f8dc747b Mon Sep 17 00:00:00 2001 From: allen0099 Date: Sat, 26 Sep 2026 22:02:48 +0000 Subject: [PATCH] fix(backends): require an int ttl and delta before any backend I/O validate_ttl accepted floats and bools. On Redis, increment(ttl=1.5) created the counter before EXPIRE failed, leaving a counter that never expires behind a misleading "not a counter" error. Only an int up to MAX_TTL (2**31 - 1) is accepted now, and increment checks delta the same way. Memcached rejects expiries past 2038-01-19, which it used to drop silently, and maps only non-numeric values to "not a counter". @cache checks ttl at decoration time. Closes #229 --- CHANGELOG.md | 12 +++++ CLAUDE.md | 2 + docs/BACKENDS.md | 28 +++++++--- docs/HTTP_CACHING.md | 2 +- fastapi_cachex/backends/base.py | 43 ++++++++++++++- fastapi_cachex/backends/memcached.py | 25 ++++++++- fastapi_cachex/backends/memory.py | 2 + fastapi_cachex/backends/redis.py | 2 + fastapi_cachex/cache.py | 12 ++++- i18n/zh-TW/docs/BACKENDS.md | 11 +++- i18n/zh-TW/docs/HTTP_CACHING.md | 2 +- tests/backends/test_memcached.py | 44 +++++++++++++++ tests/backends/test_redis.py | 18 +++++++ tests/backends/test_ttl_contract.py | 81 ++++++++++++++++++++++------ tests/test_cache.py | 14 +++++ 15 files changed, 266 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 92ee699..443f120 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -220,6 +220,18 @@ Note that 0.3.3 was never released; 0.3.4 follows 0.3.2. `del` or `pop()`, e.g. a flash message; such a session is now saved with empty data. An emptied anonymous session is still deleted. ([#227](https://github.com/allen0099/FastAPI-CacheX/issues/227)) +- **A `ttl` must be an `int` up to `MAX_TTL`, and `delta` an `int` in 64-bit + range.** On Redis, `increment(key, ttl=1.5)` created the counter and then + failed at `EXPIRE`, leaving a counter that never expired (a permanent + lockout for a rate limiter) behind an error saying the key was "not a + counter". `validate_ttl` now raises `TypeError` for `float`, `bool` and + other types, and `ValueError` above `MAX_TTL` (2**31 - 1 seconds), before + any backend I/O. A float TTL used to work on the memory backend only. + `increment` checks `delta` the same way. Memcached now raises `ValueError` + for a `ttl` whose expiry falls after 2038-01-19, which it used to accept and + then drop at once, and reports only non-numeric values as "not a counter". + `@cache` rejects such a `ttl` when the decorator is applied. + ([#229](https://github.com/allen0099/FastAPI-CacheX/issues/229)) ## [0.3.7] - 2026-09-25 diff --git a/CLAUDE.md b/CLAUDE.md index 3c5d02b..000f070 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -94,6 +94,8 @@ Four non-abstract atomic primitives live on the base class with non-atomic fallb - `set_if_absent(key, value, ttl=None) -> bool`: claim-if-free for locks/slots. Redis `SET NX EX`, Memcached `ADD`, memory under its lock. - `delete_if_equals(key, expected) -> bool`: release only while the key still holds `expected` (compared as decoded `CacheEntry`). Redis compares in Python then deletes via a Lua script that re-checks the raw bytes; Memcached uses `GETS` + `CAS` with exptime `-1` (immediate expiry), since classic `DELETE` has no CAS. +`validate_ttl` (in `backends/base.py`) accepts `None` or an `int` from 1 to `MAX_TTL` (2**31 - 1) and raises `TypeError` for floats/bools; `validate_delta` requires an `int` in signed 64-bit range. Both run before any I/O. Memcached's `_expiry` also rejects expiries after 2038-01-19. + `delete_many(keys) -> int` is the fifth non-abstract base method: a per-key loop by default, one batched operation on Redis (`DEL`) and Memory (single lock). `backends/codec.py` holds the JSON `CacheEntry` codec shared by Redis and Memcached; `decode_entry` maps a bare integer to a counter entry and every malformed value to `None`. diff --git a/docs/BACKENDS.md b/docs/BACKENDS.md index d310951..0e93e44 100644 --- a/docs/BACKENDS.md +++ b/docs/BACKENDS.md @@ -142,6 +142,8 @@ BackendProxy.set(backend) - `clear()` issues `flush_all`, which wipes the whole Memcached server, not just this namespace - A key Memcached would reject (over 250 bytes, whitespace, non-ASCII) is stored under its SHA-256 digest +- A `ttl` whose expiry falls after 2038-01-19 raises `ValueError` (see + [TTL values](#ttl-values)) - Values larger than the server's item size limit (1 MB by default, `memcached -I`) are rejected with an error. `@cache` logs it and serves the response unstored (see [When the backend fails](HTTP_CACHING.md#when-the-backend-fails)); other @@ -198,7 +200,9 @@ if await backend.set_if_absent(f"stream:{user_id}", owner, ttl=300): and the monitoring routes treat it like any other entry. Incrementing a key that holds anything else raises `CacheXError` on every backend, even a cached response whose body is a number. A counter written with - `set(key, counter_entry(n))` can be incremented on every backend. + `set(key, counter_entry(n))` can be incremented on every backend. `delta` + must be an `int` within the signed 64-bit range; anything else raises + `TypeError` or `ValueError` before the backend is touched. - `get_and_delete(key) -> CacheEntry | None` — Memory pops under its lock, Redis uses `GETDEL` (server 6.2+) and Memcached uses `GETS` + a `CAS` write with `exptime=-1` (retrying if another writer replaced the value in between). If @@ -231,12 +235,22 @@ real atomicity. Every `ttl` argument (`set`, `set_if_absent`, `increment`, and the `CacheManager` and `StateManager` methods and defaults built on them) is either `None`, meaning -the entry never expires, or a positive number of seconds. Zero and negative -values raise `ValueError`. The underlying stores disagree on what they mean: -Memcached reads an exptime of `0` as "never expire", Redis rejects `EX 0`, and -an in-process dict would expire the entry at once. A third-party backend should -call `fastapi_cachex.backends.base.validate_ttl(ttl)` in its `set` to follow -the same rule. (`@cache(ttl=0)` is separate: it sends `max-age=0` and never +the entry never expires, or an `int` number of seconds from 1 up to `MAX_TTL` +(2**31 - 1, about 68 years). The checks run before any backend I/O: + +- Zero, negative and larger values raise `ValueError`. The underlying stores + disagree on what `0` means: Memcached reads it as "never expire", Redis + rejects `EX 0`, and an in-process dict would expire the entry at once. +- A `float`, a `bool` or any other type raises `TypeError`. A float worked + only on the memory backend, and `True` was taken as one second. Convert a + `timedelta` with `int(td.total_seconds())`. +- Memcached cannot store an expiry after 2038-01-19 (its exptime is a signed + 32-bit timestamp), so the Memcached backend raises `ValueError` for a `ttl` + that reaches past it instead of accepting a write it would drop at once. + +A third-party backend should call `fastapi_cachex.backends.base.validate_ttl(ttl)` +in its `set` to follow the same rules, and `validate_delta(delta)` in +`increment`. (`@cache(ttl=0)` is separate: it sends `max-age=0` and never passes `0` to the backend; see [HTTP caching](HTTP_CACHING.md).) How each backend stores entries is described in diff --git a/docs/HTTP_CACHING.md b/docs/HTTP_CACHING.md index 8be7451..458be2b 100644 --- a/docs/HTTP_CACHING.md +++ b/docs/HTTP_CACHING.md @@ -81,7 +81,7 @@ When a cached entry is valid (within TTL): - **With `no-cache` directive**: Forces revalidation with fresh content before deciding on 304 - **With `private=True`**: Nothing is read from or written to the shared backend; the handler runs every time and only `If-None-Match` revalidation applies - **Without `ttl`** (`ttl=None`): Nothing is read from or written to the backend, as with `private=True`. The handler runs on every request, and `If-None-Match` gets a 304 only when it matches the freshly rendered response, so an old ETag never gets a 304 once the content has changed -- **With `ttl=0`**: Sends `max-age=0` and otherwise behaves like `ttl=None`. A negative `ttl` is rejected with `CacheXError` when the decorator is applied +- **With `ttl=0`**: Sends `max-age=0` and otherwise behaves like `ttl=None`. A negative `ttl`, a non-`int` one (such as `1.5` or `True`) and one above `MAX_TTL` (see [TTL values](BACKENDS.md#ttl-values)) are rejected with `CacheXError` when the decorator is applied Only successful responses are stored. A response the handler *returns* with a non-2xx status (for example `Response(..., status_code=404)`) is passed straight diff --git a/fastapi_cachex/backends/base.py b/fastapi_cachex/backends/base.py index 9a7f392..3af3132 100644 --- a/fastapi_cachex/backends/base.py +++ b/fastapi_cachex/backends/base.py @@ -35,6 +35,10 @@ def warn_if_path_shaped(pattern: str, cleared: int) -> None: ) +# The largest TTL accepted anywhere: 2**31 - 1 seconds, about 68 years. +MAX_TTL = 2**31 - 1 + + def validate_ttl(ttl: int | None) -> int | None: """Return ``ttl`` if it is ``None`` or a positive number of seconds. @@ -43,15 +47,47 @@ def validate_ttl(ttl: int | None) -> int | None: entry at once), so the library refuses them instead of letting the meaning depend on the backend. ``None`` is the way to say "no expiry". + Only an ``int`` is a TTL. A ``float`` worked on the memory backend and + failed on Redis and Memcached, and ``True`` passed as one second. + Raises: - ValueError: If ``ttl`` is zero or negative + TypeError: If ``ttl`` is not an ``int`` (``bool`` included) + ValueError: If ``ttl`` is zero, negative or larger than ``MAX_TTL`` """ - if ttl is not None and ttl <= 0: + if ttl is None: + return None + if isinstance(ttl, bool) or not isinstance(ttl, int): + msg = f"ttl must be an int number of seconds or None, got {type(ttl).__name__}" + raise TypeError(msg) + if ttl <= 0: msg = f"ttl must be a positive number of seconds or None, got {ttl!r}" raise ValueError(msg) + if ttl > MAX_TTL: + msg = f"ttl must be at most {MAX_TTL} seconds (about 68 years)" + raise ValueError(msg) return ttl +def validate_delta(delta: int) -> int: + """Return ``delta`` if it is an ``int`` a counter can be changed by. + + Redis counters are signed 64-bit integers and Memcached's are unsigned, so + a delta outside the signed 64-bit range fails on both, where it used to be + reported as the key not holding a counter. + + Raises: + TypeError: If ``delta`` is not an ``int`` (``bool`` included) + ValueError: If ``delta`` does not fit in a signed 64-bit integer + """ + if isinstance(delta, bool) or not isinstance(delta, int): + msg = f"delta must be an int, got {type(delta).__name__}" + raise TypeError(msg) + if not -(2**63) <= delta < 2**63: + msg = "delta must fit in a signed 64-bit integer" + raise ValueError(msg) + return delta + + class BaseCacheBackend(ABC): """Base class for all cache backends.""" @@ -210,7 +246,10 @@ async def increment(self, key: str, delta: int = 1, ttl: int | None = None) -> i Raises: CacheXError: If ``key`` holds a cached response instead of a counter + TypeError: If ``delta`` or ``ttl`` is not an ``int`` + ValueError: If ``ttl`` is out of range """ + validate_delta(delta) validate_ttl(ttl) current = await self.get(key) value = delta if current is None else counter_value(current) + delta diff --git a/fastapi_cachex/backends/memcached.py b/fastapi_cachex/backends/memcached.py index 425cc8d..64b205e 100644 --- a/fastapi_cachex/backends/memcached.py +++ b/fastapi_cachex/backends/memcached.py @@ -12,6 +12,7 @@ from fastapi_cachex.types import CacheEntry from .base import BaseCacheBackend +from .base import validate_delta from .base import validate_ttl logger = logging.getLogger(__name__) @@ -29,6 +30,10 @@ # not as a duration, so a longer TTL has to be converted before it is sent. _MAX_RELATIVE_TTL = 30 * 24 * 60 * 60 +# Memcached parses exptime as a signed 32-bit integer. An absolute timestamp +# past this one (2038-01-19) wraps around and the item is dropped at once. +_MAX_ABSOLUTE_EXPTIME = 2**31 - 1 + # Seconds a failed server stays out of rotation before HashClient tries it # again. Until then every call to it raises. _DEAD_TIMEOUT = 1 @@ -43,11 +48,22 @@ def _expiry(ttl: int | None) -> int: Anything past the 30-day boundary is sent as an absolute timestamp; passing it through as a duration would have Memcached read it as a moment in 1970 and expire the entry immediately. ``None`` means no expiry. + + Raises: + ValueError: If the expiry would fall after 2038-01-19, which Memcached + cannot represent: it would accept the write and drop the item. """ if ttl is None: return 0 if ttl > _MAX_RELATIVE_TTL: - return int(time.time()) + ttl + expires_at = int(time.time()) + ttl + if expires_at > _MAX_ABSOLUTE_EXPTIME: + msg = ( + f"ttl {ttl!r} expires after 2038-01-19, which Memcached cannot " + "store; use a shorter ttl or None" + ) + raise ValueError(msg) + return expires_at return ttl @@ -281,20 +297,25 @@ async def increment(self, key: str, delta: int = 1, ttl: int | None = None) -> i Memcached counters are unsigned, so a negative ``delta`` uses DECR, which stops at 0 instead of going negative. """ + validate_delta(delta) validate_ttl(ttl) from pymemcache.exceptions import MemcacheClientError prefixed_key = self._make_key(key) + # Converted up front so a ttl Memcached cannot store fails before I/O. + exptime = _expiry(ttl) try: value = await asyncio.to_thread(self._add_delta, prefixed_key, delta) if value is None: # No counter yet: ADD is atomic and a no-op when a concurrent # call created it first, so the retry always finds a counter. await asyncio.to_thread( - self.client.add, prefixed_key, b"0", _expiry(ttl), noreply=False + self.client.add, prefixed_key, b"0", exptime, noreply=False ) value = await asyncio.to_thread(self._add_delta, prefixed_key, delta) except MemcacheClientError as e: + if "non-numeric" not in str(e): + raise msg = "Cache key holds a value that is not a counter" raise CacheXError(msg) from e if value is None: diff --git a/fastapi_cachex/backends/memory.py b/fastapi_cachex/backends/memory.py index fad2b13..e46c4ec 100644 --- a/fastapi_cachex/backends/memory.py +++ b/fastapi_cachex/backends/memory.py @@ -14,6 +14,7 @@ from fastapi_cachex.types import counter_value from .base import BaseCacheBackend +from .base import validate_delta from .base import validate_ttl from .base import warn_if_path_shaped @@ -262,6 +263,7 @@ async def increment(self, key: str, delta: int = 1, ttl: int | None = None) -> i The read-modify-write happens under the backend lock, so concurrent callers on the same event loop never lose an increment. """ + validate_delta(delta) validate_ttl(ttl) self._ensure_cleanup_started() diff --git a/fastapi_cachex/backends/redis.py b/fastapi_cachex/backends/redis.py index 9138bd6..e412733 100644 --- a/fastapi_cachex/backends/redis.py +++ b/fastapi_cachex/backends/redis.py @@ -18,6 +18,7 @@ from fastapi_cachex.types import CacheEntry from .base import BaseCacheBackend +from .base import validate_delta from .base import validate_ttl from .base import warn_if_path_shaped @@ -356,6 +357,7 @@ async def increment(self, key: str, delta: int = 1, ttl: int | None = None) -> i A short Lua script makes the increment and the expiry one server-side operation; the key is stored as a plain Redis integer. """ + validate_delta(delta) validate_ttl(ttl) from redis.exceptions import ResponseError diff --git a/fastapi_cachex/cache.py b/fastapi_cachex/cache.py index 5ec7e00..23159a5 100644 --- a/fastapi_cachex/cache.py +++ b/fastapi_cachex/cache.py @@ -30,6 +30,7 @@ from starlette.status import HTTP_300_MULTIPLE_CHOICES from starlette.status import HTTP_304_NOT_MODIFIED +from .backends.base import MAX_TTL from .directives import DirectiveType from .exceptions import BackendNotFoundError from .exceptions import CacheXError @@ -481,7 +482,8 @@ def cache( Raises: CacheXError: When the decorator is applied, if ``stale`` and ``stale_ttl`` are not given together, if ``public`` and - ``private`` are both set, or if ``ttl`` is negative. + ``private`` are both set, or if ``ttl`` is not an ``int``, is + negative or is larger than ``MAX_TTL``. """ def decorator(func: HandlerCallable) -> AsyncResponseCallable: @@ -495,9 +497,17 @@ def decorator(func: HandlerCallable) -> AsyncResponseCallable: if public and private: msg = "public and private are mutually exclusive" raise CacheXError(msg) + if ttl is not None and (isinstance(ttl, bool) or not isinstance(ttl, int)): + # Checked here: at request time the backend would reject it, and + # failing open would hide that the route never caches. + msg = f"ttl must be an int number of seconds, got {type(ttl).__name__}" + raise CacheXError(msg) if ttl is not None and ttl < 0: msg = "ttl must not be negative" raise CacheXError(msg) + if ttl is not None and ttl > MAX_TTL: + msg = f"ttl must be at most {MAX_TTL} seconds" + raise CacheXError(msg) # Analyze the original function's signature sig: Signature = inspect.signature(func) diff --git a/i18n/zh-TW/docs/BACKENDS.md b/i18n/zh-TW/docs/BACKENDS.md index 02c136e..31a1c2f 100644 --- a/i18n/zh-TW/docs/BACKENDS.md +++ b/i18n/zh-TW/docs/BACKENDS.md @@ -107,6 +107,7 @@ BackendProxy.set(backend) - `clear_path()` 只會刪除完全相符的那個鍵；`include_params` 沒有作用 - `clear()` 會發出 `flush_all`，清空整台 Memcached 伺服器，而不只是這個命名空間 - Memcached 會拒絕的鍵（超過 250 位元組、含空白字元或非 ASCII 字元）會改以其 SHA-256 摘要儲存 +- 過期時間落在 2038-01-19 之後的 `ttl` 會拋出 `ValueError`（見 [TTL 值](#ttl-values)） - 超過伺服器項目大小上限（預設 1 MB，可用 `memcached -I` 調整）的值會被拒絕並拋出錯誤。`@cache` 會記錄該錯誤，並照常送出不儲存的回應（見[後端發生錯誤時](HTTP_CACHING.md#when-the-backend-fails)）；其他呼叫端則會收到該錯誤 - 若需要依模式清除快取，請考慮使用 Redis 後端 @@ -143,7 +144,7 @@ if await backend.set_if_absent(f"stream:{user_id}", owner, ttl=300): await backend.delete_if_equals(f"stream:{user_id}", owner) ``` -- `increment(key, delta=1, ttl=None) -> int`：記憶體後端在鎖內執行讀取—修改—寫入，Redis 執行 Lua 腳本（`EXISTS` + `INCRBY` + `EXPIRE`），Memcached 則使用 `ADD` + `INCR`/`DECR`（Memcached 的計數器最低停在 0）。計數器可透過 `get()` 讀到，形式為 fingerprint 為 `COUNTER_FINGERPRINT`、內容為十進位數值的 `CacheEntry`，因此 `delete`／`clear*` 與監控路由都會把它當成一般項目處理。對存放快取回應的鍵執行 increment 會拋出 `CacheXError`。 +- `increment(key, delta=1, ttl=None) -> int`：記憶體後端在鎖內執行讀取—修改—寫入，Redis 執行 Lua 腳本（`EXISTS` + `INCRBY` + `EXPIRE`），Memcached 則使用 `ADD` + `INCR`/`DECR`（Memcached 的計數器最低停在 0）。計數器可透過 `get()` 讀到，形式為 fingerprint 為 `COUNTER_FINGERPRINT`、內容為十進位數值的 `CacheEntry`，因此 `delete`／`clear*` 與監控路由都會把它當成一般項目處理。對存放快取回應的鍵執行 increment 會拋出 `CacheXError`。`delta` 必須是 signed 64 位元範圍內的 `int`，否則會在存取後端之前拋出 `TypeError` 或 `ValueError`。 - `get_and_delete(key) -> CacheEntry | None`：記憶體後端在鎖內 pop，Redis 使用 `GETDEL`（伺服器 6.2 以上），Memcached 使用 `GETS` + `exptime=-1` 的 `CAS` 寫入（若中間有其他寫入者替換了值則會重試；連續 16 次都被替換時會拋出 `CacheXError`，而不是當成鍵不存在）。`StateManager.consume_state`、`StateManager.delete_state`、`CacheManager.delete` 與 `invalidate()` 都建立在它之上。 - `set_if_absent(key, value, ttl=None) -> bool`：只在 `key` 不存在時儲存 `value`（已過期的鍵視為不存在），並回報是否有寫入。記憶體後端在鎖內檢查，Redis 使用 `SET NX EX`，Memcached 使用 `ADD`。 - `delete_if_equals(key, expected) -> bool`：只在 `key` 仍存放 `expected` 時才移除它，因此項目已過期的持有者無法釋放已被他人取得的鎖。請在你儲存的項目中放入唯一的權杖，並以同一個項目釋放。記憶體後端在鎖內比較，Redis 透過 Lua 腳本刪除，並在腳本中重新檢查先前比較過的值，Memcached 則使用 `GETS` + 一個讓項目立即過期的 `CAS` 寫入（傳統協定的 `DELETE` 不接受 CAS 權杖）。 @@ -152,6 +153,12 @@ if await backend.set_if_absent(f"stream:{user_id}", owner, ttl=300): ## TTL 值 {#ttl-values} -每個 `ttl` 參數（`set`、`set_if_absent`、`increment`，以及建立在它們之上的 `CacheManager` 與 `StateManager` 方法和預設值）只能是 `None`（表示項目永不過期），或正數秒數。零與負值會拋出 `ValueError`。底層儲存對這些值的解讀各不相同：Memcached 把 exptime `0` 視為「永不過期」，Redis 拒絕 `EX 0`，而行程內的 dict 則會立即讓項目過期。第三方後端應在其 `set` 中呼叫 `fastapi_cachex.backends.base.validate_ttl(ttl)`，以遵循相同規則。（`@cache(ttl=0)` 是另一回事：它會送出 `max-age=0`，且絕不會把 `0` 傳給後端；見 [HTTP 快取](HTTP_CACHING.md)。） +每個 `ttl` 參數（`set`、`set_if_absent`、`increment`，以及建立在它們之上的 `CacheManager` 與 `StateManager` 方法和預設值）只能是 `None`（表示項目永不過期），或介於 1 到 `MAX_TTL`（2**31 - 1，約 68 年）之間的 `int` 秒數。這些檢查都在存取後端之前進行： + +- 零、負值與更大的值會拋出 `ValueError`。底層儲存對 `0` 的解讀各不相同：Memcached 把 exptime `0` 視為「永不過期」，Redis 拒絕 `EX 0`，而行程內的 dict 則會立即讓項目過期。 +- `float`、`bool` 或其他型別會拋出 `TypeError`。float 過去只在記憶體後端上有效，而 `True` 會被當成一秒。`timedelta` 請以 `int(td.total_seconds())` 轉換。 +- Memcached 無法儲存 2038-01-19 之後的過期時間（它的 exptime 是 signed 32 位元時間戳），因此 Memcached 後端遇到超過這個時間點的 `ttl` 會拋出 `ValueError`，而不是接受一筆會立即被丟棄的寫入。 + +第三方後端應在其 `set` 中呼叫 `fastapi_cachex.backends.base.validate_ttl(ttl)`，並在 `increment` 中呼叫 `validate_delta(delta)`，以遵循相同規則。（`@cache(ttl=0)` 是另一回事：它會送出 `max-age=0`，且絕不會把 `0` 傳給後端；見 [HTTP 快取](HTTP_CACHING.md)。） 各後端如何儲存項目，請見[快取流程](CACHE_FLOW.md#backend-storage-formats)；類別本身請見 [API 參考](https://fastapi-cachex.readthedocs.io/en/latest/api/backends/)（英文）。 diff --git a/i18n/zh-TW/docs/HTTP_CACHING.md b/i18n/zh-TW/docs/HTTP_CACHING.md index aff01c8..ed08d97 100644 --- a/i18n/zh-TW/docs/HTTP_CACHING.md +++ b/i18n/zh-TW/docs/HTTP_CACHING.md @@ -66,7 +66,7 @@ async def non_store_endpoint(): - **使用 `no-cache` 指令**：先以新產生的內容強制重新驗證，再決定是否回 304 - **使用 `private=True`**：不從共用後端讀取，也不寫入；每次都執行 handler，只有 `If-None-Match` 重新驗證有效 - **未設定 `ttl`**（`ttl=None`）：快取的回應本文永遠不會直接回傳；每個請求都會執行 handler，唯一的例外是 `If-None-Match` 與已儲存 ETag 相符的請求，會得到 304 -- **使用 `ttl=0`**：送出 `max-age=0`，其餘行為與 `ttl=None` 相同。負數的 `ttl` 會在套用裝飾器時以 `CacheXError` 拒絕 +- **使用 `ttl=0`**：送出 `max-age=0`，其餘行為與 `ttl=None` 相同。負數、非 `int`（例如 `1.5` 或 `True`）或超過 `MAX_TTL`（見 [TTL 值](BACKENDS.md#ttl-values)）的 `ttl`，都會在套用裝飾器時以 `CacheXError` 拒絕 只有成功的回應會被儲存。handler *回傳* 非 2xx 狀態的回應（例如 `Response(..., status_code=404)`）會原樣傳出、永不快取，因此暫時性的錯誤不會取代或污染上一筆正常的項目。`206 Partial Content` 同樣排除在外，因為它的本文只對產生它的那個 `Range` 請求有意義。`Set-Cookie` 永遠不會被儲存或重播。 diff --git a/tests/backends/test_memcached.py b/tests/backends/test_memcached.py index e88debf..c2c6f3c 100644 --- a/tests/backends/test_memcached.py +++ b/tests/backends/test_memcached.py @@ -10,6 +10,7 @@ from fastapi_cachex.backends.codec import encode_entry from fastapi_cachex.backends.memcached import _CAS_MAX_RETRIES from fastapi_cachex.backends.memcached import _DEAD_TIMEOUT +from fastapi_cachex.backends.memcached import _expiry from fastapi_cachex.exceptions import CacheXError from fastapi_cachex.lock import CacheLock from fastapi_cachex.types import CacheEntry @@ -454,6 +455,26 @@ async def test_memcached_increment_honors_ttl( assert await memcached_backend.increment("window", ttl=1) == 1 +@pytest.mark.asyncio +async def test_memcached_increment_only_maps_non_numeric_errors_to_not_a_counter() -> ( + None +): + """Other client errors are about the request, not the stored value (#229).""" + backend = stubbed_backend() + # Looked up rather than imported: the tests do not import optional packages. + client_error = sys.modules["pymemcache.exceptions"].MemcacheClientError + backend.client.incr.side_effect = client_error(b"invalid numeric delta argument") + + with pytest.raises(client_error, match="invalid numeric delta argument"): + await backend.increment("n") + + backend.client.incr.side_effect = client_error( + b"cannot increment or decrement non-numeric value" + ) + with pytest.raises(CacheXError, match="not a counter"): + await backend.increment("n") + + @requires_memcached @pytest.mark.asyncio async def test_memcached_increment_rejects_a_cached_response( @@ -895,3 +916,26 @@ async def test_lock_lifecycle_with_memcached( assert await lock2.acquire(blocking=False) is False assert await lock1.extend(60) is True assert await lock1.release() is True + + +def test_expiry_up_to_2038_is_sent_as_a_timestamp(monkeypatch) -> None: + """Memcached reads exptime as a signed 32-bit int: 2**31 - 1 is the last second.""" + monkeypatch.setattr("fastapi_cachex.backends.memcached.time.time", lambda: 2e9) + + assert _expiry(2**31 - 1 - 2_000_000_000) == 2**31 - 1 + with pytest.raises(ValueError, match="expires after 2038-01-19"): + _expiry(2**31 - 2_000_000_000) + + +@pytest.mark.parametrize("operation", ["set", "set_if_absent", "increment"]) +@pytest.mark.asyncio +async def test_ttl_past_2038_is_rejected_before_io(operation: str) -> None: + """Such a write used to succeed while Memcached dropped the item at once (#229).""" + backend = stubbed_backend() + entry = CacheEntry(fingerprint="e", content=b"v") + args = ("k",) if operation == "increment" else ("k", entry) + + with pytest.raises(ValueError, match="expires after 2038-01-19"): + await getattr(backend, operation)(*args, ttl=2**31 - 1) + assert isinstance(backend.client, MagicMock) + assert backend.client.method_calls == [] diff --git a/tests/backends/test_redis.py b/tests/backends/test_redis.py index 4175732..3574ec3 100644 --- a/tests/backends/test_redis.py +++ b/tests/backends/test_redis.py @@ -677,6 +677,24 @@ async def test_redis_get_cache_data_with_entries( await async_redis_backend.clear() +@requires_redis +@pytest.mark.asyncio +async def test_redis_increment_with_a_float_ttl_leaves_no_counter( + async_redis_backend: AsyncRedisCacheBackend, +) -> None: + """A float ttl used to create the counter before EXPIRE failed (#229). + + Redis does not roll back a failed script, so the counter stayed at 1 with + no expiry: a rate limiter locked out for good, behind an error saying the + key was "not a counter". + """ + with pytest.raises(TypeError, match="got float"): + await async_redis_backend.increment("window", ttl=1.5) # type: ignore[arg-type] + + prefixed = async_redis_backend._make_key("window") + assert await async_redis_backend.client.exists(prefixed) == 0 + + @requires_redis @pytest.mark.asyncio async def test_redis_increment_creates_then_adds( diff --git a/tests/backends/test_ttl_contract.py b/tests/backends/test_ttl_contract.py index 35964c9..f7e5bd0 100644 --- a/tests/backends/test_ttl_contract.py +++ b/tests/backends/test_ttl_contract.py @@ -4,16 +4,24 @@ the memory backend expired the entry at once. Now `None` is the only way to say "no expiry" and zero or negative values raise `ValueError` before any backend I/O. + +Non-integer TTLs were next (#229): `True` passed as one second, a float worked +on the memory backend only, and on Redis `increment(ttl=1.5)` created the +counter before `EXPIRE` failed, leaving it with no expiry at all. Only an `int` +up to `MAX_TTL` is accepted now, and `TypeError`/`ValueError` still come +before any I/O. """ from collections.abc import Awaitable from collections.abc import Callable +from typing import Any from unittest.mock import MagicMock import pytest from fastapi_cachex.backends import AsyncRedisCacheBackend from fastapi_cachex.backends import MemcachedBackend +from fastapi_cachex.backends.base import MAX_TTL from fastapi_cachex.backends.base import BaseCacheBackend from fastapi_cachex.backends.base import validate_ttl from fastapi_cachex.backends.memory import MemoryBackend @@ -24,7 +32,17 @@ from tests.live_servers import UNCONNECTED_PORT ENTRY = CacheEntry(fingerprint="e", content=b"v") -BAD_TTLS = [0, -1] +# (ttl, exception, message) for every value validate_ttl must refuse. +BAD_TTLS = [ + pytest.param(0, ValueError, "ttl must be a positive", id="zero"), + pytest.param(-1, ValueError, "ttl must be a positive", id="negative"), + pytest.param(1.5, TypeError, "got float", id="float"), + pytest.param(60.0, TypeError, "got float", id="integral-float"), + pytest.param(True, TypeError, "got bool", id="bool"), + pytest.param("60", TypeError, "got str", id="str"), + pytest.param(MAX_TTL + 1, ValueError, "at most", id="too-large"), + pytest.param(10**400, ValueError, "at most", id="huge"), +] def make_backends() -> list[BaseCacheBackend]: @@ -48,15 +66,17 @@ def make_backends() -> list[BaseCacheBackend]: } -@pytest.mark.parametrize("ttl", BAD_TTLS) +@pytest.mark.parametrize(("ttl", "error", "match"), BAD_TTLS) @pytest.mark.parametrize("operation", ["set", "set_if_absent", "increment"]) @pytest.mark.asyncio -async def test_backends_reject_non_positive_ttl(operation: str, ttl: int) -> None: +async def test_backends_reject_invalid_ttl( + operation: str, ttl: object, error: type[Exception], match: str +) -> None: for backend in make_backends(): if isinstance(backend, DictBackend) and operation == "set": continue # a third-party set() is its own; the fallbacks are ours - with pytest.raises(ValueError, match="ttl must be a positive"): - await OPERATIONS[operation](backend, ttl) + with pytest.raises(error, match=match): + await OPERATIONS[operation](backend, ttl) # type: ignore[arg-type] if isinstance(backend, MemcachedBackend): assert isinstance(backend.client, MagicMock) assert backend.client.method_calls == [] @@ -65,39 +85,68 @@ async def test_backends_reject_non_positive_ttl(operation: str, ttl: int) -> Non backend.stop_cleanup() -@pytest.mark.parametrize("ttl", [None, 1, 3600]) +@pytest.mark.parametrize("ttl", [None, 1, 3600, MAX_TTL]) def test_validate_ttl_passes_none_and_positive(ttl: int | None) -> None: assert validate_ttl(ttl) == ttl -@pytest.mark.parametrize("ttl", BAD_TTLS) +@pytest.mark.parametrize(("ttl", "error", "match"), BAD_TTLS) @pytest.mark.asyncio -async def test_cache_manager_rejects_non_positive_ttl(ttl: int) -> None: +async def test_cache_manager_rejects_invalid_ttl( + ttl: Any, error: type[Exception], match: str +) -> None: backend = DictBackend() - with pytest.raises(ValueError, match="ttl must be a positive"): + with pytest.raises(error, match=match): CacheManager(backend, default_ttl=ttl) manager = CacheManager(backend) factory = MagicMock(return_value=1) - with pytest.raises(ValueError, match="ttl must be a positive"): + with pytest.raises(error, match=match): await manager.set("k", 1, ttl=ttl) - with pytest.raises(ValueError, match="ttl must be a positive"): + with pytest.raises(error, match=match): await manager.add("k", 1, ttl=ttl) - with pytest.raises(ValueError, match="ttl must be a positive"): + with pytest.raises(error, match=match): await manager.get_or_set("k", factory, ttl=ttl) # The ttl is checked before the factory runs. factory.assert_not_called() assert backend.store == {} -@pytest.mark.parametrize("ttl", BAD_TTLS) +@pytest.mark.parametrize(("ttl", "error", "match"), BAD_TTLS) @pytest.mark.asyncio -async def test_state_manager_rejects_non_positive_ttl(ttl: int) -> None: +async def test_state_manager_rejects_invalid_ttl( + ttl: Any, error: type[Exception], match: str +) -> None: backend = DictBackend() - with pytest.raises(ValueError, match="ttl must be a positive"): + with pytest.raises(error, match=match): StateManager(backend, default_ttl=ttl) manager = StateManager(backend) - with pytest.raises(ValueError, match="ttl must be a positive"): + with pytest.raises(error, match=match): await manager.create_state(ttl=ttl) assert backend.store == {} + + +@pytest.mark.parametrize( + ("delta", "error", "match"), + [ + pytest.param(1.5, TypeError, "delta must be an int", id="float"), + pytest.param(True, TypeError, "delta must be an int", id="bool"), + pytest.param(2**63, ValueError, "signed 64-bit", id="too-large"), + pytest.param(-(2**63) - 1, ValueError, "signed 64-bit", id="too-small"), + ], +) +@pytest.mark.asyncio +async def test_backends_reject_invalid_delta( + delta: Any, error: type[Exception], match: str +) -> None: + """Such a delta reached the server and was reported as "not a counter".""" + for backend in make_backends(): + with pytest.raises(error, match=match): + await backend.increment("n", delta=delta) + if isinstance(backend, MemcachedBackend): + assert isinstance(backend.client, MagicMock) + assert backend.client.method_calls == [] + if isinstance(backend, MemoryBackend): + assert backend.cache == {} + backend.stop_cleanup() diff --git a/tests/test_cache.py b/tests/test_cache.py index 707d05b..f6db67c 100644 --- a/tests/test_cache.py +++ b/tests/test_cache.py @@ -713,6 +713,20 @@ def test_negative_ttl_is_rejected_at_decoration(): cache(ttl=-1)(lambda: None) +@pytest.mark.parametrize( + ("ttl", "match"), + [ + pytest.param(1.5, "got float", id="float"), + pytest.param(True, "got bool", id="bool"), + pytest.param(2**31, "at most", id="too-large"), + ], +) +def test_invalid_ttl_is_rejected_at_decoration(ttl, match): + """The backend would reject it per request, and fail-open would hide that (#229).""" + with pytest.raises(CacheXError, match=match): + cache(ttl=ttl)(lambda: None) + + def test_stale_client_etag_with_changed_cache(): """If the client sends an ETag that doesn't match the cached one, return 200 with new ETag.""" import time