groovy

Groovy Template quote escape problem

Details

  • Type: Bug Bug
  • Status: Open Open
  • Priority: Major Major
  • Resolution: Unresolved
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: Templating
  • Labels:
    None
  • Number of attachments :
    3

Description

The SimpleTemplateEngine got a problem with escaping quotes inside GStrings.

"this" is tricky: ${hello + " " + hello}

will be parsed to

out.print("\"this\" is tricky: ${hello + \" \" + hello}");

which is a problem for the current lexer as a GString must be a valid groovy statement.

  1. Text File
    patch.txt
    16/Apr/04 11:21 AM
    1.0 kB
    Thomas Heller
  2. GZip Archive
    test.tar.gz
    19/Oct/04 1:02 PM
    11 kB
    Niall Gallagher
  3. GZip Archive
    test-closure.tar.gz
    21/Oct/04 7:57 AM
    13 kB
    Niall Gallagher

Issue Links

is depended upon by

Bug - A problem which impairs or prevents the functions of the product. GROOVY-980 SimpleTemplateEngine fails when enclose expression dosn't immediately evaluate to a String

  • Major - Major loss of function.
  • Closed - The issue is considered finished, the resolution is correct. Issues which are not closed can be reopened.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Thomas Heller added a comment -

patch diff to be applied from groovy/groovy-core

Show
Thomas Heller added a comment - patch diff to be applied from groovy/groovy-core
Hide
Permalink
Chris Poirier added a comment -

Thomas: I'm not clear on the purpose of that test – that GString expression will be expanded during initial parsing (and will fail, due to the first backslash). Is that what you wanted? If not, you need to escape it:
String source = "\"this\" is tricky: \$

{hello + \" \" + hello}

";

Show
Chris Poirier added a comment - Thomas: I'm not clear on the purpose of that test – that GString expression will be expanded during initial parsing (and will fail, due to the first backslash). Is that what you wanted? If not, you need to escape it: String source = "\"this\" is tricky: \$ {hello + \" \" + hello} ";
Hide
Permalink
Thomas Heller added a comment -

If you have a Groovy Template (from groovy.text.SimpleTemplateEngine) which contains (in a File for example)

"this" is tricky: $

{hello + " " + hello}

it will be parsed by the Template impl to create to stated source which is causing an exception is you pass it into the groovy compiler.

Now this creates:

out.print("\"this\" is tricky: ${hello + " " + hello}

");

which is not a valid groovy statement (as of now). I think it should be but a GString is expected to be a plain groovy statement so

out.print("\"this\" is tricky: $

{hello + " " + hello}

");

would be correct. I think the lexer needs to handle that since the first statement is correct java while the second is not.

Show
Thomas Heller added a comment - If you have a Groovy Template (from groovy.text.SimpleTemplateEngine) which contains (in a File for example) "this" is tricky: $ {hello + " " + hello} it will be parsed by the Template impl to create to stated source which is causing an exception is you pass it into the groovy compiler. Now this creates: out.print("\"this\" is tricky: ${hello + " " + hello} "); which is not a valid groovy statement (as of now). I think it should be but a GString is expected to be a plain groovy statement so out.print("\"this\" is tricky: $ {hello + " " + hello} "); would be correct. I think the lexer needs to handle that since the first statement is correct java while the second is not.
Hide
Permalink
Chris Poirier added a comment -

> I think the lexer needs to handle that since the first statement is correct java while the second is not.

Yes, because regular Java doesn't do GStrings.... In order for GString expressions to be truly useful, they have to host real Groovy code, like closures, possibly with nesting strings, possibly with nested GString expressions. How do you handle backslashes in nested GStrings? The answer is not to try. I think the handling currently in place is the correct thing to do. Double quotes delimit a nested language.

So can I assume that the intention of the attached test is to break the build?

Show
Chris Poirier added a comment - > I think the lexer needs to handle that since the first statement is correct java while the second is not. Yes, because regular Java doesn't do GStrings.... In order for GString expressions to be truly useful, they have to host real Groovy code, like closures, possibly with nesting strings, possibly with nested GString expressions. How do you handle backslashes in nested GStrings? The answer is not to try. I think the handling currently in place is the correct thing to do. Double quotes delimit a nested language. So can I assume that the intention of the attached test is to break the build?
Hide
Permalink
Thomas Heller added a comment -

Its not intended to break the build, its intended to show a bug.

I'm quite sure that you will see something like that in templates and either the lexer or the groovy template engine need to handle this case.

I inlined the template text in this testcase to not depend on external files etc. But you are welcome to move the example

$

{hello + " " + hello}

into a file and let it run from there.

I know that

$

{hello} ${hello}

is a better solution. But since GStrings are plain groovy statements its perfectly fine the run the "broken" example.

Show
Thomas Heller added a comment - Its not intended to break the build, its intended to show a bug. I'm quite sure that you will see something like that in templates and either the lexer or the groovy template engine need to handle this case. I inlined the template text in this testcase to not depend on external files etc. But you are welcome to move the example $ {hello + " " + hello} into a file and let it run from there. I know that $ {hello} ${hello} is a better solution. But since GStrings are plain groovy statements its perfectly fine the run the "broken" example.
Hide
Permalink
Chris Poirier added a comment -

Agreed. The SimpleTemplateEngine needs to be fixed.

Show
Chris Poirier added a comment - Agreed. The SimpleTemplateEngine needs to be fixed.
Hide
Permalink
Guillaume Laforge added a comment -

Sam, it looks like a problem for you.

Show
Guillaume Laforge added a comment - Sam, it looks like a problem for you.
Hide
Permalink
Thomas Heller added a comment -

A little more obvious problem:

$

{someObj.withAMethod("hi")}

won't work when used inside templates.

Show
Thomas Heller added a comment - A little more obvious problem: $ {someObj.withAMethod("hi")} won't work when used inside templates.
Hide
Permalink
Niall Gallagher added a comment -

I have implemented a solution to this problem. The solution uses a cascading translator that converts

$

{ hello + " " + hello}

to

<?= hello + " " + hello ?>

and finally, to

out.println( hello + " " + hello );

which is a valid Groovy statement. I would like to contribute and/or maintain this code for Groovy.

Show
Niall Gallagher added a comment - I have implemented a solution to this problem. The solution uses a cascading translator that converts $ { hello + " " + hello} to <?= hello + " " + hello ?> and finally, to out.println( hello + " " + hello ); which is a valid Groovy statement. I would like to contribute and/or maintain this code for Groovy.
Hide
Permalink
Guillaume Laforge added a comment -

Niall, attach your patch to one of the issues, and we'll have a look at it.

Show
Guillaume Laforge added a comment - Niall, attach your patch to one of the issues, and we'll have a look at it.
Hide
Permalink
Niall Gallagher added a comment -

The attached file is not exactly a patch but rather a rework of the parsing done for the templates. If it is reasonable at this stage to port the GStringTemplateEngine and SimpleTemplateEngine to this framework then I would be happy to do it.

Attached you will find some test drivers and an example template with some "tricky" expressions. Don't know what licence to attach to these?

Show
Niall Gallagher added a comment - The attached file is not exactly a patch but rather a rework of the parsing done for the templates. If it is reasonable at this stage to port the GStringTemplateEngine and SimpleTemplateEngine to this framework then I would be happy to do it. Attached you will find some test drivers and an example template with some "tricky" expressions. Don't know what licence to attach to these?
Hide
Permalink
Niall Gallagher added a comment -

Whoops! in haste I did not attach a parser for the closure used by the GStringTemplateEngine. I have attached it on this upload. It produces an identical script to the one produced by the current GStringTemplateEngine, except it does not escape quotes within $

{..}

expressions. The following will generate the Groovy script from a template.

java TestCascade template.groovy

Show
Niall Gallagher added a comment - Whoops! in haste I did not attach a parser for the closure used by the GStringTemplateEngine. I have attached it on this upload. It produces an identical script to the one produced by the current GStringTemplateEngine, except it does not escape quotes within $ {..} expressions. The following will generate the Groovy script from a template. java TestCascade template.groovy
Hide
Permalink
Guillaume Laforge added a comment -

Oh Template and Groovlet master!!!

Show
Guillaume Laforge added a comment - Oh Template and Groovlet master!!!
Hide
Permalink
Christian Stein added a comment -

Looking at http://jira.codehaus.org/browse/GROOVY-399 - we should write the test cases asap - before refactoring the whole parser stuff. After a first glimpse, it could help with all those char-by-char parser short-comings.

Show
Christian Stein added a comment - Looking at http://jira.codehaus.org/browse/GROOVY-399 - we should write the test cases asap - before refactoring the whole parser stuff. After a first glimpse, it could help with all those char-by-char parser short-comings.
Hide
Permalink
Paul King added a comment -

OK, I made a few changes to the existing code base so that the ugly problems with quotes inside (for now simple) GStrings goes away. Not as good as the other proposals listed here for a re-write but at least gets rid of the ugly cases for now.

I also tried grafting in the current antlr GString parsing into the template codebase and made some progress but backed out as it was going to take a lot more time to finish than I have right now.

Show
Paul King added a comment - OK, I made a few changes to the existing code base so that the ugly problems with quotes inside (for now simple) GStrings goes away. Not as good as the other proposals listed here for a re-write but at least gets rid of the ugly cases for now. I also tried grafting in the current antlr GString parsing into the template codebase and made some progress but backed out as it was going to take a lot more time to finish than I have right now.

People

  • Assignee:
    Unassigned
    Reporter:
    Thomas Heller
Vote (0)
Watch (0)

Dates

  • Created:
    Updated: