From b45650ee66833390f8e37a6747e0a8da0f8a802b Mon Sep 17 00:00:00 2001 From: allen0099 Date: Sat, 26 Sep 2026 22:25:11 +0000 Subject: [PATCH] fix(cache): percent-encode | and % in cache-key host and path The default key builder joined the raw Host header and decoded path with |||, so a Host of 'example.com|||/p' on GET /x stored the response under the key of GET /p%7C%7C%7C/x. Encode | and % in both components with escape_key_component; clear_path encodes its argument the same way and the monitoring routes decode for display. Recommend TrustedHostMiddleware in the HTTP caching guide. Closes #230 --- CHANGELOG.md | 10 +++ CLAUDE.md | 2 +- docs/CACHE_FLOW.md | 14 ++- docs/HTTP_CACHING.md | 28 +++++- fastapi_cachex/backends/memory.py | 4 +- fastapi_cachex/backends/redis.py | 4 +- fastapi_cachex/cache.py | 11 ++- fastapi_cachex/routes.py | 5 +- fastapi_cachex/types.py | 20 +++++ i18n/zh-TW/docs/CACHE_FLOW.md | 10 ++- i18n/zh-TW/docs/HTTP_CACHING.md | 18 +++- tests/backends/test_clear_pattern_contract.py | 17 ++++ tests/test_cache_key.py | 85 +++++++++++++++++++ 13 files changed, 216 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 443f120..50ca7c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -232,6 +232,16 @@ Note that 0.3.3 was never released; 0.3.4 follows 0.3.2. 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)) +- **A `|||` in the `Host` header or path can no longer poison another + path's cache entry.** The default key builder joined the raw host and + decoded path with `|||`, so `GET /x` with `Host: example.com|||/p` stored + its response under the key of `GET /p%7C%7C%7C/x`. `|` and `%` in the host + and path are now percent-encoded (`escape_key_component` in + `fastapi_cachex.types`); `clear_path()` encodes its argument the same way + and the monitoring routes decode for display. Keys whose host or path + contains `|` or `%` change, so those entries are cached afresh once. The + HTTP caching guide now recommends `TrustedHostMiddleware`. + ([#230](https://github.com/allen0099/FastAPI-CacheX/issues/230)) ## [0.3.7] - 2026-09-25 diff --git a/CLAUDE.md b/CLAUDE.md index 000f070..a9ed035 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -53,7 +53,7 @@ The library has four independent subsystems: - Cache flow: check `no-store` → check `no-cache` → check ETag (`If-None-Match`) → check TTL-based cache hit → execute handler → store result. - Fails open by default (`fail_open=True`): a backend error on `get` is logged and treated as a miss, one on `set` is logged and the response served unstored. `fail_open=False` propagates the error. - Only GET requests are cached; other methods bypass the cache entirely. -- Cache keys follow the format `method|||host|||path|||query_params` (separator defined in `types.py`). +- Cache keys follow the format `method|||host|||path|||query_params` (separator defined in `types.py`). Host and path go through `escape_key_component` (`|` → `%7C`, `%` → `%25`) so client input cannot inject the separator; `clear_path` encodes its argument and `routes.py` decodes for display. - `BackendProxy` is a non-instantiable class-level singleton (via `ProxyMeta`). Call `BackendProxy.set(backend)` at app startup; `BackendProxy.get()` raises `BackendNotFoundError` if unset. Falls back to `MemoryBackend` automatically inside `@cache` if no backend is set. - Cache values are stored as `CacheEntry(fingerprint, content, media_type)` dataclass (defined in `types.py`). diff --git a/docs/CACHE_FLOW.md b/docs/CACHE_FLOW.md index 42d68ee..f8f99b9 100644 --- a/docs/CACHE_FLOW.md +++ b/docs/CACHE_FLOW.md @@ -49,10 +49,16 @@ When a request arrives, the `@cache` decorator does the following: ```python from fastapi_cachex.types import CACHE_KEY_SEPARATOR # "|||" +from fastapi_cachex.types import escape_key_component # Cache key format (default_key_builder in fastapi_cachex/cache.py) cache_key = CACHE_KEY_SEPARATOR.join( - [request.method, request.headers.get("host", "unknown"), request.url.path, query] + [ + request.method, + escape_key_component(request.headers.get("host", "unknown")), + escape_key_component(request.url.path), + query, + ] ) # For example: @@ -64,6 +70,12 @@ The separator is `|||` rather than a colon because the host itself may contain a port (`127.0.0.1:8000`); with a colon the key could not be split reliably, and `clear_path()` needs to recover the path from the key. +The host and path are percent-encoded first: `|` becomes `%7C` and `%` becomes +`%25` (`escape_key_component` in `fastapi_cachex/types.py`). Both come from the +client, and a raw `|||` in either would shift the components so that one +request's key could equal another's. The query string is URL-encoded already. +The monitoring routes decode them again for display. + Query parameters are joined in the order the request sent them (`str(request.query_params)`) and are **not sorted**, so `?page=1&limit=10` and `?limit=10&page=1` are two separate cache entries. If you want them treated as diff --git a/docs/HTTP_CACHING.md b/docs/HTTP_CACHING.md index 458be2b..c2e9988 100644 --- a/docs/HTTP_CACHING.md +++ b/docs/HTTP_CACHING.md @@ -137,6 +137,28 @@ This ensures that: Query parameters are taken in the order the client sent them, without sorting, so `?a=1&b=2` and `?b=2&a=1` are two distinct cache entries for the same logical request. +The host and path come from the client, so `|` and `%` in them are percent-encoded +(`%7C` and `%25`). A `Host` header or path containing `|||` therefore cannot shift +the components and make one request's key equal another's. The query string is +URL-encoded already. `clear_path()` takes the path as your application sees it +(`request.url.path`) and encodes it the same way; `clear_pattern()` matches the +stored key, so write `%7C` there for a `|`. Before 0.3.8 both were stored as sent, +so after upgrading, entries for a host or path containing `|` or `%` are cached +afresh once. + +The host is still whatever the client sends. Unless a reverse proxy or load +balancer in front of the app already rejects unknown hosts, add Starlette's +`TrustedHostMiddleware`, so a forged `Host` gets a `400` instead of filling the +cache with entries no one else will request: + +```python +from starlette.middleware.trustedhost import TrustedHostMiddleware + +app.add_middleware( + TrustedHostMiddleware, allowed_hosts=["example.com", "*.example.com"] +) +``` + All backends automatically namespace keys with a prefix (e.g., `fastapi_cachex:`) to avoid conflicts with other applications. `CacheManager` (see [Application cache](APP_CACHE.md)) uses a separate, simpler `cache:`-prefixed key @@ -166,6 +188,7 @@ from fastapi import Request, Response from fastapi_cachex import cache from fastapi_cachex.types import CACHE_KEY_SEPARATOR +from fastapi_cachex.types import escape_key_component # 1. Keep it out of the shared cache entirely. @@ -183,8 +206,9 @@ def per_user_key(request: Request) -> str: user_id = getattr(request.state, "user_id", "anonymous") return ( f"{request.method}{CACHE_KEY_SEPARATOR}" - f"{request.headers.get('host', 'unknown')}{CACHE_KEY_SEPARATOR}" - f"{request.url.path}{CACHE_KEY_SEPARATOR}" + f"{escape_key_component(request.headers.get('host', 'unknown'))}" + f"{CACHE_KEY_SEPARATOR}" + f"{escape_key_component(request.url.path)}{CACHE_KEY_SEPARATOR}" f"{request.query_params}{CACHE_KEY_SEPARATOR}{user_id}" ) diff --git a/fastapi_cachex/backends/memory.py b/fastapi_cachex/backends/memory.py index e46c4ec..3c2972a 100644 --- a/fastapi_cachex/backends/memory.py +++ b/fastapi_cachex/backends/memory.py @@ -12,6 +12,7 @@ from fastapi_cachex.types import CacheItem from fastapi_cachex.types import counter_entry from fastapi_cachex.types import counter_value +from fastapi_cachex.types import escape_key_component from .base import BaseCacheBackend from .base import validate_delta @@ -311,6 +312,7 @@ async def clear_path(self, path: str, include_params: bool = False) -> int: Returns: Number of cache entries cleared """ + key_path = escape_key_component(path) def matches(key: str) -> bool: parsed = _split_http_key(key) @@ -318,7 +320,7 @@ def matches(key: str) -> bool: # Direct key match (custom key format without separators) return key == path cache_path, has_params = parsed - return cache_path == path and (include_params or not has_params) + return cache_path == key_path and (include_params or not has_params) cleared_count = await self._evict(matches) logger.debug( diff --git a/fastapi_cachex/backends/redis.py b/fastapi_cachex/backends/redis.py index e412733..9b029a8 100644 --- a/fastapi_cachex/backends/redis.py +++ b/fastapi_cachex/backends/redis.py @@ -16,6 +16,7 @@ from fastapi_cachex.exceptions import CacheXError from fastapi_cachex.types import CACHE_KEY_SEPARATOR from fastapi_cachex.types import CacheEntry +from fastapi_cachex.types import escape_key_component from .base import BaseCacheBackend from .base import validate_delta @@ -395,10 +396,11 @@ async def clear_path(self, path: str, include_params: bool = False) -> int: # exact path is matched: default_key_builder always appends a separator # after the path, so keys with no query params end with "|||". The # path is a literal, not a glob: "/files/[draft]" means those brackets. + # It is stored with "|" and "%" percent-encoded, so match it that way. suffix = "*" if include_params else "" pattern = ( f"{self._prefix_pattern}*{CACHE_KEY_SEPARATOR}" - f"{_escape_glob(path)}{CACHE_KEY_SEPARATOR}{suffix}" + f"{_escape_glob(escape_key_component(path))}{CACHE_KEY_SEPARATOR}{suffix}" ) cleared_count = await self._delete_matching(pattern) diff --git a/fastapi_cachex/cache.py b/fastapi_cachex/cache.py index 23159a5..2a4a5ee 100644 --- a/fastapi_cachex/cache.py +++ b/fastapi_cachex/cache.py @@ -40,6 +40,7 @@ from .types import CACHE_KEY_SEPARATOR from .types import CacheEntry from .types import CacheKeyBuilder +from .types import escape_key_component if TYPE_CHECKING: from fastapi.routing import APIRoute @@ -60,6 +61,11 @@ def default_key_builder(request: Request) -> str: Generates cache key in format: method|||host|||path|||query_params + ``|`` and ``%`` in the host and path are percent-encoded (see + ``escape_key_component``), so a ``Host`` header or path containing + ``|||`` cannot make one request's key equal another's. The query string + is already URL-encoded and never contains ``|``. + Args: request: The FastAPI Request object @@ -68,8 +74,9 @@ def default_key_builder(request: Request) -> str: """ key = ( f"{request.method}{CACHE_KEY_SEPARATOR}" - f"{request.headers.get('host', 'unknown')}{CACHE_KEY_SEPARATOR}" - f"{request.url.path}{CACHE_KEY_SEPARATOR}" + f"{escape_key_component(request.headers.get('host', 'unknown'))}" + f"{CACHE_KEY_SEPARATOR}" + f"{escape_key_component(request.url.path)}{CACHE_KEY_SEPARATOR}" f"{request.query_params}" ) logger.debug("Built cache key: %s", key) diff --git a/fastapi_cachex/routes.py b/fastapi_cachex/routes.py index 9a93c65..b8f6b9f 100644 --- a/fastapi_cachex/routes.py +++ b/fastapi_cachex/routes.py @@ -10,6 +10,7 @@ from .proxy import BackendProxy from .types import CACHE_KEY_SEPARATOR from .types import CacheEntry +from .types import unescape_key_component if TYPE_CHECKING: from fastapi import FastAPI @@ -107,7 +108,9 @@ def _parse_cache_key(cache_key: str) -> tuple[str, str, str, str]: """ key_parts = cache_key.split(CACHE_KEY_SEPARATOR, CACHE_KEY_MAX_PARTS) if len(key_parts) >= CACHE_KEY_MIN_PARTS: - method, host, path = key_parts[0], key_parts[1], key_parts[2] + method = key_parts[0] + host = unescape_key_component(key_parts[1]) + path = unescape_key_component(key_parts[2]) query_params = key_parts[3] if len(key_parts) > CACHE_KEY_MIN_PARTS else "" return method, host, path, query_params diff --git a/fastapi_cachex/types.py b/fastapi_cachex/types.py index 815e407..9165312 100644 --- a/fastapi_cachex/types.py +++ b/fastapi_cachex/types.py @@ -14,6 +14,26 @@ # Type for custom cache key builder function CacheKeyBuilder = Callable[[Request], str] +_KEY_ESCAPES = {"%": "%25", "|": "%7C"} +_KEY_UNESCAPES = {escaped: char for char, escaped in _KEY_ESCAPES.items()} +_KEY_UNESCAPE_RE = re.compile("%25|%7C") + + +def escape_key_component(value: str) -> str: + """Percent-encode ``|`` and ``%`` so ``value`` cannot contain the separator. + + The host header and the decoded URL path are client-controlled and may + contain ``|||``; left as is, one request's components could line up into + another request's key. Encoding ``%`` as well keeps the mapping reversible, + so two different values never share an encoding. + """ + return value.replace("%", "%25").replace("|", "%7C") + + +def unescape_key_component(value: str) -> str: + """Reverse ``escape_key_component``.""" + return _KEY_UNESCAPE_RE.sub(lambda match: _KEY_UNESCAPES[match.group()], value) + # Status replayed for entries stored before ``CacheEntry`` carried a status code. DEFAULT_STATUS_CODE = 200 diff --git a/i18n/zh-TW/docs/CACHE_FLOW.md b/i18n/zh-TW/docs/CACHE_FLOW.md index fe88af9..2d5fa2f 100644 --- a/i18n/zh-TW/docs/CACHE_FLOW.md +++ b/i18n/zh-TW/docs/CACHE_FLOW.md @@ -46,10 +46,16 @@ private？ ── 是 → 執行 handler；比對 If-None-Match 決定回傳 304 ```python from fastapi_cachex.types import CACHE_KEY_SEPARATOR # "|||" +from fastapi_cachex.types import escape_key_component # 快取鍵格式（fastapi_cachex/cache.py 中的 default_key_builder） cache_key = CACHE_KEY_SEPARATOR.join( - [request.method, request.headers.get("host", "unknown"), request.url.path, query] + [ + request.method, + escape_key_component(request.headers.get("host", "unknown")), + escape_key_component(request.url.path), + query, + ] ) # 例如： @@ -59,6 +65,8 @@ cache_key = CACHE_KEY_SEPARATOR.join( 分隔符號使用 `|||` 而不是冒號，是因為 host 本身可能包含連接埠（`127.0.0.1:8000`）；若使用冒號，快取鍵就無法可靠地拆分，而 `clear_path()` 需要從快取鍵中取回路徑。 +host 與路徑會先經過百分比編碼：`|` 變成 `%7C`，`%` 變成 `%25`（`fastapi_cachex/types.py` 中的 `escape_key_component`）。兩者都來自用戶端，其中若出現未編碼的 `|||`，各段就會錯位，使某個請求的快取鍵可能與另一個請求相同。查詢字串本來就經過 URL 編碼。監控路由顯示時會再解碼。 + 查詢參數依請求送出的順序串接（`str(request.query_params)`），**不會排序**，因此 `?page=1&limit=10` 與 `?limit=10&page=1` 是兩個不同的快取項目。若希望兩者視為同一個，請傳入自訂的 `key_builder` 將查詢字串正規化。 這個快取鍵格式讓每個維度各自獨立快取： diff --git a/i18n/zh-TW/docs/HTTP_CACHING.md b/i18n/zh-TW/docs/HTTP_CACHING.md index ed08d97..9dfd438 100644 --- a/i18n/zh-TW/docs/HTTP_CACHING.md +++ b/i18n/zh-TW/docs/HTTP_CACHING.md @@ -104,6 +104,18 @@ async def report(): 查詢參數依用戶端送出的順序取用，不會排序，因此 `?a=1&b=2` 與 `?b=2&a=1` 對同一個邏輯上的請求而言是兩筆不同的快取項目。 +host 與路徑來自用戶端，因此其中的 `|` 與 `%` 會以百分比編碼寫入（`%7C` 與 `%25`）。含有 `|||` 的 `Host` 標頭或路徑因此無法讓各段錯位，使某個請求的快取鍵與另一個請求相同。查詢字串本來就經過 URL 編碼。`clear_path()` 接受應用程式看到的路徑（`request.url.path`），並以同樣方式編碼；`clear_pattern()` 比對的是儲存的快取鍵，所以在模式中要把 `|` 寫成 `%7C`。0.3.8 之前兩者都照原樣儲存，因此升級後，host 或路徑含有 `|` 或 `%` 的項目會重新快取一次。 + +host 仍是用戶端送來的任何值。除非應用程式前方的反向代理或負載平衡器已會拒絕未知的 host，否則請加上 Starlette 的 `TrustedHostMiddleware`，讓偽造的 `Host` 得到 `400`，而不是在快取中塞滿沒有其他人會請求的項目： + +```python +from starlette.middleware.trustedhost import TrustedHostMiddleware + +app.add_middleware( + TrustedHostMiddleware, allowed_hosts=["example.com", "*.example.com"] +) +``` + 所有後端都會自動替鍵加上前綴（例如 `fastapi_cachex:`）作為命名空間，以避免與其他應用程式衝突。`CacheManager`（見[應用層快取](APP_CACHE.md)）則使用另一個較簡單、以 `cache:` 為前綴的鍵命名空間，而不是這種以 `|||` 分隔的格式，因為它的鍵與 HTTP 請求無關。 ### 需驗證身分的端點 {#authenticated-endpoints} @@ -121,6 +133,7 @@ from fastapi import Request, Response from fastapi_cachex import cache from fastapi_cachex.types import CACHE_KEY_SEPARATOR +from fastapi_cachex.types import escape_key_component # 1. 完全不放進共用快取。 @@ -137,8 +150,9 @@ def per_user_key(request: Request) -> str: user_id = getattr(request.state, "user_id", "anonymous") return ( f"{request.method}{CACHE_KEY_SEPARATOR}" - f"{request.headers.get('host', 'unknown')}{CACHE_KEY_SEPARATOR}" - f"{request.url.path}{CACHE_KEY_SEPARATOR}" + f"{escape_key_component(request.headers.get('host', 'unknown'))}" + f"{CACHE_KEY_SEPARATOR}" + f"{escape_key_component(request.url.path)}{CACHE_KEY_SEPARATOR}" f"{request.query_params}{CACHE_KEY_SEPARATOR}{user_id}" ) diff --git a/tests/backends/test_clear_pattern_contract.py b/tests/backends/test_clear_pattern_contract.py index d6d4a5b..9b618a0 100644 --- a/tests/backends/test_clear_pattern_contract.py +++ b/tests/backends/test_clear_pattern_contract.py @@ -146,3 +146,20 @@ async def test_cache_manager_clear_pattern_is_relative_to_its_namespace( assert await manager.clear_pattern("user:*") == 2 assert await manager.get("user:1") is None assert await manager.get("post:1") == {"title": "c"} + + +@pytest.mark.asyncio +@pytest.mark.parametrize("backend", ["memory", "redis"], indirect=True) +async def test_clear_path_finds_paths_with_encoded_characters( + backend: BaseCacheBackend, +) -> None: + """``clear_path`` takes the decoded path and matches the encoded key.""" + entry = CacheEntry(fingerprint="etag", content=b"x") + # Keys as default_key_builder writes them for "/a|b/100%" and a neighbour. + await backend.set("GET|||h|||/a%7Cb/100%25|||", entry) + await backend.set("GET|||h|||/a%7Cb/100%25|||v=1", entry) + await backend.set("GET|||h|||/a|||b/100%25|||", entry) + + assert await backend.clear_path("/a|b/100%") == 1 + assert await backend.clear_path("/a|b/100%", include_params=True) == 1 + assert await backend.get_all_keys() == ["GET|||h|||/a|||b/100%25|||"] diff --git a/tests/test_cache_key.py b/tests/test_cache_key.py index 747e3c2..620f301 100644 --- a/tests/test_cache_key.py +++ b/tests/test_cache_key.py @@ -1,13 +1,17 @@ """Tests for cache key generation and parsing.""" from fastapi import FastAPI +from fastapi import Request from fastapi.testclient import TestClient from fastapi_cachex.backends import MemoryBackend from fastapi_cachex.cache import cache +from fastapi_cachex.cache import default_key_builder from fastapi_cachex.proxy import BackendProxy from fastapi_cachex.routes import _parse_cache_key from fastapi_cachex.types import CACHE_KEY_SEPARATOR +from fastapi_cachex.types import escape_key_component +from fastapi_cachex.types import unescape_key_component class TestCacheKeyGeneration: @@ -228,3 +232,84 @@ async def data_endpoint(): assert key1_method == key2_method == "GET" assert key1_path == key2_path == "/api/data" + + +class TestCacheKeySeparatorInComponents: + """A ``|||`` in the Host header or path must not shift the key components.""" + + def test_host_header_cannot_poison_another_path(self) -> None: + """Host ``h|||/p`` + path ``/x`` used to share a key with path ``/p|||/x``.""" + app = FastAPI() + backend = MemoryBackend() + BackendProxy.set(backend) + + @app.get("/{p:path}") + @cache(ttl=60) + async def echo(p: str) -> dict[str, str]: + return {"p": p} + + client = TestClient(app) + poisoned = client.get("/x", headers={"host": "testserver|||/p"}) + assert poisoned.json() == {"p": "x"} + + victim = client.get("/p%7C%7C%7C/x") + assert victim.json() == {"p": "p|||/x"} + assert len(backend.cache) == 2 + + def test_percent_is_encoded_so_the_encoding_is_unambiguous(self) -> None: + """A path holding a literal ``%7C`` keeps its own key, apart from ``|``. + + Built from a raw scope: TestClient decodes the path twice, so it cannot + send a decoded path containing ``%7C`` the way a real server does. + """ + + def key_for(path: str) -> str: + scope = { + "type": "http", + "method": "GET", + "path": path, + "query_string": b"", + "headers": [(b"host", b"h")], + } + return default_key_builder(Request(scope)) + + assert key_for("/a|") == "GET|||h|||/a%7C|||" + assert key_for("/a%7C") == "GET|||h|||/a%257C|||" + + def test_ordinary_keys_are_unchanged(self) -> None: + """Only components with ``|`` or ``%`` change, so existing entries still hit.""" + app = FastAPI() + backend = MemoryBackend() + BackendProxy.set(backend) + + @app.get("/api/items") + @cache(ttl=60) + async def items() -> dict[str, str]: + return {"ok": "yes"} + + client = TestClient(app, base_url="http://127.0.0.1:8000") + client.get("/api/items", params={"q": "a|b%c"}) + assert list(backend.cache) == [ + "GET|||127.0.0.1:8000|||/api/items|||q=a%7Cb%25c" + ] + + def test_escape_round_trips_and_never_contains_the_separator(self) -> None: + """Distinct inputs get distinct encodings, and decoding restores them.""" + values = ["", "|", "|||", "%", "%7C", "%257C", "%25", "a|%b", "%%||", "%7c"] + encoded = [escape_key_component(value) for value in values] + assert len(set(encoded)) == len(values) + for value, escaped in zip(values, encoded, strict=True): + assert "|" not in escaped + assert unescape_key_component(escaped) == value + + def test_monitoring_parser_decodes_host_and_path(self) -> None: + """The monitoring routes show the host and path as the client sent them.""" + key = CACHE_KEY_SEPARATOR.join( + [ + "GET", + escape_key_component("evil|||host"), + escape_key_component("/p|||/100%"), + "q=1", + ] + ) + assert _parse_cache_key(key) == ("GET", "evil|||host", "/p|||/100%", "q=1")