Skip to content

Commit

Permalink
Merge pull request #1138 from HubSpot/limit-regex-replace
Browse files Browse the repository at this point in the history
Limit string length input for replacement filters
  • Loading branch information
jasmith-hs authored Dec 13, 2023
2 parents 61acbfb + 9b18562 commit 0c47c7f
Show file tree
Hide file tree
Showing 6 changed files with 85 additions and 2 deletions.
2 changes: 1 addition & 1 deletion src/main/java/com/hubspot/jinjava/JinjavaConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ public boolean isValidationMode() {
}

public long getMaxStringLength() {
return maxStringLength;
return maxStringLength == 0 ? getMaxOutputSize() : maxStringLength;
}

public InterpreterFactory getInterpreterFactory() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ public enum InvalidReason {
"with value '%s' must be a valid attribute of every item in the list"
),
ENUM("with value '%s' must be one of: %s"),
CIDR("with value '%s' must be a valid CIDR address");
CIDR("with value '%s' must be a valid CIDR address"),
LENGTH("with length '%s' exceeds maximum allowed length of '%s'");

private final String errorMessage;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package com.hubspot.jinjava.lib.filter;

import static com.hubspot.jinjava.lib.filter.ReplaceFilter.checkLength;

import com.google.re2j.Matcher;
import com.google.re2j.Pattern;
import com.google.re2j.PatternSyntaxException;
Expand Down Expand Up @@ -71,6 +73,10 @@ public Object filter(Object var, JinjavaInterpreter interpreter, String... args)

if (var instanceof String) {
String s = (String) var;
// Minor optimization, avoid checking short strings
if (s.length() > 100) {
checkLength(interpreter, s, this);
}
String toReplace = args[0];
String replaceWith = args[1];

Expand Down
25 changes: 25 additions & 0 deletions src/main/java/com/hubspot/jinjava/lib/filter/ReplaceFilter.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,11 @@
import com.hubspot.jinjava.doc.annotations.JinjavaDoc;
import com.hubspot.jinjava.doc.annotations.JinjavaParam;
import com.hubspot.jinjava.doc.annotations.JinjavaSnippet;
import com.hubspot.jinjava.interpret.InvalidInputException;
import com.hubspot.jinjava.interpret.InvalidReason;
import com.hubspot.jinjava.interpret.JinjavaInterpreter;
import com.hubspot.jinjava.interpret.TemplateSyntaxException;
import com.hubspot.jinjava.lib.Importable;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.math.NumberUtils;

Expand Down Expand Up @@ -66,6 +69,11 @@ public Object filter(Object var, JinjavaInterpreter interpreter, String... args)
}

String s = var.toString();
// Minor optimization, avoid checking short strings
if (s.length() > 100) {
checkLength(interpreter, s, this);
}

String toReplace = args[0];
String replaceWith = args[1];
Integer count = null;
Expand All @@ -80,4 +88,21 @@ public Object filter(Object var, JinjavaInterpreter interpreter, String... args)
return StringUtils.replace(s, toReplace, replaceWith, count);
}
}

static void checkLength(
JinjavaInterpreter interpreter,
String s,
Importable importable
) {
long maxStringLength = interpreter.getConfig().getMaxStringLength();
if (maxStringLength > 0 && s.length() > maxStringLength) {
throw new InvalidInputException(
interpreter,
importable,
InvalidReason.LENGTH,
s.length(),
maxStringLength
);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@
import static org.assertj.core.api.Assertions.assertThatThrownBy;

import com.hubspot.jinjava.BaseInterpretingTest;
import com.hubspot.jinjava.Jinjava;
import com.hubspot.jinjava.JinjavaConfig;
import com.hubspot.jinjava.interpret.InvalidArgumentException;
import com.hubspot.jinjava.interpret.InvalidInputException;
import com.hubspot.jinjava.objects.SafeString;
import org.junit.Before;
import org.junit.Test;
Expand Down Expand Up @@ -55,4 +58,26 @@ public void itMatchesRegexAndReplacesStringForSafeString() {
)
.isEqualTo("Itcosts");
}

@Test
public void itLimitsLongInput() {
StringBuilder sb = new StringBuilder();
for (int i = 0; i < 101; i++) {
sb.append('a');
}
assertThatThrownBy(
() ->
filter.filter(
sb.toString(),
new Jinjava(JinjavaConfig.newBuilder().withMaxStringLength(10).build())
.newInterpreter(),
"O",
"0"
)
)
.isInstanceOf(InvalidInputException.class)
.hasMessageContaining(
"Invalid input for 'regex_replace': input with length '101' exceeds maximum allowed length of '10'"
);
}
}
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
package com.hubspot.jinjava.lib.filter;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;

import com.hubspot.jinjava.BaseInterpretingTest;
import com.hubspot.jinjava.Jinjava;
import com.hubspot.jinjava.JinjavaConfig;
import com.hubspot.jinjava.interpret.InterpretException;
import com.hubspot.jinjava.interpret.InvalidInputException;
import com.hubspot.jinjava.objects.SafeString;
import org.junit.Before;
import org.junit.Test;
Expand Down Expand Up @@ -52,4 +56,26 @@ public void replaceBoolean() {
assertThat(filter.filter(true, interpreter, "true", "TRUEEE").toString())
.isEqualTo("TRUEEE");
}

@Test
public void itLimitsLongInput() {
StringBuilder sb = new StringBuilder();
for (int i = 0; i < 101; i++) {
sb.append('a');
}
assertThatThrownBy(
() ->
filter.filter(
sb.toString(),
new Jinjava(JinjavaConfig.newBuilder().withMaxStringLength(10).build())
.newInterpreter(),
"O",
"0"
)
)
.isInstanceOf(InvalidInputException.class)
.hasMessageContaining(
"Invalid input for 'replace': input with length '101' exceeds maximum allowed length of '10'"
);
}
}

0 comments on commit 0c47c7f

Please sign in to comment.