Add trimming note block to prevent accidentally creating expressions/tags/notes when PREVENT_ACCIDENTAL_EXPRESSIONS is enabled

Author: jasmith-hs

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<BlockPlaceholderOutputNode> 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());

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();
+    }
+  }
 }