Velocity
  1. Velocity
  2. VELOCITY-614

Impossible to escape '#' if followed by text and parenthesis( "\#something(Stuff)" renders with the '\')

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.6
    • Fix Version/s: 1.6
    • Component/s: None
    • Labels:
      None
    • Environment:
      I don't think it is relevant but still: CPU Intel Pentium D, windows XP SP3, JRE 1.6.0_07, Velocity 1.6-beta1

      Description

      "#something(Stuff)" in your templates (without the double quotes) causes a parse error.
      "#something(Stuff)" in your template (without the double quotes) renders as "#something(Stuff)"

      Either the first line should go through just fine, or the next one should not render the '\'

      This typically happens when you try to generate JavaDoc for overriding methods.

      Don't hesitate to contact me if this is not clear enough or if you are unable to reproduce this issue.

        Activity

        Hide
        Nathan Bubna added a comment -

        here's a test case which confirms it:

        [junit] [error] Parser Error: #macro() : #something(Stuff)
        [junit] Invalid arg #0 in directive #something at line 1, column 11 of #something(Stuff)
        [junit]
        [junit] org.apache.velocity.runtime.directive.MacroParseException: Invalid arg #0 in directive #something at line 1, column 11 of #something(Stuff)
        [junit] at org.apache.velocity.runtime.parser.Parser.Directive(Parser.java:798)
        [junit] at org.apache.velocity.runtime.parser.Parser.Statement(Parser.java:369)
        [junit] at org.apache.velocity.runtime.parser.Parser.process(Parser.java:307)
        [junit] at org.apache.velocity.runtime.parser.Parser.parse(Parser.java:105)
        [junit] at org.apache.velocity.runtime.RuntimeInstance.parse(RuntimeInstance.java:1119)
        [junit] at org.apache.velocity.runtime.RuntimeInstance.parse(RuntimeInstance.java:1074)
        [junit] at org.apache.velocity.runtime.RuntimeInstance.evaluate(RuntimeInstance.java:1187)
        [junit] at org.apache.velocity.runtime.RuntimeInstance.evaluate(RuntimeInstance.java:1153)
        [junit] at org.apache.velocity.app.VelocityEngine.evaluate(VelocityEngine.java:219)
        [junit] at org.apache.velocity.test.BaseEvalTestCase.evaluate(BaseEvalTestCase.java:99)
        [junit] at org.apache.velocity.test.BaseEvalTestCase.assertEvalEquals(BaseEvalTestCase.java:85)
        [junit] at org.apache.velocity.test.issues.Velocity614TestCase.testIt(Velocity614TestCase.java:37)

        Show
        Nathan Bubna added a comment - here's a test case which confirms it: [junit] [error] Parser Error: #macro() : #something(Stuff) [junit] Invalid arg #0 in directive #something at line 1, column 11 of #something(Stuff) [junit] [junit] org.apache.velocity.runtime.directive.MacroParseException: Invalid arg #0 in directive #something at line 1, column 11 of #something(Stuff) [junit] at org.apache.velocity.runtime.parser.Parser.Directive(Parser.java:798) [junit] at org.apache.velocity.runtime.parser.Parser.Statement(Parser.java:369) [junit] at org.apache.velocity.runtime.parser.Parser.process(Parser.java:307) [junit] at org.apache.velocity.runtime.parser.Parser.parse(Parser.java:105) [junit] at org.apache.velocity.runtime.RuntimeInstance.parse(RuntimeInstance.java:1119) [junit] at org.apache.velocity.runtime.RuntimeInstance.parse(RuntimeInstance.java:1074) [junit] at org.apache.velocity.runtime.RuntimeInstance.evaluate(RuntimeInstance.java:1187) [junit] at org.apache.velocity.runtime.RuntimeInstance.evaluate(RuntimeInstance.java:1153) [junit] at org.apache.velocity.app.VelocityEngine.evaluate(VelocityEngine.java:219) [junit] at org.apache.velocity.test.BaseEvalTestCase.evaluate(BaseEvalTestCase.java:99) [junit] at org.apache.velocity.test.BaseEvalTestCase.assertEvalEquals(BaseEvalTestCase.java:85) [junit] at org.apache.velocity.test.issues.Velocity614TestCase.testIt(Velocity614TestCase.java:37)
        Hide
        Nathan Bubna added a comment -

        This looks like a result of the fix allowing macros to be #parse() (RuntimeMacro et al). Now anything that looks like a macro has to have proper syntax, even if it was never meant to be a macro. Otherwise, the parser chokes.

        This would definitely be annoying for anyone generating javadoc.

        Show
        Nathan Bubna added a comment - This looks like a result of the fix allowing macros to be #parse() (RuntimeMacro et al). Now anything that looks like a macro has to have proper syntax, even if it was never meant to be a macro. Otherwise, the parser chokes. This would definitely be annoying for anyone generating javadoc.
        Hide
        Nathan Bubna added a comment -

        This won't be fixable without postponing exceptions for bad arguments to runtime-defined macros until runtime. If #something is already a known macro, it's easy enough to still throw the parse exception as currently happens. But if we want #something(Stuff) to work when it is just schmoo (i.e. there is never a #something macro), then we have to postpone the decision to render it as-is or throw a bad argument exception. We can't tell for sure at parse-time.

        I think postponing the error until runtime is acceptable, and better than the alternative of not requiring the #set( $H = '#' )$

        {H}

        something(Stuff) workaround for outputting javadoc.

        any thoughts?

        Show
        Nathan Bubna added a comment - This won't be fixable without postponing exceptions for bad arguments to runtime-defined macros until runtime. If #something is already a known macro, it's easy enough to still throw the parse exception as currently happens. But if we want #something(Stuff) to work when it is just schmoo (i.e. there is never a #something macro), then we have to postpone the decision to render it as-is or throw a bad argument exception. We can't tell for sure at parse-time. I think postponing the error until runtime is acceptable, and better than the alternative of not requiring the #set( $H = '#' )$ {H} something(Stuff) workaround for outputting javadoc. any thoughts?
        Hide
        Nathan Bubna added a comment -

        Fixed. But if #something turns out to truly be a macro, then #something(Stuff) will not throw an error until template init, rather than during parsing.

        Show
        Nathan Bubna added a comment - Fixed. But if #something turns out to truly be a macro, then #something(Stuff) will not throw an error until template init, rather than during parsing.
        Hide
        Christopher Schultz added a comment -

        Nathan,

        I'm no expert on Velocity's parser, but why would fixing this problem result in the inability to perform syntax checking?

        Shouldn't changing

        #something($foo)
        into
        #something($foo) merely change the macro-call production into plain-text? $foo is still perfectly legal, as if the user had done this:

        something($foo)

        (i.e., without the leading #).

        This suggests to me that the grammar has not been done in a natural way.

        Show
        Christopher Schultz added a comment - Nathan, I'm no expert on Velocity's parser, but why would fixing this problem result in the inability to perform syntax checking? Shouldn't changing #something($foo) into #something($foo) merely change the macro-call production into plain-text? $foo is still perfectly legal, as if the user had done this: something($foo) (i.e., without the leading #). This suggests to me that the grammar has not been done in a natural way.
        Hide
        Nathan Bubna added a comment -

        It's not #something($foo) that is the problem, it's #something(foo). The difference is that #something(foo) is perfectly legal schmoo, but if there is a #something directive or #something macro, then plain old foo (JJTWORD) is not an acceptable argument for it. it would have to be 'foo' (string literal) or $foo (reference) or ['foo'] (inline list) or

        { 'foo':$foo }

        (inline map).

        so, since it is legal schmoo and we now let you 'forward define' (aka late bind) macros (in order to allow macros to be included via #parse), we cannot tell at parse time if #something(foo) is legal schmoo or a macro called #something that is being passed an illegal argument. we have to wait until runtime to know if #something is a macro or not.

        as for the escaping, escaping is also (unfortunately) dependent on whether #something is schmoo or a valid macro. which reminds me, i didn't test how it reacts if #something is a macro and you do #something(foo). hmm. i'll have to check on that.

        anyway, hope that helps clear it up a bit.

        Show
        Nathan Bubna added a comment - It's not #something($foo) that is the problem, it's #something(foo). The difference is that #something(foo) is perfectly legal schmoo, but if there is a #something directive or #something macro, then plain old foo (JJTWORD) is not an acceptable argument for it. it would have to be 'foo' (string literal) or $foo (reference) or ['foo'] (inline list) or { 'foo':$foo } (inline map). so, since it is legal schmoo and we now let you 'forward define' (aka late bind) macros (in order to allow macros to be included via #parse), we cannot tell at parse time if #something(foo) is legal schmoo or a macro called #something that is being passed an illegal argument. we have to wait until runtime to know if #something is a macro or not. as for the escaping, escaping is also (unfortunately) dependent on whether #something is schmoo or a valid macro. which reminds me, i didn't test how it reacts if #something is a macro and you do #something(foo). hmm. i'll have to check on that. anyway, hope that helps clear it up a bit.
        Hide
        Christopher Schultz added a comment -

        Nathan,

        Oh, now I get it. Since we don't bother to statically check macro calls (because #something may or not be a macro), you have to wait until runtime to possibly fail. It's not that #something(foo) parses to "this is a macro call"... it parses to "this /might/ be a macro call... figure it out at runtime".

        Sorry for the noise.

        Show
        Christopher Schultz added a comment - Nathan, Oh, now I get it. Since we don't bother to statically check macro calls (because #something may or not be a macro), you have to wait until runtime to possibly fail. It's not that #something(foo) parses to "this is a macro call"... it parses to "this /might/ be a macro call... figure it out at runtime". Sorry for the noise.
        Hide
        Christopher Schultz added a comment -

        Actually... I'm still not convinced. This is all about escaping a # symbol, which ought to turn it into plain-text, no matter what. It's /never/ a macro reference, even if 'something' is defined as a macro at runtime.

        Show
        Christopher Schultz added a comment - Actually... I'm still not convinced. This is all about escaping a # symbol, which ought to turn it into plain-text, no matter what. It's /never/ a macro reference, even if 'something' is defined as a macro at runtime.
        Hide
        Nathan Bubna added a comment -

        Christopher, you've never been noise.

        Anyway, Guillaume's original concern was that #something(Stuff) used to work fin in pre-1.6 versions. But with the addition of the RuntimeMacro stuff in 1.6, it started throwing parse errors, because 1.6 has to treat everything that looks like a macro as if it were, etc.

        The escaping attempt was just to try and circumvent the error. Of course, that still didn't work because #something was still not a macro. You have to remember in Velocity 1.x, it has always been the case that you do not ever escape just $ or #. What you escape are references, macros and directives which happen to start with either # or $. If you put a \ in front of something that looks like a reference, macro or directive, but is not, then it does nothing whatsoever.

        Yes, this is confusing to most people who are used to just escaping characters. But it is far too late to change this in the 1.x series. When ever someone starts a 2.x branch, we can (and probably will) change that. But for now, #schmoo() will print #schmoo() and #validmacro() will print #validmacro(). There is no character escaping with \ and there never has been in Velocity. If you want character escaping, you have to use EscapeTool or poor man's style (#set( $H = '#' ) $

        {H}

        schmoo(). I've tried to clarify the docs on this for 1.6, but they could probably use more tweaking still.

        Show
        Nathan Bubna added a comment - Christopher, you've never been noise. Anyway, Guillaume's original concern was that #something(Stuff) used to work fin in pre-1.6 versions. But with the addition of the RuntimeMacro stuff in 1.6, it started throwing parse errors, because 1.6 has to treat everything that looks like a macro as if it were, etc. The escaping attempt was just to try and circumvent the error. Of course, that still didn't work because #something was still not a macro. You have to remember in Velocity 1.x, it has always been the case that you do not ever escape just $ or #. What you escape are references, macros and directives which happen to start with either # or $. If you put a \ in front of something that looks like a reference, macro or directive, but is not, then it does nothing whatsoever. Yes, this is confusing to most people who are used to just escaping characters. But it is far too late to change this in the 1.x series. When ever someone starts a 2.x branch, we can (and probably will) change that. But for now, #schmoo() will print #schmoo() and #validmacro() will print #validmacro(). There is no character escaping with \ and there never has been in Velocity. If you want character escaping, you have to use EscapeTool or poor man's style (#set( $H = '#' ) $ {H} schmoo(). I've tried to clarify the docs on this for 1.6, but they could probably use more tweaking still.
        Hide
        Christopher Schultz added a comment -

        Aah... right. I keep forgetting that escaping is not really a part of Velocity. You can put a backslash in front of a $ symbol to prevent it from being evaluated, and you can put a backslash in front of a # symbol (in a directive only) and that will prevent it from being executed (including parameters passed to the directive).

        This simply violates the principle of least surprise: all "escaping" is really just special cases that look like traditional escaping.

        It's also surprising to me that:

        #if($jazz)
        Vyacheslav Ganelin
        #end

        Yields:

        #if($ jazz )
        Vyacheslav Ganelin
        #end

        ...rather than

        #if([whatever $jazz evaluates to)
        Vyacheslav Ganelin
        #end

        Not a complaint per-se, but something to think about when the parser is re-written: escaping should work as expected by, say, a C programmer

        Show
        Christopher Schultz added a comment - Aah... right. I keep forgetting that escaping is not really a part of Velocity. You can put a backslash in front of a $ symbol to prevent it from being evaluated, and you can put a backslash in front of a # symbol (in a directive only) and that will prevent it from being executed (including parameters passed to the directive). This simply violates the principle of least surprise: all "escaping" is really just special cases that look like traditional escaping. It's also surprising to me that: #if($jazz) Vyacheslav Ganelin #end Yields: #if($ jazz ) Vyacheslav Ganelin #end ...rather than #if([whatever $jazz evaluates to) Vyacheslav Ganelin #end Not a complaint per-se, but something to think about when the parser is re-written: escaping should work as expected by, say, a C programmer

          People

          • Assignee:
            Unassigned
            Reporter:
            Guillaume Polet
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development