You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by "gianm (via GitHub)" <gi...@apache.org> on 2026/09/04 22:57:24 UTC
[PR] fix: Avoid collisions in CacheKeyBuilder. (druid)
gianm opened a new pull request, #20255:
URL: https://github.com/apache/druid/pull/20255
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 is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] fix: Avoid collisions in CacheKeyBuilder. (druid)
Posted by "FrankChen021 (via GitHub)" <gi...@apache.org>.
FrankChen021 commented on code in PR #20255:
URL: https://github.com/apache/druid/pull/20255#discussion_r3940642339
##########
processing/src/main/java/org/apache/druid/query/cache/CacheKeyBuilder.java:
##########
@@ -323,7 +327,7 @@ public byte[] build()
buffer.put(id);
for (Item item : items) {
- buffer.put(item.typeKey).put(item.item);
+ buffer.put(item.typeKey).putInt(item.item.length).put(item.item);
Review Comment:
[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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] fix: Avoid collisions in CacheKeyBuilder. (druid)
Posted by "github-advanced-security[bot] (via GitHub)" <gi...@apache.org>.
github-advanced-security[bot] commented on code in PR #20255:
URL: https://github.com/apache/druid/pull/20255#discussion_r3938533627
##########
processing/src/main/java/org/apache/druid/query/cache/CacheKeyBuilder.java:
##########
@@ -72,20 +69,11 @@
static final byte[] STRING_SEPARATOR = new byte[]{(byte) 0xFF};
static final byte[] EMPTY_BYTES = StringUtils.EMPTY_BYTES;
- private static class Item
+ private record Item(byte typeKey, byte[] item)
{
- private final byte typeKey;
- private final byte[] item;
-
- Item(byte typeKey, byte[] item)
- {
- this.typeKey = typeKey;
- this.item = item;
- }
-
int byteSize()
{
- return 1 + item.length;
+ return 1 + Integer.BYTES + item.length;
Review Comment:
## CodeQL / User-controlled data in arithmetic expression
This arithmetic expression depends on a [user-provided value](1), potentially causing an overflow.
This arithmetic expression depends on a [user-provided value](2), potentially causing an overflow.
This arithmetic expression depends on a [user-provided value](3), potentially causing an overflow.
[Show more details](https://github.com/apache/druid/security/code-scanning/11934)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org