Spring Framework
  1. Spring Framework
  2. SPR-9377

org.springframework.cache.interceptor.DefaultKeyGenerator has too weak hashing functionality

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Duplicate
    • Affects Version/s: 3.1.1
    • Fix Version/s: None
    • Component/s: Core
    • Last commented by a User:
      true

      Description

      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...

        Issue Links

          Activity

          Hide
          Hans-Peter Störr added a comment - - edited

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

          Show
          Hans-Peter Störr added a comment - - edited This seems more or less a duplicate of SPR-9036 and is IMHO a very ugly bug, not an improvement. Luckily I saw it during code review. See my suggestion there.
          Hide
          Andrey Karandey added a comment -

          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.

          Show
          Andrey Karandey added a comment - 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 .
          Hide
          Hans-Peter Störr added a comment -

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

          Show
          Hans-Peter Störr added a comment - md5 would be better, but I'd rather not use any kind of hashing - see my comment on SPR-9036 .
          Hide
          Tim Lenz added a comment -

          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.

          Show
          Tim Lenz added a comment - 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.

            People

            • Assignee:
              Phil Webb
              Reporter:
              Andrey Karandey
              Last updater:
              Phil Webb
            • Votes:
              3 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:
                Days since last comment:
                1 year, 48 weeks, 5 days ago