fix: digest memcached cache keys - #99
Open
lleirborras wants to merge 1 commit into
Open
Conversation
The cache key was the JSON-serialized request descriptor, which carries the url and query parameters verbatim. Memcached keys must be ASCII, free of whitespace and at most 250 bytes long. Dalli's meta protocol, which replaced the binary protocol in Dalli 5, works around the first two constraints by base64-encoding keys that violate them. That inflates the key by a third, and since Dalli checks the length before encoding, keys longer than 187 bytes end up over the 250 byte limit and memcached replies CLIENT_ERROR. Any request with whitespace or non-ASCII in its parameters was therefore at risk. The binary protocol sent keys as length-prefixed raw bytes, so whitespace and length were both fine and this only surfaced after upgrading Dalli. Digesting the descriptor keeps the key short and ASCII whatever the url and parameters contain. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
lleirborras
force-pushed
the
fix/memcached-safe-cache-keys
branch
from
July 31, 2026 12:57
dcbc8dd to
e983f9e
Compare
|
@lleirborras checked in IATI staging and the fix worked! |
There was a problem hiding this comment.
Pull request overview
This PR hardens Hawk’s Memcached-backed HTTP caching against Dalli 5’s meta protocol key constraints by switching from raw JSON descriptor keys to a fixed-length digest, preventing whitespace/non-ASCII key encoding blowups that can exceed Memcached’s 250-byte limit.
Changes:
- Digest HTTP cache keys using
SHA256( MultiJson.dump(descriptor) )instead of using the serialized descriptor verbatim. - Add an RSpec matcher for “memcached-safe” keys and a new spec that verifies key safety and stability across requests (including non-ASCII params).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
lib/hawk/http/caching.rb |
Switches cache key generation from raw JSON to a SHA256 hex digest to guarantee a safe, bounded key. |
spec/spec_helper.rb |
Adds a custom matcher to assert cache keys are compatible with Memcached/Dalli meta protocol constraints. |
spec/hawk/http/caching_spec.rb |
Adds specs ensuring generated keys are safe (ASCII/whitespace-free/≤250 bytes), stable for identical requests, and distinct across different requests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+28
to
+30
| match do |key| | ||
| key.is_a?(String) && key.ascii_only? && !key.match?(/\s/) && key.bytesize <= 250 | ||
| end |
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
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.
Problem
With Dalli 5, a cached
GETwhose parameters contain whitespace or non-ASCII can fail:Root cause
The cache key was
MultiJson.dump(descriptor)— the url and query parameters verbatim.Memcached keys must be ASCII, whitespace-free and at most 250 bytes. Dalli 5 replaced
the binary protocol with the line-based meta protocol, which cannot carry whitespace, so
Dalli::Protocol::Meta::KeyRegularizerbase64-encodes such keys. That inflates them by athird — and
Dalli::KeyManager#validate_keychecks the 250-byte limit before encoding,so its md5-truncation fallback never kicks in. Any key over 187 bytes then goes on the
wire above 250 bytes and the server answers
CLIENT_ERROR.JSON serialization contributes no whitespace of its own, so this needs whitespace or
non-ASCII inside a parameter value — a person's name, a search string, an accented
place name. The binary protocol sent keys as length-prefixed raw bytes, so both
whitespace and length were harmless before the Dalli upgrade.
Fix
Digest the serialized descriptor. 64 hex chars, 69 bytes once the Dalli namespace is
prepended, never base64-encoded, whatever the url and parameters contain.
Verification
New
spec/hawk/http/caching_spec.rbasserts the key is ASCII, whitespace-free and≤250 bytes — including a non-ASCII parameter case — and that it is stable per request and
distinct across requests.
45 examples, 0 failures, RuboCop clean.End-to-end against dalli 5.0.5 + memcached 1.6.45, a raw 203-byte descriptor key raises
CLIENT_ERRORwhere its 64-byte digest round-trips.Note for consumers
The key format changes, so existing cache entries go cold on deploy. Default TTL is 60
seconds.
Companion fix in people-client, where this was actually erroring in production:
https://github.com/ifad/people-client/pull/138
🤖 Generated with Claude Code