Skip to content

Commit

Permalink
Use memcmp() instead of strncmp() to compare cache buckets.
Browse files Browse the repository at this point in the history
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!)
  • Loading branch information
jparise committed Aug 31, 2011
1 parent c2146ff commit c34c374
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion JSONKit.m
Expand Up @@ -2004,7 +2004,7 @@ JK_STATIC_INLINE void jk_cache_age(JKParseState *parseState) {
for(x = 0UL; x < JK_CACHE_PROBES; x++) { for(x = 0UL; x < JK_CACHE_PROBES; x++) {
if(JK_EXPECT_F(parseState->cache.items[bucket].object == NULL)) { setBucket = 1UL; useableBucket = bucket; break; } if(JK_EXPECT_F(parseState->cache.items[bucket].object == NULL)) { setBucket = 1UL; useableBucket = bucket; break; }


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))) { 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))) {
parseState->cache.age[bucket] = (parseState->cache.age[bucket] << 1) | 1U; parseState->cache.age[bucket] = (parseState->cache.age[bucket] << 1) | 1U;
parseState->token.value.cacheItem = &parseState->cache.items[bucket]; parseState->token.value.cacheItem = &parseState->cache.items[bucket];
NSCParameterAssert(parseState->cache.items[bucket].object != NULL); NSCParameterAssert(parseState->cache.items[bucket].object != NULL);
Expand Down

0 comments on commit c34c374

Please sign in to comment.