Repository: PebbleTemplates/pebble (java)

Issue:
[bug] Sequential string literals in expression should be syntax error
A [comment](https://github.com/PebbleTemplates/pebble/blob/c0e5bde637293132a7a983f95083fcde96465ae0/pebble/src/main/java/com/mitchellbosecke/pebble/parser/ExpressionParser.java#L341) in the ExpressionParser suggests that an expression like `{{ 'one' 'two' }}` should not be allowed. Currently, it gets parsed as the concatenation of two literal strings into a single value. Here's a [test case](https://github.com/cjbrooks12/pebble/blob/ed29535174ccbfd3ac1a2415858e60e42ac90d80/pebble/src/test/java/com/mitchellbosecke/pebble/node/expression/ExpressionParserTest.java) demonstrating the issue.

I'm currently working on a fix and should have a PR for it shortly. I just wanted to open an issue now so I can link here from where this bug was reported to me.

Pull request description:
Fixes bug which allowed sequential String literals to be parsed correctly
Fixes #481, and adds unit tests for String expression parsing.

Patch:
diff --git a/pebble/src/main/java/com/mitchellbosecke/pebble/parser/ExpressionParser.java b/pebble/src/main/java/com/mitchellbosecke/pebble/parser/ExpressionParser.java
index 997be746..bcf8f9a0 100644
--- a/pebble/src/main/java/com/mitchellbosecke/pebble/parser/ExpressionParser.java
+++ b/pebble/src/main/java/com/mitchellbosecke/pebble/parser/ExpressionParser.java
@@ -337,16 +337,19 @@ public class ExpressionParser {
 
   private Expression<?> parseStringExpression() throws ParserException {
     List<Expression<?>> nodes = new ArrayList<>();
+    Token.Type previousNodeType = null;
 
     // Sequential strings are not OK, but strings can follow interpolation
     while (true) {
-      if (this.stream.current().test(Token.Type.STRING)) {
+      if (this.stream.current().test(Token.Type.STRING) && Token.Type.STRING != previousNodeType) {
         Token token = this.stream.expect(Token.Type.STRING);
         nodes.add(new LiteralStringExpression(token.getValue(), token.getLineNumber()));
+        previousNodeType = Token.Type.STRING;
       } else if (this.stream.current().test(Token.Type.STRING_INTERPOLATION_START)) {
         this.stream.expect(Token.Type.STRING_INTERPOLATION_START);
         nodes.add(this.parseExpression());
         this.stream.expect(Token.Type.STRING_INTERPOLATION_END);
+        previousNodeType = Token.Type.STRING_INTERPOLATION_END;
       } else {
         break;
       }


Test patch:
diff --git a/pebble/src/test/java/com/mitchellbosecke/pebble/node/expression/StringExpressionParserTest.java b/pebble/src/test/java/com/mitchellbosecke/pebble/node/expression/StringExpressionParserTest.java
new file mode 100644
index 00000000..483851c6
--- /dev/null
+++ b/pebble/src/test/java/com/mitchellbosecke/pebble/node/expression/StringExpressionParserTest.java
@@ -0,0 +1,215 @@
+package com.mitchellbosecke.pebble.node.expression;
+
+import com.mitchellbosecke.pebble.PebbleEngine;
+import com.mitchellbosecke.pebble.error.ParserException;
+import com.mitchellbosecke.pebble.lexer.LexerImpl;
+import com.mitchellbosecke.pebble.lexer.TokenStream;
+import com.mitchellbosecke.pebble.loader.StringLoader;
+import com.mitchellbosecke.pebble.node.PrintNode;
+import com.mitchellbosecke.pebble.node.RootNode;
+import com.mitchellbosecke.pebble.parser.Parser;
+import com.mitchellbosecke.pebble.parser.ParserImpl;
+import com.mitchellbosecke.pebble.parser.ParserOptions;
+import com.mitchellbosecke.pebble.utils.Pair;
+import org.junit.Test;
+
+import java.io.IOException;
+import java.io.StringReader;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+public class StringExpressionParserTest {
+
+    private Pair<Parser, RootNode> testParseExpression(String expression) {
+        PebbleEngine pebble = new PebbleEngine.Builder()
+                .loader(new StringLoader())
+                .strictVariables(false)
+                .build();
+
+        LexerImpl lexer = new LexerImpl(
+                pebble.getSyntax(),
+                pebble.getExtensionRegistry().getUnaryOperators().values(),
+                pebble.getExtensionRegistry().getBinaryOperators().values()
+        );
+
+        TokenStream tokenStream = lexer.tokenize(new StringReader(expression), "test.peb");
+
+        Parser parser = new ParserImpl(
+                pebble.getExtensionRegistry().getUnaryOperators(),
+                pebble.getExtensionRegistry().getBinaryOperators(),
+                pebble.getExtensionRegistry().getTokenParsers(),
+                new ParserOptions()
+        );
+
+        return new Pair<>(parser, parser.parse(tokenStream));
+    }
+
+    /*
+     * Sequential String literals are not allowed, a syntax error should be thrown
+     */
+    @Test(expected = ParserException.class)
+    public void testSequentialStrings_singleQuotes_isSyntaxError() throws IOException {
+        testParseExpression("{{ 'one' 'two' }}");
+    }
+
+    /*
+     * Sequential String literals are not allowed, a syntax error should be thrown
+     */
+    @Test(expected = ParserException.class)
+    public void testSequentialStrings_doubleQuotes_isSyntaxError() throws IOException {
+        testParseExpression("{{ \"one\" \"two\" }}");
+    }
+
+    /*
+     * + parses as Add expression. AST should be:
+     *
+     * AddExpression
+     *      LiteralStringExpression('one')
+     *      LiteralStringExpression('two')
+     */
+    @Test
+    public void testValidExpression_singleQuotes() throws IOException {
+        Pair<Parser, RootNode> underTest = testParseExpression("{{ 'one' + 'two' }}");
+
+        PrintNode printNode = (PrintNode) underTest.getRight().getBody().getChildren().get(0);
+        Expression<?> expr = printNode.getExpression();
+
+        assertThat(expr).isInstanceOf(AddExpression.class);
+
+        Expression<?> leftExpression = ((AddExpression) expr).getLeftExpression();
+        Expression<?> rightExpression = ((AddExpression) expr).getRightExpression();
+
+        assertThat(leftExpression).isInstanceOf(LiteralStringExpression.class);
+        assertThat(((LiteralStringExpression) leftExpression).getValue()).isEqualTo("one");
+
+        assertThat(rightExpression).isInstanceOf(LiteralStringExpression.class);
+        assertThat(((LiteralStringExpression) rightExpression).getValue()).isEqualTo("two");
+    }
+
+    /*
+     * + parses as Add expression. AST should be:
+     *
+     * AddExpression
+     *      LiteralStringExpression('one')
+     *      LiteralStringExpression('two')
+     */
+    @Test
+    public void testValidExpression_doubleQuotes() throws IOException {
+        Pair<Parser, RootNode> underTest = testParseExpression("{{ \"one\" + \"two\" }}");
+
+        PrintNode printNode = (PrintNode) underTest.getRight().getBody().getChildren().get(0);
+        Expression<?> expr = printNode.getExpression();
+
+        assertThat(expr).isInstanceOf(AddExpression.class);
+
+        Expression<?> leftExpression = ((AddExpression) expr).getLeftExpression();
+        Expression<?> rightExpression = ((AddExpression) expr).getRightExpression();
+
+        assertThat(leftExpression).isInstanceOf(LiteralStringExpression.class);
+        assertThat(((LiteralStringExpression) leftExpression).getValue()).isEqualTo("one");
+
+        assertThat(rightExpression).isInstanceOf(LiteralStringExpression.class);
+        assertThat(((LiteralStringExpression) rightExpression).getValue()).isEqualTo("two");
+    }
+
+    /*
+     * ~ parses as Concatenate expression. AST should be:
+     *
+     * ConcatenateExpression
+     *      LiteralStringExpression('one')
+     *      LiteralStringExpression('two')
+     */
+    @Test
+    public void testValidConcatenationExpression_singleQuotes() throws IOException {
+        Pair<Parser, RootNode> underTest = testParseExpression("{{ 'one' ~ 'two' }}");
+
+        PrintNode printNode = (PrintNode) underTest.getRight().getBody().getChildren().get(0);
+        Expression<?> expr = printNode.getExpression();
+
+        assertThat(expr).isInstanceOf(ConcatenateExpression.class);
+
+        Expression<?> leftExpression = ((ConcatenateExpression) expr).getLeftExpression();
+        Expression<?> rightExpression = ((ConcatenateExpression) expr).getRightExpression();
+
+        assertThat(leftExpression).isInstanceOf(LiteralStringExpression.class);
+        assertThat(((LiteralStringExpression) leftExpression).getValue()).isEqualTo("one");
+
+        assertThat(rightExpression).isInstanceOf(LiteralStringExpression.class);
+        assertThat(((LiteralStringExpression) rightExpression).getValue()).isEqualTo("two");
+    }
+
+    /*
+     * ~ parses as Concatenate expression. AST should be:
+     *
+     * ConcatenateExpression
+     *      LiteralStringExpression('one')
+     *      LiteralStringExpression('two')
+     */
+    @Test
+    public void testValidConcatenationExpression_doubleQuotes() throws IOException {
+        Pair<Parser, RootNode> underTest = testParseExpression("{{ \"one\" ~ \"two\" }}");
+
+        PrintNode printNode = (PrintNode) underTest.getRight().getBody().getChildren().get(0);
+        Expression<?> expr = printNode.getExpression();
+
+        assertThat(expr).isInstanceOf(ConcatenateExpression.class);
+
+        Expression<?> leftExpression = ((ConcatenateExpression) expr).getLeftExpression();
+        Expression<?> rightExpression = ((ConcatenateExpression) expr).getRightExpression();
+
+        assertThat(leftExpression).isInstanceOf(LiteralStringExpression.class);
+        assertThat(((LiteralStringExpression) leftExpression).getValue()).isEqualTo("one");
+
+        assertThat(rightExpression).isInstanceOf(LiteralStringExpression.class);
+        assertThat(((LiteralStringExpression) rightExpression).getValue()).isEqualTo("two");
+    }
+
+    /*
+     * Single-quotes does not parse interpolations. AST should be:
+     *
+     * LiteralStringExpression('one #{two} three')
+     */
+    @Test
+    public void testValidInterpolationExpression_singleQuotes() throws IOException {
+        Pair<Parser, RootNode> underTest = testParseExpression("{{ 'one #{two} three' }}");
+
+        PrintNode printNode = (PrintNode) underTest.getRight().getBody().getChildren().get(0);
+        Expression<?> expr = printNode.getExpression();
+
+        assertThat(expr).isInstanceOf(LiteralStringExpression.class);
+        assertThat(((LiteralStringExpression) expr).getValue()).isEqualTo("one #{two} three");
+    }
+
+    /*
+     * Double-quotes parses interpolations. AST should be:
+     *
+     * ConcatenateExpression
+     *      LiteralStringExpression('one ')
+     *      ConcatenateExpression
+     *          ContextVariableExpression(two)
+     *          LiteralStringExpression(' three')
+     */
+    @Test
+    public void testValidInterpolationExpression_doubleQuotes() throws IOException {
+        Pair<Parser, RootNode> underTest = testParseExpression("{{ \"one #{two} three\" }}");
+
+        PrintNode printNode = (PrintNode) underTest.getRight().getBody().getChildren().get(0);
+        Expression<?> expr = printNode.getExpression();
+
+        assertThat(expr).isInstanceOf(ConcatenateExpression.class);
+
+        Expression<?> leftExpression = ((ConcatenateExpression) expr).getLeftExpression();
+        Expression<?> rightExpression = ((ConcatenateExpression) expr).getRightExpression();
+
+        assertThat(leftExpression).isInstanceOf(LiteralStringExpression.class);
+        assertThat(((LiteralStringExpression) leftExpression).getValue()).isEqualTo("one ");
+
+        assertThat(rightExpression).isInstanceOf(ConcatenateExpression.class);
+
+        Expression<?> right_leftSubExpression = ((ConcatenateExpression) rightExpression).getLeftExpression();
+        assertThat(((ContextVariableExpression) right_leftSubExpression).getName()).isEqualTo("two");
+
+        Expression<?> right_rightSubExpression = ((ConcatenateExpression) rightExpression).getRightExpression();
+        assertThat(((LiteralStringExpression) right_rightSubExpression).getValue()).isEqualTo(" three");
+    }
+}