Skip to content

Commit c34c374

Browse files
committed
Use memcmp() instead of strncmp() to compare cache buckets.
In the rare case when a hash collision occurs between two values, the object cache's bucket-probing code needs to go as far as comparing the bytes content of the candidate bucket with the bytes content of the current token. For numeric values, 'bytes' refers to the raw bytes of the number. These byte representations may contain leading '\0' values. strncmp() is only designed for comparing strings, so characters appearing after a '\0' are not compared. Therefore, if a hash collision occurs between two numeric values that only differ in their lower bytes, they will be assigned to the same bucket, and the cache will return the first token's object for the second token's value. memcmp() is binary-safe and considers all of the numbers' bytes, avoiding this problem. This is a rare case indeed, but here is an example that reproduces the problem: JSON Input: [ 1734.75, 417.75 ] Identical Types: double (8 bytes) Identical DJB Hashes: 2510392872 JSONKit (strncmp): [ 1734.75, 1734.75 ] (boo!) JSONKit (memcmp): [ 1734.75, 417.75 ] (yay!)
1 parent c2146ff commit c34c374

File tree

1 file changed

+1
-1
lines changed

1 file changed

+1
-1
lines changed

JSONKit.m

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2004,7 +2004,7 @@ JK_STATIC_INLINE void jk_cache_age(JKParseState *parseState) {
20042004
for(x = 0UL; x < JK_CACHE_PROBES; x++) {
20052005
if(JK_EXPECT_F(parseState->cache.items[bucket].object == NULL)) { setBucket = 1UL; useableBucket = bucket; break; }
20062006

2007-
if((JK_EXPECT_T(parseState->cache.items[bucket].hash == parseState->token.value.hash)) && (JK_EXPECT_T(parseState->cache.items[bucket].size == parseState->token.value.ptrRange.length)) && (JK_EXPECT_T(parseState->cache.items[bucket].type == parseState->token.value.type)) && (JK_EXPECT_T(parseState->cache.items[bucket].bytes != NULL)) && (JK_EXPECT_T(strncmp((const char *)parseState->cache.items[bucket].bytes, (const char *)parseState->token.value.ptrRange.ptr, parseState->token.value.ptrRange.length) == 0U))) {
2007+
if((JK_EXPECT_T(parseState->cache.items[bucket].hash == parseState->token.value.hash)) && (JK_EXPECT_T(parseState->cache.items[bucket].size == parseState->token.value.ptrRange.length)) && (JK_EXPECT_T(parseState->cache.items[bucket].type == parseState->token.value.type)) && (JK_EXPECT_T(parseState->cache.items[bucket].bytes != NULL)) && (JK_EXPECT_T(memcmp(parseState->cache.items[bucket].bytes, parseState->token.value.ptrRange.ptr, parseState->token.value.ptrRange.length) == 0U))) {
20082008
parseState->cache.age[bucket] = (parseState->cache.age[bucket] << 1) | 1U;
20092009
parseState->token.value.cacheItem = &parseState->cache.items[bucket];
20102010
NSCParameterAssert(parseState->cache.items[bucket].object != NULL);

0 commit comments

Comments
 (0)