Tapestry 5
  1. Tapestry 5
  2. TAP5-1605

Template parsing of expansions can't handle map expressions

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 5.3
    • Fix Version/s: 5.3
    • Component/s: tapestry-core
    • Labels:
      None

      Description

      5.3 introduced map support into the property expression language in the form:

      {'foo': 'bar'}

      .

      Expansion parsing chokes on the syntax, however. It uses a reluctant regular expression to find the closing brace:

      private static final Pattern EXPANSION_PATTERN = Pattern.compile("\\$

      {\\s*(.*?)\\s*}

      ");

      Which means that the use of a map inside an expansion prematurely terminates the exansion:

      ${echoMap(

      {"foo": "bar"}

      )}

      The regex finds the first } and the expression evaluates as:

      echoMap({"foo": "bar"

      Which is clearly incorrect.

        Issue Links

          Activity

          Robert Zeigler created issue -
          Robert Zeigler made changes -
          Field Original Value New Value
          Assignee Robert Zeigler [ ongakugainochi ]
          Robert Zeigler made changes -
          Status Open [ 1 ] In Progress [ 3 ]
          Hide
          Robert Zeigler added a comment -

          I'm considering replacing the existing regex with one like: \${\s*(((?!\${).))\s} (
          omitted above for clarity). Basically, I've replaced the reluctant .? with a greedy . that only matches if the substrings that don't contain ${. So that introduces a new limitation to expansions: you can't do something like ${'$

          {'}

          . I'm not sure why you would do that in the first place, but here's your chance to air your use case. If there are no complaints about introducing this (rather obscure) limitation to expansions, I'll go with the above solution.

          Show
          Robert Zeigler added a comment - I'm considering replacing the existing regex with one like: \${\s*(((?!\${).) )\s } ( omitted above for clarity). Basically, I've replaced the reluctant . ? with a greedy . that only matches if the substrings that don't contain ${. So that introduces a new limitation to expansions: you can't do something like ${'$ {'} . I'm not sure why you would do that in the first place, but here's your chance to air your use case. If there are no complaints about introducing this (rather obscure) limitation to expansions, I'll go with the above solution.
          Hide
          Robert Zeigler added a comment -

          Thinking about this some more... really just trading one limitation (can't have a } anywhere inside the expansion) for a different one (can't have ${) and the latter is less likely than the former.

          Show
          Robert Zeigler added a comment - Thinking about this some more... really just trading one limitation (can't have a } anywhere inside the expansion) for a different one (can't have ${) and the latter is less likely than the former.
          Robert Zeigler made changes -
          Status In Progress [ 3 ] Resolved [ 5 ]
          Resolution Fixed [ 1 ]
          Robert Zeigler made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Hide
          Hudson added a comment -

          Integrated in tapestry-trunk-freestyle #471 (See https://builds.apache.org/job/tapestry-trunk-freestyle/471/)
          TAP5-1605: Template parsing of expansions can't handle map expressions

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

          • /tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/Index.java
          • /tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/SaxTemplateParser.java
          • /tapestry/tapestry5/trunk/tapestry-core/src/test/app1/MapExpressionInExpansions.tml
          • /tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/MapExpressionInExpansions.java
          • /tapestry/tapestry5/trunk/tapestry-core/src/test/resources/org/apache/tapestry5/internal/services/expansions_with_maps.tml
          • /tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/TemplateParserImplTest.java
          • /tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/CoreBehaviorsTests.java
          • /tapestry/tapestry5/trunk/build.gradle
          Show
          Hudson added a comment - Integrated in tapestry-trunk-freestyle #471 (See https://builds.apache.org/job/tapestry-trunk-freestyle/471/ ) TAP5-1605 : Template parsing of expansions can't handle map expressions robertdzeigler : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1156971 Files : /tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/Index.java /tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/SaxTemplateParser.java /tapestry/tapestry5/trunk/tapestry-core/src/test/app1/MapExpressionInExpansions.tml /tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/MapExpressionInExpansions.java /tapestry/tapestry5/trunk/tapestry-core/src/test/resources/org/apache/tapestry5/internal/services/expansions_with_maps.tml /tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/TemplateParserImplTest.java /tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/CoreBehaviorsTests.java /tapestry/tapestry5/trunk/build.gradle
          Hide
          Robert Zeigler added a comment -

          re-open to set the fix version.

          Show
          Robert Zeigler added a comment - re-open to set the fix version.
          Robert Zeigler made changes -
          Resolution Fixed [ 1 ]
          Status Closed [ 6 ] Reopened [ 4 ]
          Robert Zeigler made changes -
          Resolution Fixed [ 1 ]
          Status Reopened [ 4 ] Closed [ 6 ]
          Fix Version/s 5.3 [ 12316024 ]
          Robert Zeigler made changes -
          Link This issue relates to TAP5-1620 [ TAP5-1620 ]
          Transition Time In Source Status Execution Times Last Executer Last Execution Date
          Open Open In Progress In Progress
          4h 2m 1 Robert Zeigler 11/Aug/11 05:19
          In Progress In Progress Resolved Resolved
          1d 2h 34m 1 Robert Zeigler 12/Aug/11 07:53
          Resolved Resolved Closed Closed
          6s 1 Robert Zeigler 12/Aug/11 07:53
          Closed Closed Reopened Reopened
          6d 11h 54m 1 Robert Zeigler 18/Aug/11 19:48
          Reopened Reopened Closed Closed
          29s 1 Robert Zeigler 18/Aug/11 19:48

            People

            • Assignee:
              Robert Zeigler
              Reporter:
              Robert Zeigler
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development