[jira] [Commented] (WICKET-4803) Unwrapped IllegalArgumentException in WicketURLDecoder

["Martin Grigorov (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/WICKET-4803?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13476867#comment-13476867 ] 

Martin Grigorov commented on WICKET-4803:
-----------------------------------------

There is a problem with your test. It expects that both WicketRuntimeException is being thrown and that InternalErrorPage is rendered. Those are mutual exclusive.

Some details:

The stack trace is:
ain@1, prio=5, in group 'main', status: 'RUNNING'
	  at org.apache.wicket.request.UrlDecoder.decode(UrlDecoder.java:89)
	  at org.apache.wicket.request.UrlDecoder.decode(UrlDecoder.java:76)
	  at org.apache.wicket.request.Url.decodeParameter(Url.java:601)
	  at org.apache.wicket.request.Url.parseQueryParameter(Url.java:108)
	  at org.apache.wicket.request.Url.parse(Url.java:243)
	  at org.apache.wicket.util.tester.BaseWicketTester.executeUrl(BaseWicketTester.java:2675)
	  at be.dns.wicket.TestHomePage.homepageRendersSuccessfullyPageParamWithPercentSignIncorrectUsage(TestHomePage.java:48)
.....

WicketRuntimeException is in wicket-core.jar. In the stack trace the only class from -core is BaseWicketTester. Url and UrlDecoder are in wicket-request which do not see WicketRuntimeException.
Even re-working BaseWicketTester to handle this it will be a change in the behavior which will affect many applications. Currently they may expect that an exception is being thrown in some erroneous case but with this change there wont be an exception anymore but normal return with lastRenderedPage == InternalErrorPage.
I agree that InternalErrorPage is more correct but it is a bit late to change this behavior.
                
> Unwrapped IllegalArgumentException in WicketURLDecoder
> ------------------------------------------------------
>
>                 Key: WICKET-4803
>                 URL: https://issues.apache.org/jira/browse/WICKET-4803
>             Project: Wicket
>          Issue Type: Bug
>          Components: wicket
>    Affects Versions: 1.5.8
>            Reporter: Johan Heylen
>            Priority: Minor
>         Attachments: WICKET-4803-quickstart-testcase-and-example.zip
>
>
> In the class: org.apache.wicket.protocol.http.WicketURLDecoder there are two IllegalArgumentException which should be wrapped in WicketRuntimeException, otherwise they are caught by the exception handler form the servlet container (jetty, tomcat, ...) which then uses their http 500 error code configuration instead of the exception handling of wicket.
> Wrapping them would be good for consistency and help manage runtime exceptions. 
> These are the two exceptions:
> throw new IllegalArgumentException("URLDecoder: Incomplete trailing escape (%) pattern"); 
> throw new IllegalArgumentException("URLDecoder: Illegal hex characters in escape (%) pattern - " + e.getMessage()); 

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira