Skip to content

fix: Avoid collisions in CacheKeyBuilder. - #20255

Open
gianm wants to merge 1 commit into
apache:masterfrom
gianm:ckb-ambiguity
Open

fix: Avoid collisions in CacheKeyBuilder.#20255
gianm wants to merge 1 commit into
apache:masterfrom
gianm:ckb-ambiguity

Conversation

@gianm

@gianm gianm commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

This patch adds a length prefix before each item when building cache keys. This avoids ambiguity in situations where a cache key itself contains a valid type code.

This patch adds a length prefix before each item when building
cache keys. This avoids ambiguity in situations where a cache key
itself contains a valid type code.
int byteSize()
{
return 1 + item.length;
return 1 + Integer.BYTES + item.length;

@FrankChen021 FrankChen021 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Severity Findings
P0 0
P1 1
P2 0
P3 0
Total 1

Reviewed 6 of 6 changed files.


This is an automated review by Codex GPT-5.6-Luna(max)


for (Item item : items) {
buffer.put(item.typeKey).put(item.item);
buffer.put(item.typeKey).putInt(item.item.length).put(item.item);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[P1] Version the new cache-key format

This changes the serialized bytes for every CacheKeyBuilder key, but the output still uses only the caller-supplied id and does not distinguish the new format from the old one. During a rolling upgrade, shared caches such as Memcached can retain old entries; the two formats are not disjoint, so a new key can be an old-format key for a different public builder sequence (for example, old appendByte(0).appendByte(0).appendByteArray(new byte[]{v}) produces the same bytes as new appendByte(v) for the same id). That can turn a miss into a false hit and return another query's result. Add a format/version discriminator or explicitly isolate/invalidate old entries, and add a mixed-version collision regression test.

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