org.springframework.cache.interceptor.DefaultKeyGenerator has too weak hashing functionality [SPR-9377]

[spring-projects-issues]

Andrey Karandey opened SPR-9377 and commented

Key generating has a weak hashing function. Next results are equal:

generate( object1,method1,new Integer( 109 ),new Integer( 434)));
generate( object1,method1,new Integer( 110 ),new Integer( 403)));

It was pity to catch it on production...

---

Affects: 3.1.1

Issue Links:

• Cacheable key collision with DefaultKeyGenerator [SPR-10237] #14870 Cacheable key collision with DefaultKeyGenerator ("duplicates")
• Improve DefaultKeyGenerator [SPR-9036] #13675 Improve DefaultKeyGenerator
• Cacheable key collision with DefaultKeyGenerator [SPR-10237] #14870 Cacheable key collision with DefaultKeyGenerator

3 votes, 4 watchers

[spring-projects-issues]

Hans-Peter Störr commented

This seems more or less a duplicate of #13675 and is IMHO a very ugly bug, not an improvement. Luckily I saw it during code review. :-) See my suggestion there.

[spring-projects-issues]

Andrey Karandey commented

Anyway, i did not find any strong recommendation not to use this default implementation, so people might use it without any limitations.
P.S. I created my own based on md5 hash, but i`m not ready to suggest it now:).

[spring-projects-issues]

Hans-Peter Störr commented

md5 would be better, but I'd rather not use any kind of hashing - see my comment on #13675.

[spring-projects-issues]

Tim Lenz commented

Regardless of possible better implementations, I believe there is a bug in the default implementation. If you have a method with only one object parameter, that parameter is never hashed, it's simply returned.

I've made my own key generator where I changed this:

if (params.length == 1) {
return (params[0] == null ? NULL_PARAM_KEY : params[0]);
}

to this:

if (params.length == 1 && params[0] == null) {
return NULL_PARAM_KEY;
}

Now a single non-null object parameter will still be hashed, as occurred already for methods with more than one parameter.