Repository: PebbleTemplates/pebble (java)

Issue:
All Java identifiers are not supported
Fixed by #544

Pull request:
Support all Java identifiers
Fixes #545.

This is needed to call methods or use fields that are not a-z characters, for example éêȩ etc.

These are supported in Java and because Java provides useful patterns for matching starts and parts of identifiers it's better to just use them.

This issue is really important as it prevents completely valid code from running.

Patch:
diff --git a/docs/src/orchid/resources/wiki/guide/basic-usage.md b/docs/src/orchid/resources/wiki/guide/basic-usage.md
index 42f1b248..47e26160 100644
--- a/docs/src/orchid/resources/wiki/guide/basic-usage.md
+++ b/docs/src/orchid/resources/wiki/guide/basic-usage.md
@@ -68,13 +68,15 @@ characters, you can use the subscript notation ([]) instead.
 ```
 Behind the scenes `foo.bar` will attempt the following techniques to to access the `bar` attribute of the `foo`
 variable:
-- If `foo` is a map, `foo.get("bar")`
+- If `foo` is a Map, `foo.get("bar")`
 - `foo.getBar()`
 - `foo.isBar()`
 - `foo.hasBar()`
 - `foo.bar()`
 - `foo.bar`
 
+Additionally, if `foo` is a List, then `foo[0]` can be used instead of `foo.get(0)`.
+
 If the value of variable (or attribute) is null it will output an empty string.
 
 ## Type Safety
diff --git a/docs/src/orchid/resources/wiki/guide/installation.md b/docs/src/orchid/resources/wiki/guide/installation.md
index cbc71198..05373cbf 100644
--- a/docs/src/orchid/resources/wiki/guide/installation.md
+++ b/docs/src/orchid/resources/wiki/guide/installation.md
@@ -65,6 +65,7 @@ Pebble ships with the following loader implementations:
 - `FileLoader`:  Finds templates using a filesystem path.
 - `ServletLoader`:  Uses a servlet context to find the template. This is the recommended loader for use within an
 application server but is not enabled by default.
+- `Servlet5Loader`:  Same as `ServletLoader`, but for Jakarta Servlet 5.0 or newer.
 - `StringLoader`: Considers the name of the template to be the contents of the template.
 - `DelegatingLoader`: Delegates responsibility to a collection of children loaders.
 
diff --git a/pebble/pom.xml b/pebble/pom.xml
index eb780dc7..fa1cf102 100644
--- a/pebble/pom.xml
+++ b/pebble/pom.xml
@@ -52,6 +52,12 @@
       <version>${servlet-api.version}</version>
       <optional>true</optional>
     </dependency>
+    <dependency>
+      <artifactId>jakarta.servlet-api</artifactId>
+      <groupId>jakarta.servlet</groupId>
+      <version>5.0.0</version>
+      <optional>true</optional>
+    </dependency>
 
     <!-- testing dependencies -->
     <dependency>
diff --git a/pebble/src/main/java/com/mitchellbosecke/pebble/lexer/LexerImpl.java b/pebble/src/main/java/com/mitchellbosecke/pebble/lexer/LexerImpl.java
index f9f8671a..58dc7c1b 100644
--- a/pebble/src/main/java/com/mitchellbosecke/pebble/lexer/LexerImpl.java
+++ b/pebble/src/main/java/com/mitchellbosecke/pebble/lexer/LexerImpl.java
@@ -87,9 +87,9 @@ public final class LexerImpl implements Lexer {
   private boolean trimLeadingWhitespaceFromNextData = false;
 
   /**
-   * Static regular expressions for names, numbers, and punctuation.
+   * Static regular expressions for identifiers.
    */
-  private static final Pattern REGEX_NAME = Pattern.compile("^[a-zA-Z_][a-zA-Z0-9_]*");
+  private static final Pattern REGEX_IDENTIFIER = Pattern.compile("^[\\p{IsLetter}_][\\p{IsLetter}\\p{IsDigit}_]*");
 
   private static final Pattern REGEX_LONG = Pattern.compile("^[0-9]+L");
 
@@ -414,7 +414,7 @@ public final class LexerImpl implements Lexer {
     }
 
     // names
-    matcher = REGEX_NAME.matcher(this.source);
+    matcher = REGEX_IDENTIFIER.matcher(this.source);
     if (matcher.lookingAt()) {
       token = this.source.substring(matcher.end());
       this.pushToken(Token.Type.NAME, token);
@@ -662,7 +662,7 @@ public final class LexerImpl implements Lexer {
        */
       char nextChar = operator.charAt(operator.length() - 1);
       if (Character.isLetter(nextChar) || Character.getType(nextChar) == Character.LETTER_NUMBER) {
-        regex.append("(?![a-zA-Z])");
+        regex.append("(?![a-zA-Z0-9_])");
       }
     }
 
diff --git a/pebble/src/main/java/com/mitchellbosecke/pebble/loader/AbstractServletLoader.java b/pebble/src/main/java/com/mitchellbosecke/pebble/loader/AbstractServletLoader.java
new file mode 100644
index 00000000..56a9e516
--- /dev/null
+++ b/pebble/src/main/java/com/mitchellbosecke/pebble/loader/AbstractServletLoader.java
@@ -0,0 +1,128 @@
+package com.mitchellbosecke.pebble.loader;
+
+import com.mitchellbosecke.pebble.error.LoaderException;
+import com.mitchellbosecke.pebble.utils.PathUtils;
+import java.io.BufferedReader;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.io.Reader;
+import java.io.UnsupportedEncodingException;
+import java.net.MalformedURLException;
+import java.net.URL;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Abstract base class for loaders which user the servlet context to load templates.
+ *
+ * @author mbosecke
+ * @author chkal
+ */
+public abstract class AbstractServletLoader implements Loader<String> {
+
+  private static final Logger logger = LoggerFactory.getLogger(AbstractServletLoader.class);
+
+  private String prefix;
+
+  private String suffix;
+
+  private String charset = "UTF-8";
+
+  private char expectedSeparator = '/';
+
+  protected abstract InputStream getResourceAsStream(String location);
+
+  protected abstract URL getResource(String location) throws MalformedURLException;
+
+  @Override
+  public Reader getReader(String templateName) {
+
+    InputStreamReader isr = null;
+    Reader reader = null;
+
+    InputStream is = null;
+    String location = this.getLocation(templateName);
+
+    logger.debug("Looking for template in {}.", location);
+
+    is = getResourceAsStream(location);
+
+    if (is == null) {
+      throw new LoaderException(null, "Could not find template \"" + location + "\"");
+    }
+
+    try {
+      isr = new InputStreamReader(is, this.charset);
+      reader = new BufferedReader(isr);
+    } catch (UnsupportedEncodingException e) {
+    }
+
+    return reader;
+  }
+
+  private String getLocation(String templateName) {
+    // Add the prefix and make sure that it ends with a separator character
+    StringBuilder path = new StringBuilder(128);
+    if (this.getPrefix() != null) {
+
+      path.append(this.getPrefix());
+
+      // we do NOT use OS dependent separators here; getResourceAsStream
+      // explicitly requires forward slashes.
+      if (!this.getPrefix().endsWith(Character.toString(this.expectedSeparator))) {
+        path.append(this.expectedSeparator);
+      }
+    }
+    path.append(templateName);
+    if (this.getSuffix() != null) {
+      path.append(this.getSuffix());
+    }
+    return path.toString();
+  }
+
+  public String getSuffix() {
+    return this.suffix;
+  }
+
+  @Override
+  public void setSuffix(String suffix) {
+    this.suffix = suffix;
+  }
+
+  public String getPrefix() {
+    return this.prefix;
+  }
+
+  @Override
+  public void setPrefix(String prefix) {
+    this.prefix = prefix;
+  }
+
+  public String getCharset() {
+    return this.charset;
+  }
+
+  @Override
+  public void setCharset(String charset) {
+    this.charset = charset;
+  }
+
+  @Override
+  public String resolveRelativePath(String relativePath, String anchorPath) {
+    return PathUtils.resolveRelativePath(relativePath, anchorPath, this.expectedSeparator);
+  }
+
+  @Override
+  public String createCacheKey(String templateName) {
+    return templateName;
+  }
+
+  @Override
+  public boolean resourceExists(String templateName) {
+    try {
+      return getResource(this.getLocation(templateName)) != null;
+    } catch (MalformedURLException e) {
+      return false;
+    }
+  }
+}
diff --git a/pebble/src/main/java/com/mitchellbosecke/pebble/loader/Servlet5Loader.java b/pebble/src/main/java/com/mitchellbosecke/pebble/loader/Servlet5Loader.java
new file mode 100644
index 00000000..3fef106a
--- /dev/null
+++ b/pebble/src/main/java/com/mitchellbosecke/pebble/loader/Servlet5Loader.java
@@ -0,0 +1,31 @@
+package com.mitchellbosecke.pebble.loader;
+
+import jakarta.servlet.ServletContext;
+import java.io.InputStream;
+import java.net.MalformedURLException;
+import java.net.URL;
+
+/**
+ * Loader that uses a servlet context to find templates. Requires Jakarta Servlet 5.0 or newer.
+ *
+ * @author mbosecke
+ * @author chkal
+ */
+public class Servlet5Loader extends AbstractServletLoader {
+
+  private final ServletContext context;
+
+  public Servlet5Loader(ServletContext context) {
+    this.context = context;
+  }
+
+  @Override
+  protected InputStream getResourceAsStream(String location) {
+    return context.getResourceAsStream(location);
+  }
+
+  @Override
+  protected URL getResource(String location) throws MalformedURLException {
+    return context.getResource(location);
+  }
+}
diff --git a/pebble/src/main/java/com/mitchellbosecke/pebble/loader/ServletLoader.java b/pebble/src/main/java/com/mitchellbosecke/pebble/loader/ServletLoader.java
index e6268fde..3facc90f 100644
--- a/pebble/src/main/java/com/mitchellbosecke/pebble/loader/ServletLoader.java
+++ b/pebble/src/main/java/com/mitchellbosecke/pebble/loader/ServletLoader.java
@@ -1,36 +1,17 @@
 package com.mitchellbosecke.pebble.loader;
 
-import com.mitchellbosecke.pebble.error.LoaderException;
-import com.mitchellbosecke.pebble.utils.PathUtils;
-
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import java.io.BufferedReader;
 import java.io.InputStream;
-import java.io.InputStreamReader;
-import java.io.Reader;
-import java.io.UnsupportedEncodingException;
 import java.net.MalformedURLException;
-
+import java.net.URL;
 import javax.servlet.ServletContext;
 
 /**
  * Loader that uses a servlet context to find templates.
  *
  * @author mbosecke
+ * @author chkal
  */
-public class ServletLoader implements Loader<String> {
-
-  private static final Logger logger = LoggerFactory.getLogger(ServletLoader.class);
-
-  private String prefix;
-
-  private String suffix;
-
-  private String charset = "UTF-8";
-
-  private char expectedSeparator = '/';
+public class ServletLoader extends AbstractServletLoader {
 
   private final ServletContext context;
 
@@ -39,94 +20,12 @@ public class ServletLoader implements Loader<String> {
   }
 
   @Override
-  public Reader getReader(String templateName) {
-
-    InputStreamReader isr = null;
-    Reader reader = null;
-
-    InputStream is = null;
-    String location = this.getLocation(templateName);
-
-    logger.debug("Looking for template in {}.", location);
-
-    is = this.context.getResourceAsStream(location);
-
-    if (is == null) {
-      throw new LoaderException(null, "Could not find template \"" + location + "\"");
-    }
-
-    try {
-      isr = new InputStreamReader(is, this.charset);
-      reader = new BufferedReader(isr);
-    } catch (UnsupportedEncodingException e) {
-    }
-
-    return reader;
-  }
-
-  private String getLocation(String templateName) {
-    // Add the prefix and make sure that it ends with a separator character
-    StringBuilder path = new StringBuilder(128);
-    if (this.getPrefix() != null) {
-
-      path.append(this.getPrefix());
-
-      // we do NOT use OS dependent separators here; getResourceAsStream
-      // explicitly requires forward slashes.
-      if (!this.getPrefix().endsWith(Character.toString(this.expectedSeparator))) {
-        path.append(this.expectedSeparator);
-      }
-    }
-    path.append(templateName);
-    if (this.getSuffix() != null) {
-      path.append(this.getSuffix());
-    }
-    return path.toString();
-  }
-
-  public String getSuffix() {
-    return this.suffix;
-  }
-
-  @Override
-  public void setSuffix(String suffix) {
-    this.suffix = suffix;
-  }
-
-  public String getPrefix() {
-    return this.prefix;
-  }
-
-  @Override
-  public void setPrefix(String prefix) {
-    this.prefix = prefix;
-  }
-
-  public String getCharset() {
-    return this.charset;
-  }
-
-  @Override
-  public void setCharset(String charset) {
-    this.charset = charset;
-  }
-
-  @Override
-  public String resolveRelativePath(String relativePath, String anchorPath) {
-    return PathUtils.resolveRelativePath(relativePath, anchorPath, this.expectedSeparator);
-  }
-
-  @Override
-  public String createCacheKey(String templateName) {
-    return templateName;
+  protected InputStream getResourceAsStream(String location) {
+    return context.getResourceAsStream(location);
   }
 
   @Override
-  public boolean resourceExists(String templateName) {
-    try {
-      return this.context.getResource(this.getLocation(templateName)) != null;
-    } catch (MalformedURLException e) {
-      return false;
-    }
+  protected URL getResource(String location) throws MalformedURLException {
+    return context.getResource(location);
   }
 }
diff --git a/pebble/src/main/java/com/mitchellbosecke/pebble/node/ForNode.java b/pebble/src/main/java/com/mitchellbosecke/pebble/node/ForNode.java
index 6f156e71..07212831 100644
--- a/pebble/src/main/java/com/mitchellbosecke/pebble/node/ForNode.java
+++ b/pebble/src/main/java/com/mitchellbosecke/pebble/node/ForNode.java
@@ -49,6 +49,18 @@ public class ForNode extends AbstractRenderableNode {
     this.elseBody = elseBody;
   }
 
+  private static class LoopVariables {
+    public boolean first, last;
+    public LazyLength length;
+    public int index;
+    public LazyRevIndex revindex;
+
+    @Override
+    public String toString() {
+      return "{last=" + last + ", length=" + length + ", index=" + index + ", revindex=" + revindex + ", first=" + first + "}";
+    }
+  }
+
   @Override
   public void render(PebbleTemplateImpl self, Writer writer, EvaluationContextImpl context)
       throws IOException {
@@ -78,7 +90,7 @@ public class ForNode extends AbstractRenderableNode {
 
       int index = 0;
 
-      Map<String, Object> loop = null;
+      LoopVariables loop = null;
 
       boolean usingExecutorService = context.getExecutorService() != null;
 
@@ -91,23 +103,23 @@ public class ForNode extends AbstractRenderableNode {
          * would get it's own distinct copy of the context.
          */
         if (index == 0 || usingExecutorService) {
-          loop = new HashMap<>();
-          loop.put("first", index == 0);
-          loop.put("last", !iterator.hasNext());
-          loop.put("length", length);
+          loop = new LoopVariables();
+          loop.first = index == 0;
+          loop.last = !iterator.hasNext();
+          loop.length = length;
         } else if (index == 1) {
           // second iteration
-          loop.put("first", false);
+          loop.first = false;
         }
 
-        loop.put("revindex", new LazyRevIndex(index, length));
-        loop.put("index", index++);
+        loop.revindex = new LazyRevIndex(index, length);
+        loop.index = index++;
         scopeChain.put("loop", loop);
         scopeChain.put(this.variableName, iterator.next());
 
         // last iteration
         if (!iterator.hasNext()) {
-          loop.put("last", true);
+          loop.last = true;
         }
 
         this.body.render(self, writer, context);
diff --git a/pebble/src/main/java/com/mitchellbosecke/pebble/template/PebbleTemplateImpl.java b/pebble/src/main/java/com/mitchellbosecke/pebble/template/PebbleTemplateImpl.java
index d0255ab6..04bdd9b7 100644
--- a/pebble/src/main/java/com/mitchellbosecke/pebble/template/PebbleTemplateImpl.java
+++ b/pebble/src/main/java/com/mitchellbosecke/pebble/template/PebbleTemplateImpl.java
@@ -244,6 +244,12 @@ public class PebbleTemplateImpl implements PebbleTemplate {
         .getTemplate(this.resolveRelativePath(name));
     for (Pair<String, String> pair : namedMacros) {
       Macro m = templateImpl.macros.get(pair.getRight());
+
+      if (m == null) {
+        throw new PebbleException(null, "Function or Macro [" + pair.getRight() + "] referenced by alias ["
+                + pair.getLeft() + "] does not exist.");
+      }
+
       this.registerMacro(pair.getLeft(), m);
     }
   }


Test patch:
diff --git a/pebble/src/test/java/com/mitchellbosecke/pebble/lexer/IdentifierTest.java b/pebble/src/test/java/com/mitchellbosecke/pebble/lexer/IdentifierTest.java
new file mode 100644
index 00000000..d2be1fb2
--- /dev/null
+++ b/pebble/src/test/java/com/mitchellbosecke/pebble/lexer/IdentifierTest.java
@@ -0,0 +1,71 @@
+/*
+ * This file is part of Pebble.
+ *
+ * Copyright (c) 2014 by Mitchell Bösecke
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+package com.mitchellbosecke.pebble.lexer;
+
+import com.mitchellbosecke.pebble.PebbleEngine;
+import com.mitchellbosecke.pebble.error.ParserException;
+import com.mitchellbosecke.pebble.loader.StringLoader;
+import org.junit.jupiter.api.Test;
+
+import java.io.IOException;
+import java.io.StringWriter;
+import java.util.HashMap;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+public class IdentifierTest {
+
+  @Test
+  void common() throws IOException {
+    PebbleEngine pebble = new PebbleEngine.Builder().loader(new StringLoader()).strictVariables(false).build();
+    HashMap<String, Object> context = new HashMap<>();
+    context.put("fromContext", "ing");
+    StringWriter writer = new StringWriter();
+    pebble.getTemplate("{% set hello1 = \"test\" %}{{ hello1 }}{{ fromContext }}").evaluate(writer, context);
+    assertEquals("testing", writer.toString());
+  }
+
+  @Test
+  void digitStart() throws IOException {
+    PebbleEngine pebble = new PebbleEngine.Builder().loader(new StringLoader()).strictVariables(false).build();
+    StringWriter writer = new StringWriter();
+    assertThrows(ParserException.class, () -> pebble.getTemplate("{{ 0digit }}").evaluate(writer));
+  }
+
+  @Test
+  void extendedLatin() throws IOException {
+    PebbleEngine pebble = new PebbleEngine.Builder().loader(new StringLoader()).strictVariables(false).build();
+    HashMap<String, Object> context = new HashMap<>();
+    context.put("éabcêwçłë_0", "test string");
+    StringWriter writer = new StringWriter();
+    pebble.getTemplate("{{ éabcêwçłë_0 }}").evaluate(writer, context);
+    assertEquals("test string", writer.toString());
+  }
+
+  @Test
+  void arabic() throws IOException {
+    PebbleEngine pebble = new PebbleEngine.Builder().loader(new StringLoader()).strictVariables(false).build();
+    HashMap<String, Object> context = new HashMap<>();
+    context.put("شششش", "test string");
+    StringWriter writer = new StringWriter();
+    pebble.getTemplate("{{ شششش }}").evaluate(writer, context);
+    assertEquals("test string", writer.toString());
+  }
+
+  @Test
+  void chinese() throws IOException {
+    PebbleEngine pebble = new PebbleEngine.Builder().loader(new StringLoader()).strictVariables(false).build();
+    HashMap<String, Object> context = new HashMap<>();
+    context.put("水", "test string");
+    StringWriter writer = new StringWriter();
+    pebble.getTemplate("{{ 水 }}").evaluate(writer, context);
+    assertEquals("test string", writer.toString());
+  }
+}
diff --git a/pebble/src/test/java/com/mitchellbosecke/pebble/lexer/LexerImplTest.java b/pebble/src/test/java/com/mitchellbosecke/pebble/lexer/LexerImplTest.java
index decdd53e..001ba79f 100644
--- a/pebble/src/test/java/com/mitchellbosecke/pebble/lexer/LexerImplTest.java
+++ b/pebble/src/test/java/com/mitchellbosecke/pebble/lexer/LexerImplTest.java
@@ -82,6 +82,36 @@ class LexerImplTest {
     assertThat(tokenStream.peek(3).getValue()).isNull();
   }
 
+  /**
+   * Test tokenizing an expression, e.g. {{ expression }}
+   */
+  @Test
+  void testVariableNameStartingWithOperator() {
+    Loader<String> loader = new StringLoader();
+    Reader templateReader = loader.getReader("{{ is_active + contains0 }}");
+
+    TokenStream tokenStream = this.lexer.tokenize(templateReader, this.TEMPLATE_NAME);
+
+    int i = 0;
+    assertThat(tokenStream.peek(i).getType()).isEqualTo(Type.PRINT_START);
+    assertThat(tokenStream.peek(i++).getValue()).isNull();
+
+    assertThat(tokenStream.peek(i).getType()).isEqualTo(Type.NAME);
+    assertThat(tokenStream.peek(i++).getValue()).isEqualTo("is_active");
+
+    assertThat(tokenStream.peek(i).getType()).isEqualTo(Type.OPERATOR);
+    assertThat(tokenStream.peek(i++).getValue()).isEqualTo("+");
+
+    assertThat(tokenStream.peek(i).getType()).isEqualTo(Type.NAME);
+    assertThat(tokenStream.peek(i++).getValue()).isEqualTo("contains0");
+
+    assertThat(tokenStream.peek(i).getType()).isEqualTo(Type.PRINT_END);
+    assertThat(tokenStream.peek(i++).getValue()).isEqualTo("}}");
+
+    assertThat(tokenStream.peek(i).getType()).isEqualTo(Type.EOF);
+    assertThat(tokenStream.peek(i++).getValue()).isNull();
+  }
+
   /**
    * Test tokenizing Punctuation, such as the dot in item.itemType
    */
diff --git a/pebble/src/test/java/com/mitchellbosecke/pebble/macro/TestMacroCalls.java b/pebble/src/test/java/com/mitchellbosecke/pebble/macro/TestMacroCalls.java
index b9054334..074e518e 100644
--- a/pebble/src/test/java/com/mitchellbosecke/pebble/macro/TestMacroCalls.java
+++ b/pebble/src/test/java/com/mitchellbosecke/pebble/macro/TestMacroCalls.java
@@ -161,4 +161,22 @@ class TestMacroCalls {
           .startsWith("More than one named template can not share the same name"));
     }
   }
+
+  @Test
+  void testInvalidAliasReferencingUnknownMacro() throws IOException {
+    PebbleEngine pebble = new PebbleEngine.Builder().build();
+
+    try {
+      PebbleTemplate template = pebble
+          .getTemplate("templates/macros/invalid.from.unknownMacro.peb");
+      Writer writer = new StringWriter();
+      template.evaluate(writer);
+      fail("expected PebbleException");
+    } catch (PebbleException e) {
+      assertEquals(
+              "Function or Macro [iDontExist] referenced by alias [macro_test] does not exist.",
+              e.getPebbleMessage()
+      );
+    }
+  }
 }
diff --git a/pebble/src/test/resources/templates/macros/invalid.from.unknownMacro.peb b/pebble/src/test/resources/templates/macros/invalid.from.unknownMacro.peb
new file mode 100644
index 00000000..07b950c9
--- /dev/null
+++ b/pebble/src/test/resources/templates/macros/invalid.from.unknownMacro.peb
@@ -0,0 +1,4 @@
+Hello
+
+{% from "templates/macros/macro.peb" import iDontExist as macro_test %}
+Call 1: {{ macro_test() }}