Wicket
  1. Wicket
  2. WICKET-4358

BufferedWebResponse fails to add/clear cookie in redirect

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.5.4
    • Fix Version/s: 1.5.8, 6.0.0-beta3
    • Component/s: wicket
    • Labels:
    • Environment:
      Firefox 9

      Description

      bufferedWebResponse.addCookie( cookie );

      That fails under certain conditions: (1) when called on the last of three 302 redirects during OpenID login; and (2) on single redirect immediately after container startup, though it later recovers. Failure confirmed in Firebug; no cookies sent in any of the response headers. My workaround is to bypass the buffered response. This works:

      ((HttpServletResponse)bufferedWebResponse.getContainerResponse()).addCookie( cookie );

      1. WICKET-4358.patch
        2 kB
        Martin Grigorov
      2. wicket-4358.zip
        28 kB
        Bertrand Guay-Paquet
      3. wicket-bug-4358.tar.bz2
        38 kB
        Michael Allan

        Activity

        Hide
        Michael Allan added a comment -

        Believe it or not, I can no longer duplicate the fault unless I apply the patch. I can no longer duplicate it in my original quickstart (same jars, same Tomcat), nor in my current code. But if I apply the patch, then it reappears in my current code (but not in the quickstart). This is so strange, that I actually opened up the jars to double check. With my current Wicket (1.5.4) or with a newly rebuilt 1.5.4 from your repo (class files identical) it's OK. With a patched 1.5.4 (class file different), it reverts to the original behaviour. The original workaround continues to work in this case.

        Please don't spend more time on this. If I'm the only one affected, then the workaround will suffice for me.

        Show
        Michael Allan added a comment - Believe it or not, I can no longer duplicate the fault unless I apply the patch. I can no longer duplicate it in my original quickstart (same jars, same Tomcat), nor in my current code. But if I apply the patch, then it reappears in my current code (but not in the quickstart). This is so strange, that I actually opened up the jars to double check. With my current Wicket (1.5.4) or with a newly rebuilt 1.5.4 from your repo (class files identical) it's OK. With a patched 1.5.4 (class file different), it reverts to the original behaviour. The original workaround continues to work in this case. Please don't spend more time on this. If I'm the only one affected, then the workaround will suffice for me.
        Hide
        Bertrand Guay-Paquet added a comment -

        I confirm that it works with the patch

        Show
        Bertrand Guay-Paquet added a comment - I confirm that it works with the patch
        Hide
        Martin Grigorov added a comment -

        Fix for the Bertrand's quickstart is applied.
        Michael Allan's code is too hard to follow and debug.
        @Michael: please verify that the fix solves your problem.

        Show
        Martin Grigorov added a comment - Fix for the Bertrand's quickstart is applied. Michael Allan's code is too hard to follow and debug. @Michael: please verify that the fix solves your problem.
        Hide
        Martin Grigorov added a comment -

        Here is a possible patch.

        Show
        Martin Grigorov added a comment - Here is a possible patch.
        Hide
        Bertrand Guay-Paquet added a comment -

        Quickstart

        Show
        Bertrand Guay-Paquet added a comment - Quickstart
        Hide
        Bertrand Guay-Paquet added a comment -

        I stumbled upon what I think is the same bug and built a very simple quickstart reproducing the issue with Wicket 1.5.7.

        Steps to reproduce:
        1-Browse to localhost:8080
        2-The label should read "Cookie is NOT set"; if not, click on "clear cookie"
        3-Click on "Set cookie via link handler"
        4-Click on "Set cookie"
        5-The label should now read "Cookie is set"
        6-Click on "clear cookie"
        7-Clich on "Set cookie via bookmarkable link (no prompt)"
        8-The label should read "Cookie is NOT set"

        Both "cookie-setting" pages use the same code:
        HomePage.setCookie(HomePage.COOKIE_KEY, "setResponsePage");
        setResponsePage(HomePage.class);

        However, only the first page works. The case that works is when setResponsePage is called from within a ListenerInterfaceRequestHandler. The case that doesn't is when setResponsePage is called from within a RenderPageRequestHandler.

        The problem seems to be around BufferedWebResponse#renderPage(Url targetUrl, RequestCycle requestCycle) line 107.

        Show
        Bertrand Guay-Paquet added a comment - I stumbled upon what I think is the same bug and built a very simple quickstart reproducing the issue with Wicket 1.5.7. Steps to reproduce: 1-Browse to localhost:8080 2-The label should read "Cookie is NOT set"; if not, click on "clear cookie" 3-Click on "Set cookie via link handler" 4-Click on "Set cookie" 5-The label should now read "Cookie is set" 6-Click on "clear cookie" 7-Clich on "Set cookie via bookmarkable link (no prompt)" 8-The label should read "Cookie is NOT set" Both "cookie-setting" pages use the same code: HomePage.setCookie(HomePage.COOKIE_KEY, "setResponsePage"); setResponsePage(HomePage.class); However, only the first page works. The case that works is when setResponsePage is called from within a ListenerInterfaceRequestHandler. The case that doesn't is when setResponsePage is called from within a RenderPageRequestHandler. The problem seems to be around BufferedWebResponse#renderPage(Url targetUrl, RequestCycle requestCycle) line 107.
        Hide
        Michael Allan added a comment -

        Unfortunately OpenID login is complicated by lots of redirects and that kind of complexity appears to be essential to the bug. The response to the third and final 302 redirect (which you can see in Firebug) is supposed to have two cookies. They are added on lines 129 and 133 of votorola.a.web.wic.authen.WP_Login. The redirect itself is invoked on line 102.

        Are you able to confirm that it works with the workaround and fails without it? If so, then it should be easy for whoever you assign to trace the fault in BufferedWebResponse. HttpServletResponse handles the cookies OK, but BufferedWebResponse is dropping them.

        A possible clue: when the cookies are passed to BufferedWebResponse there is a mysterious EXTRA call to BufferedWebResponse.reset(). See my reply of Jan 31. This call does NOT occur when the workaround is in place, i.e. when the cookies are passed directly to HttpServletResponse. Maybe it's normal for the buffer to be flushed if cookies are added and not otherwise, but I think the problem is that it's being flushed before the cookies are successfully handed to HttpServletResponse. Else where did the cookies go?

        Show
        Michael Allan added a comment - Unfortunately OpenID login is complicated by lots of redirects and that kind of complexity appears to be essential to the bug. The response to the third and final 302 redirect (which you can see in Firebug) is supposed to have two cookies. They are added on lines 129 and 133 of votorola.a.web.wic.authen.WP_Login. The redirect itself is invoked on line 102. Are you able to confirm that it works with the workaround and fails without it? If so, then it should be easy for whoever you assign to trace the fault in BufferedWebResponse. HttpServletResponse handles the cookies OK, but BufferedWebResponse is dropping them. A possible clue: when the cookies are passed to BufferedWebResponse there is a mysterious EXTRA call to BufferedWebResponse.reset(). See my reply of Jan 31. This call does NOT occur when the workaround is in place, i.e. when the cookies are passed directly to HttpServletResponse. Maybe it's normal for the buffer to be flushed if cookies are added and not otherwise, but I think the problem is that it's being flushed before the cookies are successfully handed to HttpServletResponse. Else where did the cookies go?
        Hide
        Martin Grigorov added a comment -

        Sorry, your quickstart is too complicated and takes me too much time to analyze what you are doing.
        If you can reproduce it with less code (Wicket quickstart + what is needed to reproduce) then I'll take a look again. Otherwise I'll leave it to someone else to debug this application.

        Show
        Martin Grigorov added a comment - Sorry, your quickstart is too complicated and takes me too much time to analyze what you are doing. If you can reproduce it with less code (Wicket quickstart + what is needed to reproduce) then I'll take a look again. Otherwise I'll leave it to someone else to debug this application.
        Hide
        Michael Allan added a comment - - edited

        Quickstart attached. Tested with JDK 1.6 and 1.7, Firefox 9. Instructions:

        (1) Build and run the quickstart.

        (2) Load the home page: http://localhost:8080/wicket-bug-4358/

        (3) Click on the login link at top left.

        (4) Login with your OpenID. After 3 redirects you land back on the home page. Verify that the
        following localhost cookies ARE set on the third redirect: vo_loginPersistKey,
        vo_loginPersistUserEmail.

        (5) Open votorola.g.web.wic.WebResponseX in your editor.

        (6) Edit addCookie(). Comment out the workaround (B) and uncomment the call to BufferedWebResponse (A).

        (7) Clear the two cookies from your browser.

        (8) Rebuild and retest (1-4). This time the cookies are NOT set.

        Show
        Michael Allan added a comment - - edited Quickstart attached. Tested with JDK 1.6 and 1.7, Firefox 9. Instructions: (1) Build and run the quickstart. (2) Load the home page: http://localhost:8080/wicket-bug-4358/ (3) Click on the login link at top left. (4) Login with your OpenID. After 3 redirects you land back on the home page. Verify that the following localhost cookies ARE set on the third redirect: vo_loginPersistKey, vo_loginPersistUserEmail. (5) Open votorola.g.web.wic.WebResponseX in your editor. (6) Edit addCookie(). Comment out the workaround (B) and uncomment the call to BufferedWebResponse (A). (7) Clear the two cookies from your browser. (8) Rebuild and retest (1-4). This time the cookies are NOT set.
        Hide
        Martin Grigorov added a comment -

        The response is flushed for every response... I.e. this stack is normal.
        There must be another one that resets the response and starts writing new headers and content in it.

        Please attach a quickstart.

        Show
        Martin Grigorov added a comment - The response is flushed for every response... I.e. this stack is normal. There must be another one that resets the response and starts writing new headers and content in it. Please attach a quickstart.
        Hide
        Michael Allan added a comment -

        I don't think so, but that breakpoint IS triggered twice. Firebug says the first break holds up the response to the initial request (form POST), but I guess that one is uninteresting. The second break holds up the response to the third request (a GET) which is the one in which the cookies are supposed to be set. The cookies are not set. The break occurs after the response page is fully constructed. The constructor runs to completion and throws no exceptions. Here's the stack trace at the breakpoint:

        [1] org.apache.wicket.protocol.http.BufferedWebResponse.reset (BufferedWebResponse.java:412)
        [2] org.apache.wicket.protocol.http.HeaderBufferingWebResponse.flush(HeaderBufferingWebResponse.java:90)
        [3] org.apache.wicket.protocol.http.WicketFilter.processRequest (WicketFilter.java:172)
        [4] org.apache.wicket.protocol.http.WicketFilter.doFilter (WicketFilter.java:218)
        [5] org.apache.catalina.core.ApplicationFilterChain.internalDoFilter (null)
        [6] org.apache.catalina.core.ApplicationFilterChain.doFilter (null)
        [7] org.apache.catalina.core.StandardWrapperValve.invoke (null)
        [8] org.apache.catalina.core.StandardContextValve.invoke (null)
        [9] org.apache.catalina.valves.RequestFilterValve.process (null)
        [10] org.apache.catalina.valves.RemoteAddrValve.invoke (null)
        [11] org.apache.catalina.core.StandardHostValve.invoke (null)
        [12] org.apache.catalina.valves.ErrorReportValve.invoke (null)
        [13] org.apache.catalina.core.StandardEngineValve.invoke (null)
        [14] org.apache.catalina.connector.CoyoteAdapter.service (null)
        [15] org.apache.coyote.http11.Http11Processor.process (null)
        [16] org.apache.coyote.http11.Http11Protocol$Http11ConnectionHandler.process (null)
        [17] org.apache.tomcat.util.net.JIoEndpoint$Worker.run (null)
        [18] java.lang.Thread.run (Thread.java:722)

        But the breakpoint is NOT triggered and the cookies ARE set if I remove the call to RequestCycle.setResponsePage() by commenting out line 124: http://zelea.com/var/db/repo/votorola-wic-1.5/file/1618244d61ee/votorola/a/web/wic/authen/WP_Login.java#l124

        Hope this helps. If it's not enough, then I'll try to find time to create a quickstart.

        Show
        Michael Allan added a comment - I don't think so, but that breakpoint IS triggered twice. Firebug says the first break holds up the response to the initial request (form POST), but I guess that one is uninteresting. The second break holds up the response to the third request (a GET) which is the one in which the cookies are supposed to be set. The cookies are not set. The break occurs after the response page is fully constructed. The constructor runs to completion and throws no exceptions. Here's the stack trace at the breakpoint: [1] org.apache.wicket.protocol.http.BufferedWebResponse.reset (BufferedWebResponse.java:412) [2] org.apache.wicket.protocol.http.HeaderBufferingWebResponse.flush(HeaderBufferingWebResponse.java:90) [3] org.apache.wicket.protocol.http.WicketFilter.processRequest (WicketFilter.java:172) [4] org.apache.wicket.protocol.http.WicketFilter.doFilter (WicketFilter.java:218) [5] org.apache.catalina.core.ApplicationFilterChain.internalDoFilter (null) [6] org.apache.catalina.core.ApplicationFilterChain.doFilter (null) [7] org.apache.catalina.core.StandardWrapperValve.invoke (null) [8] org.apache.catalina.core.StandardContextValve.invoke (null) [9] org.apache.catalina.valves.RequestFilterValve.process (null) [10] org.apache.catalina.valves.RemoteAddrValve.invoke (null) [11] org.apache.catalina.core.StandardHostValve.invoke (null) [12] org.apache.catalina.valves.ErrorReportValve.invoke (null) [13] org.apache.catalina.core.StandardEngineValve.invoke (null) [14] org.apache.catalina.connector.CoyoteAdapter.service (null) [15] org.apache.coyote.http11.Http11Processor.process (null) [16] org.apache.coyote.http11.Http11Protocol$Http11ConnectionHandler.process (null) [17] org.apache.tomcat.util.net.JIoEndpoint$Worker.run (null) [18] java.lang.Thread.run (Thread.java:722) But the breakpoint is NOT triggered and the cookies ARE set if I remove the call to RequestCycle.setResponsePage() by commenting out line 124: http://zelea.com/var/db/repo/votorola-wic-1.5/file/1618244d61ee/votorola/a/web/wic/authen/WP_Login.java#l124 Hope this helps. If it's not enough, then I'll try to find time to create a quickstart.
        Hide
        Martin Grigorov added a comment -

        I don't have time to debug your code by just reading it ... :-/
        I suspect that you use some version of ResetResponseException (like RestartResponseException) that resets the WebResponse and clears the cookies.
        Add a breakpoint at org.apache.wicket.protocol.http.BufferedWebResponse#reset() and see what happens.

        Show
        Martin Grigorov added a comment - I don't have time to debug your code by just reading it ... :-/ I suspect that you use some version of ResetResponseException (like RestartResponseException) that resets the WebResponse and clears the cookies. Add a breakpoint at org.apache.wicket.protocol.http.BufferedWebResponse#reset() and see what happens.
        Hide
        Michael Allan added a comment -

        The following is not the only failure path I encountered. There's a simpler one involving a single redirect after a container restart. But I haven't investigated it much because it's not a real problem for my app. This is the path I was looking at most closely:

        Redirect (1) is by RequestCycle#scheduleRequestHandlerAfterCurrent. See page WP_Login, line 768:
        http://zelea.com/var/db/repo/votorola-wic-1.5/file/1618244d61ee/votorola/a/web/wic/authen/WP_Login.java#l768

        Redirect (2) is by the OpenID provider. In my test case, the provider is https://pip.verisignlabs.com/. Their server redirects the client to page WP_OpenIDReturn, which then retrieves the previous instance of WP_Login from the session and calls into it from line 170: http://zelea.com/var/db/repo/votorola-wic-1.5/file/1618244d61ee/votorola/a/web/wic/authen/WP_OpenIDReturn.java#l170

        Redirect (3) is by RequestCycle#setResponsePage(Class,PageParameters). See page WP_Login, line 124: http://zelea.com/var/db/repo/votorola-wic-1.5/file/1618244d61ee/votorola/a/web/wic/authen/WP_Login.java#l124

        The cookies are set immediately after, beginning at line 156: http://zelea.com/var/db/repo/votorola-wic-1.5/file/1618244d61ee/votorola/a/web/wic/authen/WP_Login.java#l156

        The cookie setting code is here. Without the workaround described here, it fails:
        http://zelea.com/var/db/repo/votorola-wic-1.5/file/1618244d61ee/votorola/g/web/wic/WebResponseX.java#l23

        I hope this is enough to trace the fault. If you still need a quickstart, I'll try to find time to install Maven and create one.

        Show
        Michael Allan added a comment - The following is not the only failure path I encountered. There's a simpler one involving a single redirect after a container restart. But I haven't investigated it much because it's not a real problem for my app. This is the path I was looking at most closely: Redirect (1) is by RequestCycle#scheduleRequestHandlerAfterCurrent. See page WP_Login, line 768: http://zelea.com/var/db/repo/votorola-wic-1.5/file/1618244d61ee/votorola/a/web/wic/authen/WP_Login.java#l768 Redirect (2) is by the OpenID provider. In my test case, the provider is https://pip.verisignlabs.com/ . Their server redirects the client to page WP_OpenIDReturn, which then retrieves the previous instance of WP_Login from the session and calls into it from line 170: http://zelea.com/var/db/repo/votorola-wic-1.5/file/1618244d61ee/votorola/a/web/wic/authen/WP_OpenIDReturn.java#l170 Redirect (3) is by RequestCycle#setResponsePage(Class,PageParameters). See page WP_Login, line 124: http://zelea.com/var/db/repo/votorola-wic-1.5/file/1618244d61ee/votorola/a/web/wic/authen/WP_Login.java#l124 The cookies are set immediately after, beginning at line 156: http://zelea.com/var/db/repo/votorola-wic-1.5/file/1618244d61ee/votorola/a/web/wic/authen/WP_Login.java#l156 The cookie setting code is here. Without the workaround described here, it fails: http://zelea.com/var/db/repo/votorola-wic-1.5/file/1618244d61ee/votorola/g/web/wic/WebResponseX.java#l23 I hope this is enough to trace the fault. If you still need a quickstart, I'll try to find time to install Maven and create one.
        Hide
        Martin Grigorov added a comment -

        How do you make the redirect ?
        Can you provide a quickstart application please.

        Show
        Martin Grigorov added a comment - How do you make the redirect ? Can you provide a quickstart application please.

          People

          • Assignee:
            Martin Grigorov
            Reporter:
            Michael Allan
          • Votes:
            1 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development