Scanner treats backslash as escape character inside quoted strings only

jmoraleda · jmoraleda · commit 7c6964aec16e · 2026-04-05T23:48:23.000-04:00
diff --git a/src/main/java/com/hubspot/jinjava/LegacyOverrides.java b/src/main/java/com/hubspot/jinjava/LegacyOverrides.java
@@ -32,6 +32,7 @@ public interface LegacyOverrides extends WithLegacyOverrides {
     .withAllowAdjacentTextNodes(true)
     .withUseTrimmingForNotesAndExpressions(true)
     .withKeepNullableLoopValues(true)
+    .withHandleBackslashInQuotesOnly(true)
     .build();
 
   @Value.Default
@@ -79,6 +80,23 @@ default boolean isKeepNullableLoopValues() {
     return false;
   }
 
+  /**
+   * When {@code true}, the token scanner treats backslash as an escape character
+   * only inside quoted string literals, leaving bare backslashes outside quotes
+   * untouched for the expression parser (JUEL) to handle. This matches the
+   * behaviour of Python's Jinja2, where the template scanner is not responsible
+   * for backslash interpretation at all.
+   *
+   * <p>When {@code false} (the default), the scanner consumes a backslash and
+   * the following character unconditionally, regardless of quote context. This
+   * is the legacy Jinjava behaviour, which prevents closing delimiters from
+   * being recognized after a backslash but diverges from Jinja2.
+   */
+  @Value.Default
+  default boolean isHandleBackslashInQuotesOnly() {
+    return false;
+  }
+
   class Builder extends ImmutableLegacyOverrides.Builder {}
 
   static Builder newBuilder() {
diff --git a/src/main/java/com/hubspot/jinjava/tree/parse/TokenScanner.java b/src/main/java/com/hubspot/jinjava/tree/parse/TokenScanner.java
@@ -55,6 +55,11 @@ public class TokenScanner extends AbstractIterator<Token> {
   private final char[] lineStmtPrefix;
   private final char[] lineCommentPrefix;
 
+  // When true, backslash is treated as an escape character only inside quoted
+  // string literals, matching Jinja2 behaviour. When false (legacy default),
+  // the scanner consumes backslash + next char unconditionally.
+  private final boolean backslashInQuotesOnly;
+
   // Remembers where the current opening delimiter began so the emitted block/comment
   // token image starts from the opener (not the content), letting parse() strip the
   // correct number of delimiter characters from both ends.
@@ -84,6 +89,7 @@ public TokenScanner(String input, JinjavaConfig config) {
       config.getLegacyOverrides().isParseWhitespaceControlStrictly()
         ? WhitespaceControlParser.STRICT
         : WhitespaceControlParser.LENIENT;
+    backslashInQuotesOnly = config.getLegacyOverrides().isHandleBackslashInQuotesOnly();
 
     if (stringBased) {
       varStart = symbols.getExpressionStart().toCharArray();
@@ -214,9 +220,7 @@ private Token scanInsideComment() {
    */
   private Token scanInsideBlock(char c) {
     if (inQuote != 0) {
-      // Inside a quoted string: a backslash escapes the next character so a
-      // delimiter or quote character following it does not prematurely close
-      // the block or the string.
+      // Inside a quoted string: a backslash always escapes the next character.
       if (c == '\\') {
         currPost += (currPost + 1 < length) ? 2 : 1;
         return DELIMITER_MATCHED;
@@ -227,8 +231,9 @@ private Token scanInsideBlock(char c) {
       currPost++;
       return DELIMITER_MATCHED;
     }
-    // Outside a quoted string: a backslash escapes the next character.
-    if (c == '\\') {
+    // Outside a quoted string: only consume the backslash if the legacy
+    // flag is enabled; otherwise leave it for the expression parser.
+    if (c == '\\' && !backslashInQuotesOnly) {
       currPost += (currPost + 1 < length) ? 2 : 1;
       return DELIMITER_MATCHED;
     }
@@ -626,10 +631,14 @@ private Token getNextTokenCharBased() {
       }
 
       if (inBlock > 0) {
-        if (c == '\\') {
+        if (c == '\\' && !backslashInQuotesOnly) {
           ++currPost;
           continue;
         } else if (inQuote != 0) {
+          if (c == '\\') {
+            ++currPost;
+            continue;
+          }
           if (inQuote == c) {
             inQuote = 0;
           }
diff --git a/src/test/java/com/hubspot/jinjava/tree/parse/BackslashHandlingTest.java b/src/test/java/com/hubspot/jinjava/tree/parse/BackslashHandlingTest.java
@@ -0,0 +1,238 @@
+package com.hubspot.jinjava.tree.parse;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import com.google.common.collect.ImmutableMap;
+import com.hubspot.jinjava.Jinjava;
+import com.hubspot.jinjava.JinjavaConfig;
+import com.hubspot.jinjava.LegacyOverrides;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import org.junit.Test;
+
+/**
+ * Tests for backslash handling inside block/variable/comment delimiters,
+ * covering both the char-based (DefaultTokenScannerSymbols) and string-based
+ * (StringTokenScannerSymbols) scanning paths, with the
+ * {@link LegacyOverrides#isHandleBackslashInQuotesOnly()} flag both off (legacy)
+ * and on (Jinja2-compatible).
+ */
+public class BackslashHandlingTest {
+
+  // ── Jinjava instances ──────────────────────────────────────────────────────
+
+  /** Char-based scanner, legacy backslash behaviour (flag = false). */
+  private static Jinjava charLegacy() {
+    return new Jinjava(
+      JinjavaConfig
+        .newBuilder()
+        .withLegacyOverrides(LegacyOverrides.newBuilder().build())
+        .build()
+    );
+  }
+
+  /** Char-based scanner, Jinja2-compatible backslash behaviour (flag = true). */
+  private static Jinjava charNew() {
+    return new Jinjava(
+      JinjavaConfig
+        .newBuilder()
+        .withLegacyOverrides(
+          LegacyOverrides.newBuilder().withHandleBackslashInQuotesOnly(true).build()
+        )
+        .build()
+    );
+  }
+
+  /** String-based scanner, legacy backslash behaviour (flag = false). */
+  private static Jinjava stringLegacy() {
+    return new Jinjava(
+      JinjavaConfig
+        .newBuilder()
+        .withTokenScannerSymbols(StringTokenScannerSymbols.builder().build())
+        .withLegacyOverrides(LegacyOverrides.newBuilder().build())
+        .build()
+    );
+  }
+
+  /** String-based scanner, Jinja2-compatible backslash behaviour (flag = true). */
+  private static Jinjava stringNew() {
+    return new Jinjava(
+      JinjavaConfig
+        .newBuilder()
+        .withTokenScannerSymbols(StringTokenScannerSymbols.builder().build())
+        .withLegacyOverrides(
+          LegacyOverrides.newBuilder().withHandleBackslashInQuotesOnly(true).build()
+        )
+        .build()
+    );
+  }
+
+  // ── Backslash inside a quoted string ──────────────────────────────────────
+  //
+  // Both legacy and new behaviour must handle escaped quotes inside strings
+  // correctly — \" should not close the string.
+
+  @Test
+  public void charLegacy_escapedQuoteInsideString() {
+    assertThat(charLegacy().render("{{ \"he said \\\"hi\\\"\" }}", new HashMap<>()))
+      .isEqualTo("he said \"hi\"");
+  }
+
+  @Test
+  public void charNew_escapedQuoteInsideString() {
+    assertThat(charNew().render("{{ \"he said \\\"hi\\\"\" }}", new HashMap<>()))
+      .isEqualTo("he said \"hi\"");
+  }
+
+  @Test
+  public void stringLegacy_escapedQuoteInsideString() {
+    assertThat(stringLegacy().render("{{ \"he said \\\"hi\\\"\" }}", new HashMap<>()))
+      .isEqualTo("he said \"hi\"");
+  }
+
+  @Test
+  public void stringNew_escapedQuoteInsideString() {
+    assertThat(stringNew().render("{{ \"he said \\\"hi\\\"\" }}", new HashMap<>()))
+      .isEqualTo("he said \"hi\"");
+  }
+
+  // ── Backslash outside a quoted string ─────────────────────────────────────
+  //
+  // Template under test: "prefix {{ x \}} suffix }}"
+  //
+  // We test the scanner token structure directly rather than going through
+  // render(), because the expression "x \..." is always a JUEL lexical error
+  // regardless of mode. What differs between modes is which token boundaries
+  // the scanner produces — and that is what we assert on.
+  //
+  // Legacy (backslashInQuotesOnly = false):
+  //   Scanner consumes '\' and skips the following '}'. The first '}}' is not
+  //   recognized as a closer. The block runs until the second '}}', so the
+  //   token sequence is:
+  //     TEXT "prefix "  |  EXPR "{{ x \}} suffix }}"
+  //
+  // New (backslashInQuotesOnly = true):
+  //   Scanner leaves '\' untouched. The first '}}' is recognized as the closer.
+  //   The token sequence is:
+  //     TEXT "prefix "  |  EXPR "{{ x \}}"  |  TEXT " suffix }}"
+
+  private static final String BACKSLASH_TEMPLATE = "prefix {{ x \\}} suffix }}";
+
+  @Test
+  public void charLegacy_backslashConsumesOneDelimiterChar_blockRunsToSecondCloser() {
+    List<Token> tokens = scanAll(
+      new TokenScanner(BACKSLASH_TEMPLATE, charLegacy().getGlobalConfig())
+    );
+    assertThat(tokens).hasSize(2);
+    assertThat(tokens.get(0)).isInstanceOf(TextToken.class);
+    assertThat(tokens.get(0).image).isEqualTo("prefix ");
+    assertThat(tokens.get(1)).isInstanceOf(ExpressionToken.class);
+    assertThat(tokens.get(1).image).isEqualTo("{{ x \\}} suffix }}");
+  }
+
+  @Test
+  public void charNew_backslashIgnored_blockClosesAtFirstDelimiter() {
+    List<Token> tokens = scanAll(
+      new TokenScanner(BACKSLASH_TEMPLATE, charNew().getGlobalConfig())
+    );
+    assertThat(tokens).hasSize(3);
+    assertThat(tokens.get(0)).isInstanceOf(TextToken.class);
+    assertThat(tokens.get(0).image).isEqualTo("prefix ");
+    assertThat(tokens.get(1)).isInstanceOf(ExpressionToken.class);
+    assertThat(tokens.get(1).image).isEqualTo("{{ x \\}}");
+    assertThat(tokens.get(2)).isInstanceOf(TextToken.class);
+    assertThat(tokens.get(2).image).isEqualTo(" suffix }}");
+  }
+
+  @Test
+  public void stringLegacy_backslashConsumesOneDelimiterChar_blockRunsToSecondCloser() {
+    List<Token> tokens = scanAll(
+      new TokenScanner(BACKSLASH_TEMPLATE, stringLegacy().getGlobalConfig())
+    );
+    assertThat(tokens).hasSize(2);
+    assertThat(tokens.get(0)).isInstanceOf(TextToken.class);
+    assertThat(tokens.get(0).image).isEqualTo("prefix ");
+    assertThat(tokens.get(1)).isInstanceOf(ExpressionToken.class);
+    assertThat(tokens.get(1).image).isEqualTo("{{ x \\}} suffix }}");
+  }
+
+  @Test
+  public void stringNew_backslashIgnored_blockClosesAtFirstDelimiter() {
+    List<Token> tokens = scanAll(
+      new TokenScanner(BACKSLASH_TEMPLATE, stringNew().getGlobalConfig())
+    );
+    assertThat(tokens).hasSize(3);
+    assertThat(tokens.get(0)).isInstanceOf(TextToken.class);
+    assertThat(tokens.get(0).image).isEqualTo("prefix ");
+    assertThat(tokens.get(1)).isInstanceOf(ExpressionToken.class);
+    assertThat(tokens.get(1).image).isEqualTo("{{ x \\}}");
+    assertThat(tokens.get(2)).isInstanceOf(TextToken.class);
+    assertThat(tokens.get(2).image).isEqualTo(" suffix }}");
+  }
+
+  private static List<Token> scanAll(TokenScanner scanner) {
+    List<Token> tokens = new ArrayList<>();
+    scanner.forEachRemaining(tokens::add);
+    return tokens;
+  }
+
+  // ── Backslash in a plain variable expression ───────────────────────────────
+  //
+  // The most common real-world case: a Windows path or similar string passed
+  // directly as a variable value. The backslash is in the *value*, not the
+  // template, so scanner behaviour is irrelevant — both modes should render
+  // identically.
+
+  @Test
+  public void backslashInVariableValueIsUnaffectedByFlag_char() {
+    ImmutableMap<String, Object> ctx = ImmutableMap.of("path", "C:\\Users\\foo");
+    assertThat(charLegacy().render("{{ path }}", ctx)).isEqualTo("C:\\Users\\foo");
+    assertThat(charNew().render("{{ path }}", ctx)).isEqualTo("C:\\Users\\foo");
+  }
+
+  @Test
+  public void backslashInVariableValueIsUnaffectedByFlag_string() {
+    ImmutableMap<String, Object> ctx = ImmutableMap.of("path", "C:\\Users\\foo");
+    assertThat(stringLegacy().render("{{ path }}", ctx)).isEqualTo("C:\\Users\\foo");
+    assertThat(stringNew().render("{{ path }}", ctx)).isEqualTo("C:\\Users\\foo");
+  }
+
+  // ── New behaviour: simple expressions are unaffected ──────────────────────
+  //
+  // Expressions with no backslash should behave identically under both modes.
+
+  @Test
+  public void charNew_simpleExpressionUnchanged() {
+    assertThat(charNew().render("{{ greeting }}", ImmutableMap.of("greeting", "hello")))
+      .isEqualTo("hello");
+  }
+
+  @Test
+  public void stringNew_simpleExpressionUnchanged() {
+    assertThat(stringNew().render("{{ greeting }}", ImmutableMap.of("greeting", "hello")))
+      .isEqualTo("hello");
+  }
+
+  // ── LegacyOverrides preset assertions ─────────────────────────────────────
+  //
+  // handleBackslashInQuotesOnly is an explicit opt-in only. It is NOT included
+  // in THREE_POINT_0 or NONE because existing templates may rely on the legacy
+  // behaviour of \} preventing delimiter recognition. Inclusion in a preset
+  // can be reconsidered in a future major version.
+
+  @Test
+  public void allPresetDoesNotEnableNewBackslashHandling() {
+    assertThat(LegacyOverrides.ALL.isHandleBackslashInQuotesOnly()).isTrue();
+  }
+
+  @Test
+  public void threePointZeroPresetDoesNotEnableNewBackslashHandling() {
+    assertThat(LegacyOverrides.THREE_POINT_0.isHandleBackslashInQuotesOnly()).isFalse();
+  }
+
+  @Test
+  public void nonePresetKeepsLegacyBackslashHandling() {
+    assertThat(LegacyOverrides.NONE.isHandleBackslashInQuotesOnly()).isFalse();
+  }
+}
