Skip to content

Commit 08a315f

Browse files
committed
limit length of strings in operators
1 parent 446cff7 commit 08a315f

5 files changed

Lines changed: 86 additions & 12 deletions

File tree

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,11 @@
33
import de.odysseus.el.misc.NumberOperations;
44
import de.odysseus.el.misc.TypeConverter;
55
import de.odysseus.el.tree.impl.ast.AstBinary;
6-
import java.util.ArrayList;
7-
import java.util.Collection;
8-
import java.util.HashMap;
9-
import java.util.List;
10-
import java.util.Map;
11-
import java.util.Objects;
6+
import java.util.*;
127

13-
public class AdditionOperator extends AstBinary.SimpleOperator {
8+
public class AdditionOperator
9+
extends AstBinary.SimpleOperator
10+
implements StringBuildingOperator {
1411

1512
@SuppressWarnings("unchecked")
1613
@Override
@@ -33,7 +30,10 @@ protected Object apply(TypeConverter converter, Object o1, Object o2) {
3330
}
3431

3532
if (o1 instanceof String || o2 instanceof String) {
36-
return Objects.toString(o1).concat(Objects.toString(o2));
33+
return getStringBuilder()
34+
.append(Objects.toString(o1))
35+
.append(Objects.toString(o2))
36+
.toString();
3737
}
3838

3939
return NumberOperations.add(converter, o1, o2);
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
package com.hubspot.jinjava.el.ext;
2+
3+
import com.hubspot.jinjava.interpret.JinjavaInterpreter;
4+
import com.hubspot.jinjava.util.LengthLimitingStringBuilder;
5+
6+
public interface StringBuildingOperator {
7+
default LengthLimitingStringBuilder getStringBuilder() {
8+
JinjavaInterpreter interpreter = JinjavaInterpreter.getCurrent();
9+
10+
long maxSize = (interpreter == null || interpreter.getConfig() == null)
11+
? 0
12+
: interpreter.getConfig().getMaxOutputSize();
13+
14+
return new LengthLimitingStringBuilder(maxSize);
15+
}
16+
}

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,16 @@
99
import de.odysseus.el.tree.impl.ast.AstBinary.SimpleOperator;
1010
import de.odysseus.el.tree.impl.ast.AstNode;
1111

12-
public class StringConcatOperator extends SimpleOperator {
12+
public class StringConcatOperator
13+
extends SimpleOperator
14+
implements StringBuildingOperator {
1315

1416
@Override
1517
protected Object apply(TypeConverter converter, Object o1, Object o2) {
1618
String o1s = converter.convert(o1, String.class);
1719
String o2s = converter.convert(o2, String.class);
1820

19-
return new StringBuilder(o1s).append(o2s).toString();
21+
return getStringBuilder().append(o1s).append(o2s).toString();
2022
}
2123

2224
@Override

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

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,10 @@
66
import com.google.common.collect.Lists;
77
import com.google.common.io.Resources;
88
import com.hubspot.jinjava.Jinjava;
9+
import com.hubspot.jinjava.JinjavaConfig;
910
import com.hubspot.jinjava.interpret.Context;
1011
import com.hubspot.jinjava.interpret.JinjavaInterpreter;
12+
import com.hubspot.jinjava.interpret.OutputTooBigException;
1113
import com.hubspot.jinjava.interpret.TemplateError.ErrorReason;
1214
import java.io.IOException;
1315
import java.nio.charset.StandardCharsets;
@@ -21,12 +23,16 @@
2123
@SuppressWarnings("unchecked")
2224
public class ExtendedSyntaxBuilderTest {
2325

26+
private static final long MAX_STRING_LENGTH = 10_000;
27+
2428
private Context context;
2529
private JinjavaInterpreter interpreter;
2630

2731
@Before
2832
public void setup() {
29-
interpreter = new Jinjava().newInterpreter();
33+
interpreter =
34+
new Jinjava(JinjavaConfig.newBuilder().withMaxOutputSize(MAX_STRING_LENGTH).build())
35+
.newInterpreter();
3036
JinjavaInterpreter.pushCurrent(interpreter);
3137

3238
context = interpreter.getContext();
@@ -96,6 +102,25 @@ public void stringConcatOperator() {
96102
assertThat(val("'foo' ~ 'bar'")).isEqualTo("foobar");
97103
}
98104

105+
@Test
106+
public void itLimitsLengthInStringConcatOperator() {
107+
StringBuilder stringBuilder = new StringBuilder();
108+
for (int i = 0; i < MAX_STRING_LENGTH - 1; i++) {
109+
stringBuilder.append("0");
110+
}
111+
112+
String longString = stringBuilder.toString();
113+
114+
context.put("longString", longString);
115+
assertThat(val("longString ~ ''")).isEqualTo(longString);
116+
assertThat(interpreter.getErrors()).isEmpty();
117+
118+
assertThat(val("longString ~ 'OVER'")).isNull();
119+
120+
assertThat(interpreter.getErrors().get(0).getMessage())
121+
.contains(OutputTooBigException.class.getSimpleName());
122+
}
123+
99124
@Test
100125
public void stringInStringOperator() {
101126
assertThat(val("'foo' in 'foobar'")).isEqualTo(true);

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

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,27 +4,58 @@
44
import static org.assertj.core.api.Assertions.entry;
55

66
import com.hubspot.jinjava.Jinjava;
7+
import com.hubspot.jinjava.JinjavaConfig;
78
import com.hubspot.jinjava.interpret.JinjavaInterpreter;
9+
import com.hubspot.jinjava.interpret.OutputTooBigException;
810
import java.util.Collection;
911
import java.util.Map;
12+
import org.junit.After;
1013
import org.junit.Before;
1114
import org.junit.Test;
1215

1316
@SuppressWarnings("unchecked")
1417
public class AdditionOperatorTest {
1518

19+
private static final long MAX_STRING_LENGTH = 10_000;
1620
private JinjavaInterpreter interpreter;
1721

1822
@Before
1923
public void setup() {
20-
interpreter = new Jinjava().newInterpreter();
24+
interpreter =
25+
new Jinjava(JinjavaConfig.newBuilder().withMaxOutputSize(MAX_STRING_LENGTH).build())
26+
.newInterpreter();
27+
JinjavaInterpreter.pushCurrent(interpreter);
28+
}
29+
30+
@After
31+
public void teardown() {
32+
JinjavaInterpreter.popCurrent();
2133
}
2234

2335
@Test
2436
public void itConcatsStrings() {
2537
assertThat(interpreter.resolveELExpression("'foo' + 'bar'", -1)).isEqualTo("foobar");
2638
}
2739

40+
@Test
41+
public void itLimitsLengthOfStrings() {
42+
StringBuilder stringBuilder = new StringBuilder();
43+
for (int i = 0; i < MAX_STRING_LENGTH; i++) {
44+
stringBuilder.append("0");
45+
}
46+
47+
String first = stringBuilder.toString();
48+
assertThat(interpreter.resolveELExpression("'" + first + "' + ''", -1))
49+
.isEqualTo(first);
50+
assertThat(interpreter.getErrors()).isEmpty();
51+
52+
assertThat(interpreter.resolveELExpression("'" + first + "' + 'TOOBIG'", -1))
53+
.isNull();
54+
55+
assertThat(interpreter.getErrors().get(0).getMessage())
56+
.contains(OutputTooBigException.class.getSimpleName());
57+
}
58+
2859
@Test
2960
public void itAddsNumbers() {
3061
assertThat(interpreter.resolveELExpression("1 + 2", -1)).isEqualTo(3L);

0 commit comments

Comments
 (0)