Spring Framework
  1. Spring Framework
  2. SPR-9303

Portlet annotation handler mapping does is not working properly because of a flaw in predicate comparison

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Complete
    • Affects Version/s: 3.1.1, 3.1.2
    • Fix Version/s: 3.1.3, 3.2 M2
    • Component/s: Web
    • Labels:
    • Last commented by a User:
      true

      Description

      Background

      Class org.springframework.web.portlet.mvc.annotation.DefaultAnnotationHandlerMapping is using PortletMappingPredicate}}s. These predicates are used to compare priorities of different mapping (e.g. mapping with attributes has a higher precedence than the one without parameters) as they support {{java.lang.Comparable interface. When selecting appropriate handler AbstractMapBasedHandlerMapping#getHandlerInternal is sorting the predicates (Collections.sort).

      Example Implementation

      I can have handler class as follows:

      Foo.java
      @RequestMapping("view")
      public class Foo {
      
          @RenderMapping()
          public String renderBar() {
              // ...
          }
      
          @ActionMapping("xyz")
          public void processXyz() {
              // ...
          }
      
          @RenderMapping(params="page=baz")
          public String renderBaz() {
              // ...
          }
      
      }
      

      If I make request with parameter page=baz, the #renderBaz() method should be invoked. This might or might not happen (explanation bellow).

      Predicate Comparison

      Registering the example handler above (in DefaultAnnotationHandlerMapping) will result in creating three handler mapping predicates:

      1. RenderMappingPredicate for renderBar without parameters
      2. ActionMappingPredicate for processXyz without parameters
      3. RenderMappingPredicate for renderBaz with parameters

      Now what happens in AbstractMapBasedHandlerMapping#getHandlerInternal is that the list with these predicates gets sorted and the first one matching the request is picked. The problem is that when they have exactly this order they are not sorted at all.

      Comparison In Detail

      This is compareTo() method implementation from RenderMappingPredicate:

      org.springframework.web.portlet.mvc.annotation.DefaultAnnotationHandlerMapping.java$RenderMappingPredicate#compareTo
      public int compareTo(Object other) {
          if (other instanceof TypeLevelMappingPredicate) {
              return 1;
          }
          else if (other instanceof RenderMappingPredicate) {
              RenderMappingPredicate otherRender = (RenderMappingPredicate) other;
              boolean hasWindowState = "".equals(this.windowState);
              boolean otherHasWindowState = "".equals(otherRender.windowState);
              if (hasWindowState != otherHasWindowState) {
                  return (hasWindowState ? -1 : 1);
              }
              else {
                  return new Integer(otherRender.params.length).compareTo(this.params.length);
              }
          }
          return (other instanceof SpecialRequestTypePredicate ? 0 : -1);
      }
      

      The code in ActionMappingPredicate is almost identical. If you check this code against the predicates from the example you will get following comparisons:

      • predicates[0].compareTo(predicates[1]) = 0 (1st RenderMappingPredicate vs ActionMappingPredicate)
      • predicates[1].compareTo(predicates[2]) = 0 (ActionMappingPredicate vs 2nd RenderMappingPredicate)

      The problem this issue is about is:

      • predicates[0].compareTo(predicates[2]) = 1 (1st RenderMappingPredicate vs 2nd RenderMappingPredicate)

      Where Is The Issue?

      The issue in comparison implementation is simple - parameter length should be compared first, regardles of the predicate type (you can see that the current implementation compares parameter length only in case predicates are of the same type).

        Issue Links

          Activity

          Hide
          Pavel Horal added a comment -

          This issue should have CRITICAL priority!

          Show
          Pavel Horal added a comment - This issue should have CRITICAL priority!
          Hide
          Pavel Horal added a comment -

          Test case showing the incorrect behavior.

          Show
          Pavel Horal added a comment - Test case showing the incorrect behavior.
          Hide
          Pavel Horal added a comment -

          While writing the test case I find out, that this issue exists only when using two handlers/controllers without (or the same) TypeLevelPredicate. Having such configuration is AFAIK not prohibited, but it is definitely not common practice. Based on this fact the priority of this issue is not CRITICAL, but probably MINOR as it is now.

          Show
          Pavel Horal added a comment - While writing the test case I find out, that this issue exists only when using two handlers/controllers without (or the same) TypeLevelPredicate. Having such configuration is AFAIK not prohibited, but it is definitely not common practice. Based on this fact the priority of this issue is not CRITICAL, but probably MINOR as it is now.
          Hide
          Pavel Horal added a comment -

          Just found similar issue SPR-7685 - it is using mapping accross two handlers without TypeLevelPredicate. What I've described here will also affect that case as well.

          Show
          Pavel Horal added a comment - Just found similar issue SPR-7685 - it is using mapping accross two handlers without TypeLevelPredicate. What I've described here will also affect that case as well.

            People

            • Assignee:
              Juergen Hoeller
              Reporter:
              Pavel Horal
              Last updater:
              Chris Beams
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:
                Days since last comment:
                2 years, 2 weeks, 5 days ago