Skip to content

fix: digest memcached cache keys - #99

Open
lleirborras wants to merge 1 commit into
masterfrom
fix/memcached-safe-cache-keys
Open

fix: digest memcached cache keys#99
lleirborras wants to merge 1 commit into
masterfrom
fix/memcached-safe-cache-keys

Conversation

@lleirborras

Copy link
Copy Markdown
Member

Problem

With Dalli 5, a cached GET whose parameters contain whitespace or non-ASCII can fail:

Response error: CLIENT_ERROR (Dalli::DalliError)
  from dalli-5.0.4/lib/dalli/protocol/response_processor.rb:199:in 'Dalli::Protocol::Meta::ResponseProcessor#error_on_unexpected!'
  from dalli-5.0.4/lib/dalli/protocol/meta.rb:33:in 'Dalli::Protocol::Meta#get'
  from dalli-5.0.4/lib/dalli/client.rb:71:in 'Dalli::Client#get'

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::KeyRegularizer base64-encodes such keys. That inflates them by a
third — and Dalli::KeyManager#validate_key checks 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.rb asserts 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_ERROR where 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

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
lleirborras force-pushed the fix/memcached-safe-cache-keys branch from dcbc8dd to e983f9e Compare July 31, 2026 12:57
@mojarra

mojarra commented Jul 31, 2026

Copy link
Copy Markdown

@lleirborras checked in IATI staging and the fix worked!

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 thread spec/spec_helper.rb
Comment on lines +28 to +30
match do |key|
key.is_a?(String) && key.ascii_only? && !key.match?(/\s/) && key.bytesize <= 250
end
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants