Merge pull request #1161 from HubSpot/jboulter-limit-length-of-string…

…s-in-operators

limit length of strings in operators

src/main/java/com/hubspot/jinjava/el/ext/AdditionOperator.java

@@ -10,7 +10,9 @@
 import java.util.Map;
 import java.util.Objects;
 
-public class AdditionOperator extends AstBinary.SimpleOperator {
+public class AdditionOperator
+  extends AstBinary.SimpleOperator
+  implements StringBuildingOperator {
 
   @SuppressWarnings("unchecked")
   @Override
@@ -33,7 +35,10 @@ protected Object apply(TypeConverter converter, Object o1, Object o2) {
     }
 
     if (o1 instanceof String || o2 instanceof String) {
-      return Objects.toString(o1).concat(Objects.toString(o2));
+      return getStringBuilder()
+        .append(Objects.toString(o1))
+        .append(Objects.toString(o2))
+        .toString();
     }
 
     return NumberOperations.add(converter, o1, o2);

src/main/java/com/hubspot/jinjava/el/ext/StringBuildingOperator.java

@@ -0,0 +1,16 @@
+package com.hubspot.jinjava.el.ext;
+
+import com.hubspot.jinjava.interpret.JinjavaInterpreter;
+import com.hubspot.jinjava.util.LengthLimitingStringBuilder;
+
+public interface StringBuildingOperator {
+  default LengthLimitingStringBuilder getStringBuilder() {
+    JinjavaInterpreter interpreter = JinjavaInterpreter.getCurrent();
+
+    long maxSize = (interpreter == null || interpreter.getConfig() == null)
+      ? 0
+      : interpreter.getConfig().getMaxOutputSize();
+
+    return new LengthLimitingStringBuilder(maxSize);
+  }
+}

src/main/java/com/hubspot/jinjava/el/ext/StringConcatOperator.java

@@ -9,14 +9,16 @@
 import de.odysseus.el.tree.impl.ast.AstBinary.SimpleOperator;
 import de.odysseus.el.tree.impl.ast.AstNode;
 
-public class StringConcatOperator extends SimpleOperator {
+public class StringConcatOperator
+  extends SimpleOperator
+  implements StringBuildingOperator {
 
   @Override
   protected Object apply(TypeConverter converter, Object o1, Object o2) {
     String o1s = converter.convert(o1, String.class);
     String o2s = converter.convert(o2, String.class);
 
-    return new StringBuilder(o1s).append(o2s).toString();
+    return getStringBuilder().append(o1s).append(o2s).toString();
   }
 
   @Override

src/test/java/com/hubspot/jinjava/el/ExtendedSyntaxBuilderTest.java

@@ -6,8 +6,10 @@
 import com.google.common.collect.Lists;
 import com.google.common.io.Resources;
 import com.hubspot.jinjava.Jinjava;
+import com.hubspot.jinjava.JinjavaConfig;
 import com.hubspot.jinjava.interpret.Context;
 import com.hubspot.jinjava.interpret.JinjavaInterpreter;
+import com.hubspot.jinjava.interpret.OutputTooBigException;
 import com.hubspot.jinjava.interpret.TemplateError.ErrorReason;
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
@@ -21,12 +23,16 @@
 @SuppressWarnings("unchecked")
 public class ExtendedSyntaxBuilderTest {
 
+  private static final long MAX_STRING_LENGTH = 10_000;
+
   private Context context;
   private JinjavaInterpreter interpreter;
 
   @Before
   public void setup() {
-    interpreter = new Jinjava().newInterpreter();
+    interpreter =
+      new Jinjava(JinjavaConfig.newBuilder().withMaxOutputSize(MAX_STRING_LENGTH).build())
+        .newInterpreter();
     JinjavaInterpreter.pushCurrent(interpreter);
 
     context = interpreter.getContext();
@@ -96,6 +102,25 @@ public void stringConcatOperator() {
     assertThat(val("'foo' ~ 'bar'")).isEqualTo("foobar");
   }
 
+  @Test
+  public void itLimitsLengthInStringConcatOperator() {
+    StringBuilder stringBuilder = new StringBuilder();
+    for (int i = 0; i < MAX_STRING_LENGTH - 1; i++) {
+      stringBuilder.append("0");
+    }
+
+    String longString = stringBuilder.toString();
+
+    context.put("longString", longString);
+    assertThat(val("longString ~ ''")).isEqualTo(longString);
+    assertThat(interpreter.getErrors()).isEmpty();
+
+    assertThat(val("longString ~ 'OVER'")).isNull();
+
+    assertThat(interpreter.getErrors().get(0).getMessage())
+      .contains(OutputTooBigException.class.getSimpleName());
+  }
+
   @Test
   public void stringInStringOperator() {
     assertThat(val("'foo' in 'foobar'")).isEqualTo(true);

src/test/java/com/hubspot/jinjava/el/ext/AdditionOperatorTest.java

@@ -4,27 +4,58 @@
 import static org.assertj.core.api.Assertions.entry;
 
 import com.hubspot.jinjava.Jinjava;
+import com.hubspot.jinjava.JinjavaConfig;
 import com.hubspot.jinjava.interpret.JinjavaInterpreter;
+import com.hubspot.jinjava.interpret.OutputTooBigException;
 import java.util.Collection;
 import java.util.Map;
+import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 
 @SuppressWarnings("unchecked")
 public class AdditionOperatorTest {
 
+  private static final long MAX_STRING_LENGTH = 10_000;
   private JinjavaInterpreter interpreter;
 
   @Before
   public void setup() {
-    interpreter = new Jinjava().newInterpreter();
+    interpreter =
+      new Jinjava(JinjavaConfig.newBuilder().withMaxOutputSize(MAX_STRING_LENGTH).build())
+        .newInterpreter();
+    JinjavaInterpreter.pushCurrent(interpreter);
+  }
+
+  @After
+  public void teardown() {
+    JinjavaInterpreter.popCurrent();
   }
 
   @Test
   public void itConcatsStrings() {
     assertThat(interpreter.resolveELExpression("'foo' + 'bar'", -1)).isEqualTo("foobar");
   }
 
+  @Test
+  public void itLimitsLengthOfStrings() {
+    StringBuilder stringBuilder = new StringBuilder();
+    for (int i = 0; i < MAX_STRING_LENGTH; i++) {
+      stringBuilder.append("0");
+    }
+
+    String first = stringBuilder.toString();
+    assertThat(interpreter.resolveELExpression("'" + first + "' + ''", -1))
+      .isEqualTo(first);
+    assertThat(interpreter.getErrors()).isEmpty();
+
+    assertThat(interpreter.resolveELExpression("'" + first + "' + 'TOOBIG'", -1))
+      .isNull();
+
+    assertThat(interpreter.getErrors().get(0).getMessage())
+      .contains(OutputTooBigException.class.getSimpleName());
+  }
+
   @Test
   public void itAddsNumbers() {
     assertThat(interpreter.resolveELExpression("1 + 2", -1)).isEqualTo(3L);