From d4a55a257b798130a89b27408198ef235e7cc67f Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Tue, 17 Apr 2018 11:10:15 +0200 Subject: [PATCH] OperatorMatches flags misguided evaluation attempts as FLAWED_PATTERN Issue: SPR-16731 --- .../VariableNotAvailableException.java | 8 ++- .../expression/spel/SpelMessage.java | 24 +++++--- .../expression/spel/ast/OperatorMatches.java | 56 ++++++++++++++++++- .../expression/spel/EvaluationTests.java | 16 ++++++ 4 files changed, 89 insertions(+), 15 deletions(-) diff --git a/spring-context/src/main/java/org/springframework/cache/interceptor/VariableNotAvailableException.java b/spring-context/src/main/java/org/springframework/cache/interceptor/VariableNotAvailableException.java index 6f7fdde3c2..52951bb3fa 100644 --- a/spring-context/src/main/java/org/springframework/cache/interceptor/VariableNotAvailableException.java +++ b/spring-context/src/main/java/org/springframework/cache/interceptor/VariableNotAvailableException.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -30,13 +30,15 @@ class VariableNotAvailableException extends EvaluationException { private final String name; + public VariableNotAvailableException(String name) { - super("Variable '" + name + "' is not available"); + super("Variable not available"); this.name = name; } - public String getName() { + public final String getName() { return this.name; } + } diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/SpelMessage.java b/spring-expression/src/main/java/org/springframework/expression/spel/SpelMessage.java index b2fa91ec64..03787c84a5 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/SpelMessage.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/SpelMessage.java @@ -156,7 +156,7 @@ public enum SpelMessage { NOT_A_REAL(Kind.ERROR, 1040, "The value ''{0}'' cannot be parsed as a double"), - MORE_INPUT(Kind.ERROR,1041, + MORE_INPUT(Kind.ERROR, 1041, "After parsing a valid expression, there is still more data in the expression: ''{0}''"), RIGHT_OPERAND_PROBLEM(Kind.ERROR, 1042, @@ -226,21 +226,22 @@ public enum SpelMessage { "A required array dimension has not been specified"), INITIALIZER_LENGTH_INCORRECT(Kind.ERROR, 1064, - "array initializer size does not match array dimensions"), + "Array initializer size does not match array dimensions"), - UNEXPECTED_ESCAPE_CHAR(Kind.ERROR, 1065, "unexpected escape character."), + UNEXPECTED_ESCAPE_CHAR(Kind.ERROR, 1065, + "Unexpected escape character"), OPERAND_NOT_INCREMENTABLE(Kind.ERROR, 1066, - "the expression component ''{0}'' does not support increment"), + "The expression component ''{0}'' does not support increment"), OPERAND_NOT_DECREMENTABLE(Kind.ERROR, 1067, - "the expression component ''{0}'' does not support decrement"), + "The expression component ''{0}'' does not support decrement"), NOT_ASSIGNABLE(Kind.ERROR, 1068, - "the expression component ''{0}'' is not assignable"), + "The expression component ''{0}'' is not assignable"), MISSING_CHARACTER(Kind.ERROR, 1069, - "missing expected character ''{0}''"), + "Missing expected character ''{0}''"), LEFT_OPERAND_PROBLEM(Kind.ERROR, 1070, "Problem parsing left operand"), @@ -248,8 +249,13 @@ public enum SpelMessage { MISSING_SELECTION_EXPRESSION(Kind.ERROR, 1071, "A required selection expression has not been specified"), - EXCEPTION_RUNNING_COMPILED_EXPRESSION(Kind.ERROR,1072, - "An exception occurred whilst evaluating a compiled expression"); + /** @since 4.1 */ + EXCEPTION_RUNNING_COMPILED_EXPRESSION(Kind.ERROR, 1072, + "An exception occurred whilst evaluating a compiled expression"), + + /** @since 4.3.17 */ + FLAWED_PATTERN(Kind.ERROR, 1073, + "Failed to efficiently evaluate pattern ''{0}'': consider redesigning it"); private final Kind kind; diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OperatorMatches.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OperatorMatches.java index f53ad97fc8..d97f18e6bb 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OperatorMatches.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OperatorMatches.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -40,6 +40,8 @@ import org.springframework.expression.spel.support.BooleanTypedValue; */ public class OperatorMatches extends Operator { + private static final int PATTERN_ACCESS_THRESHOLD = 1000000; + private final ConcurrentMap patternCache = new ConcurrentHashMap<>(); @@ -79,11 +81,59 @@ public class OperatorMatches extends Operator { pattern = Pattern.compile(rightString); this.patternCache.putIfAbsent(rightString, pattern); } - Matcher matcher = pattern.matcher(left); + Matcher matcher = pattern.matcher(new MatcherInput(left, new AccessCount())); return BooleanTypedValue.forValue(matcher.matches()); } catch (PatternSyntaxException ex) { - throw new SpelEvaluationException(rightOp.getStartPosition(), ex, SpelMessage.INVALID_PATTERN, right); + throw new SpelEvaluationException( + rightOp.getStartPosition(), ex, SpelMessage.INVALID_PATTERN, right); + } + catch (IllegalStateException ex) { + throw new SpelEvaluationException( + rightOp.getStartPosition(), ex, SpelMessage.FLAWED_PATTERN, right); + } + } + + + private static class AccessCount { + + private int count; + + public void check() throws IllegalStateException { + if (this.count++ > PATTERN_ACCESS_THRESHOLD) { + throw new IllegalStateException("Pattern access threshold exceeded"); + } + } + } + + + private static class MatcherInput implements CharSequence { + + private final CharSequence value; + + private AccessCount access; + + public MatcherInput(CharSequence value, AccessCount access) { + this.value = value; + this.access = access; + } + + public char charAt(int index) { + this.access.check(); + return this.value.charAt(index); + } + + public CharSequence subSequence(int start, int end) { + return new MatcherInput(this.value.subSequence(start, end), this.access); + } + + public int length() { + return this.value.length(); + } + + @Override + public String toString() { + return this.value.toString(); } } diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/EvaluationTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/EvaluationTests.java index f28b6a6816..338cf732a2 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/EvaluationTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/EvaluationTests.java @@ -194,6 +194,22 @@ public class EvaluationTests extends AbstractExpressionTests { evaluate("27 matches '^.*2.*$'", true, Boolean.class); // conversion int>string } + @Test // SPR-16731 + public void testMatchesWithPatternAccessThreshold() { + String pattern = "^(?=[a-z0-9-]{1,47})([a-z0-9]+[-]{0,1}){1,47}[a-z0-9]{1}$"; + String expression = "'abcde-fghijklmn-o42pasdfasdfasdf.qrstuvwxyz10x.xx.yyy.zasdfasfd' matches \'" + pattern + "\'"; + Expression expr = parser.parseExpression(expression); + try { + expr.getValue(); + fail("Should have exceeded threshold"); + } + catch (EvaluationException ee) { + SpelEvaluationException see = (SpelEvaluationException) ee; + assertEquals(SpelMessage.FLAWED_PATTERN, see.getMessageCode()); + assertTrue(see.getCause() instanceof IllegalStateException); + } + } + // mixing operators @Test public void testMixingOperators01() {