From 6fe39dfc65824fc69567ef586129b0afd8f9f97c Mon Sep 17 00:00:00 2001 From: Jack Smith Date: Fri, 20 Oct 2023 11:44:15 -0400 Subject: [PATCH 1/3] Add feature stub for preventing accidental expressions --- .../com/hubspot/jinjava/tree/output/OutputList.java | 13 +++++++++++++ 1 file changed, 13 insertions(+) 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..b3f377190 100644 --- a/src/main/java/com/hubspot/jinjava/tree/output/OutputList.java +++ b/src/main/java/com/hubspot/jinjava/tree/output/OutputList.java @@ -1,5 +1,6 @@ package com.hubspot.jinjava.tree.output; +import com.hubspot.jinjava.JinjavaConfig; import com.hubspot.jinjava.interpret.JinjavaInterpreter; import com.hubspot.jinjava.interpret.OutputTooBigException; import com.hubspot.jinjava.interpret.TemplateError; @@ -8,6 +9,8 @@ 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; @@ -46,6 +49,16 @@ public List getBlocks() { } public String getValue() { + boolean preventAccidentalExpressions = JinjavaInterpreter + .getCurrentMaybe() + .map(JinjavaInterpreter::getConfig) + .map(JinjavaConfig::getFeatures) + .map( + features -> + features.getActivationStrategy(PREVENT_ACCIDENTAL_EXPRESSIONS).isActive(null) + ) + .orElse(false); + LengthLimitingStringBuilder val = new LengthLimitingStringBuilder(maxOutputSize); for (OutputNode node : nodes) { From 1a8d43a26aaffb75f7239b5c3e5f9e63bd2079ed Mon Sep 17 00:00:00 2001 From: Jack Smith Date: Tue, 24 Oct 2023 11:30:23 -0400 Subject: [PATCH 2/3] Add trimming note block to prevent accidentally creating expressions/tags/notes when PREVENT_ACCIDENTAL_EXPRESSIONS is enabled --- .../jinjava/tree/output/OutputList.java | 69 +++++++++++++++++-- .../interpret/JinjavaInterpreterTest.java | 53 ++++++++++++++ 2 files changed, 115 insertions(+), 7 deletions(-) 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 b3f377190..5a5a39d14 100644 --- a/src/main/java/com/hubspot/jinjava/tree/output/OutputList.java +++ b/src/main/java/com/hubspot/jinjava/tree/output/OutputList.java @@ -1,9 +1,9 @@ package com.hubspot.jinjava.tree.output; -import com.hubspot.jinjava.JinjavaConfig; 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; @@ -49,18 +49,73 @@ public List getBlocks() { } public String getValue() { - boolean preventAccidentalExpressions = JinjavaInterpreter + LengthLimitingStringBuilder val = new LengthLimitingStringBuilder(maxOutputSize); + + return JinjavaInterpreter .getCurrentMaybe() .map(JinjavaInterpreter::getConfig) - .map(JinjavaConfig::getFeatures) + .filter( + config -> + config + .getFeatures() + .getActivationStrategy(PREVENT_ACCIDENTAL_EXPRESSIONS) + .isActive(null) + ) .map( - features -> - features.getActivationStrategy(PREVENT_ACCIDENTAL_EXPRESSIONS).isActive(null) + config -> joinNodesWithoutAddingExpressions(val, config.getTokenScannerSymbols()) ) - .orElse(false); + .orElseGet(() -> joinNodes(val)); + } - LengthLimitingStringBuilder val = new LengthLimitingStringBuilder(maxOutputSize); + private String joinNodesWithoutAddingExpressions( + LengthLimitingStringBuilder val, + 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(); + 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 && + ( + cur.charAt(0) == tokenScannerSymbols.getTag() || + cur.charAt(0) == tokenScannerSymbols.getExprStartChar() || + cur.charAt(0) == tokenScannerSymbols.getNoteChar() + ) + ) { + 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 String joinNodes(LengthLimitingStringBuilder val) { for (OutputNode node : nodes) { try { val.append(node.getValue()); 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(); + } + } } From 160c25461a5dae7db5f78222b657d60b6892cf4f Mon Sep 17 00:00:00 2001 From: Jack Smith Date: Fri, 27 Oct 2023 13:22:36 -0400 Subject: [PATCH 3/3] Extract static function isNoteTagOrExprChar --- .../jinjava/tree/output/OutputList.java | 31 ++++++++++--------- .../tree/parse/TokenScannerSymbols.java | 6 ++++ 2 files changed, 22 insertions(+), 15 deletions(-) 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 5a5a39d14..3ab3ec9f3 100644 --- a/src/main/java/com/hubspot/jinjava/tree/output/OutputList.java +++ b/src/main/java/com/hubspot/jinjava/tree/output/OutputList.java @@ -71,16 +71,7 @@ private String joinNodesWithoutAddingExpressions( LengthLimitingStringBuilder val, 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(); + String separator = getWhitespaceSeparator(tokenScannerSymbols); String prev = null; String cur; for (OutputNode node : nodes) { @@ -93,11 +84,7 @@ private String joinNodesWithoutAddingExpressions( ) { if ( cur.length() > 0 && - ( - cur.charAt(0) == tokenScannerSymbols.getTag() || - cur.charAt(0) == tokenScannerSymbols.getExprStartChar() || - cur.charAt(0) == tokenScannerSymbols.getNoteChar() - ) + TokenScannerSymbols.isNoteTagOrExprChar(tokenScannerSymbols, cur.charAt(0)) ) { val.append(separator); } @@ -115,6 +102,20 @@ private String joinNodesWithoutAddingExpressions( 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 { 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() + ); + } }