Tynamo
  1. Tynamo
  2. TYNAMO-124

Context path duplicated when redirecting to saved request

    Details

    • Type: Bug Bug
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: security-0.4.2
    • Fix Version/s: security-0.4.4
    • Component/s: security
    • Labels:
      None
    • Environment:
      tapestry 5.3.2
    • Number of attachments :
      1

      Description

      Context path duplicated in url when redirecting to saved request in PageServiceImpl.redirectToSavedRequest(String fallbackUrl) method. Method PageServiceImpl.createSavedRequestCookie() saves in cookie request URI with context path. Then method PageServiceImpl.redirectToSavedRequest(String fallbackUrl) call WebUtils.issueRedirect(request, response, requestUri), but this method should receive url without context path.

      1. redirect.patch
        0.5 kB
        Vladimir Velikiy

        Activity

        Hide
        Vladimir Velikiy added a comment -

        The patch is attached.

        Show
        Vladimir Velikiy added a comment - The patch is attached.
        Hide
        Kalle Korhonen added a comment -

        Thanks Vladimir! Could have gone either way with this. The other option would have been to not store the context at all with the saved request to save some bytes, but the benefit of doing it this way is that the fallback url could potentially be in a different context (saved request obviously is always in this context)

        Show
        Kalle Korhonen added a comment - Thanks Vladimir! Could have gone either way with this. The other option would have been to not store the context at all with the saved request to save some bytes, but the benefit of doing it this way is that the fallback url could potentially be in a different context (saved request obviously is always in this context)
        Hide
        Kalle Korhonen added a comment -

        Re-opening, since context is now lost on default fallback urls.

        Show
        Kalle Korhonen added a comment - Re-opening, since context is now lost on default fallback urls.
        Hide
        Lenny Primak added a comment -

        Doesn't work for request redirect itself , not fallback redirect. Sorry. Not sure why, no time to debut now.
        My patch still stands:

        I found the real culprit why my code wasn't working.
        The Cookie that was written out does not include the context,
        so when the redirect was issued, it was going against the web site root

            • Patch Attached ***

        Index: src/main/java/org/tynamo/security/internal/SecurityExceptionHandlerAssistant.java
        ===================================================================
        — src/main/java/org/tynamo/security/internal/SecurityExceptionHandlerAssistant.java (revision 2246)
        +++ src/main/java/org/tynamo/security/internal/SecurityExceptionHandlerAssistant.java (working copy)
        @@ -50,9 +50,8 @@
        // Session session = subject.getSession();
        // if (session != null) WebUtils.saveRequest(requestGlobals.getHTTPServletRequest());
        // }

        • String contextPath = servletRequest.getContextPath();
        • if ("".equals(contextPath)) contextPath = "/";
        • cookies.writeCookieValue(WebUtils.SAVED_REQUEST_KEY, WebUtils.getPathWithinApplication(servletRequest), contextPath);
          + pageService.saveRequest();
          +
          return pageService.getLoginPage();
          }
          }
        Show
        Lenny Primak added a comment - Doesn't work for request redirect itself , not fallback redirect. Sorry. Not sure why, no time to debut now. My patch still stands: I found the real culprit why my code wasn't working. The Cookie that was written out does not include the context, so when the redirect was issued, it was going against the web site root Patch Attached *** Index: src/main/java/org/tynamo/security/internal/SecurityExceptionHandlerAssistant.java =================================================================== — src/main/java/org/tynamo/security/internal/SecurityExceptionHandlerAssistant.java (revision 2246) +++ src/main/java/org/tynamo/security/internal/SecurityExceptionHandlerAssistant.java (working copy) @@ -50,9 +50,8 @@ // Session session = subject.getSession(); // if (session != null) WebUtils.saveRequest(requestGlobals.getHTTPServletRequest()); // } String contextPath = servletRequest.getContextPath(); if ("".equals(contextPath)) contextPath = "/"; cookies.writeCookieValue(WebUtils.SAVED_REQUEST_KEY, WebUtils.getPathWithinApplication(servletRequest), contextPath); + pageService.saveRequest(); + return pageService.getLoginPage(); } }
        Hide
        Kalle Korhonen added a comment -

        Reverted the earlier patch. The root cause of the issue is the context path shouldn't be stored at all since the
        default fallback urls cannot specify context. There are several places where we need to keep this in mind, TYNAMO-144 and TYNAMO-145 are related, should be all fixed and wrote a few more functional tests for verification.

        Show
        Kalle Korhonen added a comment - Reverted the earlier patch. The root cause of the issue is the context path shouldn't be stored at all since the default fallback urls cannot specify context. There are several places where we need to keep this in mind, TYNAMO-144 and TYNAMO-145 are related, should be all fixed and wrote a few more functional tests for verification.
        Hide
        Alejandro Scandroli added a comment -

        bulk closing issues that have been resolved for more than a year.

        Show
        Alejandro Scandroli added a comment - bulk closing issues that have been resolved for more than a year.

          People

          • Assignee:
            Kalle Korhonen
            Reporter:
            Vladimir Velikiy
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: