Improve diagnostics in SpEL for `matches` operator

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-30144
This commit is contained in:
Sam Brannen 2023-03-17 12:56:53 +01:00
parent 5529294ec9
commit 8010de8b63
3 changed files with 42 additions and 9 deletions

View File

@ -268,7 +268,11 @@ public enum SpelMessage {
/** @since 5.3.26 */
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.3.26 */
MAX_REGEX_LENGTH_EXCEEDED(Kind.ERROR, 1077,
"Regular expression contains too many characters, exceeding the threshold of ''{0}''");
private final Kind kind;

View File

@ -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.3.26
*/
private static final int MAX_REGEX_LENGTH = 256;
private final ConcurrentMap<String, Pattern> patternCache;
@ -78,25 +84,27 @@ public class OperatorMatches extends Operator {
public BooleanTypedValue getValueInternal(ExpressionState state) throws EvaluationException {
SpelNodeImpl leftOp = getLeftOperand();
SpelNodeImpl rightOp = getRightOperand();
String left = leftOp.getValue(state, String.class);
Object right = getRightOperand().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);
}
if (!(right instanceof String rightString)) {
Object right = rightOp.getValue(state);
if (!(right instanceof String regex)) {
throw new SpelEvaluationException(rightOp.getStartPosition(),
SpelMessage.INVALID_SECOND_OPERAND_FOR_MATCHES_OPERATOR, right);
}
try {
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) {
@ -109,6 +117,13 @@ public class OperatorMatches extends Operator {
}
}
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 {

View File

@ -482,6 +482,20 @@ class EvaluationTests extends AbstractExpressionTests {
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);
}
}
@Nested