Wicket
  1. Wicket
  2. WICKET-3551

Using web.xml <error-page> to render error pages via Wicket yields undesired behavior in Wicket 1.5

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.5-RC2
    • Fix Version/s: 1.5-RC3
    • Component/s: wicket
    • Labels:
      None

      Description

      In my applications I prefer to use Wicket to render all error pages, including my 404 "not found" page. This allows me to reuse a shared page template afforded by Wicket to create my error pages (i.e. by extending a BasePage), rather than using copy and paste to maintain static error page HTML files.

      I have been doing this as follows:

      web.xml:

      <error-page>
      <error-code>404</error-code>
      <location>/error/404</location>
      </error-page>

      Now, so long as I have an appropriate bookmarkable page mounted on the "/error/404" path, the servlet container will render my page whenever a 404 "not found" scenario is encountered.

      This works great in Wicket 1.4.x.

      However in Wicket 1.5 (RC2 and the latest SNAPSHOT as of this writing), two problems occur:

      1. By default, Wicket 1.5 automatically performs a 302 redirect before rendering the error page. This changes the URL from the invalid one (i.e. the one that generated the 404) to the mounted path (/error/404 in this example). This is not the desired behavior for a 404 error page; the original URL requested by the user should be maintained.

      2. If I attempt to work around the issue by overriding WebPageRenderer.enableRedirectForStatelessPage() to return false, the problem gets worse. Now the URL doesn't change, which is good. But Wicket gets confused about the depth of the request URL path: it seems to calculate relative URLs based on the mounted path rather than the URL that was requested. This causes all relative resources on the page (e.g. stylesheet references) to be miscalculated and break.

      I will attach two quickstarts: one showing this configuration working in 1.4, and another showing the same setup failing as described above in 1.5-RC2.

      1. WICKET-3551-wicket-1.5-RC2.tgz
        18 kB
        Matt Brictson
      2. WICKET-3551-wicket-1.4.tgz
        5 kB
        Matt Brictson

        Issue Links

          Activity

          Matt Brictson created issue -
          Matt Brictson made changes -
          Field Original Value New Value
          Attachment WICKET-3551-wicket-1.4.tgz [ 12474421 ]
          Attachment WICKET-3551-wicket-1.5-RC2.tgz [ 12474422 ]
          Hide
          Martin Grigorov added a comment -

          With trunk if I request http://localhost:8080/aaaa I still stay on that url and I see as content:

          • Note how the URL has changed to http://localhost:8080/error/404 (via a 302 redirect). This behavior is not desired. A 404 error page should render without changing the URL that was originally requested, as can be seen in this example.
          • If the application is reconfigured by overriding WebPageRenderer.enableRedirectForStatelessPage() to return false, this solves redirect problem. However, it introduces a new, more serious issue: Wicket is now confused about the depth of the URL and miscalculates the relative paths of resources on the page. For example, the stylesheet reference is now incorrect and doesn't load.

          I.e. I can't reproduce it here.

          Show
          Martin Grigorov added a comment - With trunk if I request http://localhost:8080/aaaa I still stay on that url and I see as content: Note how the URL has changed to http://localhost:8080/error/404 (via a 302 redirect). This behavior is not desired. A 404 error page should render without changing the URL that was originally requested, as can be seen in this example. If the application is reconfigured by overriding WebPageRenderer.enableRedirectForStatelessPage() to return false, this solves redirect problem. However, it introduces a new, more serious issue: Wicket is now confused about the depth of the URL and miscalculates the relative paths of resources on the page. For example, the stylesheet reference is now incorrect and doesn't load. I.e. I can't reproduce it here.
          Hide
          Martin Grigorov added a comment -

          I just tried the following optimization:

          — wicket-core/src/main/java/org/apache/wicket/request/handler/RenderPageRequestHandler.java (revision 1084741)
          +++ wicket-core/src/main/java/org/apache/wicket/request/handler/RenderPageRequestHandler.java (working copy)
          @@ -74,9 +74,22 @@
          */
          public RenderPageRequestHandler(IPageProvider pageProvider)

          { - this(pageProvider, RedirectPolicy.AUTO_REDIRECT); + this(pageProvider, computePolicy(pageProvider)); }

          + private static RedirectPolicy computePolicy(IPageProvider pageProvider2)
          + {
          + if (pageProvider2.isNewPageInstance() == false &&
          + pageProvider2.getPageInstance().isPageStateless())
          +

          { + return RedirectPolicy.NEVER_REDIRECT; + }

          + else
          +

          { + return RedirectPolicy.AUTO_REDIRECT; + }

          + }
          +

          but now org.apache.wicket.markup.html.internal.EnclosureTest.testRender10() fails. I have no time to debug what's wrong with it,

          Show
          Martin Grigorov added a comment - I just tried the following optimization: — wicket-core/src/main/java/org/apache/wicket/request/handler/RenderPageRequestHandler.java (revision 1084741) +++ wicket-core/src/main/java/org/apache/wicket/request/handler/RenderPageRequestHandler.java (working copy) @@ -74,9 +74,22 @@ */ public RenderPageRequestHandler(IPageProvider pageProvider) { - this(pageProvider, RedirectPolicy.AUTO_REDIRECT); + this(pageProvider, computePolicy(pageProvider)); } + private static RedirectPolicy computePolicy(IPageProvider pageProvider2) + { + if (pageProvider2.isNewPageInstance() == false && + pageProvider2.getPageInstance().isPageStateless()) + { + return RedirectPolicy.NEVER_REDIRECT; + } + else + { + return RedirectPolicy.AUTO_REDIRECT; + } + } + but now org.apache.wicket.markup.html.internal.EnclosureTest.testRender10() fails. I have no time to debug what's wrong with it,
          Hide
          Martin Grigorov added a comment -

          I think I just realize - initially the page is stateless but during render it becomes stateful and bevause of NEVER_REDIRECT we render the wrong markup.

          Show
          Martin Grigorov added a comment - I think I just realize - initially the page is stateless but during render it becomes stateful and bevause of NEVER_REDIRECT we render the wrong markup.
          Hide
          Matt Brictson added a comment -

          Some further thoughts on this issue:

          1. The automatic redirect policy in Wicket 1.5 in general is a good thing. But it is undesirable in this exceptional case. Is there an easy way to disable it for my error pages only, without writing a custom PageRenderer? Perhaps a flag I could override on the Page itself? (E.g. disable redirect if Page.isErrorPage()==true?)

          2. I think there is a subtle bug in how Wicket calculates relative paths that has been uncovered in this exceptional case. My understanding is that when a request yields a 404 error, the servlet container forwards the original request to the path defined in the <error-page> declaration. This forwarded request is handled by Wicket, and the relative paths are calculated by Wicket based on the forwarded path. But the user's browser URL still shows the original path. Wicket needs to be aware that the request was forwarded and unwrap it to get the path of the original request in order to calculate the proper relative paths.

          Perhaps serving <error-page> via Wicket is too problematic and should be discouraged? Do you think I going against the grain of the framework by attempting this?

          Show
          Matt Brictson added a comment - Some further thoughts on this issue: 1. The automatic redirect policy in Wicket 1.5 in general is a good thing. But it is undesirable in this exceptional case. Is there an easy way to disable it for my error pages only, without writing a custom PageRenderer? Perhaps a flag I could override on the Page itself? (E.g. disable redirect if Page.isErrorPage()==true?) 2. I think there is a subtle bug in how Wicket calculates relative paths that has been uncovered in this exceptional case. My understanding is that when a request yields a 404 error, the servlet container forwards the original request to the path defined in the <error-page> declaration. This forwarded request is handled by Wicket, and the relative paths are calculated by Wicket based on the forwarded path. But the user's browser URL still shows the original path. Wicket needs to be aware that the request was forwarded and unwrap it to get the path of the original request in order to calculate the proper relative paths. Perhaps serving <error-page> via Wicket is too problematic and should be discouraged? Do you think I going against the grain of the framework by attempting this?
          Hide
          Igor Vaynberg added a comment -

          i believe the framework should support this. the redirect policy has to be smart enough to check if the request is an error request or not, and if it is switch over to never_redirect.

          as far as relative paths being calculated correctly, in 1.4 there is a workaround for that that may need to be ported to 1.5. martin, if you have time search the 1.4 codebase for "javax.servlet.error.request_uri", i remember writing code that fixed relative paths for error urls there. ServletWebRequest i believe.

          Show
          Igor Vaynberg added a comment - i believe the framework should support this. the redirect policy has to be smart enough to check if the request is an error request or not, and if it is switch over to never_redirect. as far as relative paths being calculated correctly, in 1.4 there is a workaround for that that may need to be ported to 1.5. martin, if you have time search the 1.4 codebase for "javax.servlet.error.request_uri", i remember writing code that fixed relative paths for error urls there. ServletWebRequest i believe.
          Martin Grigorov made changes -
          Assignee Martin Grigorov [ mgrigorov ]
          Hide
          Martin Grigorov added a comment -

          The only problem here is that I cannot reproduce the problem with current trunk ...

          Show
          Martin Grigorov added a comment - The only problem here is that I cannot reproduce the problem with current trunk ...
          Igor Vaynberg made changes -
          Assignee Martin Grigorov [ mgrigorov ] Igor Vaynberg [ ivaynberg ]
          Igor Vaynberg made changes -
          Status Open [ 1 ] Resolved [ 5 ]
          Fix Version/s 1.5-RC3 [ 12316220 ]
          Resolution Fixed [ 1 ]
          Pedro Santos made changes -
          Link This issue relates to WICKET-4168 [ WICKET-4168 ]
          Sven Meier made changes -
          Link This issue is related to WICKET-5486 [ WICKET-5486 ]

            People

            • Assignee:
              Igor Vaynberg
              Reporter:
              Matt Brictson
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development