From c34c37487667b9ae1d8b50a7e946a4f086f16c8d Mon Sep 17 00:00:00 2001 From: Jon Parise Date: Tue, 30 Aug 2011 22:16:27 -0700 Subject: [PATCH] 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!) --- JSONKit.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/JSONKit.m b/JSONKit.m index 3ee1dad..1f27e95 100644 --- a/JSONKit.m +++ b/JSONKit.m @@ -2004,7 +2004,7 @@ JK_STATIC_INLINE void jk_cache_age(JKParseState *parseState) { 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_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->token.value.cacheItem = &parseState->cache.items[bucket]; NSCParameterAssert(parseState->cache.items[bucket].object != NULL);