Skip to content

Commit 85be22a

Browse files
alessio-locatelliJWCook
authored andcommitted
fix: resolve relative URLs against base_url in cache keys
Cache keys were built from the URL as passed to the request, before aiohttp resolves a relative URL against the session's `base_url`. So `GET /x` got the same key for every base URL, and sessions sharing a cache backend returned each other's cached responses. Requests, `has_url()`, and `delete_url()` now resolve relative URLs first. `ClientSession._build_url()` isn't used for this because it's private and, in older aiohttp versions, asserts that the URL is relative whenever `base_url` is set. Cache entries saved for relative URLs won't be found after upgrading. Fixes #422
1 parent f23b974 commit 85be22a

3 files changed

Lines changed: 43 additions & 4 deletions

File tree

‎HISTORY.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
configured JSON serializer, including bytes serializers.
99
- Fixed cache key collisions between request bodies, such as `data=b'false'` and `json=False`, and between request bodies and headers. Existing cache entries won't be reused after upgrading.
1010
- Fixed cache key collisions between `data=` bodies that aiohttp sends differently, such as `b'x'` and `'x'`, `{'a': 1}` and `'a=1'`, `{'a': 'b&c=d'}` and `{'a': 'b', 'c': 'd'}`, `{'a': b'x'}` and `{'a': 'x'}`, or `{'a': [1, 2]}` and `{'a': '[1, 2]'}`. Header values are now URL-encoded in cache keys too.
11+
- Fixed cache keys for relative URLs to include the session's `base_url`, so sessions with different base URLs no longer return each other's cached responses. Existing cache entries for relative URLs won't be reused after upgrading.
1112
- Added `CachedSession.has_url()` and `CachedSession.delete_url()`, which create keys like the session's requests: with its JSON serializer and, with `include_headers`, its default headers.
1213
- Requests with HTTP methods that aren't in `allowed_methods` now skip the cache entirely, so their JSON bodies are no longer serialized for a cache key. Added `CacheBackend.is_method_allowed()`.
1314
- Added `_ExpandedRequestOptions.refresh` to satisfy Pyright type checking.

‎aiohttp_client_cache/session.py‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
from aiohttp import ClientSession
1414
from aiohttp.typedefs import StrOrURL
15+
from yarl import URL
1516

1617
from aiohttp_client_cache.backends import CacheBackend, get_valid_kwargs
1718
from aiohttp_client_cache.cache_control import CacheActions, ExpirationTime, compose_refresh_headers
@@ -77,11 +78,12 @@ async def _request(
7778
# Attempt to fetch cached response
7879
headers = self._prepare_headers(kwargs.get('headers', None))
7980
kwargs['headers'] = headers
81+
url = self._resolve_url(str_or_url)
8082
key = self.cache.create_key(
81-
method, str_or_url, json_serialize=self._cache_json_serialize, **kwargs
83+
method, url, json_serialize=self._cache_json_serialize, **kwargs
8284
)
8385
actions = self.cache.create_cache_actions(
84-
key, str_or_url, expire_after=expire_after, refresh=refresh, **kwargs
86+
key, url, expire_after=expire_after, refresh=refresh, **kwargs
8587
)
8688

8789
if actions.skip_read:
@@ -169,11 +171,21 @@ async def _refresh_cached_response(
169171

170172
async def delete_url(self, url: StrOrURL, method: str = 'GET', **kwargs: Any):
171173
"""Like :py:meth:`.CacheBackend.delete_url`, with keys created like this session's requests"""
172-
await self.cache.delete_url(url, method, **self._cache_key_kwargs(**kwargs))
174+
await self.cache.delete_url(
175+
self._resolve_url(url), method, **self._cache_key_kwargs(**kwargs)
176+
)
173177

174178
async def has_url(self, url: StrOrURL, method: str = 'GET', **kwargs: Any) -> bool:
175179
"""Like :py:meth:`.CacheBackend.has_url`, with keys created like this session's requests"""
176-
return await self.cache.has_url(url, method, **self._cache_key_kwargs(**kwargs))
180+
return await self.cache.has_url(
181+
self._resolve_url(url), method, **self._cache_key_kwargs(**kwargs)
182+
)
183+
184+
def _resolve_url(self, str_or_url: StrOrURL) -> URL:
185+
url = URL(str_or_url)
186+
if self._base_url and not url.is_absolute():
187+
return self._base_url.join(url)
188+
return url
177189

178190
def _cache_key_kwargs(self, **kwargs: Any) -> dict[str, Any]:
179191
return {

‎test/unit/test_session.py‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,3 +257,29 @@ async def ok(request):
257257
assert await session.has_url(url)
258258
await session.delete_url(url)
259259
assert await cache.responses.size() == 0
260+
261+
262+
async def test_session__base_url_in_cache_key(aiohttp_server):
263+
base_urls = []
264+
for text in ['a', 'b']:
265+
266+
async def handler(request, text=text):
267+
return web.Response(text=text)
268+
269+
app = web.Application()
270+
app.router.add_get('/x', handler)
271+
base_urls.append((await aiohttp_server(app)).make_url('/'))
272+
273+
cache = CacheBackend()
274+
for base_url, expected_text in zip(base_urls, ['a', 'b'], strict=True):
275+
async with CachedSession(base_url, cache=cache) as session:
276+
response = await session.get('/x')
277+
assert await response.text() == expected_text
278+
assert response.from_cache is False
279+
assert await session.has_url('/x')
280+
assert await session.has_url(base_url / 'x')
281+
282+
async with CachedSession(base_urls[0], cache=cache) as session:
283+
await session.delete_url('/x')
284+
assert not await cache.has_url(base_urls[0] / 'x')
285+
assert await cache.has_url(base_urls[1] / 'x')

0 commit comments

Comments
 (0)