Tapestry 5
  1. Tapestry 5
  2. TAP5-1768

@ActivationRequestParameter does not encode to be URL friendly

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 5.3, 5.2
    • Fix Version/s: 5.3.1, 5.4
    • Component/s: None
    • Labels:
      None

      Description

      The @ActivationRequestParameter can contain symbols that interfere with the URL (eg. &) and these are passed straight to the browser and are not encoded. Therefore the query string in any links that are generated are all messed up.

        Activity

        Hide
        Ulrich Stärk added a comment -

        The problem is that StringValueEncoder passes server-side strings to client-side strings without doing any conversion. This works fine as long as there are no characters that are reserved as per RFC 3986. For these, percent-encoding is required.

        Should we fix this in StringValueEncoder or should we explicitly do this for this case only? I'm a bit afraid that doing it in StringValueEncoder will give us problems elsewhere.

        Uli

        Show
        Ulrich Stärk added a comment - The problem is that StringValueEncoder passes server-side strings to client-side strings without doing any conversion. This works fine as long as there are no characters that are reserved as per RFC 3986. For these, percent-encoding is required. Should we fix this in StringValueEncoder or should we explicitly do this for this case only? I'm a bit afraid that doing it in StringValueEncoder will give us problems elsewhere. Uli
        Hide
        Ulrich Stärk added a comment -

        Resolving as fixed.

        Show
        Ulrich Stärk added a comment - Resolving as fixed.
        Hide
        Hudson added a comment -

        Integrated in tapestry-trunk-freestyle #639 (See https://builds.apache.org/job/tapestry-trunk-freestyle/639/)
        TAP5-1768 - @ActivationRequestParameter does not encode to be URL friendly

        uli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1212379
        Files :

        • /tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/ActivationRequestParameterWorker.java
        • /tapestry/tapestry5/trunk/tapestry-core/src/test/app1/ActivationRequestParameterDemo.tml
        • /tapestry/tapestry5/trunk/tapestry-core/src/test/groovy/org/apache/tapestry5/integration/app1/ActivationRequestParameterTests.groovy
        • /tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/ActivationRequestParameterDemo.java
        Show
        Hudson added a comment - Integrated in tapestry-trunk-freestyle #639 (See https://builds.apache.org/job/tapestry-trunk-freestyle/639/ ) TAP5-1768 - @ActivationRequestParameter does not encode to be URL friendly uli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1212379 Files : /tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/ActivationRequestParameterWorker.java /tapestry/tapestry5/trunk/tapestry-core/src/test/app1/ActivationRequestParameterDemo.tml /tapestry/tapestry5/trunk/tapestry-core/src/test/groovy/org/apache/tapestry5/integration/app1/ActivationRequestParameterTests.groovy /tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/ActivationRequestParameterDemo.java
        Hide
        Massimo Lusetti added a comment -

        Just to fix the correct version

        Show
        Massimo Lusetti added a comment - Just to fix the correct version
        Hide
        Christian Riedel added a comment -

        If you have a property in you page, let's say this one:

        @ActivationRequestParameter("error_description")
        private String errorDescription;

        and the error description is encoded like this:
        ...?error_description=This+Is+Some+Error+Description

        then the ActivationRequestParameterWorker or to be more precise the URLEncoderImpl will throw an exception telling me there's an illegal character in my URL.
        The '+' resolves to a space and spaces are not within the 'safe' BitSet within URLEncoderImpl. I encountered this while trying to read facebook error messages in my app (they are always passed as query parameters attached to a defined redirectURI).
        I'm not able to change the way FB adds these parameters.

        Would you mind changing the behaviour again to be less strict about the value of query parameters?

        Cheers
        Christian

        Show
        Christian Riedel added a comment - If you have a property in you page, let's say this one: @ActivationRequestParameter("error_description") private String errorDescription; and the error description is encoded like this: ...?error_description=This+Is+Some+Error+Description then the ActivationRequestParameterWorker or to be more precise the URLEncoderImpl will throw an exception telling me there's an illegal character in my URL. The '+' resolves to a space and spaces are not within the 'safe' BitSet within URLEncoderImpl. I encountered this while trying to read facebook error messages in my app (they are always passed as query parameters attached to a defined redirectURI). I'm not able to change the way FB adds these parameters. Would you mind changing the behaviour again to be less strict about the value of query parameters? Cheers Christian
        Hide
        Ulrich Stärk added a comment -

        The culprit here is URLEncoder. Instead of the standard URL encoding rules Tapestry uses it's own encoding scheme in order to also be able to encode null values. We'll need to discuss this on the dev list.

        Uli

        Show
        Ulrich Stärk added a comment - The culprit here is URLEncoder. Instead of the standard URL encoding rules Tapestry uses it's own encoding scheme in order to also be able to encode null values. We'll need to discuss this on the dev list. Uli

          People

          • Assignee:
            Ulrich Stärk
            Reporter:
            Ryan How
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development