Merge pull request #1123 from HubSpot/prevent-accidental-expressions

Prevent accidental expressions from being output
HubSpot · Oct 28, 2023 · 264ac2b · 264ac2b
2 parents 7a60dca + 160c254
commit 264ac2b
Show file tree

Hide file tree

Showing 3 changed files with 128 additions and 0 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
@@ -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<OutputNode> nodes = new LinkedList<>();
   private final List<BlockPlaceholderOutputNode> blocks = new LinkedList<>();
   private final long maxOutputSize;
@@ -48,6 +51,72 @@ public List<BlockPlaceholderOutputNode> 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/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
@@ -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();
+    }
+  }
 }