Uploaded image for project: 'Struts 2'
  1. Struts 2
  2. WW-4433

ConventionUnknownHandler change breaks exception handling in interceptors.

    Details

      Description

      Struts 2.3.20 appears to have caused a regression that prevents exceptions thrown from convention-plugin actions from reaching ExceptionMappingInterceptor. This breaks exception handling when using the convention-plugin.

      To Reproduce:

      • Generate a project struts2-archetype-convention archetype using 2.3.20
      • Throw exception in the action. With 2.3.20, a blank page is shown.
      • Change to 2.3.16.3 and you will get the standard struts2 error page.

      The breaking change appears to have been made in WW-4331. This causes error interceptor code to break (showing a blank page when exceptions are thrown) as DefaultActionInvocation does not catch an exception from the default UnknownHandler implementation execution, which would previously re-throw the original exception back up for the interceptors to catch.

      Workaround:

      We've created our own UnknownHandler implementation that just throws a new NoSuchMethodException, allowing DefaultActionInvocation to re-throw the original exception so that our error interceptor can again catch it.

        Issue Links

          Activity

          Hide
          lukaszlenart Lukasz Lenart added a comment -

          Check the docs
          http://struts.apache.org/docs/unknown-handlers.html

          You must implement com.opensymphony.xwork2.UnknownHandler and register your instance as follow

          <bean type="com.opensymphony.xwork2.UnknownHandler" name="handler1" class="com.opensymphony.xwork2.config.providers.SomeUnknownHandler"/>
          

          then reference it in stack

          <unknown-handler-stack>
             <unknown-handler-ref name="handler1" />
          </unknown-handler-stack>
          
          Show
          lukaszlenart Lukasz Lenart added a comment - Check the docs http://struts.apache.org/docs/unknown-handlers.html You must implement com.opensymphony.xwork2.UnknownHandler and register your instance as follow <bean type= "com.opensymphony.xwork2.UnknownHandler" name= "handler1" class= "com.opensymphony.xwork2.config.providers.SomeUnknownHandler" /> then reference it in stack <unknown-handler-stack> <unknown-handler-ref name= "handler1" /> </unknown-handler-stack>
          Hide
          lilibit1985 Marzieh Beiraghdar added a comment -

          Thank you for the update. It fixes the problem. Still i like to know how to register a customize unknown handler.
          Please let me know what i am doing wrong.

          Show
          lilibit1985 Marzieh Beiraghdar added a comment - Thank you for the update. It fixes the problem. Still i like to know how to register a customize unknown handler. Please let me know what i am doing wrong.
          Hide
          lukaszlenart Lukasz Lenart added a comment -

          You can use 2.3.24 version which fixes this problem

          Show
          lukaszlenart Lukasz Lenart added a comment - You can use 2.3.24 version which fixes this problem
          Hide
          lilibit1985 Marzieh Beiraghdar added a comment -

          I have trouble registering customize unknown handler. I have CustomUnknownHandlerManager class which implements com.opensymphony.xwork2.UnknownHandler interface and throw NoSuchMethodException . Also i have added these lines in my struts.xml file:

          <unknown-handler-stack>
          <unknown-handler-ref name="net.iranet.isc.saba.utils.CustomUnknownHandlerManager" />
          </unknown-handler-stack>

          CustomUnknownHandlerManager calss seems not to registered properly. Please help me out.

          Show
          lilibit1985 Marzieh Beiraghdar added a comment - I have trouble registering customize unknown handler. I have CustomUnknownHandlerManager class which implements com.opensymphony.xwork2.UnknownHandler interface and throw NoSuchMethodException . Also i have added these lines in my struts.xml file: <unknown-handler-stack> <unknown-handler-ref name="net.iranet.isc.saba.utils.CustomUnknownHandlerManager" /> </unknown-handler-stack> CustomUnknownHandlerManager calss seems not to registered properly. Please help me out.
          Hide
          lukaszlenart Lukasz Lenart added a comment -

          2.3.23 test version was cancelled as there were some other issues discovered, 2.3.24 will be pushed to test soon.

          Show
          lukaszlenart Lukasz Lenart added a comment - 2.3.23 test version was cancelled as there were some other issues discovered, 2.3.24 will be pushed to test soon.
          Hide
          gmarshall56 Gary M. added a comment -

          <version>2.3.23</version> is not in Maven Central. The most recent I see there is 2.3.20. Am I looking in the wrong place? If not, when will this version be available for use by my maven application? Thank you

          Show
          gmarshall56 Gary M. added a comment - <version>2.3.23</version> is not in Maven Central. The most recent I see there is 2.3.20. Am I looking in the wrong place? If not, when will this version be available for use by my maven application? Thank you
          Hide
          lukaszlenart Lukasz Lenart added a comment - - edited

          Just implement com.opensymphony.xwork2.UnknownHandler interface and throw NoSuchMethodException - that's all and now register the new handler like this http://struts.apache.org/docs/unknown-handlers.html

          BTW. you can help testing 2.3.23 http://markmail.org/thread/oqtssgeesejrobko

          Show
          lukaszlenart Lukasz Lenart added a comment - - edited Just implement com.opensymphony.xwork2.UnknownHandler interface and throw NoSuchMethodException - that's all and now register the new handler like this http://struts.apache.org/docs/unknown-handlers.html BTW. you can help testing 2.3.23 http://markmail.org/thread/oqtssgeesejrobko
          Hide
          afattahi Alireza Fattahi added a comment -

          Me too ! Looking for workaround code !

          Show
          afattahi Alireza Fattahi added a comment - Me too ! Looking for workaround code !
          Hide
          williams_david3 David Williams added a comment -

          ok cool. Thanks!
          Has anybody posted the code for the workaround? I would like to add the workaround to my code and test it.

          Show
          williams_david3 David Williams added a comment - ok cool. Thanks! Has anybody posted the code for the workaround? I would like to add the workaround to my code and test it.
          Hide
          williams_david3 David Williams added a comment -

          ok cool. Thanks!
          Has anybody posted the code for the workaround? I would like to add the workaround to my code and test it.

          Show
          williams_david3 David Williams added a comment - ok cool. Thanks! Has anybody posted the code for the workaround? I would like to add the workaround to my code and test it.
          Hide
          lukaszlenart Lukasz Lenart added a comment -

          2.3.22 was pushed for test but some issues were discovered during test period so this version was dropped and now I'm starting with 2.3.23
          http://markmail.org/thread/i72dhopvlitfoq4w#query:+page:1+mid:nxq7uet4j75mpqpk+state:results

          Show
          lukaszlenart Lukasz Lenart added a comment - 2.3.22 was pushed for test but some issues were discovered during test period so this version was dropped and now I'm starting with 2.3.23 http://markmail.org/thread/i72dhopvlitfoq4w#query:+page:1+mid:nxq7uet4j75mpqpk+state:results
          Hide
          williams_david3 David Williams added a comment -

          Anybody, is there a projected date for the 2.3.22 release?

          Show
          williams_david3 David Williams added a comment - Anybody, is there a projected date for the 2.3.22 release?
          Hide
          lukaszlenart Lukasz Lenart added a comment -

          Alireza Fattahi but fix for this issue is quite simple and it's always better to test even 2.3.20 as you can discover some other problems that won't be addressed in 2.3.21 (because you didn't mention them )

          Show
          lukaszlenart Lukasz Lenart added a comment - Alireza Fattahi but fix for this issue is quite simple and it's always better to test even 2.3.20 as you can discover some other problems that won't be addressed in 2.3.21 (because you didn't mention them )
          Hide
          afattahi Alireza Fattahi added a comment -

          I don't know it should be asked here or not !
          But this bug fix is very important for us, and we could not upgrade until the 2.3.21 is released.
          So, when do you think the 2.3.21 will be release ( and put in maven)

          Thanks!

          Show
          afattahi Alireza Fattahi added a comment - I don't know it should be asked here or not ! But this bug fix is very important for us, and we could not upgrade until the 2.3.21 is released. So, when do you think the 2.3.21 will be release ( and put in maven) Thanks!
          Hide
          lukaszlenart Lukasz Lenart added a comment -

          Solved with

          • 5551a9bb60e86fced2c9cf2850a46f045d6728df
          • 3288096c2b33a35d5cfa18fc75f19d9ad3b7b60f
          • bf7714f7ae648ef9e7e59ae00a9bbf34c746fffb
          • 61ab8137ebbafd33a8c0c96d2c177db3e5267a0d
          • 702738693ce9206f3023903d73094fe1522cb91c
          Show
          lukaszlenart Lukasz Lenart added a comment - Solved with 5551a9bb60e86fced2c9cf2850a46f045d6728df 3288096c2b33a35d5cfa18fc75f19d9ad3b7b60f bf7714f7ae648ef9e7e59ae00a9bbf34c746fffb 61ab8137ebbafd33a8c0c96d2c177db3e5267a0d 702738693ce9206f3023903d73094fe1522cb91c
          Hide
          lukaszlenart Lukasz Lenart added a comment -

          One more case:

          • method doesn't exist but doMethod exists and throws an exception

          I think support for the do prefix should be removed to simplify logic

          Show
          lukaszlenart Lukasz Lenart added a comment - One more case: method doesn't exist but doMethod exists and throws an exception I think support for the do prefix should be removed to simplify logic
          Hide
          lukaszlenart Lukasz Lenart added a comment -

          I identified three use cases here:

          • action method exists
          • action method exists yet throws exception
          • action method doesn't exist

          Is there any other case I have missed?

          Show
          lukaszlenart Lukasz Lenart added a comment - I identified three use cases here: action method exists action method exists yet throws exception action method doesn't exist Is there any other case I have missed?
          Hide
          senen Senen de Diego added a comment - - edited

          I was going to notify the same bug (spring security exceptions not reaching the FilterSecurityInterceptor) but I see it's already detected and diagnosed. I just want to add a suggestion: I think the OgnlException should be passed as a parameter to UnknownHandlerManager.handleUnknownMethod in order to help the handler decide if it does something or not.

          Or better than this:
          The UnknownHandlerManger should only be called if the exception encapsulated in OgnlException is a NoSuchMethodException.

          Show
          senen Senen de Diego added a comment - - edited I was going to notify the same bug (spring security exceptions not reaching the FilterSecurityInterceptor) but I see it's already detected and diagnosed. I just want to add a suggestion: I think the OgnlException should be passed as a parameter to UnknownHandlerManager.handleUnknownMethod in order to help the handler decide if it does something or not. Or better than this: The UnknownHandlerManger should only be called if the exception encapsulated in OgnlException is a NoSuchMethodException.
          Hide
          lukaszlenart Lukasz Lenart added a comment -

          Regarding deprecation you're right - it should be clear like this

               * @deprecated @throws NoSuchMethodException If the method cannot be found should return null instead,
               *                                           don't throw exception as other UnknownHandles won't be invoked
          
          Show
          lukaszlenart Lukasz Lenart added a comment - Regarding deprecation you're right - it should be clear like this * @deprecated @ throws NoSuchMethodException If the method cannot be found should return null instead, * don't throw exception as other UnknownHandles won't be invoked
          Hide
          lukaszlenart Lukasz Lenart added a comment - - edited

          Now when I'm checking the flow I see the problem but it wasn't in UnknownHandler's default implementation - method must return null to allow other UnknownHandlers be invoked

          The problem is in DefaultActionInvocation which should have re-thrown the original exception if after examining all the UnknownHandlers the result is still null.

          // throw the original exception as UnknownHandlers weren't able to handle invocation as well
          if (methodResult == null) {
              throw e;
          }
          
          Show
          lukaszlenart Lukasz Lenart added a comment - - edited Now when I'm checking the flow I see the problem but it wasn't in UnknownHandler 's default implementation - method must return null to allow other UnknownHandlers be invoked The problem is in DefaultActionInvocation which should have re-thrown the original exception if after examining all the UnknownHandlers the result is still null. // throw the original exception as UnknownHandlers weren't able to handle invocation as well if (methodResult == null ) { throw e; }
          Hide
          jwolschon Joseph Wolschon added a comment -

          An improper deprecation was also added to the interface that makes it unclear if this is an intentional deprecation to UnknownHandler implementation or a change to default Struts behavior: https://fisheye6.atlassian.com/browse/struts/xwork-core/src/main/java/com/opensymphony/xwork2/UnknownHandler.java?r1=0c543aef318341ca9bd482e15f1637497b8a4dfd&r2=ff100ce2d9c939e9cf4867bc46966a7dfecb6770.

          Show
          jwolschon Joseph Wolschon added a comment - An improper deprecation was also added to the interface that makes it unclear if this is an intentional deprecation to UnknownHandler implementation or a change to default Struts behavior: https://fisheye6.atlassian.com/browse/struts/xwork-core/src/main/java/com/opensymphony/xwork2/UnknownHandler.java?r1=0c543aef318341ca9bd482e15f1637497b8a4dfd&r2=ff100ce2d9c939e9cf4867bc46966a7dfecb6770 .

            People

            • Assignee:
              lukaszlenart Lukasz Lenart
              Reporter:
              jwolschon Joseph Wolschon
            • Votes:
              2 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development