Wicket
  1. Wicket
  2. WICKET-4387

StringIndexOutOfBoundsException when forwarding requests

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 1.5.4
    • Fix Version/s: 1.5.5, 1.5.7, 6.0.0-beta1
    • Component/s: wicket
    • Labels:
      None

      Description

      We're getting StringIndexOutOfBoundsException from wicket when forwarding a request from our servlet filter (using request dispatcher) to wicket.
      The problem occurs whenever the original URI is shorter than the wicket filter mapping.
      I created an example webapp (based on the quickstart) in which a ForwardFilter is mapped to /f/* and it forwards all the requests to /wicket/ (see web.xml snippet below).
      With this webapp a request to "http://localhost:8081/wicket/f/" results in the following exception:

      ERROR - RequestCycle               - Error during processing error message
      java.lang.StringIndexOutOfBoundsException: String index out of range: -5
              at java.lang.String.substring(String.java:1958)
              at java.lang.String.substring(String.java:1925)
              at org.apache.wicket.protocol.http.servlet.ServletWebRequest.getContextRelativeUrl(ServletWebRequest.java:180)
              at org.apache.wicket.protocol.http.servlet.ServletWebRequest.getClientUrl(ServletWebRequest.java:140)
              at org.apache.wicket.request.UrlRenderer.<init>(UrlRenderer.java:59)
              at org.apache.wicket.request.cycle.RequestCycle.newUrlRenderer(RequestCycle.java:148)
              at org.apache.wicket.request.cycle.RequestCycle.getUrlRenderer(RequestCycle.java:172)
              at org.apache.wicket.request.handler.render.WebPageRenderer.respond(WebPageRenderer.java:145)
              at org.apache.wicket.request.handler.RenderPageRequestHandler.respond(RenderPageRequestHandler.java:167)
              at org.apache.wicket.request.cycle.RequestCycle$HandlerExecutor.respond(RequestCycle.java:781)
              at org.apache.wicket.request.RequestHandlerStack.execute(RequestHandlerStack.java:64)
              at org.apache.wicket.request.cycle.RequestCycle.executeExceptionRequestHandler(RequestCycle.java:304)
              at org.apache.wicket.request.cycle.RequestCycle.executeExceptionRequestHandler(RequestCycle.java:313)
              at org.apache.wicket.request.cycle.RequestCycle.executeExceptionRequestHandler(RequestCycle.java:313)
              at org.apache.wicket.request.cycle.RequestCycle.executeExceptionRequestHandler(RequestCycle.java:313)
              at org.apache.wicket.request.cycle.RequestCycle.executeExceptionRequestHandler(RequestCycle.java:313)
              at org.apache.wicket.request.cycle.RequestCycle.executeExceptionRequestHandler(RequestCycle.java:313)
              at org.apache.wicket.request.cycle.RequestCycle.executeExceptionRequestHandler(RequestCycle.java:313)
              at org.apache.wicket.request.cycle.RequestCycle.executeExceptionRequestHandler(RequestCycle.java:313)
              at org.apache.wicket.request.cycle.RequestCycle.executeExceptionRequestHandler(RequestCycle.java:313)
              at org.apache.wicket.request.cycle.RequestCycle.executeExceptionRequestHandler(RequestCycle.java:313)
              at org.apache.wicket.request.cycle.RequestCycle.executeExceptionRequestHandler(RequestCycle.java:313)
              at org.apache.wicket.request.cycle.RequestCycle.processRequest(RequestCycle.java:227)
              at org.apache.wicket.request.cycle.RequestCycle.processRequestAndDetach(RequestCycle.java:283)
              at org.apache.wicket.protocol.http.WicketFilter.processRequest(WicketFilter.java:162)
              at org.apache.wicket.protocol.http.WicketFilter.doFilter(WicketFilter.java:218)
              at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:243)
              at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:210)
              at org.apache.catalina.core.ApplicationDispatcher.invoke(ApplicationDispatcher.java:684)
              at org.apache.catalina.core.ApplicationDispatcher.processRequest(ApplicationDispatcher.java:471)
              at org.apache.catalina.core.ApplicationDispatcher.doForward(ApplicationDispatcher.java:402)
              at org.apache.catalina.core.ApplicationDispatcher.forward(ApplicationDispatcher.java:329)
              at org.jfrog.ForwardFilter.doFilter(ForwardFilter.java:22)
              at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:243)
              at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:210)
              at org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:240)
              at org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:164)
              at org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:498)
              at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:164)
              at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:100)
              at org.apache.catalina.valves.AccessLogValve.invoke(AccessLogValve.java:562)
              at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:118)
              at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:394)
              at org.apache.coyote.http11.Http11AprProcessor.process(Http11AprProcessor.java:284)
              at org.apache.coyote.http11.Http11AprProtocol$Http11ConnectionHandler.process(Http11AprProtocol.java:322)
              at org.apache.tomcat.util.net.AprEndpoint$SocketProcessor.run(AprEndpoint.java:1714)
              at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1110)
              at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:603)
              at java.lang.Thread.run(Thread.java:722)
      
      web.xml snippet
      	<filter>
      		<filter-name>forward</filter-name>
      		<filter-class>org.jfrog.ForwardFilter</filter-class>
      	</filter>
      
      	<filter>
      		<filter-name>wicket.wicket</filter-name>
      		<filter-class>org.apache.wicket.protocol.http.WicketFilter</filter-class>
      		<init-param>
      			<param-name>applicationClassName</param-name>
      			<param-value>org.jfrog.WicketApplication</param-value>
      		</init-param>
      	</filter>
      
      	<filter-mapping>
      		<filter-name>forward</filter-name>
      		<url-pattern>/f/*</url-pattern>
      	</filter-mapping>
      
      	<filter-mapping>
      		<filter-name>wicket.wicket</filter-name>
      		<url-pattern>/wicket/*</url-pattern>
              <dispatcher>FORWARD</dispatcher>
              <dispatcher>REQUEST</dispatcher>
      	</filter-mapping>
      

      This bug is opened here following a bug we found in Artifactory after upgrading to Wicket 1.5

      1. wicket.zip
        23 kB
        Yossi Shaul
      2. wicket4387.zip
        23 kB
        Maarten Bosteels
      3. TEST-org.apache.wicket.markup.html.border.ComponentBorderTest.xml
        8 kB
        Darryl L. Miles

        Issue Links

          Activity

          Hide
          Yossi Shaul added a comment -

          Attaching a quickstart that reproduce this error. Tested on Tomcat.

          Show
          Yossi Shaul added a comment - Attaching a quickstart that reproduce this error. Tested on Tomcat.
          Hide
          Yossi Shaul added a comment -

          The relative URLs (when not getting out of bound exception) is not correct.

          Show
          Yossi Shaul added a comment - The relative URLs (when not getting out of bound exception) is not correct.
          Hide
          qxo added a comment -

          fix patch in org.apache.wicket.protocol.http.servlet.ServletWebRequest:

          	private Url getContextRelativeUrl(String uri, String filterPrefix)
          	{
          		if (filterPrefix.length() > 0 && !filterPrefix.endsWith("/"))
          		{
          			filterPrefix += "/";
          		}
          		StringBuilder url = new StringBuilder();
          		uri = Strings.stripJSessionId(uri);
          		final int start = httpServletRequest.getContextPath().length() + filterPrefix.length() + 1;
          		url.append(uri.length()<start ? uri: uri.substring(start)); //for WICKET-4387
          
          		if (errorAttributes == null)
          		{
          			String query = httpServletRequest.getQueryString();
          			if (!Strings.isEmpty(query))
          			{
          				url.append('?');
          				url.append(query);
          			}
          		}
          
          		return setParameters(Url.parse(url.toString(), getCharset()));
          	}
          
          Show
          qxo added a comment - fix patch in org.apache.wicket.protocol.http.servlet.ServletWebRequest: private Url getContextRelativeUrl( String uri, String filterPrefix) { if (filterPrefix.length() > 0 && !filterPrefix.endsWith( "/" )) { filterPrefix += "/" ; } StringBuilder url = new StringBuilder(); uri = Strings.stripJSessionId(uri); final int start = httpServletRequest.getContextPath().length() + filterPrefix.length() + 1; url.append(uri.length()<start ? uri: uri.substring(start)); // for WICKET-4387 if (errorAttributes == null ) { String query = httpServletRequest.getQueryString(); if (!Strings.isEmpty(query)) { url.append('?'); url.append(query); } } return setParameters(Url.parse(url.toString(), getCharset())); }
          Hide
          Al Thompson added a comment -

          Here's the patch I applied to 1.5.3. Functionally equivalent to qxo's but perhaps slightly more readable:

          180c181,186
          < url.append(uri.substring(start));

          > if(start > uri.length())

          { > url.append(uri); > }

          else

          { > url.append(uri.substring(start)); > }
          Show
          Al Thompson added a comment - Here's the patch I applied to 1.5.3. Functionally equivalent to qxo's but perhaps slightly more readable: 180c181,186 < url.append(uri.substring(start)); — > if(start > uri.length()) { > url.append(uri); > } else { > url.append(uri.substring(start)); > }
          Hide
          Al Thompson added a comment -

          Slight line numbering fixup:

          org/apache/wicket/protocol/http/servlet/ServletWebRequest.java
          180c181,185
          < url.append(uri.substring(start));

          > if(start > uri.length())

          { > url.append(uri); > }

          else

          { > url.append(uri.substring(start)); > }
          Show
          Al Thompson added a comment - Slight line numbering fixup: org/apache/wicket/protocol/http/servlet/ServletWebRequest.java 180c181,185 < url.append(uri.substring(start)); — > if(start > uri.length()) { > url.append(uri); > } else { > url.append(uri.substring(start)); > }
          Hide
          Martin Grigorov added a comment -

          Please test with latest -SNAPSHOT the applied fix. Thanks!

          Show
          Martin Grigorov added a comment - Please test with latest -SNAPSHOT the applied fix. Thanks!
          Hide
          Martin Grigorov added a comment -

          Hi,

          Can you confirm that the fix in 1.5.5 solves the problem for you?

          Show
          Martin Grigorov added a comment - Hi, Can you confirm that the fix in 1.5.5 solves the problem for you?
          Hide
          Maarten Bosteels added a comment -

          We were bitten by this bug as well.
          We are using wicket 1.5.5
          I was able to reproduce the issue in 1.5.5, 1.5.6 and 6.0-SNAPSHOT with the attached quickstart.

          Only changes to a stock quickstart:

          • add ERROR and REQUEST dispatchers in web.xml
          • add an <error-page> tag in web.xml
          • mount a notfoundpage
          • do not use / as context path in Start.java
          Show
          Maarten Bosteels added a comment - We were bitten by this bug as well. We are using wicket 1.5.5 I was able to reproduce the issue in 1.5.5, 1.5.6 and 6.0-SNAPSHOT with the attached quickstart. Only changes to a stock quickstart: add ERROR and REQUEST dispatchers in web.xml add an <error-page> tag in web.xml mount a notfoundpage do not use / as context path in Start.java
          Hide
          Darryl L. Miles added a comment - - edited

          I think the bug is that +1 is added to the data.

          // The inputs I am able to observe from browser URL http://127.0.0.1:8080/myapp/foobar/
          String contextPath="/myapp"
          String filterPrefix="/foobar/"
          String uri="/myapp/foobar/"

          // From Wicket 1.5.6 source ServletWebRequest:201
          final int start = contextPath.length() + filterPrefix.length() + 1; // 6 + 8 + 1 = 15
          url.append(uri.substring(start)); // StringIndexOutOfBoundsException("String index out of range: -1")

          Looking at the code I can not see how adding +1 (in the computation of 'start') is useful since it would remove some extra character (that may or may not be present).

          Show
          Darryl L. Miles added a comment - - edited I think the bug is that +1 is added to the data. // The inputs I am able to observe from browser URL http://127.0.0.1:8080/myapp/foobar/ String contextPath="/myapp" String filterPrefix="/foobar/" String uri="/myapp/foobar/" // From Wicket 1.5.6 source ServletWebRequest:201 final int start = contextPath.length() + filterPrefix.length() + 1; // 6 + 8 + 1 = 15 url.append(uri.substring(start)); // StringIndexOutOfBoundsException("String index out of range: -1") Looking at the code I can not see how adding +1 (in the computation of 'start') is useful since it would remove some extra character (that may or may not be present).
          Hide
          Darryl L. Miles added a comment -

          I can confirm that with the inputs:

          String contextPath="/myapp"
          String filterPrefix="/foobar/"
          String uri="/myapp/foobar/abc"

          The output URL becomes "bc" with Wicket 1.5.7 where the leading "a" was chopped off.

          However in removing the "+ 1" addition a large number of unit tests start failing.

          So I can only describe the matter as some kind of "voodoo" in that it is doing a non-obvious thing and at the point of the non-obvious action it is not documented in the source with comments.

          Enclosed a file of the first unit test to fail wicket-core/target/surefire-reports/TEST-org.apache.wicket.markup.html.border.ComponentBorderTest.xml

          Show
          Darryl L. Miles added a comment - I can confirm that with the inputs: String contextPath="/myapp" String filterPrefix="/foobar/" String uri="/myapp/foobar/abc" The output URL becomes "bc" with Wicket 1.5.7 where the leading "a" was chopped off. However in removing the "+ 1" addition a large number of unit tests start failing. So I can only describe the matter as some kind of "voodoo" in that it is doing a non-obvious thing and at the point of the non-obvious action it is not documented in the source with comments. Enclosed a file of the first unit test to fail wicket-core/target/surefire-reports/TEST-org.apache.wicket.markup.html.border.ComponentBorderTest.xml
          Hide
          Darryl L. Miles added a comment - - edited

          First failing unit test information

          ??? where did the file attachment go ??? This comment should have a file attachment if it does not show up anywhere for you please comment back.

          Show
          Darryl L. Miles added a comment - - edited First failing unit test information ??? where did the file attachment go ??? This comment should have a file attachment if it does not show up anywhere for you please comment back.
          Hide
          Igor Vaynberg added a comment -

          i can see it in the Attachments section above...

          Show
          Igor Vaynberg added a comment - i can see it in the Attachments section above...

            People

            • Assignee:
              Martin Grigorov
              Reporter:
              Yossi Shaul
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development