manjuraj
commented
11 years ago @Homer512 you are right in asserting that the check might be unnecessary. In fact as it exists right now, the code doesn't do that check, but it is something I have planned to introduce it (See: https://github.com/twitter/fatcache/blob/master/src/fc_request.c#L172).
also remember that checking only requires a in-memory comparission of the key string which is not such a performance overhead
In your summary, you write that you have code in place to check for SHA-1 collisions. I suggest getting rid of that code and its performance overhead.
There are simply no collisions for reasonably short keys. The chance of accidentally producing a hash collision for SHA-1 is astronomically small. Even purposely finding SHA-1 collisions with brute force takes ca. 2^80 instructions. Just do the math on how long your system would need to be running for that to happen.
The only reason for doing the check would be to protect against more elaborate attacks which a user might be able to inject. It is unlikely that SHA-1 will be susceptible to preimage attacks (which would be necessary to, for example, retrieve other chosen data from the cache) any time soon (not even MD5 is). More likely but still a far fetch would be a collision attack on SHA-1 that would allow a complexity attack on your system. If that ever becomes a potential threat, you should migrate to a better hash as checking won't help you.