Skip to content

Commit

Permalink
Improve diagnostics in SpEL for matches operator
Browse files Browse the repository at this point in the history
Supplying a large regular expression to the `matches` operator in a
SpEL expression can result in errors that are not very helpful to the
user.

This commit improves the diagnostics in SpEL for the `matches` operator
by throwing a SpelEvaluationException with a meaningful error message
to better assist the user.

Closes gh-30150
  • Loading branch information
sbrannen committed Mar 20, 2023
1 parent 4542b53 commit b9b31af
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,11 @@ public enum SpelMessage {

/** @since 5.2.23 */
MAX_REPEATED_TEXT_SIZE_EXCEEDED(Kind.ERROR, 1076,
"Repeated text results in too many characters, exceeding the threshold of ''{0}''");
"Repeated text results in too many characters, exceeding the threshold of ''{0}''"),

/** @since 5.2.23 */
MAX_REGEX_LENGTH_EXCEEDED(Kind.ERROR, 1077,
"Regular expression contains too many characters, exceeding the threshold of ''{0}''");


private final Kind kind;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,12 @@ public class OperatorMatches extends Operator {

private static final int PATTERN_ACCESS_THRESHOLD = 1000000;

/**
* Maximum number of characters permitted in a regular expression.
* @since 5.2.23
*/
private static final int MAX_REGEX_LENGTH = 256;

private final ConcurrentMap<String, Pattern> patternCache;


Expand Down Expand Up @@ -78,26 +84,28 @@ public OperatorMatches(ConcurrentMap<String, Pattern> patternCache, int startPos
public BooleanTypedValue getValueInternal(ExpressionState state) throws EvaluationException {
SpelNodeImpl leftOp = getLeftOperand();
SpelNodeImpl rightOp = getRightOperand();
String left = leftOp.getValue(state, String.class);
Object right = rightOp.getValue(state);

if (left == null) {
String input = leftOp.getValue(state, String.class);
if (input == null) {
throw new SpelEvaluationException(leftOp.getStartPosition(),
SpelMessage.INVALID_FIRST_OPERAND_FOR_MATCHES_OPERATOR, (Object) null);
}

Object right = rightOp.getValue(state);
if (!(right instanceof String)) {
throw new SpelEvaluationException(rightOp.getStartPosition(),
SpelMessage.INVALID_SECOND_OPERAND_FOR_MATCHES_OPERATOR, right);
}
String regex = (String) right;

try {
String rightString = (String) right;
Pattern pattern = this.patternCache.get(rightString);
Pattern pattern = this.patternCache.get(regex);
if (pattern == null) {
pattern = Pattern.compile(rightString);
this.patternCache.putIfAbsent(rightString, pattern);
checkRegexLength(regex);
pattern = Pattern.compile(regex);
this.patternCache.putIfAbsent(regex, pattern);
}
Matcher matcher = pattern.matcher(new MatcherInput(left, new AccessCount()));
Matcher matcher = pattern.matcher(new MatcherInput(input, new AccessCount()));
return BooleanTypedValue.forValue(matcher.matches());
}
catch (PatternSyntaxException ex) {
Expand All @@ -110,6 +118,13 @@ public BooleanTypedValue getValueInternal(ExpressionState state) throws Evaluati
}
}

private void checkRegexLength(String regex) {
if (regex.length() > MAX_REGEX_LENGTH) {
throw new SpelEvaluationException(getStartPosition(),
SpelMessage.MAX_REGEX_LENGTH_EXCEEDED, MAX_REGEX_LENGTH);
}
}


private static class AccessCount {

Expand All @@ -127,7 +142,7 @@ private static class MatcherInput implements CharSequence {

private final CharSequence value;

private AccessCount access;
private final AccessCount access;

public MatcherInput(CharSequence value, AccessCount access) {
this.value = value;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,20 @@ void matchesWithPatternAccessThreshold() {
evaluateAndCheckError(expression, SpelMessage.FLAWED_PATTERN);
}

@Test
void matchesWithPatternLengthThreshold() {
String pattern = "(0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789" +
"0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789" +
"01234567890123456789012345678901234567890123456789|abc)";
assertThat(pattern).hasSize(256);
Expression expr = parser.parseExpression("'abc' matches '" + pattern + "'");
assertThat(expr.getValue(context, Boolean.class)).isTrue();

pattern += "?";
assertThat(pattern).hasSize(257);
evaluateAndCheckError("'abc' matches '" + pattern + "'", Boolean.class, SpelMessage.MAX_REGEX_LENGTH_EXCEEDED);
}

// mixing operators
@Test
public void testMixingOperators01() {
Expand Down

0 comments on commit b9b31af

Please sign in to comment.