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.

        Issue Links

          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