Skip to content

Commit 9a9ee9f

Browse files
committed
Scanner treats backslash as escape character inside quoted strings only
1 parent 4d42df5 commit 9a9ee9f

5 files changed

Lines changed: 283 additions & 6 deletions

File tree

src/main/java/com/hubspot/jinjava/LegacyOverrides.java

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ public interface LegacyOverrides extends WithLegacyOverrides {
2121
.withAllowAdjacentTextNodes(true)
2222
.withUseTrimmingForNotesAndExpressions(true)
2323
.withKeepNullableLoopValues(true)
24+
.withHandleBackslashInQuotesOnly(true)
2425
.build();
2526
LegacyOverrides ALL = new Builder()
2627
.withEvaluateMapKeys(true)
@@ -32,6 +33,7 @@ public interface LegacyOverrides extends WithLegacyOverrides {
3233
.withAllowAdjacentTextNodes(true)
3334
.withUseTrimmingForNotesAndExpressions(true)
3435
.withKeepNullableLoopValues(true)
36+
.withHandleBackslashInQuotesOnly(true)
3537
.build();
3638

3739
@Value.Default
@@ -79,6 +81,23 @@ default boolean isKeepNullableLoopValues() {
7981
return false;
8082
}
8183

84+
/**
85+
* When {@code true}, the token scanner treats backslash as an escape character
86+
* only inside quoted string literals, leaving bare backslashes outside quotes
87+
* untouched for the expression parser (JUEL) to handle. This matches the
88+
* behaviour of Python's Jinja2, where the template scanner is not responsible
89+
* for backslash interpretation at all.
90+
*
91+
* <p>When {@code false} (the default), the scanner consumes a backslash and
92+
* the following character unconditionally, regardless of quote context. This
93+
* is the legacy Jinjava behaviour, which prevents closing delimiters from
94+
* being recognized after a backslash but diverges from Jinja2.
95+
*/
96+
@Value.Default
97+
default boolean isHandleBackslashInQuotesOnly() {
98+
return false;
99+
}
100+
82101
class Builder extends ImmutableLegacyOverrides.Builder {}
83102

84103
static Builder newBuilder() {

src/main/java/com/hubspot/jinjava/tree/parse/StringTokenScanner.java

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,11 @@ public class StringTokenScanner extends AbstractIterator<Token> {
6868
private final char[] lineStmtPrefix;
6969
private final char[] lineCommentPrefix;
7070

71+
// When true, backslash is treated as an escape character only inside quoted
72+
// string literals, matching Jinja2 behaviour. When false (legacy default),
73+
// the scanner consumes backslash + next char unconditionally.
74+
private final boolean backslashInQuotesOnly;
75+
7176
// Remembers where the current opening delimiter began so the emitted block/comment
7277
// token image starts from the opener (not the content), letting parse() strip the
7378
// correct number of delimiter characters from both ends.
@@ -97,6 +102,8 @@ public StringTokenScanner(String input, JinjavaConfig config) {
97102

98103
String lcp = symbols.getLineCommentPrefix();
99104
lineCommentPrefix = (lcp != null && !lcp.isEmpty()) ? lcp.toCharArray() : null;
105+
106+
backslashInQuotesOnly = config.getLegacyOverrides().isHandleBackslashInQuotesOnly();
100107
}
101108

102109
// ── Core scanning loop ────────────────────────────────────────────────────
@@ -202,9 +209,7 @@ private Token scanInsideComment() {
202209
*/
203210
private Token scanInsideBlock(char c) {
204211
if (inQuote != 0) {
205-
// Inside a quoted string: a backslash escapes the next character so a
206-
// delimiter or quote character following it does not prematurely close
207-
// the block or the string.
212+
// Inside a quoted string: a backslash always escapes the next character.
208213
if (c == '\\') {
209214
currPost += (currPost + 1 < length) ? 2 : 1;
210215
return DELIMITER_MATCHED;
@@ -215,8 +220,9 @@ private Token scanInsideBlock(char c) {
215220
currPost++;
216221
return DELIMITER_MATCHED;
217222
}
218-
// Outside a quoted string: a backslash escapes the next character.
219-
if (c == '\\') {
223+
// Outside a quoted string: only consume the backslash if the legacy
224+
// flag is enabled; otherwise leave it for the expression parser.
225+
if (c == '\\' && !backslashInQuotesOnly) {
220226
currPost += (currPost + 1 < length) ? 2 : 1;
221227
return DELIMITER_MATCHED;
222228
}

src/main/java/com/hubspot/jinjava/tree/parse/TokenScanner.java

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,11 @@ public class TokenScanner extends AbstractIterator<Token> {
4949
private final TokenScannerSymbols symbols;
5050
private final WhitespaceControlParser whitespaceControlParser;
5151

52+
// When true, backslash is treated as an escape character only inside quoted
53+
// string literals, matching Jinja2 behaviour. When false (legacy default),
54+
// the scanner consumes backslash + next char unconditionally.
55+
private final boolean backslashInQuotesOnly;
56+
5257
public TokenScanner(String input, JinjavaConfig config) {
5358
this.config = config;
5459

@@ -71,6 +76,7 @@ public TokenScanner(String input, JinjavaConfig config) {
7176
config.getLegacyOverrides().isParseWhitespaceControlStrictly()
7277
? WhitespaceControlParser.STRICT
7378
: WhitespaceControlParser.LENIENT;
79+
backslashInQuotesOnly = config.getLegacyOverrides().isHandleBackslashInQuotesOnly();
7480
}
7581

7682
private Token getNextToken() {
@@ -82,10 +88,14 @@ private Token getNextToken() {
8288
}
8389

8490
if (inBlock > 0) {
85-
if (c == '\\') {
91+
if (c == '\\' && !backslashInQuotesOnly) {
8692
++currPost;
8793
continue;
8894
} else if (inQuote != 0) {
95+
if (c == '\\') {
96+
++currPost;
97+
continue;
98+
}
8999
if (inQuote == c) {
90100
inQuote = 0;
91101
}
Lines changed: 234 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,234 @@
1+
package com.hubspot.jinjava.tree.parse;
2+
3+
import static org.assertj.core.api.Assertions.assertThat;
4+
5+
import com.google.common.collect.AbstractIterator;
6+
import com.google.common.collect.ImmutableMap;
7+
import com.hubspot.jinjava.Jinjava;
8+
import com.hubspot.jinjava.JinjavaConfig;
9+
import com.hubspot.jinjava.LegacyOverrides;
10+
import java.util.ArrayList;
11+
import java.util.HashMap;
12+
import java.util.List;
13+
import org.junit.Test;
14+
15+
/**
16+
* Tests for backslash handling inside block/variable/comment delimiters,
17+
* covering both the char-based (DefaultTokenScannerSymbols) and string-based
18+
* (StringTokenScannerSymbols) scanning paths, with the
19+
* {@link LegacyOverrides#isHandleBackslashInQuotesOnly()} flag both off (legacy)
20+
* and on (Jinja2-compatible).
21+
*/
22+
public class BackslashHandlingTest {
23+
24+
// ── Jinjava instances ──────────────────────────────────────────────────────
25+
26+
/** Char-based scanner, legacy backslash behaviour (flag = false). */
27+
private static Jinjava charLegacy() {
28+
return new Jinjava(
29+
JinjavaConfig
30+
.newBuilder()
31+
.withLegacyOverrides(LegacyOverrides.newBuilder().build())
32+
.build()
33+
);
34+
}
35+
36+
/** Char-based scanner, Jinja2-compatible backslash behaviour (flag = true). */
37+
private static Jinjava charNew() {
38+
return new Jinjava(
39+
JinjavaConfig
40+
.newBuilder()
41+
.withLegacyOverrides(
42+
LegacyOverrides.newBuilder().withHandleBackslashInQuotesOnly(true).build()
43+
)
44+
.build()
45+
);
46+
}
47+
48+
/** String-based scanner, legacy backslash behaviour (flag = false). */
49+
private static Jinjava stringLegacy() {
50+
return new Jinjava(
51+
JinjavaConfig
52+
.newBuilder()
53+
.withTokenScannerSymbols(StringTokenScannerSymbols.builder().build())
54+
.withLegacyOverrides(LegacyOverrides.newBuilder().build())
55+
.build()
56+
);
57+
}
58+
59+
/** String-based scanner, Jinja2-compatible backslash behaviour (flag = true). */
60+
private static Jinjava stringNew() {
61+
return new Jinjava(
62+
JinjavaConfig
63+
.newBuilder()
64+
.withTokenScannerSymbols(StringTokenScannerSymbols.builder().build())
65+
.withLegacyOverrides(
66+
LegacyOverrides.newBuilder().withHandleBackslashInQuotesOnly(true).build()
67+
)
68+
.build()
69+
);
70+
}
71+
72+
// ── Backslash inside a quoted string ──────────────────────────────────────
73+
//
74+
// Both legacy and new behaviour must handle escaped quotes inside strings
75+
// correctly — \" should not close the string.
76+
77+
@Test
78+
public void charLegacy_escapedQuoteInsideString() {
79+
assertThat(charLegacy().render("{{ \"he said \\\"hi\\\"\" }}", new HashMap<>()))
80+
.isEqualTo("he said \"hi\"");
81+
}
82+
83+
@Test
84+
public void charNew_escapedQuoteInsideString() {
85+
assertThat(charNew().render("{{ \"he said \\\"hi\\\"\" }}", new HashMap<>()))
86+
.isEqualTo("he said \"hi\"");
87+
}
88+
89+
@Test
90+
public void stringLegacy_escapedQuoteInsideString() {
91+
assertThat(stringLegacy().render("{{ \"he said \\\"hi\\\"\" }}", new HashMap<>()))
92+
.isEqualTo("he said \"hi\"");
93+
}
94+
95+
@Test
96+
public void stringNew_escapedQuoteInsideString() {
97+
assertThat(stringNew().render("{{ \"he said \\\"hi\\\"\" }}", new HashMap<>()))
98+
.isEqualTo("he said \"hi\"");
99+
}
100+
101+
// ── Backslash outside a quoted string ─────────────────────────────────────
102+
//
103+
// Template under test: "prefix {{ x \}} suffix }}"
104+
//
105+
// We test the scanner token structure directly rather than going through
106+
// render(), because the expression "x \..." is always a JUEL lexical error
107+
// regardless of mode. What differs between modes is which token boundaries
108+
// the scanner produces — and that is what we assert on.
109+
//
110+
// Legacy (backslashInQuotesOnly = false):
111+
// Scanner consumes '\' and skips the following '}'. The first '}}' is not
112+
// recognized as a closer. The block runs until the second '}}', so the
113+
// token sequence is:
114+
// TEXT "prefix " | EXPR "{{ x \}} suffix }}"
115+
//
116+
// New (backslashInQuotesOnly = true):
117+
// Scanner leaves '\' untouched. The first '}}' is recognized as the closer.
118+
// The token sequence is:
119+
// TEXT "prefix " | EXPR "{{ x \}}" | TEXT " suffix }}"
120+
121+
private static final String BACKSLASH_TEMPLATE = "prefix {{ x \\}} suffix }}";
122+
123+
@Test
124+
public void charLegacy_backslashConsumesOneDelimiterChar_blockRunsToSecondCloser() {
125+
List<Token> tokens = scanAll(
126+
new TokenScanner(BACKSLASH_TEMPLATE, charLegacy().getGlobalConfig())
127+
);
128+
assertThat(tokens).hasSize(2);
129+
assertThat(tokens.get(0)).isInstanceOf(TextToken.class);
130+
assertThat(tokens.get(0).image).isEqualTo("prefix ");
131+
assertThat(tokens.get(1)).isInstanceOf(ExpressionToken.class);
132+
assertThat(tokens.get(1).image).isEqualTo("{{ x \\}} suffix }}");
133+
}
134+
135+
@Test
136+
public void charNew_backslashIgnored_blockClosesAtFirstDelimiter() {
137+
List<Token> tokens = scanAll(
138+
new TokenScanner(BACKSLASH_TEMPLATE, charNew().getGlobalConfig())
139+
);
140+
assertThat(tokens).hasSize(3);
141+
assertThat(tokens.get(0)).isInstanceOf(TextToken.class);
142+
assertThat(tokens.get(0).image).isEqualTo("prefix ");
143+
assertThat(tokens.get(1)).isInstanceOf(ExpressionToken.class);
144+
assertThat(tokens.get(1).image).isEqualTo("{{ x \\}}");
145+
assertThat(tokens.get(2)).isInstanceOf(TextToken.class);
146+
assertThat(tokens.get(2).image).isEqualTo(" suffix }}");
147+
}
148+
149+
@Test
150+
public void stringLegacy_backslashConsumesOneDelimiterChar_blockRunsToSecondCloser() {
151+
List<Token> tokens = scanAll(
152+
new StringTokenScanner(BACKSLASH_TEMPLATE, stringLegacy().getGlobalConfig())
153+
);
154+
assertThat(tokens).hasSize(2);
155+
assertThat(tokens.get(0)).isInstanceOf(TextToken.class);
156+
assertThat(tokens.get(0).image).isEqualTo("prefix ");
157+
assertThat(tokens.get(1)).isInstanceOf(ExpressionToken.class);
158+
assertThat(tokens.get(1).image).isEqualTo("{{ x \\}} suffix }}");
159+
}
160+
161+
@Test
162+
public void stringNew_backslashIgnored_blockClosesAtFirstDelimiter() {
163+
List<Token> tokens = scanAll(
164+
new StringTokenScanner(BACKSLASH_TEMPLATE, stringNew().getGlobalConfig())
165+
);
166+
assertThat(tokens).hasSize(3);
167+
assertThat(tokens.get(0)).isInstanceOf(TextToken.class);
168+
assertThat(tokens.get(0).image).isEqualTo("prefix ");
169+
assertThat(tokens.get(1)).isInstanceOf(ExpressionToken.class);
170+
assertThat(tokens.get(1).image).isEqualTo("{{ x \\}}");
171+
assertThat(tokens.get(2)).isInstanceOf(TextToken.class);
172+
assertThat(tokens.get(2).image).isEqualTo(" suffix }}");
173+
}
174+
175+
private static List<Token> scanAll(AbstractIterator<Token> scanner) {
176+
List<Token> tokens = new ArrayList<>();
177+
scanner.forEachRemaining(tokens::add);
178+
return tokens;
179+
}
180+
181+
// ── Backslash in a plain variable expression ───────────────────────────────
182+
//
183+
// The most common real-world case: a Windows path or similar string passed
184+
// directly as a variable value. The backslash is in the *value*, not the
185+
// template, so scanner behaviour is irrelevant — both modes should render
186+
// identically.
187+
188+
@Test
189+
public void backslashInVariableValueIsUnaffectedByFlag_char() {
190+
ImmutableMap<String, Object> ctx = ImmutableMap.of("path", "C:\\Users\\foo");
191+
assertThat(charLegacy().render("{{ path }}", ctx)).isEqualTo("C:\\Users\\foo");
192+
assertThat(charNew().render("{{ path }}", ctx)).isEqualTo("C:\\Users\\foo");
193+
}
194+
195+
@Test
196+
public void backslashInVariableValueIsUnaffectedByFlag_string() {
197+
ImmutableMap<String, Object> ctx = ImmutableMap.of("path", "C:\\Users\\foo");
198+
assertThat(stringLegacy().render("{{ path }}", ctx)).isEqualTo("C:\\Users\\foo");
199+
assertThat(stringNew().render("{{ path }}", ctx)).isEqualTo("C:\\Users\\foo");
200+
}
201+
202+
// ── New behaviour: simple expressions are unaffected ──────────────────────
203+
//
204+
// Expressions with no backslash should behave identically under both modes.
205+
206+
@Test
207+
public void charNew_simpleExpressionUnchanged() {
208+
assertThat(charNew().render("{{ greeting }}", ImmutableMap.of("greeting", "hello")))
209+
.isEqualTo("hello");
210+
}
211+
212+
@Test
213+
public void stringNew_simpleExpressionUnchanged() {
214+
assertThat(stringNew().render("{{ greeting }}", ImmutableMap.of("greeting", "hello")))
215+
.isEqualTo("hello");
216+
}
217+
218+
// ── LegacyOverrides preset assertions ─────────────────────────────────────
219+
220+
@Test
221+
public void allPresetDoesNotEnableNewBackslashHandling() {
222+
assertThat(LegacyOverrides.ALL.isHandleBackslashInQuotesOnly()).isTrue();
223+
}
224+
225+
@Test
226+
public void threePointZeroPresetDoesNotEnableNewBackslashHandling() {
227+
assertThat(LegacyOverrides.THREE_POINT_0.isHandleBackslashInQuotesOnly()).isTrue();
228+
}
229+
230+
@Test
231+
public void nonePresetKeepsLegacyBackslashHandling() {
232+
assertThat(LegacyOverrides.NONE.isHandleBackslashInQuotesOnly()).isFalse();
233+
}
234+
}

src/test/java/com/hubspot/jinjava/tree/parse/TokenScannerTest.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import com.google.common.io.Resources;
99
import com.hubspot.jinjava.BaseJinjavaTest;
1010
import com.hubspot.jinjava.JinjavaConfig;
11+
import com.hubspot.jinjava.LegacyOverrides;
1112
import com.hubspot.jinjava.features.BuiltInFeatures;
1213
import com.hubspot.jinjava.features.FeatureConfig;
1314
import com.hubspot.jinjava.features.FeatureStrategies;
@@ -319,6 +320,13 @@ public void testLstripBlocks() {
319320

320321
@Test
321322
public void itTreatsEscapedQuotesSameWhenNotInQuotes() {
323+
config =
324+
BaseJinjavaTest
325+
.newConfigBuilder()
326+
.withLegacyOverrides(
327+
LegacyOverrides.newBuilder().withHandleBackslashInQuotesOnly(false).build()
328+
)
329+
.build();
322330
List<Token> tokens = tokens("tag-with-all-escaped-quotes");
323331
assertThat(tokens).hasSize(8);
324332
assertThat(tokens.stream().map(Token::getType).collect(Collectors.toList()))

0 commit comments

Comments
 (0)