diff --git a/src/main/java/com/hubspot/jinjava/LegacyOverrides.java b/src/main/java/com/hubspot/jinjava/LegacyOverrides.java index 5ff01314f..f38e0f681 100644 --- a/src/main/java/com/hubspot/jinjava/LegacyOverrides.java +++ b/src/main/java/com/hubspot/jinjava/LegacyOverrides.java @@ -3,9 +3,21 @@ /** * This class allows Jinjava to be configured to override legacy behaviour. * LegacyOverrides.NONE signifies that none of the legacy functionality will be overridden. + * LegacyOverrides.ALL signifies that all new functionality will be used; avoid legacy "bugs". */ public class LegacyOverrides { public static final LegacyOverrides NONE = new LegacyOverrides.Builder().build(); + public static final LegacyOverrides ALL = new LegacyOverrides.Builder() + .withEvaluateMapKeys(true) + .withIterateOverMapKeys(true) + .withUsePyishObjectMapper(true) + .withUseSnakeCasePropertyNaming(true) + .withWhitespaceRequiredWithinTokens(true) + .withUseNaturalOperatorPrecedence(true) + .withParseWhitespaceControlStrictly(true) + .withAllowAdjacentTextNodes(true) + .withUseTrimmingForNotesAndExpressions(true) + .build(); private final boolean evaluateMapKeys; private final boolean iterateOverMapKeys; private final boolean usePyishObjectMapper; @@ -13,6 +25,8 @@ public class LegacyOverrides { private final boolean whitespaceRequiredWithinTokens; private final boolean useNaturalOperatorPrecedence; private final boolean parseWhitespaceControlStrictly; + private final boolean allowAdjacentTextNodes; + private final boolean useTrimmingForNotesAndExpressions; private LegacyOverrides(Builder builder) { evaluateMapKeys = builder.evaluateMapKeys; @@ -22,6 +36,8 @@ private LegacyOverrides(Builder builder) { whitespaceRequiredWithinTokens = builder.whitespaceRequiredWithinTokens; useNaturalOperatorPrecedence = builder.useNaturalOperatorPrecedence; parseWhitespaceControlStrictly = builder.parseWhitespaceControlStrictly; + allowAdjacentTextNodes = builder.allowAdjacentTextNodes; + useTrimmingForNotesAndExpressions = builder.useTrimmingForNotesAndExpressions; } public static Builder newBuilder() { @@ -56,6 +72,14 @@ public boolean isParseWhitespaceControlStrictly() { return parseWhitespaceControlStrictly; } + public boolean isAllowAdjacentTextNodes() { + return allowAdjacentTextNodes; + } + + public boolean isUseTrimmingForNotesAndExpressions() { + return useTrimmingForNotesAndExpressions; + } + public static class Builder { private boolean evaluateMapKeys = false; private boolean iterateOverMapKeys = false; @@ -64,6 +88,8 @@ public static class Builder { private boolean whitespaceRequiredWithinTokens = false; private boolean useNaturalOperatorPrecedence = false; private boolean parseWhitespaceControlStrictly = false; + private boolean allowAdjacentTextNodes = false; + private boolean useTrimmingForNotesAndExpressions = false; private Builder() {} @@ -83,6 +109,10 @@ public static Builder from(LegacyOverrides legacyOverrides) { .withUseNaturalOperatorPrecedence(legacyOverrides.useNaturalOperatorPrecedence) .withParseWhitespaceControlStrictly( legacyOverrides.parseWhitespaceControlStrictly + ) + .withAllowAdjacentTextNodes(legacyOverrides.allowAdjacentTextNodes) + .withUseTrimmingForNotesAndExpressions( + legacyOverrides.useTrimmingForNotesAndExpressions ); } @@ -126,5 +156,17 @@ public Builder withParseWhitespaceControlStrictly( this.parseWhitespaceControlStrictly = parseWhitespaceControlStrictly; return this; } + + public Builder withAllowAdjacentTextNodes(boolean allowAdjacentTextNodes) { + this.allowAdjacentTextNodes = allowAdjacentTextNodes; + return this; + } + + public Builder withUseTrimmingForNotesAndExpressions( + boolean useTrimmingForNotesAndExpressions + ) { + this.useTrimmingForNotesAndExpressions = useTrimmingForNotesAndExpressions; + return this; + } } } diff --git a/src/main/java/com/hubspot/jinjava/tree/TreeParser.java b/src/main/java/com/hubspot/jinjava/tree/TreeParser.java index 07fbbb142..ca739829f 100644 --- a/src/main/java/com/hubspot/jinjava/tree/TreeParser.java +++ b/src/main/java/com/hubspot/jinjava/tree/TreeParser.java @@ -17,6 +17,7 @@ import com.google.common.collect.Iterators; import com.google.common.collect.PeekingIterator; +import com.hubspot.jinjava.JinjavaConfig; import com.hubspot.jinjava.interpret.DisabledException; import com.hubspot.jinjava.interpret.JinjavaInterpreter; import com.hubspot.jinjava.interpret.MissingEndTagException; @@ -62,9 +63,15 @@ public Node buildTree() { Node node = nextNode(); if (node != null) { - if (node instanceof TextNode && getLastSibling() instanceof TextNode) { + if ( + node instanceof TextNode && + getLastSibling() instanceof TextNode && + !interpreter.getConfig().getLegacyOverrides().isAllowAdjacentTextNodes() + ) { // merge adjacent text nodes so whitespace control properly applies - getLastSibling().getMaster().mergeImageAndContent(node.getMaster()); + ((TextToken) getLastSibling().getMaster()).mergeImageAndContent( + (TextToken) node.getMaster() + ); } else { parent.getChildren().add(node); } @@ -96,6 +103,12 @@ public Node buildTree() { private Node nextNode() { Token token = scanner.next(); + if (token.isLeftTrim() && isTrimmingEnabledForToken(token, interpreter.getConfig())) { + final Node lastSibling = getLastSibling(); + if (lastSibling instanceof TextNode) { + lastSibling.getMaster().setRightTrim(true); + } + } if (token.getType() == symbols.getFixed()) { if (token instanceof UnclosedToken) { @@ -170,7 +183,11 @@ private Node text(TextToken textToken) { final Node lastSibling = getLastSibling(); // if last sibling was a tag and has rightTrimAfterEnd, strip whitespace - if (lastSibling instanceof TagNode && isRightTrim((TagNode) lastSibling)) { + if ( + lastSibling != null && + isRightTrim(lastSibling) && + isTrimmingEnabledForToken(lastSibling.getMaster(), interpreter.getConfig()) + ) { textToken.setLeftTrim(true); } @@ -186,18 +203,21 @@ private Node text(TextToken textToken) { return n; } - private boolean isRightTrim(TagNode lastSibling) { - return ( - lastSibling.getEndName() == null || - ( - lastSibling.getTag() instanceof FlexibleTag && - !((FlexibleTag) lastSibling.getTag()).hasEndTag( - (TagToken) lastSibling.getMaster() - ) + private boolean isRightTrim(Node lastSibling) { + if (lastSibling instanceof TagNode) { + return ( + ((TagNode) lastSibling).getEndName() == null || + ( + ((TagNode) lastSibling).getTag() instanceof FlexibleTag && + !((FlexibleTag) ((TagNode) lastSibling).getTag()).hasEndTag( + (TagToken) lastSibling.getMaster() + ) + ) ) - ) - ? lastSibling.getMaster().isRightTrim() - : lastSibling.getMaster().isRightTrimAfterEnd(); + ? lastSibling.getMaster().isRightTrim() + : lastSibling.getMaster().isRightTrimAfterEnd(); + } + return lastSibling.getMaster().isRightTrim(); } private Node expression(ExpressionToken expressionToken) { @@ -242,14 +262,6 @@ private Node tag(TagToken tagToken) { if (tag instanceof EndTag) { endTag(tag, tagToken); return null; - } else { - // if a tag has left trim, mark the last sibling to trim right whitespace - if (tagToken.isLeftTrim()) { - final Node lastSibling = getLastSibling(); - if (lastSibling instanceof TextNode) { - lastSibling.getMaster().setRightTrim(true); - } - } } TagNode node = new TagNode(tag, tagToken, symbols); @@ -268,16 +280,6 @@ private Node tag(TagToken tagToken) { } private void endTag(Tag tag, TagToken tagToken) { - final Node lastSibling = getLastSibling(); - - if ( - parent instanceof TagNode && - tagToken.isLeftTrim() && - lastSibling instanceof TextNode - ) { - lastSibling.getMaster().setRightTrim(true); - } - if (parent.getMaster() != null) { // root node parent.getMaster().setRightTrimAfterEnd(tagToken.isRightTrim()); } @@ -318,4 +320,11 @@ private void endTag(Tag tag, TagToken tagToken) { ); } } + + private boolean isTrimmingEnabledForToken(Token token, JinjavaConfig jinjavaConfig) { + if (token instanceof TagToken || token instanceof TextToken) { + return true; + } + return jinjavaConfig.getLegacyOverrides().isUseTrimmingForNotesAndExpressions(); + } } diff --git a/src/main/java/com/hubspot/jinjava/tree/output/OutputList.java b/src/main/java/com/hubspot/jinjava/tree/output/OutputList.java index 738e9062c..3ab3ec9f3 100644 --- a/src/main/java/com/hubspot/jinjava/tree/output/OutputList.java +++ b/src/main/java/com/hubspot/jinjava/tree/output/OutputList.java @@ -3,11 +3,14 @@ import com.hubspot.jinjava.interpret.JinjavaInterpreter; import com.hubspot.jinjava.interpret.OutputTooBigException; import com.hubspot.jinjava.interpret.TemplateError; +import com.hubspot.jinjava.tree.parse.TokenScannerSymbols; import com.hubspot.jinjava.util.LengthLimitingStringBuilder; import java.util.LinkedList; import java.util.List; public class OutputList { + public static final String PREVENT_ACCIDENTAL_EXPRESSIONS = + "PREVENT_ACCIDENTAL_EXPRESSIONS"; private final List nodes = new LinkedList<>(); private final List blocks = new LinkedList<>(); private final long maxOutputSize; @@ -48,6 +51,72 @@ public List getBlocks() { public String getValue() { LengthLimitingStringBuilder val = new LengthLimitingStringBuilder(maxOutputSize); + return JinjavaInterpreter + .getCurrentMaybe() + .map(JinjavaInterpreter::getConfig) + .filter( + config -> + config + .getFeatures() + .getActivationStrategy(PREVENT_ACCIDENTAL_EXPRESSIONS) + .isActive(null) + ) + .map( + config -> joinNodesWithoutAddingExpressions(val, config.getTokenScannerSymbols()) + ) + .orElseGet(() -> joinNodes(val)); + } + + private String joinNodesWithoutAddingExpressions( + LengthLimitingStringBuilder val, + TokenScannerSymbols tokenScannerSymbols + ) { + String separator = getWhitespaceSeparator(tokenScannerSymbols); + String prev = null; + String cur; + for (OutputNode node : nodes) { + try { + cur = node.getValue(); + if ( + prev != null && + prev.length() > 0 && + prev.charAt(prev.length() - 1) == tokenScannerSymbols.getExprStartChar() + ) { + if ( + cur.length() > 0 && + TokenScannerSymbols.isNoteTagOrExprChar(tokenScannerSymbols, cur.charAt(0)) + ) { + val.append(separator); + } + } + prev = cur; + val.append(node.getValue()); + } catch (OutputTooBigException e) { + JinjavaInterpreter + .getCurrent() + .addError(TemplateError.fromOutputTooBigException(e)); + return val.toString(); + } + } + + return val.toString(); + } + + private static String getWhitespaceSeparator(TokenScannerSymbols tokenScannerSymbols) { + @SuppressWarnings("StringBufferReplaceableByString") + String separator = new StringBuilder() + .append('\n') + .append(tokenScannerSymbols.getPrefixChar()) + .append(tokenScannerSymbols.getNoteChar()) + .append(tokenScannerSymbols.getTrimChar()) + .append(' ') + .append(tokenScannerSymbols.getNoteChar()) + .append(tokenScannerSymbols.getExprEndChar()) + .toString(); + return separator; + } + + private String joinNodes(LengthLimitingStringBuilder val) { for (OutputNode node : nodes) { try { val.append(node.getValue()); diff --git a/src/main/java/com/hubspot/jinjava/tree/parse/NoteToken.java b/src/main/java/com/hubspot/jinjava/tree/parse/NoteToken.java index fe98c4dda..c57f8e124 100644 --- a/src/main/java/com/hubspot/jinjava/tree/parse/NoteToken.java +++ b/src/main/java/com/hubspot/jinjava/tree/parse/NoteToken.java @@ -15,6 +15,8 @@ **********************************************************************/ package com.hubspot.jinjava.tree.parse; +import org.apache.commons.lang3.StringUtils; + public class NoteToken extends Token { private static final long serialVersionUID = -3859011447900311329L; @@ -37,6 +39,9 @@ public int getType() { */ @Override protected void parse() { + if (StringUtils.isNotEmpty(image)) { + handleTrim(image.substring(2, image.length() - 2)); + } content = ""; } diff --git a/src/main/java/com/hubspot/jinjava/tree/parse/TextToken.java b/src/main/java/com/hubspot/jinjava/tree/parse/TextToken.java index dc3d64b42..edfbb4d3e 100644 --- a/src/main/java/com/hubspot/jinjava/tree/parse/TextToken.java +++ b/src/main/java/com/hubspot/jinjava/tree/parse/TextToken.java @@ -29,6 +29,13 @@ public TextToken( super(image, lineNumber, startPosition, symbols); } + public void mergeImageAndContent(TextToken otherToken) { + String thisOutput = output(); + String otherTokenOutput = otherToken.output(); + this.image = thisOutput + otherTokenOutput; + this.content = image; + } + @Override public int getType() { return getSymbols().getFixed(); diff --git a/src/main/java/com/hubspot/jinjava/tree/parse/Token.java b/src/main/java/com/hubspot/jinjava/tree/parse/Token.java index dda5bd63b..487987626 100644 --- a/src/main/java/com/hubspot/jinjava/tree/parse/Token.java +++ b/src/main/java/com/hubspot/jinjava/tree/parse/Token.java @@ -53,11 +53,6 @@ public String getImage() { return image; } - public void mergeImageAndContent(Token otherToken) { - this.image = image + otherToken.image; - this.content = content + otherToken.content; - } - public int getLineNumber() { return lineNumber; } diff --git a/src/main/java/com/hubspot/jinjava/tree/parse/TokenScannerSymbols.java b/src/main/java/com/hubspot/jinjava/tree/parse/TokenScannerSymbols.java index ed499556b..5f561d198 100644 --- a/src/main/java/com/hubspot/jinjava/tree/parse/TokenScannerSymbols.java +++ b/src/main/java/com/hubspot/jinjava/tree/parse/TokenScannerSymbols.java @@ -122,4 +122,10 @@ public String getClosingComment() { } return closingComment; } + + public static boolean isNoteTagOrExprChar(TokenScannerSymbols symbols, char c) { + return ( + c == symbols.getNote() || c == symbols.getTag() || c == symbols.getExprStartChar() + ); + } } diff --git a/src/test/java/com/hubspot/jinjava/interpret/JinjavaInterpreterTest.java b/src/test/java/com/hubspot/jinjava/interpret/JinjavaInterpreterTest.java index 6b6b7733b..16e3d990b 100644 --- a/src/test/java/com/hubspot/jinjava/interpret/JinjavaInterpreterTest.java +++ b/src/test/java/com/hubspot/jinjava/interpret/JinjavaInterpreterTest.java @@ -8,15 +8,19 @@ import com.google.common.collect.Lists; import com.hubspot.jinjava.Jinjava; import com.hubspot.jinjava.JinjavaConfig; +import com.hubspot.jinjava.features.FeatureConfig; +import com.hubspot.jinjava.features.FeatureStrategies; import com.hubspot.jinjava.interpret.JinjavaInterpreter.InterpreterScopeClosable; import com.hubspot.jinjava.interpret.TemplateError.ErrorItem; import com.hubspot.jinjava.interpret.TemplateError.ErrorReason; import com.hubspot.jinjava.interpret.TemplateError.ErrorType; +import com.hubspot.jinjava.mode.EagerExecutionMode; import com.hubspot.jinjava.mode.PreserveRawExecutionMode; import com.hubspot.jinjava.objects.date.FormattedDate; import com.hubspot.jinjava.objects.date.StrftimeFormatter; import com.hubspot.jinjava.tree.TextNode; import com.hubspot.jinjava.tree.output.BlockInfo; +import com.hubspot.jinjava.tree.output.OutputList; import com.hubspot.jinjava.tree.parse.TextToken; import com.hubspot.jinjava.tree.parse.TokenScannerSymbols; import java.time.ZoneId; @@ -503,4 +507,53 @@ public void itFiltersDuplicateErrors() { assertThat(interpreter.getErrors()).containsExactly(error1, error2); } + + @Test + public void itPreventsAccidentalExpressions() { + String makeExpression = "if (true) {\n{%- print deferred -%}\n}"; + String makeTag = "if (true) {\n{%- print '% print 123 %' -%}\n}"; + String makeNote = "if (true) {\n{%- print '# note #' -%}\n}"; + jinjava.getGlobalContext().put("deferred", DeferredValue.instance()); + + JinjavaInterpreter normalInterpreter = new JinjavaInterpreter( + jinjava, + jinjava.getGlobalContext(), + JinjavaConfig.newBuilder().withExecutionMode(EagerExecutionMode.instance()).build() + ); + JinjavaInterpreter preventingInterpreter = new JinjavaInterpreter( + jinjava, + jinjava.getGlobalContext(), + JinjavaConfig + .newBuilder() + .withFeatureConfig( + FeatureConfig + .newBuilder() + .add(OutputList.PREVENT_ACCIDENTAL_EXPRESSIONS, FeatureStrategies.ACTIVE) + .build() + ) + .withExecutionMode(EagerExecutionMode.instance()) + .build() + ); + JinjavaInterpreter.pushCurrent(normalInterpreter); + try { + assertThat(normalInterpreter.render(makeExpression)) + .isEqualTo("if (true) {{% print deferred %}}"); + assertThat(normalInterpreter.render(makeTag)) + .isEqualTo("if (true) {% print 123 %}"); + assertThat(normalInterpreter.render(makeNote)).isEqualTo("if (true) {# note #}"); + } finally { + JinjavaInterpreter.popCurrent(); + } + JinjavaInterpreter.pushCurrent(preventingInterpreter); + try { + assertThat(preventingInterpreter.render(makeExpression)) + .isEqualTo("if (true) {\n" + "{#- #}{% print deferred %}}"); + assertThat(preventingInterpreter.render(makeTag)) + .isEqualTo("if (true) {\n" + "{#- #}% print 123 %}"); + assertThat(preventingInterpreter.render(makeNote)) + .isEqualTo("if (true) {\n" + "{#- #}# note #}"); + } finally { + JinjavaInterpreter.popCurrent(); + } + } } diff --git a/src/test/java/com/hubspot/jinjava/tree/TreeParserTest.java b/src/test/java/com/hubspot/jinjava/tree/TreeParserTest.java index 6ba9065ca..6c15f396e 100644 --- a/src/test/java/com/hubspot/jinjava/tree/TreeParserTest.java +++ b/src/test/java/com/hubspot/jinjava/tree/TreeParserTest.java @@ -6,6 +6,7 @@ import com.hubspot.jinjava.BaseInterpretingTest; import com.hubspot.jinjava.Jinjava; import com.hubspot.jinjava.JinjavaConfig; +import com.hubspot.jinjava.LegacyOverrides; import com.hubspot.jinjava.interpret.TemplateError.ErrorType; import java.nio.charset.StandardCharsets; import org.junit.Test; @@ -224,6 +225,76 @@ public void itWarnsTwiceAgainstUnclosedBlockTag() { assertThat(interpreter.getErrors().get(1).getLineno()).isEqualTo(1); } + @Test + public void itTrimsNotes() { + String expression = "A\n{#- note -#}\nB"; + final Node tree = new TreeParser(interpreter, expression).buildTree(); + assertThat(interpreter.render(tree)).isEqualTo("A\n\nB"); + interpreter = + new Jinjava( + JinjavaConfig + .newBuilder() + .withLegacyOverrides( + LegacyOverrides + .newBuilder() + .withUseTrimmingForNotesAndExpressions(true) + .build() + ) + .build() + ) + .newInterpreter(); + final Node newTree = new TreeParser(interpreter, expression).buildTree(); + assertThat(interpreter.render(newTree)).isEqualTo("AB"); + } + + @Test + public void itMergesTextNodesWhileRespectingTrim() { + String expression = "{% print 'A' -%}\n{#- note -#}\nB\n{%- print 'C' %}"; + final Node tree = new TreeParser(interpreter, expression).buildTree(); + assertThat(interpreter.render(tree)).isEqualTo("ABC"); + } + + @Test + public void itTrimsExpressions() { + String expression = "A\n{{- 'B' -}}\nC"; + final Node tree = new TreeParser(interpreter, expression).buildTree(); + assertThat(interpreter.render(tree)).isEqualTo("A\nB\nC"); + interpreter = + new Jinjava( + JinjavaConfig + .newBuilder() + .withLegacyOverrides( + LegacyOverrides + .newBuilder() + .withUseTrimmingForNotesAndExpressions(true) + .build() + ) + .build() + ) + .newInterpreter(); + final Node newTree = new TreeParser(interpreter, expression).buildTree(); + assertThat(interpreter.render(newTree)).isEqualTo("ABC"); + } + + @Test + public void itDoesNotMergeAdjacentTextNodesWhenLegacyOverrideIsApplied() { + String expression = "A\n{%- if true -%}\n{# comment #}\nB{% endif %}"; + final Node tree = new TreeParser(interpreter, expression).buildTree(); + assertThat(interpreter.render(tree)).isEqualTo("AB"); + interpreter = + new Jinjava( + JinjavaConfig + .newBuilder() + .withLegacyOverrides( + LegacyOverrides.newBuilder().withAllowAdjacentTextNodes(true).build() + ) + .build() + ) + .newInterpreter(); + final Node overriddenTree = new TreeParser(interpreter, expression).buildTree(); + assertThat(interpreter.render(overriddenTree)).isEqualTo("A\nB"); + } + Node parse(String fixture) { try { return new TreeParser(