Skip to content

Commit cda07c0

Browse files
jasmith-hsjasmith-hs
committed
Revert "Merge pull request #1125 from HubSpot/revert-whitespace-changes"
This reverts commit f3b8912, reversing changes made to 3b99a0a.
1 parent f3b8912 commit cda07c0

9 files changed

Lines changed: 235 additions & 38 deletions

File tree

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

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,28 @@
33
/**
44
* This class allows Jinjava to be configured to override legacy behaviour.
55
* LegacyOverrides.NONE signifies that none of the legacy functionality will be overridden.
6+
* LegacyOverrides.ALL signifies that all new functionality will be used; avoid legacy "bugs".
67
*/
78
public class LegacyOverrides {
89
public static final LegacyOverrides NONE = new LegacyOverrides.Builder().build();
10+
public static final LegacyOverrides ALL = new LegacyOverrides.Builder()
11+
.withEvaluateMapKeys(true)
12+
.withIterateOverMapKeys(true)
13+
.withUsePyishObjectMapper(true)
14+
.withUseSnakeCasePropertyNaming(true)
15+
.withWhitespaceRequiredWithinTokens(true)
16+
.withUseNaturalOperatorPrecedence(true)
17+
.withParseWhitespaceControlStrictly(true)
18+
.withAllowAdjacentTextNodes(true)
19+
.build();
920
private final boolean evaluateMapKeys;
1021
private final boolean iterateOverMapKeys;
1122
private final boolean usePyishObjectMapper;
1223
private final boolean useSnakeCasePropertyNaming;
1324
private final boolean whitespaceRequiredWithinTokens;
1425
private final boolean useNaturalOperatorPrecedence;
1526
private final boolean parseWhitespaceControlStrictly;
27+
private final boolean allowAdjacentTextNodes;
1628

1729
private LegacyOverrides(Builder builder) {
1830
evaluateMapKeys = builder.evaluateMapKeys;
@@ -22,6 +34,7 @@ private LegacyOverrides(Builder builder) {
2234
whitespaceRequiredWithinTokens = builder.whitespaceRequiredWithinTokens;
2335
useNaturalOperatorPrecedence = builder.useNaturalOperatorPrecedence;
2436
parseWhitespaceControlStrictly = builder.parseWhitespaceControlStrictly;
37+
allowAdjacentTextNodes = builder.allowAdjacentTextNodes;
2538
}
2639

2740
public static Builder newBuilder() {
@@ -56,6 +69,10 @@ public boolean isParseWhitespaceControlStrictly() {
5669
return parseWhitespaceControlStrictly;
5770
}
5871

72+
public boolean isAllowAdjacentTextNodes() {
73+
return allowAdjacentTextNodes;
74+
}
75+
5976
public static class Builder {
6077
private boolean evaluateMapKeys = false;
6178
private boolean iterateOverMapKeys = false;
@@ -64,6 +81,7 @@ public static class Builder {
6481
private boolean whitespaceRequiredWithinTokens = false;
6582
private boolean useNaturalOperatorPrecedence = false;
6683
private boolean parseWhitespaceControlStrictly = false;
84+
private boolean allowAdjacentTextNodes = false;
6785

6886
private Builder() {}
6987

@@ -83,7 +101,8 @@ public static Builder from(LegacyOverrides legacyOverrides) {
83101
.withUseNaturalOperatorPrecedence(legacyOverrides.useNaturalOperatorPrecedence)
84102
.withParseWhitespaceControlStrictly(
85103
legacyOverrides.parseWhitespaceControlStrictly
86-
);
104+
)
105+
.withAllowAdjacentTextNodes(legacyOverrides.allowAdjacentTextNodes);
87106
}
88107

89108
public Builder withEvaluateMapKeys(boolean evaluateMapKeys) {
@@ -126,5 +145,10 @@ public Builder withParseWhitespaceControlStrictly(
126145
this.parseWhitespaceControlStrictly = parseWhitespaceControlStrictly;
127146
return this;
128147
}
148+
149+
public Builder withAllowAdjacentTextNodes(boolean allowAdjacentTextNodes) {
150+
this.allowAdjacentTextNodes = allowAdjacentTextNodes;
151+
return this;
152+
}
129153
}
130154
}

‎src/main/java/com/hubspot/jinjava/tree/TreeParser.java‎

Lines changed: 29 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,15 @@ public Node buildTree() {
6262
Node node = nextNode();
6363

6464
if (node != null) {
65-
if (node instanceof TextNode && getLastSibling() instanceof TextNode) {
65+
if (
66+
node instanceof TextNode &&
67+
getLastSibling() instanceof TextNode &&
68+
!interpreter.getConfig().getLegacyOverrides().isAllowAdjacentTextNodes()
69+
) {
6670
// merge adjacent text nodes so whitespace control properly applies
67-
getLastSibling().getMaster().mergeImageAndContent(node.getMaster());
71+
((TextToken) getLastSibling().getMaster()).mergeImageAndContent(
72+
(TextToken) node.getMaster()
73+
);
6874
} else {
6975
parent.getChildren().add(node);
7076
}
@@ -96,6 +102,12 @@ public Node buildTree() {
96102

97103
private Node nextNode() {
98104
Token token = scanner.next();
105+
if (token.isLeftTrim()) {
106+
final Node lastSibling = getLastSibling();
107+
if (lastSibling instanceof TextNode) {
108+
lastSibling.getMaster().setRightTrim(true);
109+
}
110+
}
99111

100112
if (token.getType() == symbols.getFixed()) {
101113
if (token instanceof UnclosedToken) {
@@ -170,7 +182,7 @@ private Node text(TextToken textToken) {
170182
final Node lastSibling = getLastSibling();
171183

172184
// if last sibling was a tag and has rightTrimAfterEnd, strip whitespace
173-
if (lastSibling instanceof TagNode && isRightTrim((TagNode) lastSibling)) {
185+
if (lastSibling != null && isRightTrim(lastSibling)) {
174186
textToken.setLeftTrim(true);
175187
}
176188

@@ -186,18 +198,21 @@ private Node text(TextToken textToken) {
186198
return n;
187199
}
188200

189-
private boolean isRightTrim(TagNode lastSibling) {
190-
return (
191-
lastSibling.getEndName() == null ||
192-
(
193-
lastSibling.getTag() instanceof FlexibleTag &&
194-
!((FlexibleTag) lastSibling.getTag()).hasEndTag(
195-
(TagToken) lastSibling.getMaster()
196-
)
201+
private boolean isRightTrim(Node lastSibling) {
202+
if (lastSibling instanceof TagNode) {
203+
return (
204+
((TagNode) lastSibling).getEndName() == null ||
205+
(
206+
((TagNode) lastSibling).getTag() instanceof FlexibleTag &&
207+
!((FlexibleTag) ((TagNode) lastSibling).getTag()).hasEndTag(
208+
(TagToken) lastSibling.getMaster()
209+
)
210+
)
197211
)
198-
)
199-
? lastSibling.getMaster().isRightTrim()
200-
: lastSibling.getMaster().isRightTrimAfterEnd();
212+
? lastSibling.getMaster().isRightTrim()
213+
: lastSibling.getMaster().isRightTrimAfterEnd();
214+
}
215+
return lastSibling.getMaster().isRightTrim();
201216
}
202217

203218
private Node expression(ExpressionToken expressionToken) {
@@ -242,14 +257,6 @@ private Node tag(TagToken tagToken) {
242257
if (tag instanceof EndTag) {
243258
endTag(tag, tagToken);
244259
return null;
245-
} else {
246-
// if a tag has left trim, mark the last sibling to trim right whitespace
247-
if (tagToken.isLeftTrim()) {
248-
final Node lastSibling = getLastSibling();
249-
if (lastSibling instanceof TextNode) {
250-
lastSibling.getMaster().setRightTrim(true);
251-
}
252-
}
253260
}
254261

255262
TagNode node = new TagNode(tag, tagToken, symbols);
@@ -268,16 +275,6 @@ private Node tag(TagToken tagToken) {
268275
}
269276

270277
private void endTag(Tag tag, TagToken tagToken) {
271-
final Node lastSibling = getLastSibling();
272-
273-
if (
274-
parent instanceof TagNode &&
275-
tagToken.isLeftTrim() &&
276-
lastSibling instanceof TextNode
277-
) {
278-
lastSibling.getMaster().setRightTrim(true);
279-
}
280-
281278
if (parent.getMaster() != null) { // root node
282279
parent.getMaster().setRightTrimAfterEnd(tagToken.isRightTrim());
283280
}

‎src/main/java/com/hubspot/jinjava/tree/output/OutputList.java‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,14 @@
33
import com.hubspot.jinjava.interpret.JinjavaInterpreter;
44
import com.hubspot.jinjava.interpret.OutputTooBigException;
55
import com.hubspot.jinjava.interpret.TemplateError;
6+
import com.hubspot.jinjava.tree.parse.TokenScannerSymbols;
67
import com.hubspot.jinjava.util.LengthLimitingStringBuilder;
78
import java.util.LinkedList;
89
import java.util.List;
910

1011
public class OutputList {
12+
public static final String PREVENT_ACCIDENTAL_EXPRESSIONS =
13+
"PREVENT_ACCIDENTAL_EXPRESSIONS";
1114
private final List<OutputNode> nodes = new LinkedList<>();
1215
private final List<BlockPlaceholderOutputNode> blocks = new LinkedList<>();
1316
private final long maxOutputSize;
@@ -48,6 +51,72 @@ public List<BlockPlaceholderOutputNode> getBlocks() {
4851
public String getValue() {
4952
LengthLimitingStringBuilder val = new LengthLimitingStringBuilder(maxOutputSize);
5053

54+
return JinjavaInterpreter
55+
.getCurrentMaybe()
56+
.map(JinjavaInterpreter::getConfig)
57+
.filter(
58+
config ->
59+
config
60+
.getFeatures()
61+
.getActivationStrategy(PREVENT_ACCIDENTAL_EXPRESSIONS)
62+
.isActive(null)
63+
)
64+
.map(
65+
config -> joinNodesWithoutAddingExpressions(val, config.getTokenScannerSymbols())
66+
)
67+
.orElseGet(() -> joinNodes(val));
68+
}
69+
70+
private String joinNodesWithoutAddingExpressions(
71+
LengthLimitingStringBuilder val,
72+
TokenScannerSymbols tokenScannerSymbols
73+
) {
74+
String separator = getWhitespaceSeparator(tokenScannerSymbols);
75+
String prev = null;
76+
String cur;
77+
for (OutputNode node : nodes) {
78+
try {
79+
cur = node.getValue();
80+
if (
81+
prev != null &&
82+
prev.length() > 0 &&
83+
prev.charAt(prev.length() - 1) == tokenScannerSymbols.getExprStartChar()
84+
) {
85+
if (
86+
cur.length() > 0 &&
87+
TokenScannerSymbols.isNoteTagOrExprChar(tokenScannerSymbols, cur.charAt(0))
88+
) {
89+
val.append(separator);
90+
}
91+
}
92+
prev = cur;
93+
val.append(node.getValue());
94+
} catch (OutputTooBigException e) {
95+
JinjavaInterpreter
96+
.getCurrent()
97+
.addError(TemplateError.fromOutputTooBigException(e));
98+
return val.toString();
99+
}
100+
}
101+
102+
return val.toString();
103+
}
104+
105+
private static String getWhitespaceSeparator(TokenScannerSymbols tokenScannerSymbols) {
106+
@SuppressWarnings("StringBufferReplaceableByString")
107+
String separator = new StringBuilder()
108+
.append('\n')
109+
.append(tokenScannerSymbols.getPrefixChar())
110+
.append(tokenScannerSymbols.getNoteChar())
111+
.append(tokenScannerSymbols.getTrimChar())
112+
.append(' ')
113+
.append(tokenScannerSymbols.getNoteChar())
114+
.append(tokenScannerSymbols.getExprEndChar())
115+
.toString();
116+
return separator;
117+
}
118+
119+
private String joinNodes(LengthLimitingStringBuilder val) {
51120
for (OutputNode node : nodes) {
52121
try {
53122
val.append(node.getValue());

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

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

18+
import org.apache.commons.lang3.StringUtils;
19+
1820
public class NoteToken extends Token {
1921
private static final long serialVersionUID = -3859011447900311329L;
2022

@@ -37,6 +39,9 @@ public int getType() {
3739
*/
3840
@Override
3941
protected void parse() {
42+
if (StringUtils.isNotEmpty(image)) {
43+
handleTrim(image.substring(2, image.length() - 2));
44+
}
4045
content = "";
4146
}
4247

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,13 @@ public TextToken(
2929
super(image, lineNumber, startPosition, symbols);
3030
}
3131

32+
public void mergeImageAndContent(TextToken otherToken) {
33+
String thisOutput = output();
34+
String otherTokenOutput = otherToken.output();
35+
this.image = thisOutput + otherTokenOutput;
36+
this.content = image;
37+
}
38+
3239
@Override
3340
public int getType() {
3441
return getSymbols().getFixed();

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

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,11 +53,6 @@ public String getImage() {
5353
return image;
5454
}
5555

56-
public void mergeImageAndContent(Token otherToken) {
57-
this.image = image + otherToken.image;
58-
this.content = content + otherToken.content;
59-
}
60-
6156
public int getLineNumber() {
6257
return lineNumber;
6358
}

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,4 +122,10 @@ public String getClosingComment() {
122122
}
123123
return closingComment;
124124
}
125+
126+
public static boolean isNoteTagOrExprChar(TokenScannerSymbols symbols, char c) {
127+
return (
128+
c == symbols.getNote() || c == symbols.getTag() || c == symbols.getExprStartChar()
129+
);
130+
}
125131
}

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

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,19 @@
88
import com.google.common.collect.Lists;
99
import com.hubspot.jinjava.Jinjava;
1010
import com.hubspot.jinjava.JinjavaConfig;
11+
import com.hubspot.jinjava.features.FeatureConfig;
12+
import com.hubspot.jinjava.features.FeatureStrategies;
1113
import com.hubspot.jinjava.interpret.JinjavaInterpreter.InterpreterScopeClosable;
1214
import com.hubspot.jinjava.interpret.TemplateError.ErrorItem;
1315
import com.hubspot.jinjava.interpret.TemplateError.ErrorReason;
1416
import com.hubspot.jinjava.interpret.TemplateError.ErrorType;
17+
import com.hubspot.jinjava.mode.EagerExecutionMode;
1518
import com.hubspot.jinjava.mode.PreserveRawExecutionMode;
1619
import com.hubspot.jinjava.objects.date.FormattedDate;
1720
import com.hubspot.jinjava.objects.date.StrftimeFormatter;
1821
import com.hubspot.jinjava.tree.TextNode;
1922
import com.hubspot.jinjava.tree.output.BlockInfo;
23+
import com.hubspot.jinjava.tree.output.OutputList;
2024
import com.hubspot.jinjava.tree.parse.TextToken;
2125
import com.hubspot.jinjava.tree.parse.TokenScannerSymbols;
2226
import java.time.ZoneId;
@@ -503,4 +507,53 @@ public void itFiltersDuplicateErrors() {
503507

504508
assertThat(interpreter.getErrors()).containsExactly(error1, error2);
505509
}
510+
511+
@Test
512+
public void itPreventsAccidentalExpressions() {
513+
String makeExpression = "if (true) {\n{%- print deferred -%}\n}";
514+
String makeTag = "if (true) {\n{%- print '% print 123 %' -%}\n}";
515+
String makeNote = "if (true) {\n{%- print '# note #' -%}\n}";
516+
jinjava.getGlobalContext().put("deferred", DeferredValue.instance());
517+
518+
JinjavaInterpreter normalInterpreter = new JinjavaInterpreter(
519+
jinjava,
520+
jinjava.getGlobalContext(),
521+
JinjavaConfig.newBuilder().withExecutionMode(EagerExecutionMode.instance()).build()
522+
);
523+
JinjavaInterpreter preventingInterpreter = new JinjavaInterpreter(
524+
jinjava,
525+
jinjava.getGlobalContext(),
526+
JinjavaConfig
527+
.newBuilder()
528+
.withFeatureConfig(
529+
FeatureConfig
530+
.newBuilder()
531+
.add(OutputList.PREVENT_ACCIDENTAL_EXPRESSIONS, FeatureStrategies.ACTIVE)
532+
.build()
533+
)
534+
.withExecutionMode(EagerExecutionMode.instance())
535+
.build()
536+
);
537+
JinjavaInterpreter.pushCurrent(normalInterpreter);
538+
try {
539+
assertThat(normalInterpreter.render(makeExpression))
540+
.isEqualTo("if (true) {{% print deferred %}}");
541+
assertThat(normalInterpreter.render(makeTag))
542+
.isEqualTo("if (true) {% print 123 %}");
543+
assertThat(normalInterpreter.render(makeNote)).isEqualTo("if (true) {# note #}");
544+
} finally {
545+
JinjavaInterpreter.popCurrent();
546+
}
547+
JinjavaInterpreter.pushCurrent(preventingInterpreter);
548+
try {
549+
assertThat(preventingInterpreter.render(makeExpression))
550+
.isEqualTo("if (true) {\n" + "{#- #}{% print deferred %}}");
551+
assertThat(preventingInterpreter.render(makeTag))
552+
.isEqualTo("if (true) {\n" + "{#- #}% print 123 %}");
553+
assertThat(preventingInterpreter.render(makeNote))
554+
.isEqualTo("if (true) {\n" + "{#- #}# note #}");
555+
} finally {
556+
JinjavaInterpreter.popCurrent();
557+
}
558+
}
506559
}

0 commit comments

Comments
 (0)