MyFaces Core
  1. MyFaces Core
  2. MYFACES-3229

ServletExternalContextImpl.encodeRedirectURL() doesn't handle existing query parameters correctly

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.1.1
    • Fix Version/s: 2.0.8, 2.1.2
    • Component/s: JSR-314
    • Labels:
      None

      Description

      ServletExternalContextImpl.encodeRedirectURL() doesn't work correctly if the base URL already contains encoded query parameters. In this case the resulting URL contains the query parameter encoded twice, which completely breaks them.

      Take a look at this example:

      externalContext.encodeRedirectURL("/test?p1=a+b", null);

      Result ----> /test?p1=a%2Bb

      Another example are encoded ampersand:

      externalContext.encodeRedirectURL("/test?p1=a%26b", null);

      Result ----> /test?p1=a%2526b

      The root cause of the problem seems to be that ServletExternalContextImpl.encodeURL() doesn't decode the query parameters while parsing the base URL.

      1. MYFACES-3229-failing-unit-test.patch
        1 kB
        Christian Kaltepoth
      2. MYFACES-3229-fix.patch
        1 kB
        Christian Kaltepoth

        Activity

        Hide
        Christian Kaltepoth added a comment -

        Unit test reproducing this issue

        Show
        Christian Kaltepoth added a comment - Unit test reproducing this issue
        Hide
        Christian Kaltepoth added a comment -

        Sorry, I forgot to mention that both MyFaces 2.0.x and 2.1.x seem to be affected!

        Show
        Christian Kaltepoth added a comment - Sorry, I forgot to mention that both MyFaces 2.0.x and 2.1.x seem to be affected!
        Hide
        Christian Kaltepoth added a comment -

        Proposed patch!

        Show
        Christian Kaltepoth added a comment - Proposed patch!
        Hide
        Leonardo Uribe added a comment -

        thanks to Christian Kaltepoth for provide this patch

        Show
        Leonardo Uribe added a comment - thanks to Christian Kaltepoth for provide this patch

          People

          • Assignee:
            Leonardo Uribe
            Reporter:
            Christian Kaltepoth
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development