Skip to content

Commit b21ca24

Browse files
tkindytkindy
authored
Reintroduce whitespace control parsing fix with legacy override (#903)
1 parent 5cb58e9 commit b21ca24

9 files changed

Lines changed: 206 additions & 21 deletions

File tree

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

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,15 @@ public class LegacyOverrides {
1111
private final boolean usePyishObjectMapper;
1212
private final boolean whitespaceRequiredWithinTokens;
1313
private final boolean useNaturalOperatorPrecedence;
14+
private final boolean parseWhitespaceControlStrictly;
1415

1516
private LegacyOverrides(Builder builder) {
1617
evaluateMapKeys = builder.evaluateMapKeys;
1718
iterateOverMapKeys = builder.iterateOverMapKeys;
1819
usePyishObjectMapper = builder.usePyishObjectMapper;
1920
whitespaceRequiredWithinTokens = builder.whitespaceRequiredWithinTokens;
2021
useNaturalOperatorPrecedence = builder.useNaturalOperatorPrecedence;
22+
parseWhitespaceControlStrictly = builder.parseWhitespaceControlStrictly;
2123
}
2224

2325
public static Builder newBuilder() {
@@ -44,12 +46,17 @@ public boolean isUseNaturalOperatorPrecedence() {
4446
return useNaturalOperatorPrecedence;
4547
}
4648

49+
public boolean isParseWhitespaceControlStrictly() {
50+
return parseWhitespaceControlStrictly;
51+
}
52+
4753
public static class Builder {
4854
private boolean evaluateMapKeys = false;
4955
private boolean iterateOverMapKeys = false;
5056
private boolean usePyishObjectMapper = false;
5157
private boolean whitespaceRequiredWithinTokens = false;
5258
private boolean useNaturalOperatorPrecedence = false;
59+
private boolean parseWhitespaceControlStrictly = false;
5360

5461
private Builder() {}
5562

@@ -65,7 +72,10 @@ public static Builder from(LegacyOverrides legacyOverrides) {
6572
.withWhitespaceRequiredWithinTokens(
6673
legacyOverrides.whitespaceRequiredWithinTokens
6774
)
68-
.withUseNaturalOperatorPrecedence(legacyOverrides.useNaturalOperatorPrecedence);
75+
.withUseNaturalOperatorPrecedence(legacyOverrides.useNaturalOperatorPrecedence)
76+
.withParseWhitespaceControlStrictly(
77+
legacyOverrides.parseWhitespaceControlStrictly
78+
);
6979
}
7080

7181
public Builder withEvaluateMapKeys(boolean evaluateMapKeys) {
@@ -96,5 +106,12 @@ public Builder withUseNaturalOperatorPrecedence(
96106
this.useNaturalOperatorPrecedence = useNaturalOperatorPrecedence;
97107
return this;
98108
}
109+
110+
public Builder withParseWhitespaceControlStrictly(
111+
boolean parseWhitespaceControlStrictly
112+
) {
113+
this.parseWhitespaceControlStrictly = parseWhitespaceControlStrictly;
114+
return this;
115+
}
99116
}
100117
}

‎src/main/java/com/hubspot/jinjava/tree/parse/ExpressionToken.java‎

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -44,16 +44,7 @@ public int getType() {
4444
@Override
4545
protected void parse() {
4646
this.expr = WhitespaceUtils.unwrap(image, "{{", "}}");
47-
48-
if (WhitespaceUtils.startsWith(expr, "-")) {
49-
setLeftTrim(true);
50-
this.expr = WhitespaceUtils.unwrap(expr, "-", "");
51-
}
52-
if (WhitespaceUtils.endsWith(expr, "-")) {
53-
setRightTrim(true);
54-
this.expr = WhitespaceUtils.unwrap(expr, "", "-");
55-
}
56-
47+
this.expr = handleTrim(expr);
5748
this.expr = StringUtils.trimToEmpty(this.expr);
5849
}
5950

‎src/main/java/com/hubspot/jinjava/tree/parse/LenientWhitespaceControlParser.java‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
package com.hubspot.jinjava.tree.parse;
2+
3+
import com.hubspot.jinjava.util.WhitespaceUtils;
4+
5+
public class LenientWhitespaceControlParser implements WhitespaceControlParser {
6+
7+
@Override
8+
public boolean hasLeftTrim(String unwrapped) {
9+
return WhitespaceUtils.startsWith(unwrapped, "-");
10+
}
11+
12+
@Override
13+
public String stripLeft(String unwrapped) {
14+
return WhitespaceUtils.unwrap(unwrapped, "-", "");
15+
}
16+
17+
@Override
18+
public boolean hasRightTrim(String unwrapped) {
19+
return WhitespaceUtils.endsWith(unwrapped, "-");
20+
}
21+
22+
@Override
23+
public String stripRight(String unwrapped) {
24+
return WhitespaceUtils.unwrap(unwrapped, "", "-");
25+
}
26+
}

‎src/main/java/com/hubspot/jinjava/tree/parse/StrictWhitespaceControlParser.java‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
package com.hubspot.jinjava.tree.parse;
2+
3+
public class StrictWhitespaceControlParser implements WhitespaceControlParser {
4+
5+
@Override
6+
public boolean hasLeftTrim(String unwrapped) {
7+
return unwrapped.charAt(0) == '-';
8+
}
9+
10+
@Override
11+
public String stripLeft(String unwrapped) {
12+
return unwrapped.substring(1);
13+
}
14+
15+
@Override
16+
public boolean hasRightTrim(String unwrapped) {
17+
return unwrapped.charAt(unwrapped.length() - 1) == '-';
18+
}
19+
20+
@Override
21+
public String stripRight(String unwrapped) {
22+
return unwrapped.substring(0, unwrapped.length() - 1);
23+
}
24+
}

‎src/main/java/com/hubspot/jinjava/tree/parse/TagToken.java‎

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
package com.hubspot.jinjava.tree.parse;
1717

1818
import com.hubspot.jinjava.interpret.TemplateSyntaxException;
19-
import com.hubspot.jinjava.util.WhitespaceUtils;
2019

2120
public class TagToken extends Token {
2221
private static final long serialVersionUID = -4927751270481832992L;
@@ -54,15 +53,7 @@ protected void parse() {
5453
}
5554

5655
content = image.substring(2, image.length() - 2);
57-
58-
if (WhitespaceUtils.startsWith(content, "-")) {
59-
setLeftTrim(true);
60-
content = WhitespaceUtils.unwrap(content, "-", "");
61-
}
62-
if (WhitespaceUtils.endsWith(content, "-")) {
63-
setRightTrim(true);
64-
content = WhitespaceUtils.unwrap(content, "", "-");
65-
}
56+
content = handleTrim(content);
6657

6758
int nameStart = -1, pos = 0, len = content.length();
6859

‎src/main/java/com/hubspot/jinjava/tree/parse/Token.java‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@
1515
**********************************************************************/
1616
package com.hubspot.jinjava.tree.parse;
1717

18+
import com.hubspot.jinjava.JinjavaConfig;
19+
import com.hubspot.jinjava.LegacyOverrides;
20+
import com.hubspot.jinjava.interpret.JinjavaInterpreter;
1821
import com.hubspot.jinjava.interpret.UnexpectedTokenException;
1922
import java.io.Serializable;
2023

@@ -83,6 +86,36 @@ public void setRightTrimAfterEnd(boolean rightTrimAfterEnd) {
8386
this.rightTrimAfterEnd = rightTrimAfterEnd;
8487
}
8588

89+
/**
90+
* Handle any whitespace control characters, capturing whether leading or trailing
91+
* whitespace should be stripped.
92+
* @param unwrapped the content of the block stripped of its delimeters
93+
* @return the content stripped of any whitespace control characters.
94+
*/
95+
protected final String handleTrim(String unwrapped) {
96+
boolean parseWhitespaceControlStrictly = JinjavaInterpreter
97+
.getCurrentMaybe()
98+
.map(JinjavaInterpreter::getConfig)
99+
.map(JinjavaConfig::getLegacyOverrides)
100+
.map(LegacyOverrides::isParseWhitespaceControlStrictly)
101+
.orElse(false);
102+
103+
WhitespaceControlParser parser = parseWhitespaceControlStrictly
104+
? WhitespaceControlParser.STRICT
105+
: WhitespaceControlParser.LENIENT;
106+
107+
String result = unwrapped;
108+
if (parser.hasLeftTrim(result)) {
109+
setLeftTrim(true);
110+
result = parser.stripLeft(result);
111+
}
112+
if (parser.hasRightTrim(result)) {
113+
setRightTrim(true);
114+
result = parser.stripRight(result);
115+
}
116+
return result;
117+
}
118+
86119
public int getStartPosition() {
87120
return startPosition;
88121
}

‎src/main/java/com/hubspot/jinjava/tree/parse/WhitespaceControlParser.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
package com.hubspot.jinjava.tree.parse;
2+
3+
public interface WhitespaceControlParser {
4+
WhitespaceControlParser LENIENT = new LenientWhitespaceControlParser();
5+
WhitespaceControlParser STRICT = new StrictWhitespaceControlParser();
6+
7+
boolean hasLeftTrim(String unwrapped);
8+
String stripLeft(String unwrapped);
9+
10+
boolean hasRightTrim(String unwrapped);
11+
String stripRight(String unwrapped);
12+
}

‎src/test/java/com/hubspot/jinjava/interpret/JinjavaInterpreterTest.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -343,4 +343,14 @@ public void itInterpretsFilterChainsInOrder() {
343343
assertThat(interpreter.render("{{ 'foo' | upper | replace('O', 'A') }}"))
344344
.isEqualTo("FAA");
345345
}
346+
347+
@Test
348+
public void itInterpretsWhitespaceControl() {
349+
assertThat(interpreter.render(". {%- set x = 5 -%} .")).isEqualTo("..");
350+
}
351+
352+
@Test
353+
public void itInterpretsEmptyExpressions() {
354+
assertThat(interpreter.render("{{}}")).isEqualTo("");
355+
}
346356
}

‎src/test/java/com/hubspot/jinjava/interpret/LegacyWhitespaceControlParsingTest.java‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
package com.hubspot.jinjava.interpret;
2+
3+
import static org.assertj.core.api.Assertions.assertThat;
4+
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
5+
6+
import com.hubspot.jinjava.Jinjava;
7+
import com.hubspot.jinjava.JinjavaConfig;
8+
import com.hubspot.jinjava.LegacyOverrides;
9+
import java.util.HashMap;
10+
import org.junit.Before;
11+
import org.junit.Test;
12+
13+
public class LegacyWhitespaceControlParsingTest {
14+
Jinjava legacy;
15+
Jinjava modern;
16+
17+
@Before
18+
public void setUp() throws Exception {
19+
legacy =
20+
new Jinjava(
21+
JinjavaConfig.newBuilder().withLegacyOverrides(LegacyOverrides.NONE).build()
22+
);
23+
modern =
24+
new Jinjava(
25+
JinjavaConfig
26+
.newBuilder()
27+
.withLegacyOverrides(
28+
LegacyOverrides.newBuilder().withParseWhitespaceControlStrictly(true).build()
29+
)
30+
.build()
31+
);
32+
}
33+
34+
@Test
35+
public void itInterpretsStandaloneNegatives() {
36+
String template = "{{ -10 }}";
37+
38+
assertThat(legacy.render(template, new HashMap<>())).isEqualTo("10");
39+
assertThat(modern.render(template, new HashMap<>())).isEqualTo("-10");
40+
}
41+
42+
@Test
43+
public void itInterpretsStandaloneNegativesWithWhitespace() {
44+
String template = "{{ - 10 }}";
45+
46+
assertThat(legacy.render(template, new HashMap<>())).isEqualTo("10");
47+
48+
// Somewhat surprising, but this aligns with Jinja
49+
assertThat(modern.render(template, new HashMap<>())).isEqualTo("-10");
50+
}
51+
52+
@Test
53+
public void itErrorsOnTrailingDash() {
54+
String template = "{{ 10- }}";
55+
56+
assertThat(legacy.render(template, new HashMap<>())).isEqualTo("10");
57+
assertThatExceptionOfType(FatalTemplateErrorsException.class)
58+
.isThrownBy(() -> modern.render(template, new HashMap<>()))
59+
.withMessageContaining("syntax error at position 6");
60+
}
61+
62+
@Test
63+
public void itErrorsOnSpacedTrailingDash() {
64+
String template = "{{ 10 - }}";
65+
66+
assertThat(legacy.render(template, new HashMap<>())).isEqualTo("10");
67+
assertThatExceptionOfType(FatalTemplateErrorsException.class)
68+
.isThrownBy(() -> modern.render(template, new HashMap<>()))
69+
.withMessageContaining("syntax error at position 7");
70+
}
71+
72+
@Test
73+
public void itErrorsOnSpacedDashesOnBothSides() {
74+
String template = "{{ - 10 - }}";
75+
76+
assertThat(legacy.render(template, new HashMap<>())).isEqualTo("10");
77+
assertThatExceptionOfType(FatalTemplateErrorsException.class)
78+
.isThrownBy(() -> modern.render(template, new HashMap<>()))
79+
.withMessageContaining("syntax error at position 9");
80+
}
81+
}

0 commit comments

Comments
 (0)