From 147cb49083935ed91640e0dfb0b5929198c48a3d Mon Sep 17 00:00:00 2001 From: allen0099 Date: Fri, 25 Sep 2026 12:38:13 +0000 Subject: [PATCH] fix(redis): match the key prefix and clear_path path literally in SCAN SCAN MATCH patterns interpolated key_prefix and the clear_path path raw, so glob characters in them were live: clear_path("/files/[draft]") missed the entry for that path, and a prefix with ? or * reached other prefixes' keys. Escape *?[]\ in both; only the clear_pattern argument stays a glob. Closes #106 --- CHANGELOG.md | 7 +++++ docs/BACKENDS.md | 3 ++ fastapi_cachex/backends/redis.py | 35 ++++++++++++++++----- tests/backends/test_redis.py | 54 ++++++++++++++++++++++++++++++++ 4 files changed, 92 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e4bcd31..25b0353 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,13 @@ Note that 0.3.3 was never released; 0.3.4 follows 0.3.2. ### Fixed +- The Redis backend now matches its key prefix and the path given to + `clear_path()` literally when it builds `SCAN` patterns. Glob characters in + them used to be live: `clear_path("/files/[draft]")` missed the cached entry + for that path, and a `key_prefix` containing `?` or `*` let `clear()`, + `get_all_keys()` and `clear_pattern()` reach keys under other prefixes. Only + the pattern passed to `clear_pattern()` is still a glob. + - `StateManager` no longer writes the raw OAuth state to its logs. The state comes from the callback query string, so logging it leaked live tokens and let a caller forge log lines with CR/LF. Log lines now carry `state_ref`, the diff --git a/docs/BACKENDS.md b/docs/BACKENDS.md index 7178b16..047519f 100644 --- a/docs/BACKENDS.md +++ b/docs/BACKENDS.md @@ -50,6 +50,9 @@ BackendProxy.set(backend) - Uses SCAN instead of KEYS for safe production use (non-blocking) - Namespaced with `fastapi_cachex:` prefix by default; pass `key_prefix="myapp:cache:"` for multi-tenant scenarios +- Only the pattern you pass to `clear_pattern()` is a glob. The key prefix and the path + given to `clear_path()` are matched literally, so `*`, `?`, `[` or `]` in them cannot + reach keys outside the prefix or miss the path **Configuring from a model**: `RedisConfig` is a pydantic model with the same settings and validation, which is handy when they come from environment diff --git a/fastapi_cachex/backends/redis.py b/fastapi_cachex/backends/redis.py index e8d05c0..7596e95 100644 --- a/fastapi_cachex/backends/redis.py +++ b/fastapi_cachex/backends/redis.py @@ -31,6 +31,15 @@ # SCAN page size and DEL batch size; keeps individual commands small. _BATCH_SIZE = 100 +# Characters that are live in a Redis glob pattern. +_GLOB_SPECIAL = frozenset("*?[]\\") + + +def _escape_glob(text: str) -> str: + """Backslash-escape ``text`` so a Redis glob pattern matches it literally.""" + return "".join(f"\\{ch}" if ch in _GLOB_SPECIAL else ch for ch in text) + + # INCRBY that attaches a TTL only when it creates the key, so a counter lives in # a fixed window. KEYS[1] = key, ARGV[1] = delta, ARGV[2] = ttl (0 = none). _INCREMENT_SCRIPT = """ @@ -154,6 +163,11 @@ def _make_key(self, key: str) -> str: """Add prefix to cache key.""" return f"{self.key_prefix}{key}" + @property + def _prefix_pattern(self) -> str: + """The key prefix as a literal glob, so ``*``/``?``/``[`` in it stay inert.""" + return _escape_glob(self.key_prefix) + async def _scan_keys(self, pattern: str) -> list[str]: """Collect every key matching ``pattern`` (a full, prefixed glob). @@ -271,7 +285,9 @@ async def clear(self) -> None: Only deletes keys within this backend's prefix. """ - removed = await self._delete_keys(await self._scan_keys(f"{self.key_prefix}*")) + removed = await self._delete_keys( + await self._scan_keys(f"{self._prefix_pattern}*") + ) logger.debug("Redis CLEAR; removed=%s", removed) async def clear_path(self, path: str, include_params: bool = False) -> int: @@ -286,9 +302,13 @@ async def clear_path(self, path: str, include_params: bool = False) -> int: """ # Keys are method|||host|||path|||query. Without include_params only the # exact path is matched: default_key_builder always appends a separator - # after the path, so keys with no query params end with "|||". + # after the path, so keys with no query params end with "|||". The + # path is a literal, not a glob: "/files/[draft]" means those brackets. suffix = "*" if include_params else "" - pattern = f"{self.key_prefix}*{CACHE_KEY_SEPARATOR}{path}{CACHE_KEY_SEPARATOR}{suffix}" + pattern = ( + f"{self._prefix_pattern}*{CACHE_KEY_SEPARATOR}" + f"{_escape_glob(path)}{CACHE_KEY_SEPARATOR}{suffix}" + ) keys = await self._scan_keys(pattern) # Also match direct keys (custom key formats without separators) @@ -309,15 +329,16 @@ async def clear_path(self, path: str, include_params: bool = False) -> int: async def clear_pattern(self, pattern: str) -> int: """Clear cached responses matching a pattern. + Only ``pattern`` is a live glob; the backend's key prefix is matched + literally, whether or not ``pattern`` repeats it. + Args: pattern: A glob pattern to match cache keys against Returns: Number of cache entries cleared """ - full_pattern = ( - pattern if pattern.startswith(self.key_prefix) else self._make_key(pattern) - ) + full_pattern = self._prefix_pattern + pattern.removeprefix(self.key_prefix) cleared_count = await self._delete_keys(await self._scan_keys(full_pattern)) warn_if_path_shaped(pattern, cleared_count) logger.debug( @@ -331,7 +352,7 @@ async def get_all_keys(self) -> list[str]: Returns: List of logical cache keys (without the backend key prefix) """ - keys = await self._scan_keys(f"{self.key_prefix}*") + keys = await self._scan_keys(f"{self._prefix_pattern}*") logical_keys = [k.removeprefix(self.key_prefix) for k in keys] logger.debug("Redis GET_ALL_KEYS; count=%s", len(logical_keys)) return logical_keys diff --git a/tests/backends/test_redis.py b/tests/backends/test_redis.py index 3a1b6af..8e457b5 100644 --- a/tests/backends/test_redis.py +++ b/tests/backends/test_redis.py @@ -921,3 +921,57 @@ async def ttl_zero() -> dict[str, str]: assert revalidated.status_code == 304 finally: BackendProxy.set(previous) + + +@requires_redis +@pytest.mark.asyncio +async def test_redis_clear_path_matches_glob_characters_literally( + async_redis_backend: AsyncRedisCacheBackend, +) -> None: + """A path with glob metacharacters clears its HTTP entries, and only those.""" + entry = CacheEntry(fingerprint="etag", content=b"x") + path = "/files/[draft]*?\\" + await async_redis_backend.set(f"GET|||host|||{path}|||", entry) + await async_redis_backend.set(f"GET|||host|||{path}|||v=1", entry) + # Keys the unescaped pattern would have caught. + await async_redis_backend.set("GET|||host|||/files/d|||", entry) + await async_redis_backend.set("GET|||host|||/files/[draft]xy\\|||", entry) + + assert await async_redis_backend.clear_path(path) == 1 + assert await async_redis_backend.clear_path(path, include_params=True) == 1 + assert sorted(await async_redis_backend.get_all_keys()) == [ + "GET|||host|||/files/[draft]xy\\|||", + "GET|||host|||/files/d|||", + ] + + +@pytest.mark.asyncio +async def test_redis_glob_characters_in_prefix_do_not_reach_other_prefixes() -> None: + """clear/get_all_keys/clear_pattern stay inside a prefix containing ``?``/``*``.""" + reason = redis_skip_reason() + if reason is not None: + pytest.skip(reason) + + def make(prefix: str) -> AsyncRedisCacheBackend: + return AsyncRedisCacheBackend( + host=REDIS_HOST, port=REDIS_PORT, key_prefix=prefix + ) + + globbed, other = make("cachex-test?*:"), make("cachex-testX-other:") + entry = CacheEntry(fingerprint="etag", content=b"x") + try: + await other.set("keep", entry) + await globbed.set("mine", entry) + + assert await globbed.get_all_keys() == ["mine"] + assert await globbed.clear_pattern("*") == 1 + await globbed.set("mine", entry) + assert await globbed.clear_pattern("cachex-test?*:*") == 1 + await globbed.set("mine", entry) + await globbed.clear() + + assert await globbed.get_all_keys() == [] + assert await other.get("keep") is not None + finally: + await globbed.clear() + await other.clear()