Repository navigation
Conversation
alessio-locatelli
force-pushed
the
fix/cache-key-review-followups
branch
from
October 6, 2026 15:34
da2040a to
eed531c
Compare
Bytes of one key component could pass for bytes of its neighbor, so
requests that aiohttp sends differently shared a cache key, e.g.
`data=b'false'` (application/octet-stream) and `json=False`
(application/json), or `data=b'x=1'` and `headers={'x': '1'}` with
include_headers. With POST caching enabled, one request could receive
the other's cached response.
A length prefix on every component rules out shifts at every
boundary, whereas tagging only `data` and `json` would leave the
boundary to the headers open. The prefixed components are fed to the
hash one at a time rather than joined first, so a large body isn't
copied just to compute its key.
Every cache key changes, so caches created by earlier versions are
not reused after upgrading.
Only the session knows which JSON serializer and default headers produced a cached request's key, but CacheBackend.has_url() and delete_url() default to stdlib json.dumps() and no headers. With a custom serializer, or with include_headers and session default headers, has_url() returned False for cached requests and delete_url() left them in the cache, unless the caller repeated the session's configuration in every call. The session versions supply that configuration. The backend does not store it: a backend can be shared by sessions with different serializers, and helpers that followed whichever session was created last would depend on unrelated session construction order.
Creating a cache key serializes the JSON body, and aiohttp serializes it again to send it. Requests with methods outside allowed_methods are never cached, POST by default, so for them the first serialization was pure overhead, which grows with large bodies or expensive custom serializers. These requests also no longer wait on the per-key lock, which only exists to avoid duplicate cache fills. Cached responses for a method later removed from allowed_methods are no longer deleted when that method is requested; they remain until delete_expired_responses() purges them after expiry, or delete_url() or clear() removes them. The session and is_cacheable() share CacheBackend.is_method_allowed() so they can't disagree about which methods are cached. It uppercases the method because the session receives it as the caller wrote it, while aiohttp uppercases it on the response. Cookie restoration moved into a method because the new branch pushed _request() over ruff's complexity limit (C901).
aiohttp chooses the content type of a `data=` body from its Python
type. A mapping becomes a URL-encoded form, unless a field value is
bytes-like or a file object, which makes the form multipart. Cache
keys ignored all of this and joined mapping pairs without escaping,
so requests that aiohttp sends differently shared a key:
- `b'x'` (application/octet-stream) and `'x'` (text/plain)
- `{'a': 1}` (form-urlencoded) and `'a=1'` (text/plain)
- `{'a': 'b&c=d'}` (sent as `a=b%26c%3Dd`) and `{'a': 'b', 'c': 'd'}`
- `{'a': b'x'}` (multipart/form-data) and `{'a': 'x'}`
(form-urlencoded)
- `{'a': [1, 2]}` (sent as `a=1&a=2`) and `{'a': '[1, 2]'}`
With POST caching enabled, one request could receive the other's
cached response. Empty bodies are tagged too, since aiohttp still
sends their content type.
Form fields are URL-encoded with `doseq=True`, as aiohttp encodes
them, so a sequence value matches the repeated fields actually sent.
Multipart fields keep each value's own tag, because aiohttp sends
string fields and bytes fields as different parts.
Fields are sorted by name only, so repeated names keep the order in
which aiohttp sends them.
The tag and the body are separate key components, and multipart
fields are hashed into a digest, so a large body is hashed in place.
Concatenating a tag onto it would copy the whole body just to compute
the key.
bytearray and memoryview share the bytes tag because aiohttp sends
them exactly like bytes, so such requests can reuse each other's
responses. Header values go through the same escaping, which removes
the equivalent collision for headers containing `&` or `=`.
alessio-locatelli
force-pushed
the
fix/cache-key-review-followups
branch
from
October 6, 2026 15:56
eed531c to
f0add2f
Compare
alessio-locatelli
changed the title
alessio-locatelli
marked this pull request as ready for review
October 6, 2026 16:14
Member
|
Thanks for reviewing cache key behavior! I'll look this over after work today. I'd like to publish a release (0.15.0) after merging this. Are there any other changes you'd like to make before releasing? |
Collaborator
Author
Yes, I was planning to address #422 today or tomorrow. If you think #422 is not urgent, we can include it in next patch release (for example, Also, Python 3.10 has reached end-of-life, but this project still supports Python 3.9. Would you like to include the Python 3.9 and 3.10 removal in |
Member
Sure, let's include both #422 and retiring python <3.11 in v0.15.0. I'll also review the doc build and dependencies before releasing. We may or may not be able to update to Sphinx 9 depending on extension compatibility. |
JWCook
reviewed
Oct 6, 2026
Comment on lines
+209
to
+215
| tracemalloc.start() | ||
| try: | ||
| create_key('POST', 'https://example.com', data=data) | ||
| _, peak = tracemalloc.get_traced_memory() | ||
| finally: | ||
| tracemalloc.stop() | ||
| assert peak < BODY_SIZE // 10 |
Member
There was a problem hiding this comment.
Neat, I didn't know this was doable with just the stdlib!
JWCook
approved these changes
Oct 6, 2026
JWCook
left a comment
JWCook
left a comment
Member
There was a problem hiding this comment.
All of these changes look good to me. Thank you!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #420.
Cache key collisions
Before this PR, requests that aiohttp sends differently could get the same cache key. With POST caching enabled, one such request could receive the other's cached response. This PR fixes two causes:
Unframed key components. The method, URL,
data,json, and headers were hashed as one continuous byte sequence, so bytes could move from one component to the next without changing the key. For example,data=b'false'(sent asapplication/octet-stream) andjson=False(sent asapplication/json) got the same key. Each component is now prefixed with its length before hashing.data=bodies keyed without their type. aiohttp picks a body's content type from its Python type, but the cache key ignored the type and joined form fields without escaping. These pairs got the same key even though aiohttp sends them differently:b'x'and'x'{'a': 1}and'a=1'{'a': 'b&c=d'}and{'a': 'b', 'c': 'd'}{'a': b'x'}(multipart) and{'a': 'x'}{'a': [1, 2]}and{'a': '[1, 2]'}The key now includes the body type and encodes form fields the way aiohttp does. Large raw bodies and multipart fields are hashed without being copied.
All cache keys change, so existing cache entries won't be reused after upgrading.
CachedSession.has_url()andCachedSession.delete_url()CacheBackend.has_url()andCacheBackend.delete_url()build keys with the stdlibjson.dumps()and without the session's default headers. So they can't find requests cached by a session that uses a custom JSON serializer, or by a session with default headers wheninclude_headers=True. The newCachedSession.has_url()andCachedSession.delete_url()build keys the same way the session's requests do.The backend doesn't store the serializer. Several sessions with different serializers can share one backend, and the backend helpers shouldn't change behavior depending on which session was created last.
Requests with uncached methods skip the cache
Building a cache key serializes a request's JSON body, and aiohttp serializes it again to send the request. Requests whose method isn't in
allowed_methods(by default, every method except GET and HEAD) are never cached, soCachedSessionnow sends them directly without building a key. The newCacheBackend.is_method_allowed()method is used both here and byis_cacheable(), so the two checks always agree.Two behavior changes follow from this:
allowed_methods, its existing cache entries are no longer deleted when that method is requested. They stay untildelete_expired_responses()removes them after they expire, or untildelete_url()orclear()removes them.Testing
New unit tests cover each collision, the session helpers, the cache bypass for uncached methods, and peak memory while hashing a 10 MiB body. For the multipart and list-value cases, tests against a local aiohttp server check that aiohttp sends the two bodies differently and that they're now cached separately. Every added or changed line is covered by unit tests.