diff --git a/framework-docs/modules/ROOT/pages/appendix.adoc b/framework-docs/modules/ROOT/pages/appendix.adoc index b65a9ab4c45..8c2cbd703ff 100644 --- a/framework-docs/modules/ROOT/pages/appendix.adoc +++ b/framework-docs/modules/ROOT/pages/appendix.adoc @@ -74,6 +74,11 @@ expressions used in XML bean definitions, `@Value`, etc. | The mode to use when compiling expressions for the xref:core/expressions/evaluation.adoc#expressions-compiler-configuration[Spring Expression Language]. +| `spring.expression.maxOperations` +| The default maximum number of operations permitted during +xref:core/expressions/evaluation.adoc#expressions-parser-configuration[Spring Expression Language] +expression evaluation. + | `spring.getenv.ignore` | Instructs Spring to ignore operating system environment variables if a Spring `Environment` property -- for example, a placeholder in a configuration String -- isn't diff --git a/framework-docs/modules/ROOT/pages/core/expressions/evaluation.adoc b/framework-docs/modules/ROOT/pages/core/expressions/evaluation.adoc index 2501e72e4b3..e8b1af95788 100644 --- a/framework-docs/modules/ROOT/pages/core/expressions/evaluation.adoc +++ b/framework-docs/modules/ROOT/pages/core/expressions/evaluation.adoc @@ -395,6 +395,16 @@ set a JVM system property or Spring property named `spring.context.expression.ma to the maximum expression length needed by your application (see xref:appendix.adoc#appendix-spring-properties[Supported Spring Properties]). +Similarly, the number of operations performed during the evaluation of a SpEL expression +cannot exceed 10,000 by default; however, the `maxOperations` value is configurable. If +you create a `SpelExpressionParser` programmatically (the recommend approach), you can +specify a custom `maxOperations` value when creating the `SpelParserConfiguration` that +you provide to the `SpelExpressionParser`. If you are not able to configure an explicit +value for `maxOperations` via `SpelParserConfiguration`, you can set a JVM system +property or Spring property named `spring.expression.maxOperations` to the maximum number +of operations required by your application (see +xref:appendix.adoc#appendix-spring-properties[Supported Spring Properties]). + [[expressions-spel-compilation]] == SpEL Compilation diff --git a/spring-core/src/main/java/org/springframework/core/SpringProperties.java b/spring-core/src/main/java/org/springframework/core/SpringProperties.java index e4ed3c6e8b7..bcdc21dd127 100644 --- a/spring-core/src/main/java/org/springframework/core/SpringProperties.java +++ b/spring-core/src/main/java/org/springframework/core/SpringProperties.java @@ -44,6 +44,7 @@ import org.jspecify.annotations.Nullable; * @see org.springframework.beans.factory.support.DefaultListableBeanFactory#STRICT_LOCKING_PROPERTY_NAME * @see org.springframework.core.env.AbstractEnvironment#IGNORE_GETENV_PROPERTY_NAME * @see org.springframework.expression.spel.SpelParserConfiguration#SPRING_EXPRESSION_COMPILER_MODE_PROPERTY_NAME + * @see org.springframework.expression.spel.SpelParserConfiguration#SPRING_EXPRESSION_MAX_OPERATIONS_PROPERTY_NAME * @see org.springframework.jdbc.core.StatementCreatorUtils#IGNORE_GETPARAMETERTYPE_PROPERTY_NAME * @see org.springframework.jndi.JndiLocatorDelegate#IGNORE_JNDI_PROPERTY_NAME * @see org.springframework.objenesis.SpringObjenesis#IGNORE_OBJENESIS_PROPERTY_NAME diff --git a/spring-core/src/main/java/org/springframework/util/AntPathMatcher.java b/spring-core/src/main/java/org/springframework/util/AntPathMatcher.java index 684046caf12..90b31a0f12b 100644 --- a/spring-core/src/main/java/org/springframework/util/AntPathMatcher.java +++ b/spring-core/src/main/java/org/springframework/util/AntPathMatcher.java @@ -723,7 +723,7 @@ public class AntPathMatcher implements PathMatcher { return this.caseSensitive ? this.rawPattern.equals(str) : this.rawPattern.equalsIgnoreCase(str); } else if (this.pattern != null) { - Matcher matcher = this.pattern.matcher(str); + Matcher matcher = this.pattern.matcher(new MaxAttemptsCharSequence(str)); if (matcher.matches()) { if (uriTemplateVariables != null) { if (this.variableNames.size() != matcher.groupCount()) { @@ -748,6 +748,60 @@ public class AntPathMatcher implements PathMatcher { return false; } + + private static class MaxAttemptsCharSequence implements CharSequence { + + private static final int MAX_ATTEMPTS = 1_000_000; + + private final String text; + + private final Counter counter; + + + MaxAttemptsCharSequence(String text) { + this(text, new Counter()); + } + + private MaxAttemptsCharSequence(String text, Counter counter) { + this.text = text; + this.counter = counter; + } + + + @Override + public int length() { + return this.text.length(); + } + + @Override + public char charAt(int index) { + if (this.counter.value++ >= MAX_ATTEMPTS) { + throw new IllegalStateException( + "Too many character access attempts encountered during pattern matching"); + } + return this.text.charAt(index); + } + + @Override + public boolean isEmpty() { + return this.text.isEmpty(); + } + + @Override + public CharSequence subSequence(int start, int end) { + return new MaxAttemptsCharSequence(this.text.substring(start, end), this.counter); + } + + @Override + public String toString() { + return this.text; + } + + + private static class Counter { + private int value; + } + } } diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ExpressionState.java b/spring-expression/src/main/java/org/springframework/expression/spel/ExpressionState.java index c03080a8aa8..b3669fd4fa2 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ExpressionState.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ExpressionState.java @@ -60,6 +60,8 @@ public class ExpressionState { private final SpelParserConfiguration configuration; + private int operationsCount; + private @Nullable Deque contextObjects; // When entering a new scope there is a new base object which should be used @@ -140,6 +142,7 @@ public class ExpressionState { * @see EvaluationContext#assignVariable(String, Supplier) */ public TypedValue assignVariable(String name, Supplier valueSupplier) { + trackOperation(); return this.relatedContext.assignVariable(name, valueSupplier); } @@ -164,6 +167,7 @@ public class ExpressionState { * @see #setVariable(String, Object) */ public TypedValue lookupVariable(String name) { + trackOperation(); Object value = this.relatedContext.lookupVariable(name); return (value != null ? new TypedValue(value) : TypedValue.NULL); } @@ -225,6 +229,7 @@ public class ExpressionState { public TypedValue operate(Operation op, @Nullable Object left, @Nullable Object right) throws EvaluationException { OperatorOverloader overloader = this.relatedContext.getOperatorOverloader(); if (overloader.overridesOperation(op, left, right)) { + trackOperation(); Object returnValue = overloader.operate(op, left, right); return new TypedValue(returnValue); } @@ -247,4 +252,16 @@ public class ExpressionState { return this.configuration; } + /** + * Track an operation during expression evaluation. + * @since 6.2.19 + * @see SpelParserConfiguration#getMaximumOperations() + */ + public void trackOperation() { + int maxOperations = this.configuration.getMaximumOperations(); + if (++this.operationsCount >= maxOperations) { + throw new SpelEvaluationException(SpelMessage.MAX_OPERATIONS_EXCEEDED, maxOperations); + } + } + } 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 1067eddc0ce..d58c89e7aea 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 @@ -303,8 +303,11 @@ public enum SpelMessage { /** @since 6.2 */ EXCEPTION_DURING_INDEX_WRITE(Kind.ERROR, 1084, - "A problem occurred while attempting to write index ''{0}'' in ''{1}''"); + "A problem occurred while attempting to write index ''{0}'' in ''{1}''"), + /** @since 6.2.19 */ + MAX_OPERATIONS_EXCEEDED(Kind.ERROR, 1085, + "SpEL expression evaluation exceeded the threshold of ''{0}'' operations"); private final Kind kind; diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/SpelParserConfiguration.java b/spring-expression/src/main/java/org/springframework/expression/spel/SpelParserConfiguration.java index c1ee8ecdc88..aa2a3db8c74 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/SpelParserConfiguration.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/SpelParserConfiguration.java @@ -21,6 +21,8 @@ import java.util.Locale; import org.jspecify.annotations.Nullable; import org.springframework.core.SpringProperties; +import org.springframework.util.Assert; +import org.springframework.util.StringUtils; /** * Configuration object for the SpEL expression parser. @@ -40,9 +42,38 @@ public class SpelParserConfiguration { */ public static final int DEFAULT_MAX_EXPRESSION_LENGTH = 10_000; - /** System property to configure the default compiler mode for SpEL expression parsers: {@value}. */ + /** + * Default maximum number of operations permitted during SpEL expression evaluation: {@value}. + * @since 6.2.19 + * @see #SPRING_EXPRESSION_MAX_OPERATIONS_PROPERTY_NAME + */ + public static final int DEFAULT_MAX_OPERATIONS = 10_000; + + /** + * System property to configure the default compiler mode for SpEL expression parsers: {@value}. + *

NOTE: Instead of relying on a global default, applications + * and frameworks should ideally set an explicit custom value via the + * {@link #SpelParserConfiguration(SpelCompilerMode, ClassLoader, boolean, boolean, int, int, int)} + * constructor which provides complete configuration control and the ability + * to override global defaults per use case. + *

Can also be configured via the {@link SpringProperties} mechanism. + */ public static final String SPRING_EXPRESSION_COMPILER_MODE_PROPERTY_NAME = "spring.expression.compiler.mode"; + /** + * System property to configure the default maximum number of operations permitted + * during SpEL expression evaluation: {@value}. + *

NOTE: Instead of relying on a global default, applications + * and frameworks should ideally set an explicit custom value via the + * {@link #SpelParserConfiguration(SpelCompilerMode, ClassLoader, boolean, boolean, int, int, int)} + * constructor which provides complete configuration control and the ability + * to override global defaults per use case. + *

Can also be configured via the {@link SpringProperties} mechanism. + * @since 6.2.19 + * @see #DEFAULT_MAX_OPERATIONS + */ + public static final String SPRING_EXPRESSION_MAX_OPERATIONS_PROPERTY_NAME = "spring.expression.maxOperations"; + private static final SpelCompilerMode defaultCompilerMode; @@ -65,9 +96,17 @@ public class SpelParserConfiguration { private final int maximumExpressionLength; + private final int maximumOperations; + /** * Create a new {@code SpelParserConfiguration} instance with default settings. + *

NOTE: Favor the + * {@link #SpelParserConfiguration(SpelCompilerMode, ClassLoader, boolean, boolean, int, int, int)} + * constructor for complete configuration control and the ability to override + * global defaults per use case. + * @see #SPRING_EXPRESSION_COMPILER_MODE_PROPERTY_NAME + * @see #SPRING_EXPRESSION_MAX_OPERATIONS_PROPERTY_NAME */ public SpelParserConfiguration() { this(null, null, false, false, Integer.MAX_VALUE); @@ -75,8 +114,16 @@ public class SpelParserConfiguration { /** * Create a new {@code SpelParserConfiguration} instance. - * @param compilerMode the compiler mode for the parser - * @param compilerClassLoader the ClassLoader to use as the basis for expression compilation + *

NOTE: Favor the + * {@link #SpelParserConfiguration(SpelCompilerMode, ClassLoader, boolean, boolean, int, int, int)} + * constructor for complete configuration control and the ability to override + * global defaults per use case. + * @param compilerMode the compiler mode that parsers using this configuration + * should use; or {@code null} to use the default mode + * @param compilerClassLoader the {@code ClassLoader} to use as the basis for + * expression compilation; or {@code null} to use the default {@code ClassLoader} + * @see #SPRING_EXPRESSION_COMPILER_MODE_PROPERTY_NAME + * @see #SPRING_EXPRESSION_MAX_OPERATIONS_PROPERTY_NAME */ public SpelParserConfiguration(@Nullable SpelCompilerMode compilerMode, @Nullable ClassLoader compilerClassLoader) { this(compilerMode, compilerClassLoader, false, false, Integer.MAX_VALUE); @@ -84,9 +131,14 @@ public class SpelParserConfiguration { /** * Create a new {@code SpelParserConfiguration} instance. + *

NOTE: Favor the + * {@link #SpelParserConfiguration(SpelCompilerMode, ClassLoader, boolean, boolean, int, int, int)} + * constructor for complete configuration control and the ability to override + * global defaults per use case. * @param autoGrowNullReferences if null references should automatically grow * @param autoGrowCollections if collections should automatically grow - * @see #SpelParserConfiguration(boolean, boolean, int) + * @see #SPRING_EXPRESSION_COMPILER_MODE_PROPERTY_NAME + * @see #SPRING_EXPRESSION_MAX_OPERATIONS_PROPERTY_NAME */ public SpelParserConfiguration(boolean autoGrowNullReferences, boolean autoGrowCollections) { this(null, null, autoGrowNullReferences, autoGrowCollections, Integer.MAX_VALUE); @@ -94,9 +146,15 @@ public class SpelParserConfiguration { /** * Create a new {@code SpelParserConfiguration} instance. + *

NOTE: Favor the + * {@link #SpelParserConfiguration(SpelCompilerMode, ClassLoader, boolean, boolean, int, int, int)} + * constructor for complete configuration control and the ability to override + * global defaults per use case. * @param autoGrowNullReferences if null references should automatically grow * @param autoGrowCollections if collections should automatically grow - * @param maximumAutoGrowSize the maximum size that the collection can auto grow + * @param maximumAutoGrowSize the maximum size to which a collection can auto grow + * @see #SPRING_EXPRESSION_COMPILER_MODE_PROPERTY_NAME + * @see #SPRING_EXPRESSION_MAX_OPERATIONS_PROPERTY_NAME */ public SpelParserConfiguration(boolean autoGrowNullReferences, boolean autoGrowCollections, int maximumAutoGrowSize) { this(null, null, autoGrowNullReferences, autoGrowCollections, maximumAutoGrowSize); @@ -104,11 +162,19 @@ public class SpelParserConfiguration { /** * Create a new {@code SpelParserConfiguration} instance. - * @param compilerMode the compiler mode that parsers using this configuration object should use - * @param compilerClassLoader the ClassLoader to use as the basis for expression compilation + *

NOTE: Favor the + * {@link #SpelParserConfiguration(SpelCompilerMode, ClassLoader, boolean, boolean, int, int, int)} + * constructor for complete configuration control and the ability to override + * global defaults per use case. + * @param compilerMode the compiler mode that parsers using this configuration + * should use; or {@code null} to use the default mode + * @param compilerClassLoader the {@code ClassLoader} to use as the basis for + * expression compilation; or {@code null} to use the default {@code ClassLoader} * @param autoGrowNullReferences if null references should automatically grow * @param autoGrowCollections if collections should automatically grow - * @param maximumAutoGrowSize the maximum size that the collection can auto grow + * @param maximumAutoGrowSize the maximum size to which a collection can auto grow + * @see #SPRING_EXPRESSION_COMPILER_MODE_PROPERTY_NAME + * @see #SPRING_EXPRESSION_MAX_OPERATIONS_PROPERTY_NAME */ public SpelParserConfiguration(@Nullable SpelCompilerMode compilerMode, @Nullable ClassLoader compilerClassLoader, boolean autoGrowNullReferences, boolean autoGrowCollections, int maximumAutoGrowSize) { @@ -119,24 +185,60 @@ public class SpelParserConfiguration { /** * Create a new {@code SpelParserConfiguration} instance. - * @param compilerMode the compiler mode that parsers using this configuration object should use - * @param compilerClassLoader the ClassLoader to use as the basis for expression compilation + *

NOTE: Favor the + * {@link #SpelParserConfiguration(SpelCompilerMode, ClassLoader, boolean, boolean, int, int, int)} + * constructor for complete configuration control and the ability to override + * global defaults per use case. + * @param compilerMode the compiler mode that parsers using this configuration + * should use; or {@code null} to use the default mode + * @param compilerClassLoader the {@code ClassLoader} to use as the basis for + * expression compilation; or {@code null} to use the default {@code ClassLoader} * @param autoGrowNullReferences if null references should automatically grow * @param autoGrowCollections if collections should automatically grow - * @param maximumAutoGrowSize the maximum size that a collection can auto grow + * @param maximumAutoGrowSize the maximum size to which a collection can auto grow * @param maximumExpressionLength the maximum length of a SpEL expression; * must be a positive number * @since 5.2.25 + * @see #SPRING_EXPRESSION_COMPILER_MODE_PROPERTY_NAME + * @see #SPRING_EXPRESSION_MAX_OPERATIONS_PROPERTY_NAME */ public SpelParserConfiguration(@Nullable SpelCompilerMode compilerMode, @Nullable ClassLoader compilerClassLoader, boolean autoGrowNullReferences, boolean autoGrowCollections, int maximumAutoGrowSize, int maximumExpressionLength) { - this.compilerMode = (compilerMode != null ? compilerMode : defaultCompilerMode); + this((compilerMode != null ? compilerMode : defaultCompilerMode), compilerClassLoader, autoGrowNullReferences, + autoGrowCollections, maximumAutoGrowSize, maximumExpressionLength, retrieveMaxOperations()); + } + + /** + * Create a new {@code SpelParserConfiguration} instance. + * @param compilerMode the compiler mode that parsers using this configuration + * should use; must not be {@code null} + * @param compilerClassLoader the {@code ClassLoader} to use as the basis for + * expression compilation; or {@code null} to use the default {@code ClassLoader} + * @param autoGrowNullReferences if null references should automatically grow + * @param autoGrowCollections if collections should automatically grow + * @param maximumAutoGrowSize the maximum size to which a collection can auto grow + * @param maximumExpressionLength the maximum length of a SpEL expression; + * must be a positive number + * @param maximumOperations the maximum number of operations permitted during + * SpEL expression evaluation; must be a positive number + * @since 6.2.19 + */ + public SpelParserConfiguration(SpelCompilerMode compilerMode, @Nullable ClassLoader compilerClassLoader, + boolean autoGrowNullReferences, boolean autoGrowCollections, int maximumAutoGrowSize, int maximumExpressionLength, + int maximumOperations) { + + Assert.notNull(compilerMode, "'compilerMode' must not be null"); + Assert.isTrue(maximumExpressionLength > 0, "'maximumExpressionLength' must be a positive number"); + Assert.isTrue(maximumOperations > 0, "'maximumOperations' must be a positive number"); + + this.compilerMode = compilerMode; this.compilerClassLoader = compilerClassLoader; this.autoGrowNullReferences = autoGrowNullReferences; this.autoGrowCollections = autoGrowCollections; this.maximumAutoGrowSize = maximumAutoGrowSize; this.maximumExpressionLength = maximumExpressionLength; + this.maximumOperations = maximumOperations; } @@ -148,7 +250,7 @@ public class SpelParserConfiguration { } /** - * Return the ClassLoader to use as the basis for expression compilation. + * Return the {@code ClassLoader} to use as the basis for expression compilation. */ public @Nullable ClassLoader getCompilerClassLoader() { return this.compilerClassLoader; @@ -169,7 +271,7 @@ public class SpelParserConfiguration { } /** - * Return the maximum size that a collection can auto grow. + * Return the maximum size to which a collection can auto grow. */ public int getMaximumAutoGrowSize() { return this.maximumAutoGrowSize; @@ -183,4 +285,32 @@ public class SpelParserConfiguration { return this.maximumExpressionLength; } + /** + * Return the maximum number of operations permitted during SpEL expression + * evaluation. + * @since 6.2.19 + */ + public int getMaximumOperations() { + return this.maximumOperations; + } + + + private static int retrieveMaxOperations() { + String value = SpringProperties.getProperty(SPRING_EXPRESSION_MAX_OPERATIONS_PROPERTY_NAME); + if (!StringUtils.hasText(value)) { + return DEFAULT_MAX_OPERATIONS; + } + + try { + int maxOperations = Integer.parseInt(value.trim()); + Assert.isTrue(maxOperations > 0, () -> "Value [" + maxOperations + "] for system property [" + + SPRING_EXPRESSION_MAX_OPERATIONS_PROPERTY_NAME + "] must be positive"); + return maxOperations; + } + catch (NumberFormatException ex) { + throw new IllegalArgumentException("Failed to parse value for system property [" + + SPRING_EXPRESSION_MAX_OPERATIONS_PROPERTY_NAME + "]: " + ex.getMessage(), ex); + } + } + } diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/Assign.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/Assign.java index e19d758fc7f..2ba31b54e9e 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/Assign.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/Assign.java @@ -44,6 +44,7 @@ public class Assign extends SpelNodeImpl { if (!state.getEvaluationContext().isAssignmentEnabled()) { throw new SpelEvaluationException(getStartPosition(), SpelMessage.NOT_ASSIGNABLE, toStringAST()); } + state.trackOperation(); return this.children[0].setValueInternal(state, () -> this.children[1].getValueInternal(state)); } diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/BeanReference.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/BeanReference.java index a2f9fc36211..ee6cbe40c51 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/BeanReference.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/BeanReference.java @@ -65,6 +65,7 @@ public class BeanReference extends SpelNodeImpl { getStartPosition(), SpelMessage.NO_BEAN_RESOLVER_REGISTERED, this.beanName); } + state.trackOperation(); try { return new TypedValue(beanResolver.resolve(state.getEvaluationContext(), this.beanName)); } diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/ConstructorReference.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/ConstructorReference.java index 3e5cafcfe61..b2a0457e05d 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/ConstructorReference.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/ConstructorReference.java @@ -109,6 +109,7 @@ public class ConstructorReference extends SpelNodeImpl { */ @Override public TypedValue getValueInternal(ExpressionState state) throws EvaluationException { + state.trackOperation(); if (this.isArrayConstructor) { return createArray(state); } diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/Elvis.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/Elvis.java index 482355f41c1..b2f65812d3d 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/Elvis.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/Elvis.java @@ -60,17 +60,17 @@ public class Elvis extends SpelNodeImpl { /** - * If the left-hand operand is neither neither {@code null}, an empty - * {@link Optional}, nor an empty {@link String}, return its value, or the - * value contained in the {@code Optional}. If the left-hand operand is - * {@code null}, an empty {@code Optional}, or an empty {@code String}, - * return the other value. + * If the left-hand operand is neither {@code null}, an empty {@link Optional}, + * nor an empty {@link String}, return its value, or the value contained in the + * {@code Optional}. If the left-hand operand is {@code null}, an empty + * {@code Optional}, or an empty {@code String}, return the other value. * @param state the expression state * @throws EvaluationException if the null/empty check does not evaluate correctly * or there is a problem evaluating the alternative */ @Override public TypedValue getValueInternal(ExpressionState state) throws EvaluationException { + state.trackOperation(); TypedValue result; TypedValue leftHandTypedValue = this.children[0].getValueInternal(state); Object leftHandValue = leftHandTypedValue.getValue(); diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/FunctionReference.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/FunctionReference.java index 7b75b466a23..3f1c0e49df0 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/FunctionReference.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/FunctionReference.java @@ -84,6 +84,7 @@ public class FunctionReference extends SpelNodeImpl { // Note: "javaMethod" cannot be named "method" due to a bug in Checkstyle. if (function instanceof Method javaMethod) { try { + state.trackOperation(); return executeFunctionViaMethod(state, javaMethod); } catch (SpelEvaluationException ex) { @@ -95,6 +96,7 @@ public class FunctionReference extends SpelNodeImpl { // Function registered via a MethodHandle. if (function instanceof MethodHandle methodHandle) { try { + state.trackOperation(); return executeFunctionViaMethodHandle(state, methodHandle); } catch (SpelEvaluationException ex) { diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java index 464a73f07e9..ff94b719c45 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java @@ -218,14 +218,14 @@ public class Indexer extends SpelNodeImpl { if (target.getClass().isArray()) { int intIndex = convertIndexToInt(state, index); this.indexedType = IndexedType.ARRAY; - return new ArrayIndexingValueRef(state.getTypeConverter(), target, intIndex, targetDescriptor); + return new ArrayIndexingValueRef(state, state.getTypeConverter(), target, intIndex, targetDescriptor); } // Indexing into a List if (target instanceof List list) { int intIndex = convertIndexToInt(state, index); this.indexedType = IndexedType.LIST; - return new CollectionIndexingValueRef(list, intIndex, targetDescriptor, + return new CollectionIndexingValueRef(state, list, intIndex, targetDescriptor, state.getTypeConverter(), state.getConfiguration().isAutoGrowCollections(), state.getConfiguration().getMaximumAutoGrowSize()); } @@ -238,14 +238,14 @@ public class Indexer extends SpelNodeImpl { key = state.convertValue(key, mapKeyTypeDescriptor); } this.indexedType = IndexedType.MAP; - return new MapIndexingValueRef(state.getTypeConverter(), map, key, targetDescriptor); + return new MapIndexingValueRef(state, state.getTypeConverter(), map, key, targetDescriptor); } // Indexing into a String if (target instanceof String string) { int intIndex = convertIndexToInt(state, index); this.indexedType = IndexedType.STRING; - return new StringIndexingValueRef(string, intIndex, targetDescriptor); + return new StringIndexingValueRef(state, string, intIndex, targetDescriptor); } // Check for a custom IndexAccessor. @@ -257,7 +257,7 @@ public class Indexer extends SpelNodeImpl { for (IndexAccessor indexAccessor : accessorsToTry) { if (indexAccessor.canRead(evalContext, target, index)) { this.indexedType = IndexedType.CUSTOM; - return new IndexAccessorValueRef(target, index, evalContext, targetDescriptor); + return new IndexAccessorValueRef(state, target, index, evalContext, targetDescriptor); } } } @@ -272,7 +272,7 @@ public class Indexer extends SpelNodeImpl { for (IndexAccessor indexAccessor : accessorsToTry) { if (indexAccessor.canWrite(evalContext, target, index)) { this.indexedType = IndexedType.CUSTOM; - return new IndexAccessorValueRef(target, index, evalContext, targetDescriptor); + return new IndexAccessorValueRef(state, target, index, evalContext, targetDescriptor); } } } @@ -286,7 +286,7 @@ public class Indexer extends SpelNodeImpl { // Fallback indexing support for collections if (target instanceof Collection collection) { int intIndex = convertIndexToInt(state, index); - return new CollectionIndexingValueRef(collection, intIndex, targetDescriptor, + return new CollectionIndexingValueRef(state, collection, intIndex, targetDescriptor, state.getTypeConverter(), state.getConfiguration().isAutoGrowCollections(), state.getConfiguration().getMaximumAutoGrowSize()); } @@ -296,7 +296,7 @@ public class Indexer extends SpelNodeImpl { if (valueType != null && String.class == valueType.getType()) { this.indexedType = IndexedType.OBJECT; return new PropertyAccessorValueRef( - target, (String) index, state.getEvaluationContext(), targetDescriptor); + state, target, (String) index, state.getEvaluationContext(), targetDescriptor); } throw new SpelEvaluationException( @@ -501,6 +501,8 @@ public class Indexer extends SpelNodeImpl { private class ArrayIndexingValueRef implements ValueRef { + private final ExpressionState expressionState; + private final TypeConverter typeConverter; private final Object array; @@ -509,7 +511,10 @@ public class Indexer extends SpelNodeImpl { private final TypeDescriptor typeDescriptor; - ArrayIndexingValueRef(TypeConverter typeConverter, Object array, int index, TypeDescriptor typeDescriptor) { + ArrayIndexingValueRef(ExpressionState expressionState, TypeConverter typeConverter, + Object array, int index, TypeDescriptor typeDescriptor) { + + this.expressionState = expressionState; this.typeConverter = typeConverter; this.array = array; this.index = index; @@ -535,6 +540,7 @@ public class Indexer extends SpelNodeImpl { } private Object getArrayElement(Object ctx, int idx) throws SpelEvaluationException { + this.expressionState.trackOperation(); Class arrayComponentType = ctx.getClass().componentType(); if (arrayComponentType == boolean.class) { boolean[] array = (boolean[]) ctx; @@ -605,6 +611,7 @@ public class Indexer extends SpelNodeImpl { private void setArrayElement(TypeConverter converter, Object ctx, int idx, @Nullable Object newValue, Class arrayComponentType) throws EvaluationException { + this.expressionState.trackOperation(); if (arrayComponentType == boolean.class) { boolean[] array = (boolean[]) ctx; checkAccess(array.length, idx); @@ -668,13 +675,14 @@ public class Indexer extends SpelNodeImpl { } return result; } - } @SuppressWarnings({"rawtypes", "unchecked"}) private class MapIndexingValueRef implements ValueRef { + private final ExpressionState expressionState; + private final TypeConverter typeConverter; private final Map map; @@ -683,9 +691,10 @@ public class Indexer extends SpelNodeImpl { private final TypeDescriptor mapEntryDescriptor; - public MapIndexingValueRef( - TypeConverter typeConverter, Map map, @Nullable Object key, TypeDescriptor mapEntryDescriptor) { + public MapIndexingValueRef(ExpressionState expressionState, TypeConverter typeConverter, + Map map, @Nullable Object key, TypeDescriptor mapEntryDescriptor) { + this.expressionState = expressionState; this.typeConverter = typeConverter; this.map = map; this.key = key; @@ -694,6 +703,7 @@ public class Indexer extends SpelNodeImpl { @Override public TypedValue getValue() { + this.expressionState.trackOperation(); Object value = this.map.get(this.key); exitTypeDescriptor = CodeFlow.toDescriptor(Object.class); return new TypedValue(value, this.mapEntryDescriptor.getMapValueTypeDescriptor(value)); @@ -705,6 +715,7 @@ public class Indexer extends SpelNodeImpl { newValue = this.typeConverter.convertValue(newValue, TypeDescriptor.forObject(newValue), this.mapEntryDescriptor.getMapValueTypeDescriptor()); } + this.expressionState.trackOperation(); this.map.put(this.key, newValue); } @@ -717,6 +728,8 @@ public class Indexer extends SpelNodeImpl { private class PropertyAccessorValueRef implements ValueRef { + private final ExpressionState expressionState; + private final Object targetObject; private final String name; @@ -725,9 +738,10 @@ public class Indexer extends SpelNodeImpl { private final TypeDescriptor targetObjectTypeDescriptor; - public PropertyAccessorValueRef(Object targetObject, String name, + public PropertyAccessorValueRef(ExpressionState expressionState, Object targetObject, String name, EvaluationContext evaluationContext, TypeDescriptor targetObjectTypeDescriptor) { + this.expressionState = expressionState; this.targetObject = targetObject; this.name = name; this.evaluationContext = evaluationContext; @@ -745,6 +759,7 @@ public class Indexer extends SpelNodeImpl { // Is it OK to use the cached accessor? if (cachedPropertyName.equals(this.name) && cachedTargetType.equals(targetType)) { PropertyAccessor accessor = cachedPropertyReadState.accessor; + this.expressionState.trackOperation(); return accessor.read(this.evaluationContext, this.targetObject, this.name); } // If the above code block did not use a cached accessor and return a value, @@ -759,6 +774,7 @@ public class Indexer extends SpelNodeImpl { accessor = reflectivePropertyAccessor.createOptimalAccessor( this.evaluationContext, this.targetObject, this.name); } + this.expressionState.trackOperation(); TypedValue result = accessor.read(this.evaluationContext, this.targetObject, this.name); Indexer.this.cachedPropertyReadState = new CachedPropertyState(accessor, targetType, this.name); if (accessor instanceof CompilablePropertyAccessor compilablePropertyAccessor) { @@ -787,6 +803,7 @@ public class Indexer extends SpelNodeImpl { // Is it OK to use the cached accessor? if (cachedPropertyName.equals(this.name) && cachedTargetType.equals(targetType)) { PropertyAccessor accessor = cachedPropertyWriteState.accessor; + this.expressionState.trackOperation(); accessor.write(this.evaluationContext, this.targetObject, this.name, newValue); return; } @@ -798,6 +815,7 @@ public class Indexer extends SpelNodeImpl { AccessorUtils.getAccessorsToTry(targetType, this.evaluationContext.getPropertyAccessors()); for (PropertyAccessor accessor : accessorsToTry) { if (accessor.canWrite(this.evaluationContext, this.targetObject, this.name)) { + this.expressionState.trackOperation(); accessor.write(this.evaluationContext, this.targetObject, this.name, newValue); Indexer.this.cachedPropertyWriteState = new CachedPropertyState(accessor, targetType, this.name); return; @@ -822,6 +840,8 @@ public class Indexer extends SpelNodeImpl { @SuppressWarnings({"rawtypes", "unchecked"}) private class CollectionIndexingValueRef implements ValueRef { + private final ExpressionState expressionState; + private final Collection collection; private final int index; @@ -834,9 +854,11 @@ public class Indexer extends SpelNodeImpl { private final int maximumSize; - public CollectionIndexingValueRef(Collection collection, int index, TypeDescriptor collectionEntryDescriptor, - TypeConverter typeConverter, boolean growCollection, int maximumSize) { + public CollectionIndexingValueRef(ExpressionState expressionState, Collection collection, int index, + TypeDescriptor collectionEntryDescriptor, TypeConverter typeConverter, + boolean growCollection, int maximumSize) { + this.expressionState = expressionState; this.collection = collection; this.index = index; this.collectionEntryDescriptor = collectionEntryDescriptor; @@ -849,6 +871,7 @@ public class Indexer extends SpelNodeImpl { public TypedValue getValue() { growCollectionIfNecessary(); if (this.collection instanceof List list) { + this.expressionState.trackOperation(); Object o = list.get(this.index); exitTypeDescriptor = CodeFlow.toDescriptor(Object.class); return new TypedValue(o, this.collectionEntryDescriptor.elementTypeDescriptor(o)); @@ -856,6 +879,7 @@ public class Indexer extends SpelNodeImpl { int pos = 0; for (Object o : this.collection) { + this.expressionState.trackOperation(); if (pos == this.index) { return new TypedValue(o, this.collectionEntryDescriptor.elementTypeDescriptor(o)); } @@ -877,6 +901,7 @@ public class Indexer extends SpelNodeImpl { newValue = this.typeConverter.convertValue(newValue, TypeDescriptor.forObject(newValue), this.collectionEntryDescriptor.getElementTypeDescriptor()); } + this.expressionState.trackOperation(); list.set(this.index, newValue); } @@ -898,6 +923,7 @@ public class Indexer extends SpelNodeImpl { Constructor ctor = getDefaultConstructor(elementType.getType()); int newElements = this.index - this.collection.size(); while (newElements >= 0) { + this.expressionState.trackOperation(); // Insert a null value if the element type does not have a default constructor. this.collection.add(ctor != null ? ctor.newInstance() : null); newElements--; @@ -927,13 +953,18 @@ public class Indexer extends SpelNodeImpl { private class StringIndexingValueRef implements ValueRef { + private final ExpressionState expressionState; + private final String target; private final int index; private final TypeDescriptor typeDescriptor; - public StringIndexingValueRef(String target, int index, TypeDescriptor typeDescriptor) { + public StringIndexingValueRef( + ExpressionState expressionState, String target, int index, TypeDescriptor typeDescriptor) { + + this.expressionState = expressionState; this.target = target; this.index = index; this.typeDescriptor = typeDescriptor; @@ -945,6 +976,7 @@ public class Indexer extends SpelNodeImpl { throw new SpelEvaluationException(getStartPosition(), SpelMessage.STRING_INDEX_OUT_OF_BOUNDS, this.target.length(), this.index); } + this.expressionState.trackOperation(); return new TypedValue(String.valueOf(this.target.charAt(this.index))); } @@ -963,6 +995,8 @@ public class Indexer extends SpelNodeImpl { private class IndexAccessorValueRef implements ValueRef { + private final ExpressionState expressionState; + private final Object target; private final Object index; @@ -972,9 +1006,10 @@ public class Indexer extends SpelNodeImpl { private final TypeDescriptor typeDescriptor; - IndexAccessorValueRef(Object target, Object index, EvaluationContext evaluationContext, - TypeDescriptor typeDescriptor) { + IndexAccessorValueRef(ExpressionState expressionState, Object target, Object index, + EvaluationContext evaluationContext, TypeDescriptor typeDescriptor) { + this.expressionState = expressionState; this.target = target; this.index = index; this.evaluationContext = evaluationContext; @@ -996,6 +1031,7 @@ public class Indexer extends SpelNodeImpl { IndexAccessor accessor = cachedIndexReadState.accessor; if (this.evaluationContext.getIndexAccessors().contains(accessor)) { try { + this.expressionState.trackOperation(); return accessor.read(this.evaluationContext, this.target, this.index); } catch (Exception ex) { @@ -1014,6 +1050,7 @@ public class Indexer extends SpelNodeImpl { AccessorUtils.getAccessorsToTry(this.target, this.evaluationContext.getIndexAccessors()); for (IndexAccessor indexAccessor : accessorsToTry) { if (indexAccessor.canRead(this.evaluationContext, this.target, this.index)) { + this.expressionState.trackOperation(); TypedValue result = indexAccessor.read(this.evaluationContext, this.target, this.index); Indexer.this.cachedIndexReadState = new CachedIndexState(indexAccessor, targetType, this.index); if (indexAccessor instanceof CompilableIndexAccessor compilableIndexAccessor) { @@ -1050,6 +1087,7 @@ public class Indexer extends SpelNodeImpl { IndexAccessor accessor = cachedIndexWriteState.accessor; if (this.evaluationContext.getIndexAccessors().contains(accessor)) { try { + this.expressionState.trackOperation(); accessor.write(this.evaluationContext, this.target, this.index, newValue); return; } @@ -1069,6 +1107,7 @@ public class Indexer extends SpelNodeImpl { AccessorUtils.getAccessorsToTry(this.target, this.evaluationContext.getIndexAccessors()); for (IndexAccessor indexAccessor : accessorsToTry) { if (indexAccessor.canWrite(this.evaluationContext, this.target, this.index)) { + this.expressionState.trackOperation(); indexAccessor.write(this.evaluationContext, this.target, this.index, newValue); Indexer.this.cachedIndexWriteState = new CachedIndexState(indexAccessor, targetType, this.index); return; diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/InlineList.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/InlineList.java index 5ae75d813e7..6c674bac178 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/InlineList.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/InlineList.java @@ -29,7 +29,7 @@ import org.springframework.expression.TypedValue; import org.springframework.expression.spel.CodeFlow; import org.springframework.expression.spel.ExpressionState; import org.springframework.expression.spel.SpelNode; -import org.springframework.expression.spel.support.StandardEvaluationContext; +import org.springframework.expression.spel.support.SimpleEvaluationContext; import org.springframework.util.Assert; /** @@ -43,67 +43,63 @@ import org.springframework.util.Assert; */ public class InlineList extends SpelNodeImpl { - private final @Nullable TypedValue constant; + private final boolean isConstant; + + private volatile @Nullable TypedValue constant; public InlineList(int startPos, int endPos, SpelNodeImpl... args) { super(startPos, endPos, args); - this.constant = computeConstantValue(); + this.isConstant = determineIfConstant(); } /** - * If all the components of the list are constants, or lists that themselves - * contain constants, then a constant list can be built to represent this node. - *

This will speed up later getValue calls and reduce the amount of garbage - * created. + * Determine whether this list is structurally eligible to be a constant + * value: whether all of its components are themselves constants or lists + * that contain only constants. + *

The actual constant value is created lazily on the first call to + * {@link #getValueInternal(ExpressionState)}. */ - private @Nullable TypedValue computeConstantValue() { + private boolean determineIfConstant() { for (int c = 0, max = getChildCount(); c < max; c++) { SpelNode child = getChild(c); - if (!(child instanceof Literal)) { - if (child instanceof InlineList inlineList) { - if (!inlineList.isConstant()) { - return null; - } - } - else if (!(child instanceof OpMinus opMinus) || !opMinus.isNegativeNumberLiteral()) { - return null; - } + if (child instanceof Literal) { + continue; } + if (child instanceof InlineList inlineList && inlineList.isConstant()) { + continue; + } + if (child instanceof OpMinus opMinus && opMinus.isNegativeNumberLiteral()) { + continue; + } + return false; } - - List constantList = new ArrayList<>(); - int childcount = getChildCount(); - ExpressionState expressionState = new ExpressionState(new StandardEvaluationContext()); - for (int c = 0; c < childcount; c++) { - SpelNode child = getChild(c); - if (child instanceof Literal literal) { - constantList.add(literal.getLiteralValue().getValue()); - } - else if (child instanceof InlineList inlineList) { - constantList.add(inlineList.getConstantValue()); - } - else if (child instanceof OpMinus) { - constantList.add(child.getValue(expressionState)); - } - } - return new TypedValue(Collections.unmodifiableList(constantList)); + return true; } @Override public TypedValue getValueInternal(ExpressionState expressionState) throws EvaluationException { - if (this.constant != null) { - return this.constant; + TypedValue result = this.constant; + if (result != null) { + return result; } - else { - int childCount = getChildCount(); - List returnValue = new ArrayList<>(childCount); - for (int c = 0; c < childCount; c++) { - returnValue.add(getChild(c).getValue(expressionState)); - } - return new TypedValue(returnValue); + result = createList(expressionState); + if (this.isConstant) { + this.constant = result; } + return result; + } + + private TypedValue createList(ExpressionState expressionState) throws EvaluationException { + int childCount = getChildCount(); + expressionState.trackOperation(); + List list = new ArrayList<>(childCount); + for (int c = 0; c < childCount; c++) { + expressionState.trackOperation(); + list.add(getChild(c).getValue(expressionState)); + } + return new TypedValue(this.isConstant ? Collections.unmodifiableList(list) : list); } @Override @@ -117,21 +113,37 @@ public class InlineList extends SpelNodeImpl { } /** - * Return whether this list is a constant value. + * Return whether this list is structurally a constant value. + *

Note that the resulting constant value is created lazily on the + * first call to {@link #getValueInternal(ExpressionState)} or + * {@link #getConstantValue()}. */ public boolean isConstant() { - return (this.constant != null); + return this.isConstant; } + /** + * Return the cached constant {@link List} value for this inline list, + * lazily creating it on first access. + * @see #isConstant() + * @deprecated as of Spring Framework 6.2.19; this method was only intended for + * testing purposes and will be removed in a future version of the framework + */ @SuppressWarnings("unchecked") + @Deprecated(since = "6.2.19") public @Nullable List getConstantValue() { - Assert.state(this.constant != null, "No constant"); - return (List) this.constant.getValue(); + Assert.state(this.isConstant, "Not a constant"); + TypedValue result = this.constant; + if (result == null) { + result = createList(new ExpressionState(SimpleEvaluationContext.forReadOnlyDataBinding().build())); + this.constant = result; + } + return (List) result.getValue(); } @Override public boolean isCompilable() { - return isConstant(); + return this.isConstant; } @Override diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/InlineMap.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/InlineMap.java index b2cfe3e6f66..6ec8e236467 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/InlineMap.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/InlineMap.java @@ -26,7 +26,7 @@ import org.springframework.expression.EvaluationException; import org.springframework.expression.TypedValue; import org.springframework.expression.spel.ExpressionState; import org.springframework.expression.spel.SpelNode; -import org.springframework.expression.spel.support.StandardEvaluationContext; +import org.springframework.expression.spel.support.SimpleEvaluationContext; import org.springframework.util.Assert; /** @@ -40,103 +40,79 @@ import org.springframework.util.Assert; */ public class InlineMap extends SpelNodeImpl { - private final @Nullable TypedValue constant; + private final boolean isConstant; + + private volatile @Nullable TypedValue constant; public InlineMap(int startPos, int endPos, SpelNodeImpl... args) { super(startPos, endPos, args); - this.constant = computeConstantValue(); + this.isConstant = determineIfConstant(); } /** - * If all the components of the map are constants, or lists/maps that themselves - * contain constants, then a constant map can be built to represent this node. - *

This will speed up later getValue calls and reduce the amount of garbage - * created. + * Determine whether this map is structurally eligible to be a constant + * value: whether all of its components are themselves constants or lists/maps + * that contain only constants. + *

The actual constant value is created lazily on the first call to + * {@link #getValueInternal(ExpressionState)}. */ - private @Nullable TypedValue computeConstantValue() { - for (int c = 0, max = getChildCount(); c < max; c++) { - SpelNode child = getChild(c); - if (!(child instanceof Literal)) { - if (child instanceof InlineList inlineList) { - if (!inlineList.isConstant()) { - return null; - } - } - else if (child instanceof InlineMap inlineMap) { - if (!inlineMap.isConstant()) { - return null; - } - } - else if (!(c % 2 == 0 && child instanceof PropertyOrFieldReference)) { - if (!(child instanceof OpMinus opMinus) || !opMinus.isNegativeNumberLiteral()) { - return null; - } - } - } - } - - Map constantMap = new LinkedHashMap<>(); + private boolean determineIfConstant() { int childCount = getChildCount(); - ExpressionState expressionState = new ExpressionState(new StandardEvaluationContext()); for (int c = 0; c < childCount; c++) { - SpelNode keyChild = getChild(c++); - Object key; - if (keyChild instanceof Literal literal) { - key = literal.getLiteralValue().getValue(); + SpelNode child = getChild(c); + if (child instanceof Literal) { + continue; } - else if (keyChild instanceof PropertyOrFieldReference propertyOrFieldReference) { - key = propertyOrFieldReference.getName(); + if (child instanceof InlineList inlineList && inlineList.isConstant()) { + continue; } - else if (keyChild instanceof OpMinus) { - key = keyChild.getValue(expressionState); + if (child instanceof InlineMap inlineMap && inlineMap.isConstant()) { + continue; } - else { - return null; + if (c % 2 == 0 && child instanceof PropertyOrFieldReference) { + continue; } - - SpelNode valueChild = getChild(c); - Object value = null; - if (valueChild instanceof Literal literal) { - value = literal.getLiteralValue().getValue(); + if (child instanceof OpMinus opMinus && opMinus.isNegativeNumberLiteral()) { + continue; } - else if (valueChild instanceof InlineList inlineList) { - value = inlineList.getConstantValue(); - } - else if (valueChild instanceof InlineMap inlineMap) { - value = inlineMap.getConstantValue(); - } - else if (valueChild instanceof OpMinus) { - value = valueChild.getValue(expressionState); - } - constantMap.put(key, value); + return false; } - return new TypedValue(Collections.unmodifiableMap(constantMap)); + return true; } @Override public TypedValue getValueInternal(ExpressionState expressionState) throws EvaluationException { - if (this.constant != null) { - return this.constant; + TypedValue result = this.constant; + if (result != null) { + return result; } - else { - Map returnValue = new LinkedHashMap<>(); - int childcount = getChildCount(); - for (int c = 0; c < childcount; c++) { - SpelNode keyChild = getChild(c++); - Object key = null; - if (keyChild instanceof PropertyOrFieldReference reference) { - key = reference.getName(); - } - else { - key = keyChild.getValue(expressionState); - } - Object value = getChild(c).getValue(expressionState); - returnValue.put(key, value); + result = createMap(expressionState); + if (this.isConstant) { + this.constant = result; + } + return result; + } + + private TypedValue createMap(ExpressionState expressionState) throws EvaluationException { + expressionState.trackOperation(); + Map map = new LinkedHashMap<>(); + int childCount = getChildCount(); + for (int c = 0; c < childCount; c++) { + SpelNode keyChild = getChild(c++); + Object key; + if (keyChild instanceof PropertyOrFieldReference reference) { + key = reference.getName(); } - return new TypedValue(returnValue); + else { + key = keyChild.getValue(expressionState); + } + Object value = getChild(c).getValue(expressionState); + expressionState.trackOperation(); + map.put(key, value); } + return new TypedValue(this.isConstant ? Collections.unmodifiableMap(map) : map); } @Override @@ -155,16 +131,32 @@ public class InlineMap extends SpelNodeImpl { } /** - * Return whether this map is a constant value. + * Return whether this map is structurally a constant value. + *

Note that the resulting constant value is created lazily on the + * first call to {@link #getValueInternal(ExpressionState)} or + * {@link #getConstantValue()}. */ public boolean isConstant() { - return this.constant != null; + return this.isConstant; } + /** + * Return the cached constant {@link Map} value for this inline map, + * lazily creating it on first access. + * @see #isConstant() + * @deprecated as of Spring Framework 6.2.19; this method was only intended for + * testing purposes and will be removed in a future version of the framework + */ @SuppressWarnings("unchecked") + @Deprecated(since = "6.2.19") public @Nullable Map getConstantValue() { - Assert.state(this.constant != null, "No constant"); - return (Map) this.constant.getValue(); + Assert.state(this.isConstant, "Not a constant"); + TypedValue result = this.constant; + if (result == null) { + result = createMap(new ExpressionState(SimpleEvaluationContext.forReadOnlyDataBinding().build())); + this.constant = result; + } + return (Map) result.getValue(); } } diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/MethodReference.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/MethodReference.java index ace09f1edd2..963104d4eaa 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/MethodReference.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/MethodReference.java @@ -131,13 +131,13 @@ public class MethodReference extends SpelNodeImpl { Object target = contextObject.getValue(); TypeDescriptor targetType = contextObject.getTypeDescriptor(); @Nullable Object[] arguments = getArguments(state); - TypedValue result = getValueInternal(evaluationContext, target, targetType, arguments); + TypedValue result = getValueInternal(state, evaluationContext, target, targetType, arguments); updateExitTypeDescriptor(); return result; } - private TypedValue getValueInternal(EvaluationContext evaluationContext, @Nullable Object target, - @Nullable TypeDescriptor targetType, @Nullable Object[] arguments) { + private TypedValue getValueInternal(ExpressionState state, EvaluationContext evaluationContext, + @Nullable Object target, @Nullable TypeDescriptor targetType, @Nullable Object[] arguments) { List argumentTypes = getArgumentTypes(arguments); Optional fallbackOptionalTarget = null; @@ -167,6 +167,7 @@ public class MethodReference extends SpelNodeImpl { MethodExecutor executorToUse = getCachedExecutor(evaluationContext, target, targetType, argumentTypes); if (executorToUse != null) { try { + state.trackOperation(); return executorToUse.execute(evaluationContext, target, arguments); } catch (AccessException ex) { @@ -232,6 +233,7 @@ public class MethodReference extends SpelNodeImpl { this.cachedExecutor = new CachedMethodExecutor( executorToUse, (targetToUse instanceof Class clazz ? clazz : null), targetType, argumentTypes); try { + state.trackOperation(); return executorToUse.execute(evaluationContext, targetToUse, arguments); } catch (AccessException ex) { @@ -450,6 +452,8 @@ public class MethodReference extends SpelNodeImpl { private class MethodValueRef implements ValueRef { + private final ExpressionState expressionState; + private final EvaluationContext evaluationContext; private final @Nullable Object target; @@ -459,6 +463,7 @@ public class MethodReference extends SpelNodeImpl { private final @Nullable Object[] arguments; public MethodValueRef(ExpressionState state, @Nullable Object[] arguments) { + this.expressionState = state; this.evaluationContext = state.getEvaluationContext(); this.target = state.getActiveContextObject().getValue(); this.targetType = state.getActiveContextObject().getTypeDescriptor(); @@ -468,7 +473,7 @@ public class MethodReference extends SpelNodeImpl { @Override public TypedValue getValue() { TypedValue result = MethodReference.this.getValueInternal( - this.evaluationContext, this.target, this.targetType, this.arguments); + this.expressionState, this.evaluationContext, this.target, this.targetType, this.arguments); updateExitTypeDescriptor(); return result; } diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpAnd.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpAnd.java index 59f03f3b7a3..fa73c74904f 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpAnd.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpAnd.java @@ -47,6 +47,7 @@ public class OpAnd extends Operator { @Override public TypedValue getValueInternal(ExpressionState state) throws EvaluationException { + state.trackOperation(); if (!getBooleanValue(state, getLeftOperand())) { // no need to evaluate right operand return BooleanTypedValue.FALSE; diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpDec.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpDec.java index b0c664679ae..9133772b649 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpDec.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpDec.java @@ -68,6 +68,7 @@ public class OpDec extends Operator { TypedValue newValue = null; if (operandValue instanceof Number op1) { + state.trackOperation(); if (op1 instanceof BigDecimal bigDecimal) { newValue = new TypedValue(bigDecimal.subtract(BigDecimal.ONE), operandTypedValue.getTypeDescriptor()); } @@ -114,8 +115,9 @@ public class OpDec extends Operator { } } - // set the new value + state.trackOperation(); try { + // set the new value lvalue.setValue(newValue.getValue()); } catch (SpelEvaluationException see) { diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpDivide.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpDivide.java index 238caacf7b2..32270fef212 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpDivide.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpDivide.java @@ -51,6 +51,7 @@ public class OpDivide extends Operator { Object rightOperand = getRightOperand().getValueInternal(state).getValue(); if (leftOperand instanceof Number leftNumber && rightOperand instanceof Number rightNumber) { + state.trackOperation(); if (leftNumber instanceof BigDecimal || rightNumber instanceof BigDecimal) { BigDecimal leftBigDecimal = NumberUtils.convertNumberToTargetClass(leftNumber, BigDecimal.class); BigDecimal rightBigDecimal = NumberUtils.convertNumberToTargetClass(rightNumber, BigDecimal.class); diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpEQ.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpEQ.java index 03e017b17b4..2342b36a221 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpEQ.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpEQ.java @@ -43,6 +43,7 @@ public class OpEQ extends Operator { Object right = getRightOperand().getValueInternal(state).getValue(); this.leftActualDescriptor = CodeFlow.toDescriptorFromObject(left); this.rightActualDescriptor = CodeFlow.toDescriptorFromObject(right); + state.trackOperation(); return BooleanTypedValue.forValue(equalityCheck(state.getEvaluationContext(), left, right)); } diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpGE.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpGE.java index 58da70ced0f..b0feebc1290 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpGE.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpGE.java @@ -46,6 +46,7 @@ public class OpGE extends Operator { public BooleanTypedValue getValueInternal(ExpressionState state) throws EvaluationException { Object left = getLeftOperand().getValueInternal(state).getValue(); Object right = getRightOperand().getValueInternal(state).getValue(); + state.trackOperation(); this.leftActualDescriptor = CodeFlow.toDescriptorFromObject(left); this.rightActualDescriptor = CodeFlow.toDescriptorFromObject(right); diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpGT.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpGT.java index f570883cd09..59778577a1d 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpGT.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpGT.java @@ -46,6 +46,7 @@ public class OpGT extends Operator { public BooleanTypedValue getValueInternal(ExpressionState state) throws EvaluationException { Object left = getLeftOperand().getValueInternal(state).getValue(); Object right = getRightOperand().getValueInternal(state).getValue(); + state.trackOperation(); this.leftActualDescriptor = CodeFlow.toDescriptorFromObject(left); this.rightActualDescriptor = CodeFlow.toDescriptorFromObject(right); diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpInc.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpInc.java index 44a0002ad6a..8b0acedcea2 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpInc.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpInc.java @@ -66,6 +66,7 @@ public class OpInc extends Operator { TypedValue newValue = null; if (value instanceof Number op1) { + state.trackOperation(); if (op1 instanceof BigDecimal bigDecimal) { newValue = new TypedValue(bigDecimal.add(BigDecimal.ONE), typedValue.getTypeDescriptor()); } @@ -110,8 +111,9 @@ public class OpInc extends Operator { } } - // set the new value + state.trackOperation(); try { + // set the new value valueRef.setValue(newValue.getValue()); } catch (SpelEvaluationException see) { diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpLE.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpLE.java index 8b5125457f3..aa3d9ac7073 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpLE.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpLE.java @@ -46,6 +46,7 @@ public class OpLE extends Operator { public BooleanTypedValue getValueInternal(ExpressionState state) throws EvaluationException { Object left = getLeftOperand().getValueInternal(state).getValue(); Object right = getRightOperand().getValueInternal(state).getValue(); + state.trackOperation(); this.leftActualDescriptor = CodeFlow.toDescriptorFromObject(left); this.rightActualDescriptor = CodeFlow.toDescriptorFromObject(right); diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpLT.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpLT.java index efeb4a42ddf..ce0dfb8ed97 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpLT.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpLT.java @@ -46,6 +46,7 @@ public class OpLT extends Operator { public BooleanTypedValue getValueInternal(ExpressionState state) throws EvaluationException { Object left = getLeftOperand().getValueInternal(state).getValue(); Object right = getRightOperand().getValueInternal(state).getValue(); + state.trackOperation(); this.leftActualDescriptor = CodeFlow.toDescriptorFromObject(left); this.rightActualDescriptor = CodeFlow.toDescriptorFromObject(right); diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpMinus.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpMinus.java index ef42289c76f..a926c54b6a7 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpMinus.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpMinus.java @@ -51,6 +51,13 @@ public class OpMinus extends Operator { public OpMinus(int startPos, int endPos, SpelNodeImpl... operands) { super("-", startPos, endPos, operands); + // If this is a unary negation of a number literal, the exit type descriptor + // can be derived statically from the literal. Doing so up front lets the + // compiler determine that this node is compilable without first evaluating + // the expression. + if (isNegativeNumberLiteral()) { + this.exitTypeDescriptor = ((Literal) operands[0]).exitTypeDescriptor; + } } @@ -72,6 +79,7 @@ public class OpMinus extends Operator { if (this.children.length < 2) { // if only one operand, then this is unary minus Object operand = leftOp.getValueInternal(state).getValue(); if (operand instanceof Number number) { + state.trackOperation(); if (number instanceof BigDecimal bigDecimal) { return new TypedValue(bigDecimal.negate()); } @@ -112,6 +120,7 @@ public class OpMinus extends Operator { Object right = getRightOperand().getValueInternal(state).getValue(); if (left instanceof Number leftNumber && right instanceof Number rightNumber) { + state.trackOperation(); if (leftNumber instanceof BigDecimal || rightNumber instanceof BigDecimal) { BigDecimal leftBigDecimal = NumberUtils.convertNumberToTargetClass(leftNumber, BigDecimal.class); BigDecimal rightBigDecimal = NumberUtils.convertNumberToTargetClass(rightNumber, BigDecimal.class); @@ -145,6 +154,7 @@ public class OpMinus extends Operator { } if (left instanceof String theString && right instanceof Integer theInteger && theString.length() == 1) { + state.trackOperation(); // Implements character - int (ie. b - 1 = a) return new TypedValue(Character.toString((char) (theString.charAt(0) - theInteger))); } diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpModulus.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpModulus.java index 4823f0284c5..6facafd94f3 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpModulus.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpModulus.java @@ -49,6 +49,7 @@ public class OpModulus extends Operator { Object rightOperand = getRightOperand().getValueInternal(state).getValue(); if (leftOperand instanceof Number leftNumber && rightOperand instanceof Number rightNumber) { + state.trackOperation(); if (leftNumber instanceof BigDecimal || rightNumber instanceof BigDecimal) { BigDecimal leftBigDecimal = NumberUtils.convertNumberToTargetClass(leftNumber, BigDecimal.class); BigDecimal rightBigDecimal = NumberUtils.convertNumberToTargetClass(rightNumber, BigDecimal.class); diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpMultiply.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpMultiply.java index 75a5bb10ccc..5f7f53f7115 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpMultiply.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpMultiply.java @@ -82,6 +82,7 @@ public class OpMultiply extends Operator { Object rightOperand = getRightOperand().getValueInternal(state).getValue(); if (leftOperand instanceof Number leftNumber && rightOperand instanceof Number rightNumber) { + state.trackOperation(); if (leftNumber instanceof BigDecimal || rightNumber instanceof BigDecimal) { BigDecimal leftBigDecimal = NumberUtils.convertNumberToTargetClass(leftNumber, BigDecimal.class); BigDecimal rightBigDecimal = NumberUtils.convertNumberToTargetClass(rightNumber, BigDecimal.class); @@ -116,6 +117,7 @@ public class OpMultiply extends Operator { if (leftOperand instanceof String text && rightOperand instanceof Integer count) { checkRepeatedTextSize(text, count); + state.trackOperation(); return new TypedValue(text.repeat(count)); } @@ -127,9 +129,9 @@ public class OpMultiply extends Operator { throw new SpelEvaluationException(getStartPosition(), SpelMessage.NEGATIVE_REPEATED_TEXT_COUNT, count); } - int result = text.length() * count; + long result = (long) text.length() * (long) count; if (result < 0 || result > MAX_REPEATED_TEXT_SIZE) { - throw new SpelEvaluationException(getStartPosition(), + throw new SpelEvaluationException(getRightOperand().getStartPosition(), SpelMessage.MAX_REPEATED_TEXT_SIZE_EXCEEDED, MAX_REPEATED_TEXT_SIZE); } } diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpNE.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpNE.java index 77e62a1fffc..5f656f47000 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpNE.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpNE.java @@ -44,6 +44,7 @@ public class OpNE extends Operator { Object rightValue = getRightOperand().getValueInternal(state).getValue(); this.leftActualDescriptor = CodeFlow.toDescriptorFromObject(leftValue); this.rightActualDescriptor = CodeFlow.toDescriptorFromObject(rightValue); + state.trackOperation(); return BooleanTypedValue.forValue(!equalityCheck(state.getEvaluationContext(), leftValue, rightValue)); } diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpOr.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpOr.java index e3798c4f6db..b8c4e629db1 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpOr.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpOr.java @@ -46,6 +46,7 @@ public class OpOr extends Operator { @Override public BooleanTypedValue getValueInternal(ExpressionState state) throws EvaluationException { + state.trackOperation(); if (getBooleanValue(state, getLeftOperand())) { // no need to evaluate right operand return BooleanTypedValue.TRUE; diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpPlus.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpPlus.java index f037ef7d522..858bdff0a21 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpPlus.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpPlus.java @@ -74,6 +74,7 @@ public class OpPlus extends Operator { if (this.children.length < 2) { // if only one operand, then this is unary plus Object operandOne = leftOp.getValueInternal(state).getValue(); if (operandOne instanceof Number) { + state.trackOperation(); if (operandOne instanceof Double) { this.exitTypeDescriptor = "D"; } @@ -97,6 +98,7 @@ public class OpPlus extends Operator { Object rightOperand = operandTwoValue.getValue(); if (leftOperand instanceof Number leftNumber && rightOperand instanceof Number rightNumber) { + state.trackOperation(); if (leftNumber instanceof BigDecimal || rightNumber instanceof BigDecimal) { BigDecimal leftBigDecimal = NumberUtils.convertNumberToTargetClass(leftNumber, BigDecimal.class); BigDecimal rightBigDecimal = NumberUtils.convertNumberToTargetClass(rightNumber, BigDecimal.class); @@ -133,37 +135,41 @@ public class OpPlus extends Operator { this.exitTypeDescriptor = "Ljava/lang/String"; checkStringLength(leftString); checkStringLength(rightString); - return concatenate(leftString, rightString); + return concatenate(state, leftString, rightString); } if (leftOperand instanceof String leftString) { checkStringLength(leftString); String rightString = (rightOperand == null ? "null" : convertTypedValueToString(operandTwoValue, state)); checkStringLength(rightString); - return concatenate(leftString, rightString); + return concatenate(state, leftString, rightString); } if (rightOperand instanceof String rightString) { checkStringLength(rightString); String leftString = (leftOperand == null ? "null" : convertTypedValueToString(operandOneValue, state)); checkStringLength(leftString); - return concatenate(leftString, rightString); + return concatenate(state, leftString, rightString); } return state.operate(Operation.ADD, leftOperand, rightOperand); } private void checkStringLength(String string) { - if (string.length() > MAX_CONCATENATED_STRING_LENGTH) { + checkStringLength(string.length()); + } + + private void checkStringLength(int stringLength) { + if (stringLength > MAX_CONCATENATED_STRING_LENGTH) { throw new SpelEvaluationException(getStartPosition(), SpelMessage.MAX_CONCATENATED_STRING_LENGTH_EXCEEDED, MAX_CONCATENATED_STRING_LENGTH); } } - private TypedValue concatenate(String leftString, String rightString) { - String result = leftString + rightString; - checkStringLength(result); - return new TypedValue(result); + private TypedValue concatenate(ExpressionState state, String leftString, String rightString) { + checkStringLength(leftString.length() + rightString.length()); + state.trackOperation(); + return new TypedValue(leftString + rightString); } @Override diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OperatorBetween.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OperatorBetween.java index f742a60dca2..6a1607205bb 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OperatorBetween.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OperatorBetween.java @@ -68,6 +68,7 @@ public class OperatorBetween extends Operator { Object high = list.get(1); TypeComparator comp = state.getTypeComparator(); try { + state.trackOperation(); return BooleanTypedValue.forValue(comp.compare(left, low) >= 0 && comp.compare(left, high) <= 0); } catch (SpelEvaluationException ex) { diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OperatorInstanceof.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OperatorInstanceof.java index 9c74b7c2524..72303465ce5 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OperatorInstanceof.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OperatorInstanceof.java @@ -62,6 +62,7 @@ public class OperatorInstanceof extends Operator { Object leftValue = left.getValue(); Object rightValue = right.getValue(); BooleanTypedValue result; + state.trackOperation(); if (!(rightValue instanceof Class rightClass)) { throw new SpelEvaluationException(getRightOperand().getStartPosition(), SpelMessage.INSTANCEOF_OPERATOR_NEEDS_CLASS_OPERAND, 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 21e8b998e13..3d9dd80594e 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 @@ -16,7 +16,6 @@ package org.springframework.expression.spel.ast; -import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -27,6 +26,7 @@ import org.springframework.expression.spel.ExpressionState; import org.springframework.expression.spel.SpelEvaluationException; import org.springframework.expression.spel.SpelMessage; import org.springframework.expression.spel.support.BooleanTypedValue; +import org.springframework.util.ConcurrentLruCache; /** * Implements the matches operator. Matches takes two operands: @@ -41,6 +41,12 @@ import org.springframework.expression.spel.support.BooleanTypedValue; */ public class OperatorMatches extends Operator { + /** + * Maximum number of compiled regular expressions in the pattern cache: {@value}. + * @since 6.2.19 + */ + public static final int MAX_PATTERN_CACHE_SIZE = 256; + private static final int PATTERN_ACCESS_THRESHOLD = 1000000; /** @@ -49,29 +55,47 @@ public class OperatorMatches extends Operator { */ private static final int MAX_REGEX_LENGTH = 1000; - private final ConcurrentMap patternCache; + + private final ConcurrentLruCache patternCache; /** * Create a new {@link OperatorMatches} instance. - * @deprecated as of Spring Framework 5.2.23 in favor of invoking - * {@link #OperatorMatches(ConcurrentMap, int, int, SpelNodeImpl...)} - * with a shared pattern cache instead + * @deprecated as of Spring Framework 5.2.23; for removal in Spring Framework 7.1; invoke + * {@link #OperatorMatches(ConcurrentLruCache, int, int, SpelNodeImpl...)} instead */ - @Deprecated(since = "5.2.23") + @Deprecated(since = "5.2.23", forRemoval = true) public OperatorMatches(int startPos, int endPos, SpelNodeImpl... operands) { - this(new ConcurrentHashMap<>(), startPos, endPos, operands); + this(new ConcurrentLruCache<>(MAX_PATTERN_CACHE_SIZE, Pattern::compile), startPos, endPos, operands); } /** * Create a new {@link OperatorMatches} instance with a shared pattern cache. + *

As of Spring Framework 6.2.19, the supplied {@code patternCacheMap} will + * be ignored. * @since 5.2.23 + * @deprecated as of Spring Framework 6.2.19; for removal in Spring Framework 7.1; invoke + * {@link #OperatorMatches(ConcurrentLruCache, int, int, SpelNodeImpl...)} instead */ - public OperatorMatches(ConcurrentMap patternCache, int startPos, int endPos, SpelNodeImpl... operands) { + @Deprecated(since = "6.2.19", forRemoval = true) + public OperatorMatches(ConcurrentMap patternCacheMap, + int startPos, int endPos, SpelNodeImpl... operands) { + + this(startPos, endPos, operands); + } + + /** + * Create a new {@link OperatorMatches} instance with a shared pattern cache. + * @since 6.2.19 + */ + public OperatorMatches(ConcurrentLruCache patternCache, + int startPos, int endPos, SpelNodeImpl... operands) { + super("matches", startPos, endPos, operands); this.patternCache = patternCache; } + /** * Check the first operand matches the regex specified as the second operand. * @param state the expression state @@ -96,12 +120,14 @@ public class OperatorMatches extends Operator { throw new SpelEvaluationException(rightOp.getStartPosition(), SpelMessage.INVALID_SECOND_OPERAND_FOR_MATCHES_OPERATOR, right); } + if (regex.length() > MAX_REGEX_LENGTH) { + throw new SpelEvaluationException(rightOp.getStartPosition(), + SpelMessage.MAX_REGEX_LENGTH_EXCEEDED, MAX_REGEX_LENGTH); + } + state.trackOperation(); try { - Pattern pattern = this.patternCache.computeIfAbsent(regex, key -> { - checkRegexLength(key); - return Pattern.compile(key); - }); + Pattern pattern = this.patternCache.get(regex); Matcher matcher = pattern.matcher(new MatcherInput(input, new AccessCount())); return BooleanTypedValue.forValue(matcher.matches()); } @@ -115,13 +141,6 @@ public class OperatorMatches extends Operator { } } - private void checkRegexLength(String regex) { - if (regex.length() > MAX_REGEX_LENGTH) { - throw new SpelEvaluationException(getRightOperand().getStartPosition(), - SpelMessage.MAX_REGEX_LENGTH_EXCEEDED, MAX_REGEX_LENGTH); - } - } - private static class AccessCount { diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OperatorNot.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OperatorNot.java index ec7c37b4984..091dc67dbba 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OperatorNot.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OperatorNot.java @@ -48,6 +48,7 @@ public class OperatorNot extends SpelNodeImpl { // Not is a unary operator so d if (value == null) { throw new SpelEvaluationException(SpelMessage.TYPE_CONVERSION_ERROR, "null", "boolean"); } + state.trackOperation(); return BooleanTypedValue.forValue(!value); } catch (SpelEvaluationException ex) { diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OperatorPower.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OperatorPower.java index fcc68e4c9b6..1891db890f6 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OperatorPower.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OperatorPower.java @@ -48,6 +48,7 @@ public class OperatorPower extends Operator { Object rightOperand = rightOp.getValueInternal(state).getValue(); if (leftOperand instanceof Number leftNumber && rightOperand instanceof Number rightNumber) { + state.trackOperation(); if (leftNumber instanceof BigDecimal) { BigDecimal leftBigDecimal = NumberUtils.convertNumberToTargetClass(leftNumber, BigDecimal.class); return new TypedValue(leftBigDecimal.pow(rightNumber.intValue())); diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/Projection.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/Projection.java index d21e722d3ac..7aca05ffd04 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/Projection.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/Projection.java @@ -110,11 +110,13 @@ public class Projection extends SpelNodeImpl { // that can be referenced in the operation -- for example, // {'a':'y', 'b':'n'}.![value == 'y' ? key : null] evaluates to ['a', null]. if (operand instanceof Map mapData) { + state.trackOperation(); List result = new ArrayList<>(); for (Map.Entry entry : mapData.entrySet()) { try { state.pushActiveContextObject(new TypedValue(entry)); state.enterScope(); + state.trackOperation(); result.add(this.children[0].getValueInternal(state).getValue()); } finally { @@ -130,6 +132,7 @@ public class Projection extends SpelNodeImpl { Iterable data = (operand instanceof Iterable iterable ? iterable : Arrays.asList(ObjectUtils.toObjectArray(operand))); + state.trackOperation(); List result = new ArrayList<>(); Class arrayElementType = null; for (Object element : data) { @@ -140,6 +143,7 @@ public class Projection extends SpelNodeImpl { if (value != null && operandIsArray) { arrayElementType = determineCommonType(arrayElementType, value.getClass()); } + state.trackOperation(); result.add(value); } finally { diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/PropertyOrFieldReference.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/PropertyOrFieldReference.java index b5f5ff27f69..e7761c310fe 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/PropertyOrFieldReference.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/PropertyOrFieldReference.java @@ -110,14 +110,14 @@ public class PropertyOrFieldReference extends SpelNodeImpl { @Override public ValueRef getValueRef(ExpressionState state) throws EvaluationException { - return new AccessorValueRef(this, state.getActiveContextObject(), state.getEvaluationContext(), + return new AccessorValueRef(this, state.getActiveContextObject(), state, state.getEvaluationContext(), state.getConfiguration().isAutoGrowNullReferences()); } @Override public TypedValue getValueInternal(ExpressionState state) throws EvaluationException { - TypedValue tv = getValueInternal(state.getActiveContextObject(), state.getEvaluationContext(), - state.getConfiguration().isAutoGrowNullReferences()); + TypedValue tv = getValueInternal(state, state.getActiveContextObject(), + state.getEvaluationContext(), state.getConfiguration().isAutoGrowNullReferences()); PropertyAccessor accessorToUse = this.cachedReadAccessor; if (accessorToUse instanceof CompilablePropertyAccessor compilablePropertyAccessor) { setExitTypeDescriptor(CodeFlow.toDescriptor(compilablePropertyAccessor.getPropertyType())); @@ -125,10 +125,10 @@ public class PropertyOrFieldReference extends SpelNodeImpl { return tv; } - private TypedValue getValueInternal(TypedValue contextObject, EvaluationContext evalContext, - boolean isAutoGrowNullReferences) throws EvaluationException { + private TypedValue getValueInternal(ExpressionState state, TypedValue contextObject, + EvaluationContext evalContext, boolean isAutoGrowNullReferences) throws EvaluationException { - TypedValue result = readProperty(contextObject, evalContext, this.name); + TypedValue result = readProperty(state, contextObject, evalContext, this.name); // Dynamically create the objects if the user has requested that optional behavior if (result.getValue() == null && isAutoGrowNullReferences && @@ -138,16 +138,18 @@ public class PropertyOrFieldReference extends SpelNodeImpl { // Create a new collection or map ready for the indexer if (List.class == resultDescriptor.getType()) { if (isWritableProperty(this.name, contextObject, evalContext)) { + state.trackOperation(); List newList = new ArrayList<>(); - writeProperty(contextObject, evalContext, this.name, newList); - result = readProperty(contextObject, evalContext, this.name); + writeProperty(state, contextObject, evalContext, this.name, newList); + result = readProperty(state, contextObject, evalContext, this.name); } } else if (Map.class == resultDescriptor.getType()) { if (isWritableProperty(this.name,contextObject, evalContext)) { + state.trackOperation(); Map newMap = new HashMap<>(); - writeProperty(contextObject, evalContext, this.name, newMap); - result = readProperty(contextObject, evalContext, this.name); + writeProperty(state, contextObject, evalContext, this.name, newMap); + result = readProperty(state, contextObject, evalContext, this.name); } } else { @@ -155,9 +157,10 @@ public class PropertyOrFieldReference extends SpelNodeImpl { try { if (isWritableProperty(this.name,contextObject, evalContext)) { Class clazz = resultDescriptor.getType(); + state.trackOperation(); Object newObject = ReflectionUtils.accessibleConstructor(clazz).newInstance(); - writeProperty(contextObject, evalContext, this.name, newObject); - result = readProperty(contextObject, evalContext, this.name); + writeProperty(state, contextObject, evalContext, this.name, newObject); + result = readProperty(state, contextObject, evalContext, this.name); } } catch (InvocationTargetException ex) { @@ -178,7 +181,7 @@ public class PropertyOrFieldReference extends SpelNodeImpl { throws EvaluationException { TypedValue typedValue = valueSupplier.get(); - writeProperty(state.getActiveContextObject(), state.getEvaluationContext(), this.name, typedValue.getValue()); + writeProperty(state, state.getActiveContextObject(), state.getEvaluationContext(), this.name, typedValue.getValue()); return typedValue; } @@ -197,8 +200,8 @@ public class PropertyOrFieldReference extends SpelNodeImpl { * @return the value of the property * @throws EvaluationException if any problem accessing the property, or if it cannot be found */ - private TypedValue readProperty(TypedValue contextObject, EvaluationContext evalContext, String name) - throws EvaluationException { + private TypedValue readProperty(ExpressionState state, TypedValue contextObject, + EvaluationContext evalContext, String name) throws EvaluationException { final Object originalTarget = contextObject.getValue(); Object target = originalTarget; @@ -226,6 +229,7 @@ public class PropertyOrFieldReference extends SpelNodeImpl { if (accessorToUse != null) { if (evalContext.getPropertyAccessors().contains(accessorToUse)) { try { + state.trackOperation(); return accessorToUse.read(evalContext, target, name); } catch (Exception ex) { @@ -250,6 +254,7 @@ public class PropertyOrFieldReference extends SpelNodeImpl { evalContext, target, name); } this.cachedReadAccessor = accessor; + state.trackOperation(); return accessor.read(evalContext, target, name); } // Second, attempt to find the property on the original Optional instance. @@ -262,11 +267,15 @@ public class PropertyOrFieldReference extends SpelNodeImpl { // If we end up using a property on the original Optional instance, // we don't need to unwrap the Optional in the compiled expression. this.unwrapOptional = false; + state.trackOperation(); return accessor.read(evalContext, fallbackOptionalTarget, name); } } } catch (Exception ex) { + if (ex instanceof SpelEvaluationException see) { + throw see; + } throw new SpelEvaluationException(ex, SpelMessage.EXCEPTION_DURING_PROPERTY_READ, name, ex.getMessage()); } @@ -286,9 +295,8 @@ public class PropertyOrFieldReference extends SpelNodeImpl { } } - private void writeProperty( - TypedValue contextObject, EvaluationContext evalContext, String name, @Nullable Object newValue) - throws EvaluationException { + private void writeProperty(ExpressionState state, TypedValue contextObject, EvaluationContext evalContext, + String name, @Nullable Object newValue) throws EvaluationException { Object target = contextObject.getValue(); if (target == null) { @@ -303,6 +311,7 @@ public class PropertyOrFieldReference extends SpelNodeImpl { if (accessorToUse != null) { if (evalContext.getPropertyAccessors().contains(accessorToUse)) { try { + state.trackOperation(); accessorToUse.write(evalContext, target, name, newValue); return; } @@ -320,6 +329,7 @@ public class PropertyOrFieldReference extends SpelNodeImpl { for (PropertyAccessor accessor : accessorsToTry) { if (accessor.canWrite(evalContext, target, name)) { this.cachedWriteAccessor = accessor; + state.trackOperation(); accessor.write(evalContext, target, name, newValue); return; } @@ -416,15 +426,18 @@ public class PropertyOrFieldReference extends SpelNodeImpl { private final TypedValue contextObject; + private final ExpressionState expressionState; + private final EvaluationContext evalContext; private final boolean autoGrowNullReferences; public AccessorValueRef(PropertyOrFieldReference propertyOrFieldReference, TypedValue activeContextObject, - EvaluationContext evalContext, boolean autoGrowNullReferences) { + ExpressionState expressionState, EvaluationContext evalContext, boolean autoGrowNullReferences) { this.ref = propertyOrFieldReference; this.contextObject = activeContextObject; + this.expressionState = expressionState; this.evalContext = evalContext; this.autoGrowNullReferences = autoGrowNullReferences; } @@ -432,7 +445,7 @@ public class PropertyOrFieldReference extends SpelNodeImpl { @Override public TypedValue getValue() { TypedValue value = - this.ref.getValueInternal(this.contextObject, this.evalContext, this.autoGrowNullReferences); + this.ref.getValueInternal(this.expressionState, this.contextObject, this.evalContext, this.autoGrowNullReferences); if (this.ref.cachedReadAccessor instanceof CompilablePropertyAccessor compilablePropertyAccessor) { this.ref.setExitTypeDescriptor(CodeFlow.toDescriptor(compilablePropertyAccessor.getPropertyType())); } @@ -441,7 +454,7 @@ public class PropertyOrFieldReference extends SpelNodeImpl { @Override public void setValue(@Nullable Object newValue) { - this.ref.writeProperty(this.contextObject, this.evalContext, this.ref.name, newValue); + this.ref.writeProperty(this.expressionState, this.contextObject, this.evalContext, this.ref.name, newValue); } @Override diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/Selection.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/Selection.java index a24645c7e2e..6e638a93de3 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/Selection.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/Selection.java @@ -130,6 +130,7 @@ public class Selection extends SpelNodeImpl { SpelNodeImpl selectionCriteria = this.children[0]; if (operand instanceof Map mapdata) { + state.trackOperation(); Map result = new HashMap<>(); Object lastKey = null; @@ -141,6 +142,7 @@ public class Selection extends SpelNodeImpl { Object val = selectionCriteria.getValueInternal(state).getValue(); if (val instanceof Boolean b) { if (b) { + state.trackOperation(); result.put(entry.getKey(), entry.getValue()); if (this.variant == FIRST) { return new ValueRef.TypedValueHolderValueRef(new TypedValue(result), this); @@ -177,6 +179,7 @@ public class Selection extends SpelNodeImpl { Iterable data = (operand instanceof Iterable iterable ? iterable : Arrays.asList(ObjectUtils.toObjectArray(operand))); + state.trackOperation(); List result = new ArrayList<>(); for (Object element : data) { try { @@ -188,6 +191,7 @@ public class Selection extends SpelNodeImpl { if (this.variant == FIRST) { return new ValueRef.TypedValueHolderValueRef(new TypedValue(element), this); } + state.trackOperation(); result.add(element); } } diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/Ternary.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/Ternary.java index b28800bcb46..0b2988a0887 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/Ternary.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/Ternary.java @@ -57,6 +57,7 @@ public class Ternary extends SpelNodeImpl { throw new SpelEvaluationException(getChild(0).getStartPosition(), SpelMessage.TYPE_CONVERSION_ERROR, "null", "boolean"); } + state.trackOperation(); TypedValue result = this.children[condition ? 1 : 2].getValueInternal(state); computeExitTypeDescriptor(); return result; diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/TypeReference.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/TypeReference.java index 6230e0b9adf..b06c2ff7fde 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/TypeReference.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/TypeReference.java @@ -58,6 +58,7 @@ public class TypeReference extends SpelNodeImpl { // TODO Possible optimization: if we cache the discovered type reference, but can we do that? String typeName = (String) this.children[0].getValueInternal(state).getValue(); Assert.state(typeName != null, "No type name"); + state.trackOperation(); if (!typeName.contains(".") && Character.isLowerCase(typeName.charAt(0))) { TypeCode tc = TypeCode.valueOf(typeName.toUpperCase(Locale.ROOT)); if (tc != TypeCode.OBJECT) { diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/standard/InternalSpelExpressionParser.java b/spring-expression/src/main/java/org/springframework/expression/spel/standard/InternalSpelExpressionParser.java index dc95f9fb956..c94bea7277e 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/standard/InternalSpelExpressionParser.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/standard/InternalSpelExpressionParser.java @@ -22,8 +22,6 @@ import java.util.Collections; import java.util.Deque; import java.util.List; import java.util.Locale; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentMap; import java.util.regex.Pattern; import org.jspecify.annotations.Nullable; @@ -80,6 +78,7 @@ import org.springframework.expression.spel.ast.Ternary; import org.springframework.expression.spel.ast.TypeReference; import org.springframework.expression.spel.ast.VariableReference; import org.springframework.lang.Contract; +import org.springframework.util.ConcurrentLruCache; import org.springframework.util.StringUtils; /** @@ -101,7 +100,9 @@ class InternalSpelExpressionParser extends TemplateAwareExpressionParser { private final Deque constructedNodes = new ArrayDeque<>(); // Shared cache for compiled regex patterns - private final ConcurrentMap patternCache = new ConcurrentHashMap<>(); + private final ConcurrentLruCache patternCache = + new ConcurrentLruCache<>(OperatorMatches.MAX_PATTERN_CACHE_SIZE, Pattern::compile); + // The expression being parsed private String expressionString = ""; diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectivePropertyAccessor.java b/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectivePropertyAccessor.java index 06f93226093..b9e2fcbd077 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectivePropertyAccessor.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectivePropertyAccessor.java @@ -374,10 +374,11 @@ public class ReflectivePropertyAccessor implements PropertyAccessor { * Find a getter method for the specified property. */ protected @Nullable Method findGetterForProperty(String propertyName, Class clazz, boolean mustBeStatic) { - Method method = findMethodForProperty(getPropertyMethodSuffixes(propertyName), + String[] methodSuffixes = getPropertyMethodSuffixes(propertyName); + Method method = findMethodForProperty(methodSuffixes, "get", clazz, mustBeStatic, 0, ANY_TYPES); if (method == null) { - method = findMethodForProperty(getPropertyMethodSuffixes(propertyName), + method = findMethodForProperty(methodSuffixes, "is", clazz, mustBeStatic, 0, BOOLEAN_TYPES); if (method == null) { // Record-style plain accessor method, for example, name() @@ -385,6 +386,9 @@ public class ReflectivePropertyAccessor implements PropertyAccessor { "", clazz, mustBeStatic, 0, ANY_TYPES); } } + if (method != null && method.getReturnType() == void.class) { + method = null; // not a valid accessor method + } return method; } 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 f5f9cafb190..06290373568 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 @@ -27,6 +27,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; +import org.springframework.core.SpringProperties; import org.springframework.expression.AccessException; import org.springframework.expression.BeanResolver; import org.springframework.expression.EvaluationContext; @@ -47,6 +48,7 @@ import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.assertThatIllegalStateException; import static org.assertj.core.api.Assertions.within; import static org.springframework.expression.spel.SpelMessage.BETWEEN_RIGHT_OPERAND_MUST_BE_TWO_ELEMENT_LIST; +import static org.springframework.expression.spel.SpelParserConfiguration.SPRING_EXPRESSION_MAX_OPERATIONS_PROPERTY_NAME; /** * Tests the evaluation of real expressions in a real context. @@ -97,6 +99,96 @@ class EvaluationTests extends AbstractExpressionTests { evaluateAndCheckError(parser, expression, String.class, SpelMessage.MAX_EXPRESSION_LENGTH_EXCEEDED); } + @Test + void maxOperationsIsConfigurableViaSpelParserConfiguration() { + int maxLength = 100; + int maxOperations = 10; + + ExpressionParser parser = new SpelExpressionParser(); + + // The following expression contains 10 tracked operations: + String expression = "('foo' + 'bar').length >= 6 && {1, 1 + 1, 3}.contains(3)"; + Expression expr1 = parser.parseExpression(expression); + assertThat(expr1.getValue(Boolean.class)).isTrue(); + + SpelParserConfiguration configuration = + new SpelParserConfiguration(SpelCompilerMode.OFF, null, false, false, 0, maxLength, maxOperations); + parser = new SpelExpressionParser(configuration); + Expression expr2 = parser.parseExpression(expression); + assertThatExceptionOfType(SpelEvaluationException.class) + .isThrownBy(() -> expr2.getValue(Boolean.class)) + .satisfies(ex -> { + assertThat(ex.getMessageCode()).isEqualTo(SpelMessage.MAX_OPERATIONS_EXCEEDED); + assertThat(ex.getInserts()).as("inserts").containsExactly(maxOperations); + }); + } + + @Test + void maxOperationsIsConfigurableViaSpringProperty() { + int maxOperations = 10; + + ExpressionParser parser = new SpelExpressionParser(); + + // The following expression contains 10 tracked operations: + String expression = "('foo' + 'bar').length >= 6 && {1, 1 + 1, 3}.contains(3)"; + Expression expr1 = parser.parseExpression(expression); + assertThat(expr1.getValue(Boolean.class)).isTrue(); + + try { + SpringProperties.setProperty(SPRING_EXPRESSION_MAX_OPERATIONS_PROPERTY_NAME, "" + maxOperations); + + parser = new SpelExpressionParser(); + Expression expr2 = parser.parseExpression(expression); + assertThatExceptionOfType(SpelEvaluationException.class) + .isThrownBy(() -> expr2.getValue(Boolean.class)) + .satisfies(ex -> { + assertThat(ex.getMessageCode()).isEqualTo(SpelMessage.MAX_OPERATIONS_EXCEEDED); + assertThat(ex.getInserts()).as("inserts").containsExactly(maxOperations); + }); + } + finally { + SpringProperties.setProperty(SPRING_EXPRESSION_MAX_OPERATIONS_PROPERTY_NAME, null); + } + } + + @Test + void maxOperationsConfiguredViaSpelParserConfigurationOverridesSpringProperty() { + int maxLength = 100; + int maxOperations = 10; + + ExpressionParser parser = new SpelExpressionParser(); + + // The following expression contains 10 tracked operations: + String expression = "('foo' + 'bar').length >= 6 && {1, 1 + 1, 3}.contains(3)"; + Expression expr1 = parser.parseExpression(expression); + assertThat(expr1.getValue(Boolean.class)).isTrue(); + + try { + SpringProperties.setProperty(SPRING_EXPRESSION_MAX_OPERATIONS_PROPERTY_NAME, "" + maxOperations / 2); + + // maxOperations + 1 should override maxOperations / 2 + SpelParserConfiguration configuration = + new SpelParserConfiguration(SpelCompilerMode.OFF, null, false, false, 0, maxLength, maxOperations + 1); + parser = new SpelExpressionParser(configuration); + expr1 = parser.parseExpression(expression); + assertThat(expr1.getValue(Boolean.class)).isTrue(); + + configuration = new SpelParserConfiguration(SpelCompilerMode.OFF, null, false, false, 0, maxLength, maxOperations); + parser = new SpelExpressionParser(configuration); + Expression expr2 = parser.parseExpression(expression); + assertThatExceptionOfType(SpelEvaluationException.class) + .isThrownBy(() -> expr2.getValue(Boolean.class)) + .satisfies(ex -> { + assertThat(ex.getMessageCode()).isEqualTo(SpelMessage.MAX_OPERATIONS_EXCEEDED); + // Should fail due to maxOperations, not maxOperations / 2 + assertThat(ex.getInserts()).as("inserts").containsExactly(maxOperations); + }); + } + finally { + SpringProperties.setProperty(SPRING_EXPRESSION_MAX_OPERATIONS_PROPERTY_NAME, null); + } + } + @Test void createListsOnAttemptToIndexNull01() throws EvaluationException, ParseException { ExpressionParser parser = new SpelExpressionParser(new SpelParserConfiguration(true, true)); diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/OperatorTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/OperatorTests.java index 2059be38634..aadc76ad90a 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/OperatorTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/OperatorTests.java @@ -574,21 +574,26 @@ class OperatorTests extends AbstractExpressionTests { @Test void stringRepeat() { - evaluate("'abc' * 0", "", String.class); - evaluate("'abc' * 1", "abc", String.class); - evaluate("'abc' * 2", "abcabc", String.class); + String EMPTY = ""; + evaluate("'' * 0", EMPTY, String.class); + evaluate("'' * 2", EMPTY, String.class); + evaluate("'abc' * 0", EMPTY, String.class); + + evaluate("'Abc' * 1", "Abc", String.class); + evaluate("'Abc' * 2", "AbcAbc", String.class); + evaluate("'Abc' * 3", "AbcAbcAbc", String.class); Expression expr = parser.parseExpression("'a' * 256"); assertThat(expr.getValue(context, String.class)).hasSize(256); - // 4 is the position of the '*' (repeat operator) - evaluateAndCheckError("'a' * 257", String.class, MAX_REPEATED_TEXT_SIZE_EXCEEDED, 4); + // 6 is the position of the repeatCount + evaluateAndCheckError("'a' * 257", String.class, MAX_REPEATED_TEXT_SIZE_EXCEEDED, 6); // Integer overflow: 2 * ((Integer.MAX_VALUE / 2) + 1) --> integer overflow int repeatCount = (Integer.MAX_VALUE / 2) + 1; assertThat(2 * repeatCount).isNegative(); - // 5 is the position of the '*' (repeat operator) - evaluateAndCheckError("'ab' * " + repeatCount, String.class, MAX_REPEATED_TEXT_SIZE_EXCEEDED, 5); + // 7 is the position of the repeatCount + evaluateAndCheckError("'ab' * " + repeatCount, String.class, MAX_REPEATED_TEXT_SIZE_EXCEEDED, 7); } @Test @@ -623,12 +628,19 @@ class OperatorTests extends AbstractExpressionTests { assertThat(expr.getValue(context, String.class)).hasSize(maxSize); // Text is too big + context.setVariable("text1", createString(maxSize)); + context.setVariable("text2", createString(maxSize)); + evaluateAndCheckError("#text1 + #text2", String.class, MAX_CONCATENATED_STRING_LENGTH_EXCEEDED, 7); + context.setVariable("text1", createString(maxSize + 1)); evaluateAndCheckError("#text1 + ''", String.class, MAX_CONCATENATED_STRING_LENGTH_EXCEEDED, 7); evaluateAndCheckError("#text1 + true", String.class, MAX_CONCATENATED_STRING_LENGTH_EXCEEDED, 7); evaluateAndCheckError("'' + #text1", String.class, MAX_CONCATENATED_STRING_LENGTH_EXCEEDED, 3); evaluateAndCheckError("true + #text1", String.class, MAX_CONCATENATED_STRING_LENGTH_EXCEEDED, 5); + context.setVariable("text1", createString(maxSize - 1)); + evaluateAndCheckError("#text1 + 'YZ'", String.class, MAX_CONCATENATED_STRING_LENGTH_EXCEEDED, 7); + context.setVariable("text1", createString(maxSize / 2)); context.setVariable("text2", createString((maxSize / 2) + 1)); evaluateAndCheckError("#text1 + #text2", String.class, MAX_CONCATENATED_STRING_LENGTH_EXCEEDED, 7); diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/PropertyAccessTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/PropertyAccessTests.java index a5ab11cbb51..2209f56cd09 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/PropertyAccessTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/PropertyAccessTests.java @@ -16,6 +16,7 @@ package org.springframework.expression.spel; +import java.lang.reflect.Method; import java.time.ZoneId; import java.util.ArrayList; import java.util.Collections; @@ -39,6 +40,7 @@ import org.springframework.expression.spel.support.StandardEvaluationContext; import org.springframework.expression.spel.testresources.Inventor; import org.springframework.expression.spel.testresources.Person; import org.springframework.expression.spel.testresources.RecordPerson; +import org.springframework.util.ClassUtils; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; @@ -202,6 +204,16 @@ class PropertyAccessTests extends AbstractExpressionTests { target.setName("p2"); assertThat(expr.getValue(context, target)).isEqualTo("p2"); + assertThatSpelEvaluationException() + .isThrownBy(() -> parser.parseExpression("nonexistent").getValue(context, target)) + .extracting(SpelEvaluationException::getMessageCode).isEqualTo(SpelMessage.PROPERTY_OR_FIELD_NOT_READABLE); + + Method getInvalid = ClassUtils.getMethod(Person.class, "getInvalid"); + assertThat(getInvalid.getReturnType()).isEqualTo(void.class); + assertThatSpelEvaluationException() + .isThrownBy(() -> parser.parseExpression("invalid").getValue(context, target)) + .extracting(SpelEvaluationException::getMessageCode).isEqualTo(SpelMessage.PROPERTY_OR_FIELD_NOT_READABLE); + assertThatSpelEvaluationException() .isThrownBy(() -> parser.parseExpression("name='p3'").getValue(context, target)) .extracting(SpelEvaluationException::getMessageCode).isEqualTo(SpelMessage.NOT_ASSIGNABLE); diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/ast/InlineCollectionTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/ast/InlineCollectionTests.java index 0c122f5aee3..d3c436a6061 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/ast/InlineCollectionTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/ast/InlineCollectionTests.java @@ -30,6 +30,7 @@ import org.springframework.expression.spel.standard.SpelExpressionParser; import org.springframework.expression.spel.support.StandardEvaluationContext; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; /** * Tests for {@link InlineList} and {@link InlineMap}. @@ -47,17 +48,35 @@ class InlineCollectionTests { class InlineListTests { @Test - void listIsCached() { - InlineList list = parseList("{1, -2, 3, 4}"); - assertThat(list.isConstant()).isTrue(); - assertThat(list.getConstantValue()).isEqualTo(List.of(1, -2, 3, 4)); + @SuppressWarnings({ "rawtypes", "unchecked", "deprecation" }) + void getConstantValue() { + InlineList inlineList = parseList("{1, -2, 3, 4}"); + assertThat(inlineList.isConstant()).isTrue(); + List list1 = inlineList.getConstantValue(); + List list2 = inlineList.getValue(expressionState(), List.class); + assertThat(list1).containsExactly(1, -2, 3, 4).isSameAs(list2); + } + + @Test + @SuppressWarnings({ "rawtypes", "unchecked" }) + void constantListIsCached() { + ExpressionState expressionState = expressionState(); + + InlineList inlineList = parseList("{1, -2, 3, 4}"); + assertThat(inlineList.isConstant()).isTrue(); + List list1 = inlineList.getValue(expressionState, List.class); + List list2 = inlineList.getValue(expressionState, List.class); + assertThat(list1).containsExactly(1, -2, 3, 4).isSameAs(list2); + // Constant inline lists are immutable. + assertThatExceptionOfType(UnsupportedOperationException.class) + .isThrownBy(() -> list1.add(999)); } @Test void dynamicListIsNotCached() { InlineList list = parseList("{1, (5 - 3), 3, 4}"); assertThat(list.isConstant()).isFalse(); - assertThat(list.getValue(null)).isEqualTo(List.of(1, 2, 3, 4)); + assertThat(list.getValue(expressionState())).isEqualTo(List.of(1, 2, 3, 4)); } @Test @@ -110,11 +129,28 @@ class InlineCollectionTests { class InlineMapTests { @Test - void mapIsCached() { - InlineMap map = parseMap("{1 : 2, 3 : 4}"); - assertThat(map.isConstant()).isTrue(); - Map expected = Map.of(1, 2, 3, 4); - assertThat(map.getValue(null)).isEqualTo(expected); + @SuppressWarnings({ "rawtypes", "unchecked", "deprecation" }) + void getConstantValue() { + InlineMap inlineMap = parseMap("{1 : 2, 3 : 4}"); + assertThat(inlineMap.isConstant()).isTrue(); + Map map1 = inlineMap.getConstantValue(); + Map map2 = inlineMap.getValue(expressionState(), Map.class); + assertThat(map1).isEqualTo(Map.of(1, 2, 3, 4)).isSameAs(map2); + } + + @Test + @SuppressWarnings({ "rawtypes", "unchecked" }) + void constantMapIsCached() { + ExpressionState expressionState = expressionState(); + + InlineMap inlineMap = parseMap("{1 : 2, 3 : 4}"); + assertThat(inlineMap.isConstant()).isTrue(); + Map map1 = inlineMap.getValue(expressionState, Map.class); + Map map2 = inlineMap.getValue(expressionState, Map.class); + assertThat(map1).isEqualTo(Map.of(1, 2, 3, 4)).isSameAs(map2); + // Constant inline maps are immutable. + assertThatExceptionOfType(UnsupportedOperationException.class) + .isThrownBy(() -> map1.put(99, "X")); } @Test @@ -122,7 +158,7 @@ class InlineCollectionTests { InlineMap map = parseMap("{-1 : 2, (-2 - 1) : -4}"); assertThat(map.isConstant()).isFalse(); Map expected = Map.of(-1, 2, -3, -4); - assertThat(map.getValue(null)).isEqualTo(expected); + assertThat(map.getValue(expressionState())).isEqualTo(expected); } @Test @@ -155,7 +191,7 @@ class InlineCollectionTests { InlineMap map = parseMap("{-1 : 2, -3 : 4}"); assertThat(map.isConstant()).isTrue(); Map expected = Map.of(-1, 2, -3, 4); - assertThat(map.getValue(null)).isEqualTo(expected); + assertThat(map.getValue(expressionState())).isEqualTo(expected); } @Test @@ -163,7 +199,7 @@ class InlineCollectionTests { InlineMap map = parseMap("{1 : -2, 3 : -4}"); assertThat(map.isConstant()).isTrue(); Map expected = Map.of(1, -2, 3, -4); - assertThat(map.getValue(null)).isEqualTo(expected); + assertThat(map.getValue(expressionState())).isEqualTo(expected); } @Test @@ -171,7 +207,7 @@ class InlineCollectionTests { InlineMap map = parseMap("{1L : -2L, 3L : -4L}"); assertThat(map.isConstant()).isTrue(); Map expected = Map.of(1L, -2L, 3L, -4L); - assertThat(map.getValue(null)).isEqualTo(expected); + assertThat(map.getValue(expressionState())).isEqualTo(expected); } @Test @@ -179,7 +215,7 @@ class InlineCollectionTests { InlineMap map = parseMap("{-1.0f : -2.0f, -3.0f : -4.0f}"); assertThat(map.isConstant()).isTrue(); Map expected = Map.of(-1.0f, -2.0f, -3.0f, -4.0f); - assertThat(map.getValue(null)).isEqualTo(expected); + assertThat(map.getValue(expressionState())).isEqualTo(expected); } @Test @@ -187,7 +223,7 @@ class InlineCollectionTests { InlineMap map = parseMap("{-1.0 : -2.0, -3.0 : -4.0}"); assertThat(map.isConstant()).isTrue(); Map expected = Map.of(-1.0, -2.0, -3.0, -4.0); - assertThat(map.getValue(null)).isEqualTo(expected); + assertThat(map.getValue(expressionState())).isEqualTo(expected); } @Test @@ -195,7 +231,7 @@ class InlineCollectionTests { InlineMap map = parseMap("{-1 : -2, -3 : -4}"); assertThat(map.isConstant()).isTrue(); Map expected = Map.of(-1, -2, -3, -4); - assertThat(map.getValue(null)).isEqualTo(expected); + assertThat(map.getValue(expressionState())).isEqualTo(expected); } private InlineMap parseMap(String s) { @@ -210,6 +246,10 @@ class InlineCollectionTests { return (SpelExpression) parser.parseExpression(s); } + private static ExpressionState expressionState() { + return new ExpressionState(new StandardEvaluationContext()); + } + private static class NumberHolder { @SuppressWarnings("unused") diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/testresources/Person.java b/spring-expression/src/test/java/org/springframework/expression/spel/testresources/Person.java index 017bef65612..8237f9f33d8 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/testresources/Person.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/testresources/Person.java @@ -59,4 +59,8 @@ public class Person { return company; } + public void getInvalid() { + // no-op + } + } diff --git a/spring-jms/src/main/java/org/springframework/jms/support/converter/JacksonJsonMessageConverter.java b/spring-jms/src/main/java/org/springframework/jms/support/converter/JacksonJsonMessageConverter.java index a8a70006672..bb9f5f1977a 100644 --- a/spring-jms/src/main/java/org/springframework/jms/support/converter/JacksonJsonMessageConverter.java +++ b/spring-jms/src/main/java/org/springframework/jms/support/converter/JacksonJsonMessageConverter.java @@ -21,6 +21,7 @@ import java.io.IOException; import java.io.OutputStreamWriter; import java.io.StringWriter; import java.io.UnsupportedEncodingException; +import java.util.Arrays; import java.util.HashMap; import java.util.Map; @@ -48,6 +49,8 @@ import org.springframework.util.ClassUtils; * {@link #setTargetType targetType} is set to {@link MessageType#TEXT}. * Converts from a {@link TextMessage} or {@link BytesMessage} to an object. * + *

For untrusted environments, use {@link #setTrustedPackages(String...)}. + * * @author Sebastien Deleuze * @since 7.0 */ @@ -73,6 +76,8 @@ public class JacksonJsonMessageConverter implements SmartMessageConverter, BeanC private final Map, String> classIdMappings = new HashMap<>(); + private String @Nullable [] trustedPackages; + private @Nullable ClassLoader beanClassLoader; @@ -80,6 +85,7 @@ public class JacksonJsonMessageConverter implements SmartMessageConverter, BeanC * Construct a new instance with a {@link JsonMapper} customized with the * {@link tools.jackson.databind.JacksonModule}s found by * {@link MapperBuilder#findModules(ClassLoader)}. + * @see #setTrustedPackages(String...) */ public JacksonJsonMessageConverter() { this(JsonMapper.builder()); @@ -89,6 +95,8 @@ public class JacksonJsonMessageConverter implements SmartMessageConverter, BeanC * Construct a new instance with the provided {@link JsonMapper.Builder} * customized with the {@link tools.jackson.databind.JacksonModule}s found * by {@link MapperBuilder#findModules(ClassLoader)}. + * @param builder the mapper builder to use + * @see #setTrustedPackages(String...) * @see JsonMapper#builder() */ public JacksonJsonMessageConverter(JsonMapper.Builder builder) { @@ -98,6 +106,8 @@ public class JacksonJsonMessageConverter implements SmartMessageConverter, BeanC /** * Construct a new instance with the provided {@link JsonMapper}. + * @param mapper the mapper to use + * @see #setTrustedPackages(String...) * @see JsonMapper#builder() */ public JacksonJsonMessageConverter(JsonMapper mapper) { @@ -105,6 +115,15 @@ public class JacksonJsonMessageConverter implements SmartMessageConverter, BeanC this.mapper = mapper; } + /** + * Specify the trusted Java packages for deserialization. + * @param trustedPackages the trusted Java packages for deserialization + * @since 7.0.8 + */ + public void setTrustedPackages(String... trustedPackages) { + this.trustedPackages = trustedPackages.clone(); + } + /** * Specify whether {@link #toMessage(Object, Session)} should marshal to a * {@link BytesMessage} or a {@link TextMessage}. @@ -168,6 +187,23 @@ public class JacksonJsonMessageConverter implements SmartMessageConverter, BeanC }); } + private boolean isTrustedPackage(String requestedType) { + if (this.trustedPackages != null) { + String packageName = ClassUtils.getPackageName(requestedType); + int lastBracketIndex = packageName.lastIndexOf('['); + if (lastBracketIndex != -1 && packageName.length() > lastBracketIndex + 1 && packageName.charAt(lastBracketIndex + 1) == 'L') { + packageName = packageName.substring(lastBracketIndex + 2); + } + for (String trustedPackage : this.trustedPackages) { + if (packageName.equals(trustedPackage)) { + return true; + } + } + return false; + } + return true; + } + @Override public void setBeanClassLoader(ClassLoader classLoader) { this.beanClassLoader = classLoader; @@ -445,6 +481,10 @@ public class JacksonJsonMessageConverter implements SmartMessageConverter, BeanC if (mappedClass != null) { return this.mapper.constructType(mappedClass); } + if (!isTrustedPackage(typeId)) { + throw new MessageConversionException("The class '" + typeId + "' is not in the trusted packages: " + + Arrays.toString(this.trustedPackages)); + } try { Class typeClass = ClassUtils.forName(typeId, this.beanClassLoader); return this.mapper.constructType(typeClass); diff --git a/spring-jms/src/main/java/org/springframework/jms/support/converter/MappingJackson2MessageConverter.java b/spring-jms/src/main/java/org/springframework/jms/support/converter/MappingJackson2MessageConverter.java index c74ef2a81ab..e842f023496 100644 --- a/spring-jms/src/main/java/org/springframework/jms/support/converter/MappingJackson2MessageConverter.java +++ b/spring-jms/src/main/java/org/springframework/jms/support/converter/MappingJackson2MessageConverter.java @@ -21,6 +21,7 @@ import java.io.IOException; import java.io.OutputStreamWriter; import java.io.StringWriter; import java.io.UnsupportedEncodingException; +import java.util.Arrays; import java.util.HashMap; import java.util.Map; @@ -49,6 +50,8 @@ import org.springframework.util.ClassUtils; * {@link #setTargetType targetType} is set to {@link MessageType#TEXT}. * Converts from a {@link TextMessage} or {@link BytesMessage} to an object. * + *

For untrusted environments, use {@link #setTrustedPackages(String...)}. + * *

It customizes Jackson's default properties with the following ones: *

    *
  • {@link MapperFeature#DEFAULT_VIEW_INCLUSION} is disabled
  • @@ -59,6 +62,7 @@ import org.springframework.util.ClassUtils; * @author Dave Syer * @author Juergen Hoeller * @author Stephane Nicoll + * @author Sebastien Deleuze * @since 3.1.4 * @deprecated since 7.0 in favor of {@link JacksonJsonMessageConverter} */ @@ -85,11 +89,14 @@ public class MappingJackson2MessageConverter implements SmartMessageConverter, B private final Map, String> classIdMappings = new HashMap<>(); + private String @Nullable [] trustedPackages; + private @Nullable ClassLoader beanClassLoader; /** * Construct a {@code MappingJackson2MessageConverter} with a default {@link ObjectMapper}. + * @see #setTrustedPackages(String...) */ @SuppressWarnings("deprecation") // on Jackson 2.13: configure(MapperFeature, boolean) public MappingJackson2MessageConverter() { @@ -102,12 +109,21 @@ public class MappingJackson2MessageConverter implements SmartMessageConverter, B * Construct a {@code MappingJackson2MessageConverter} with a custom {@link ObjectMapper}. * @param objectMapper the {@code ObjectMapper} to use * @since 6.1 + * @see #setTrustedPackages(String...) */ public MappingJackson2MessageConverter(ObjectMapper objectMapper) { Assert.notNull(objectMapper, "ObjectMapper must not be null"); this.objectMapper = objectMapper; } + /** + * Specify the trusted Java packages for deserialization. + * @param trustedPackages the trusted Java packages for deserialization + * @since 6.2.19 + */ + public void setTrustedPackages(String... trustedPackages) { + this.trustedPackages = trustedPackages.clone(); + } /** * Set the {@code ObjectMapper} for this converter. @@ -181,6 +197,23 @@ public class MappingJackson2MessageConverter implements SmartMessageConverter, B }); } + private boolean isTrustedPackage(String requestedType) { + if (this.trustedPackages != null) { + String packageName = ClassUtils.getPackageName(requestedType); + int lastBracketIndex = packageName.lastIndexOf('['); + if (lastBracketIndex != -1 && packageName.length() > lastBracketIndex + 1 && packageName.charAt(lastBracketIndex + 1) == 'L') { + packageName = packageName.substring(lastBracketIndex + 2); + } + for (String trustedPackage : this.trustedPackages) { + if (packageName.equals(trustedPackage)) { + return true; + } + } + return false; + } + return true; + } + @Override public void setBeanClassLoader(ClassLoader classLoader) { this.beanClassLoader = classLoader; @@ -461,6 +494,10 @@ public class MappingJackson2MessageConverter implements SmartMessageConverter, B if (mappedClass != null) { return this.objectMapper.constructType(mappedClass); } + if (!isTrustedPackage(typeId)) { + throw new MessageConversionException("The class '" + typeId + "' is not in the trusted packages: " + + Arrays.toString(this.trustedPackages)); + } try { Class typeClass = ClassUtils.forName(typeId, this.beanClassLoader); return this.objectMapper.constructType(typeClass); diff --git a/spring-jms/src/test/java/org/springframework/jms/support/converter/JacksonJsonMessageConverterTests.java b/spring-jms/src/test/java/org/springframework/jms/support/converter/JacksonJsonMessageConverterTests.java index b01f421512e..d67b68003bd 100644 --- a/spring-jms/src/test/java/org/springframework/jms/support/converter/JacksonJsonMessageConverterTests.java +++ b/spring-jms/src/test/java/org/springframework/jms/support/converter/JacksonJsonMessageConverterTests.java @@ -37,6 +37,7 @@ import org.mockito.stubbing.Answer; import org.springframework.core.MethodParameter; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.isA; @@ -131,6 +132,86 @@ class JacksonJsonMessageConverterTests { assertThat(unmarshalled).as("Invalid result").isEqualTo(result); } + @Test + void fromTextMessageUntrusted() throws Exception { + converter = new JacksonJsonMessageConverter(); + converter.setTrustedPackages("java.lang"); + converter.setTypeIdPropertyName("__typeid__"); + TextMessage textMessageMock = mock(); + + String text = "{\"foo\":\"bar\"}"; + given(textMessageMock.getStringProperty("__typeid__")).willReturn(MyBean.class.getName()); + given(textMessageMock.getText()).willReturn(text); + + assertThatExceptionOfType(MessageConversionException.class) + .isThrownBy(() -> converter.fromMessage(textMessageMock)) + .withMessageContaining("is not in the trusted packages"); + } + + @Test + void fromTextMessageTrusted() throws Exception { + converter = new JacksonJsonMessageConverter(); + converter.setTrustedPackages("java.lang", "org.springframework.jms.support.converter"); + converter.setTypeIdPropertyName("__typeid__"); + TextMessage textMessageMock = mock(); + MyBean unmarshalled = new MyBean("bar"); + + String text = "{\"foo\":\"bar\"}"; + given(textMessageMock.getStringProperty("__typeid__")).willReturn(MyBean.class.getName()); + given(textMessageMock.getText()).willReturn(text); + + MyBean result = (MyBean) converter.fromMessage(textMessageMock); + assertThat(unmarshalled).as("Invalid result").isEqualTo(result); + } + + @Test + void fromTextMessageTrustedEmpty() throws Exception { + converter = new JacksonJsonMessageConverter(); + converter.setTrustedPackages(); + converter.setTypeIdPropertyName("__typeid__"); + TextMessage textMessageMock = mock(); + + String text = "{\"foo\":\"bar\"}"; + given(textMessageMock.getStringProperty("__typeid__")).willReturn(MyBean.class.getName()); + given(textMessageMock.getText()).willReturn(text); + + assertThatExceptionOfType(MessageConversionException.class) + .isThrownBy(() -> converter.fromMessage(textMessageMock)) + .withMessageContaining("is not in the trusted packages"); + } + + @Test + void fromTextMessageTrusted1DArray() throws Exception { + converter = new JacksonJsonMessageConverter(); + converter.setTrustedPackages("org.springframework.jms.support.converter"); + converter.setTypeIdPropertyName("__typeid__"); + TextMessage textMessageMock = mock(); + MyBean[] unmarshalled = new MyBean[] { new MyBean("bar") }; + + String text = "[{\"foo\":\"bar\"}]"; + given(textMessageMock.getStringProperty("__typeid__")).willReturn("[L" + MyBean.class.getName() + ";"); + given(textMessageMock.getText()).willReturn(text); + + MyBean[] result = (MyBean[]) converter.fromMessage(textMessageMock); + assertThat(unmarshalled).as("Invalid result").isEqualTo(result); + } + + @Test + void fromTextMessageTrusted2DArray() throws Exception { + converter = new JacksonJsonMessageConverter(); + converter.setTrustedPackages("org.springframework.jms.support.converter"); + converter.setTypeIdPropertyName("__typeid__"); + TextMessage textMessageMock = mock(); + MyBean[][] unmarshalled = new MyBean[][] { { new MyBean("bar") } }; + + String text = "[[{\"foo\":\"bar\"}]]"; + given(textMessageMock.getStringProperty("__typeid__")).willReturn("[[L" + MyBean.class.getName() + ";"); + given(textMessageMock.getText()).willReturn(text); + + MyBean[][] result = (MyBean[][]) converter.fromMessage(textMessageMock); + assertThat(unmarshalled).as("Invalid result").isEqualTo(result); + } + @Test void fromTextMessageWithUnknownProperty() throws Exception { TextMessage textMessageMock = mock(); diff --git a/spring-jms/src/test/java/org/springframework/jms/support/converter/MappingJackson2MessageConverterTests.java b/spring-jms/src/test/java/org/springframework/jms/support/converter/MappingJackson2MessageConverterTests.java index f93f292f1aa..52bd6274538 100644 --- a/spring-jms/src/test/java/org/springframework/jms/support/converter/MappingJackson2MessageConverterTests.java +++ b/spring-jms/src/test/java/org/springframework/jms/support/converter/MappingJackson2MessageConverterTests.java @@ -37,6 +37,7 @@ import org.mockito.stubbing.Answer; import org.springframework.core.MethodParameter; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.isA; @@ -48,6 +49,7 @@ import static org.mockito.Mockito.verify; * @author Arjen Poutsma * @author Dave Syer * @author Stephane Nicoll + * @author Sebastien Deleuze */ @SuppressWarnings("removal") class MappingJackson2MessageConverterTests { @@ -134,6 +136,86 @@ class MappingJackson2MessageConverterTests { assertThat(unmarshalled).as("Invalid result").isEqualTo(result); } + @Test + void fromTextMessageUntrusted() throws Exception { + converter = new MappingJackson2MessageConverter(); + converter.setTrustedPackages("java.lang"); + converter.setTypeIdPropertyName("__typeid__"); + TextMessage textMessageMock = mock(); + + String text = "{\"foo\":\"bar\"}"; + given(textMessageMock.getStringProperty("__typeid__")).willReturn(MyBean.class.getName()); + given(textMessageMock.getText()).willReturn(text); + + assertThatExceptionOfType(MessageConversionException.class) + .isThrownBy(() -> converter.fromMessage(textMessageMock)) + .withMessageContaining("is not in the trusted packages"); + } + + @Test + void fromTextMessageTrusted() throws Exception { + converter = new MappingJackson2MessageConverter(); + converter.setTrustedPackages("java.lang", "org.springframework.jms.support.converter"); + converter.setTypeIdPropertyName("__typeid__"); + TextMessage textMessageMock = mock(); + MyBean unmarshalled = new MyBean("bar"); + + String text = "{\"foo\":\"bar\"}"; + given(textMessageMock.getStringProperty("__typeid__")).willReturn(MyBean.class.getName()); + given(textMessageMock.getText()).willReturn(text); + + MyBean result = (MyBean) converter.fromMessage(textMessageMock); + assertThat(unmarshalled).as("Invalid result").isEqualTo(result); + } + + @Test + void fromTextMessageTrustedEmpty() throws Exception { + converter = new MappingJackson2MessageConverter(); + converter.setTrustedPackages(); + converter.setTypeIdPropertyName("__typeid__"); + TextMessage textMessageMock = mock(); + + String text = "{\"foo\":\"bar\"}"; + given(textMessageMock.getStringProperty("__typeid__")).willReturn(MyBean.class.getName()); + given(textMessageMock.getText()).willReturn(text); + + assertThatExceptionOfType(MessageConversionException.class) + .isThrownBy(() -> converter.fromMessage(textMessageMock)) + .withMessageContaining("is not in the trusted packages"); + } + + @Test + void fromTextMessageTrusted1DArray() throws Exception { + converter = new MappingJackson2MessageConverter(); + converter.setTrustedPackages("org.springframework.jms.support.converter"); + converter.setTypeIdPropertyName("__typeid__"); + TextMessage textMessageMock = mock(); + MyBean[] unmarshalled = new MyBean[] { new MyBean("bar") }; + + String text = "[{\"foo\":\"bar\"}]"; + given(textMessageMock.getStringProperty("__typeid__")).willReturn("[L" + MyBean.class.getName() + ";"); + given(textMessageMock.getText()).willReturn(text); + + MyBean[] result = (MyBean[]) converter.fromMessage(textMessageMock); + assertThat(unmarshalled).as("Invalid result").isEqualTo(result); + } + + @Test + void fromTextMessageTrusted2DArray() throws Exception { + converter = new MappingJackson2MessageConverter(); + converter.setTrustedPackages("org.springframework.jms.support.converter"); + converter.setTypeIdPropertyName("__typeid__"); + TextMessage textMessageMock = mock(); + MyBean[][] unmarshalled = new MyBean[][] { { new MyBean("bar") } }; + + String text = "[[{\"foo\":\"bar\"}]]"; + given(textMessageMock.getStringProperty("__typeid__")).willReturn("[[L" + MyBean.class.getName() + ";"); + given(textMessageMock.getText()).willReturn(text); + + MyBean[][] result = (MyBean[][]) converter.fromMessage(textMessageMock); + assertThat(unmarshalled).as("Invalid result").isEqualTo(result); + } + @Test void fromTextMessageWithUnknownProperty() throws Exception { TextMessage textMessageMock = mock(); diff --git a/spring-web/src/main/java/org/springframework/web/util/JavaScriptUtils.java b/spring-web/src/main/java/org/springframework/web/util/JavaScriptUtils.java index 6ad75260336..0a01be51384 100644 --- a/spring-web/src/main/java/org/springframework/web/util/JavaScriptUtils.java +++ b/spring-web/src/main/java/org/springframework/web/util/JavaScriptUtils.java @@ -27,6 +27,7 @@ package org.springframework.web.util; * @author Juergen Hoeller * @author Rob Harrop * @author Rossen Stoyanchev + * @author Sebastien Deleuze * @since 1.1.1 */ public abstract class JavaScriptUtils { @@ -89,6 +90,12 @@ public abstract class JavaScriptUtils { else if (c == '\u2029') { filtered.append("\\u2029"); } + else if (c == '`') { + filtered.append("\\u0060"); + } + else if (c == '$') { + filtered.append("\\u0024"); + } else { filtered.append(c); } diff --git a/spring-web/src/main/java/org/springframework/web/util/RfcUriParser.java b/spring-web/src/main/java/org/springframework/web/util/RfcUriParser.java index c87c2a7b123..3198e93590a 100644 --- a/spring-web/src/main/java/org/springframework/web/util/RfcUriParser.java +++ b/spring-web/src/main/java/org/springframework/web/util/RfcUriParser.java @@ -241,12 +241,16 @@ abstract class RfcUriParser { parser.index(++i); parser.captureHost(); if (parser.hasNext()) { - if (parser.charAtIndex() == ':') { + char next = parser.charAtIndex(); + if (next == ':') { parser.advanceTo(PORT, i + 1); } - else { + else if (next == '/') { parser.advanceTo(PATH, i); } + else { + fail(parser, "Bad authority"); + } } break; case ':': diff --git a/spring-web/src/test/java/org/springframework/web/util/JavaScriptUtilsTests.java b/spring-web/src/test/java/org/springframework/web/util/JavaScriptUtilsTests.java index b91f1b825b3..b2c49163491 100644 --- a/spring-web/src/test/java/org/springframework/web/util/JavaScriptUtilsTests.java +++ b/spring-web/src/test/java/org/springframework/web/util/JavaScriptUtilsTests.java @@ -62,4 +62,14 @@ class JavaScriptUtilsTests { assertThat(JavaScriptUtils.javaScriptEscape("<>")).isEqualTo("\\u003C\\u003E"); } + @Test + void escapeBacktick() { + assertThat(JavaScriptUtils.javaScriptEscape("`")).isEqualTo("\\u0060"); + } + + @Test + void escapeDollar() { + assertThat(JavaScriptUtils.javaScriptEscape("$")).isEqualTo("\\u0024"); + } + } diff --git a/spring-web/src/test/java/org/springframework/web/util/UriComponentsBuilderTests.java b/spring-web/src/test/java/org/springframework/web/util/UriComponentsBuilderTests.java index e8729d97d9b..fc0a40a54c3 100644 --- a/spring-web/src/test/java/org/springframework/web/util/UriComponentsBuilderTests.java +++ b/spring-web/src/test/java/org/springframework/web/util/UriComponentsBuilderTests.java @@ -267,7 +267,10 @@ class UriComponentsBuilderTests { @EnumSource void fromUriStringInvalidIPv6Host(ParserType parserType) { assertThatIllegalArgumentException().isThrownBy(() -> - UriComponentsBuilder.fromUriString("http://[1abc:2abc:3abc::5ABC:6abc:8080/resource", parserType)); + UriComponentsBuilder.fromUriString("https://[1abc:2abc:3abc::5ABC:6abc:8080/resource", parserType)); + + assertThatIllegalArgumentException().isThrownBy(() -> + UriComponentsBuilder.fromUriString("https://[1abc:2abc:3abc::5ABC:6abc]resource", parserType)); } @ParameterizedTest // gh-36759 diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandler.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandler.java index d4fc2e86c57..5147d2c59c6 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandler.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandler.java @@ -44,6 +44,7 @@ import org.springframework.core.io.buffer.DataBuffer; import org.springframework.core.io.buffer.DataBufferFactory; import org.springframework.core.io.buffer.DataBufferUtils; import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpStatus; import org.springframework.http.HttpStatusCode; import org.springframework.http.MediaType; import org.springframework.http.codec.ServerSentEvent; @@ -61,6 +62,7 @@ import org.springframework.web.reactive.HandlerResultHandler; import org.springframework.web.reactive.accept.RequestedContentTypeResolver; import org.springframework.web.reactive.result.HandlerResultHandlerSupport; import org.springframework.web.server.NotAcceptableStatusException; +import org.springframework.web.server.ResponseStatusException; import org.springframework.web.server.ServerWebExchange; /** @@ -89,6 +91,7 @@ import org.springframework.web.server.ServerWebExchange; * presence of annotations, for example, for {@code @ResponseBody}. * * @author Rossen Stoyanchev + * @author Sebastien Deleuze * @since 5.0 */ public class ViewResolutionResultHandler extends HandlerResultHandlerSupport implements HandlerResultHandler, Ordered { @@ -256,65 +259,70 @@ public class ViewResolutionResultHandler extends HandlerResultHandlerSupport imp clazz = FragmentsRendering.class; } - if (returnValue == NO_VALUE || ClassUtils.isVoidType(clazz)) { - viewsMono = resolveViews(getDefaultViewName(exchange), locale); - } - else if (CharSequence.class.isAssignableFrom(clazz) && !hasModelAnnotation(parameter)) { - viewsMono = resolveViews(returnValue.toString(), locale); - } - else if (Rendering.class.isAssignableFrom(clazz)) { - Rendering render = (Rendering) returnValue; - HttpStatusCode status = render.status(); - if (status != null) { - exchange.getResponse().setStatusCode(status); + try { + if (returnValue == NO_VALUE || ClassUtils.isVoidType(clazz)) { + viewsMono = resolveViews(getDefaultViewName(exchange), locale); } - exchange.getResponse().getHeaders().putAll(render.headers()); - model.addAllAttributes(render.modelAttributes()); - Object view = render.view(); - if (view == null) { - view = getDefaultViewName(exchange); + else if (CharSequence.class.isAssignableFrom(clazz) && !hasModelAnnotation(parameter)) { + viewsMono = resolveViews(returnValue.toString(), locale); } - viewsMono = (view instanceof String viewName ? resolveViews(viewName, locale) : - Mono.just(Collections.singletonList((View) view))); - } - else if (FragmentsRendering.class.isAssignableFrom(clazz)) { - ServerHttpResponse response = exchange.getResponse(); - FragmentsRendering render = (FragmentsRendering) returnValue; - HttpStatusCode status = render.status(); - if (status != null) { - response.setStatusCode(status); + else if (Rendering.class.isAssignableFrom(clazz)) { + Rendering render = (Rendering) returnValue; + HttpStatusCode status = render.status(); + if (status != null) { + exchange.getResponse().setStatusCode(status); + } + exchange.getResponse().getHeaders().putAll(render.headers()); + model.addAllAttributes(render.modelAttributes()); + Object view = render.view(); + if (view == null) { + view = getDefaultViewName(exchange); + } + viewsMono = (view instanceof String viewName ? resolveViews(viewName, locale) : + Mono.just(Collections.singletonList((View) view))); } - response.getHeaders().putAll(render.headers()); - bindingContext.updateModel(exchange); + else if (FragmentsRendering.class.isAssignableFrom(clazz)) { + ServerHttpResponse response = exchange.getResponse(); + FragmentsRendering render = (FragmentsRendering) returnValue; + HttpStatusCode status = render.status(); + if (status != null) { + response.setStatusCode(status); + } + response.getHeaders().putAll(render.headers()); + bindingContext.updateModel(exchange); - StreamHandler streamHandler = - (this.sseHandler.supports(exchange.getRequest()) ? this.sseHandler : null); + StreamHandler streamHandler = + (this.sseHandler.supports(exchange.getRequest()) ? this.sseHandler : null); - if (streamHandler != null) { - streamHandler.updateResponse(exchange); + if (streamHandler != null) { + streamHandler.updateResponse(exchange); + } + + Flux> renderFlux = render.fragments() + .concatMap(fragment -> renderFragment(fragment, null, streamHandler, locale, bindingContext, exchange)) + .doOnDiscard(DataBuffer.class, DataBufferUtils::release); + + return response.writeAndFlushWith(renderFlux); + } + else if (Model.class.isAssignableFrom(clazz)) { + model.addAllAttributes(((Model) returnValue).asMap()); + viewsMono = resolveViews(getDefaultViewName(exchange), locale); + } + else if (Map.class.isAssignableFrom(clazz) && !hasModelAnnotation(parameter)) { + model.addAllAttributes((Map) returnValue); + viewsMono = resolveViews(getDefaultViewName(exchange), locale); + } + else if (View.class.isAssignableFrom(clazz)) { + viewsMono = Mono.just(Collections.singletonList((View) returnValue)); + } + else { + String name = getNameForReturnValue(parameter); + model.addAttribute(name, returnValue); + viewsMono = resolveViews(getDefaultViewName(exchange), locale); } - - Flux> renderFlux = render.fragments() - .concatMap(fragment -> renderFragment(fragment, null, streamHandler, locale, bindingContext, exchange)) - .doOnDiscard(DataBuffer.class, DataBufferUtils::release); - - return response.writeAndFlushWith(renderFlux); } - else if (Model.class.isAssignableFrom(clazz)) { - model.addAllAttributes(((Model) returnValue).asMap()); - viewsMono = resolveViews(getDefaultViewName(exchange), locale); - } - else if (Map.class.isAssignableFrom(clazz) && !hasModelAnnotation(parameter)) { - model.addAllAttributes((Map) returnValue); - viewsMono = resolveViews(getDefaultViewName(exchange), locale); - } - else if (View.class.isAssignableFrom(clazz)) { - viewsMono = Mono.just(Collections.singletonList((View) returnValue)); - } - else { - String name = getNameForReturnValue(parameter); - model.addAttribute(name, returnValue); - viewsMono = resolveViews(getDefaultViewName(exchange), locale); + catch (ResponseStatusException ex) { + return Mono.error(ex); } bindingContext.updateModel(exchange); return viewsMono.flatMap(views -> render(views, model.asMap(), null, bindingContext, exchange)); @@ -324,12 +332,16 @@ public class ViewResolutionResultHandler extends HandlerResultHandlerSupport imp /** * Select a default view name when a controller did not specify it. * Use the request path the leading and trailing slash stripped. + * @throws ResponseStatusException with a 400 error code if the path contains a "redirect:" prefix */ private String getDefaultViewName(ServerWebExchange exchange) { String path = exchange.getRequest().getPath().pathWithinApplication().value(); if (path.startsWith("/")) { path = path.substring(1); } + if (path.startsWith(UrlBasedViewResolver.REDIRECT_URL_PREFIX)) { + throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "Rejected path '" + path + "' with 'redirect:' prefix"); + } if (path.endsWith("/")) { path = path.substring(0, path.length() - 1); } diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandlerTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandlerTests.java index 97199c88451..6e88b61ab96 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandlerTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandlerTests.java @@ -52,6 +52,7 @@ import org.springframework.web.reactive.HandlerResult; import org.springframework.web.reactive.accept.HeaderContentTypeResolver; import org.springframework.web.reactive.accept.RequestedContentTypeResolver; import org.springframework.web.server.NotAcceptableStatusException; +import org.springframework.web.server.ResponseStatusException; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.testfixture.http.server.reactive.MockServerHttpResponse; import org.springframework.web.testfixture.server.MockServerWebExchange; @@ -256,6 +257,20 @@ class ViewResolutionResultHandlerTests { assertResponseBody(exchange, "account: {id=123}"); } + @Test + void defaultViewNameWithRedirectPrefixFails() { + MethodParameter returnType = on(Handler.class).resolveReturnType(Mono.class, String.class); + HandlerResult result = new HandlerResult(new Object(), Mono.empty(), returnType, this.bindingContext); + ViewResolutionResultHandler handler = resultHandler(new TestViewResolver("account")); + + MockServerWebExchange exchange = MockServerWebExchange.from(get("/redirect:account")); + Mono mono = handler.handleResult(exchange, result); + StepVerifier.create(mono) + .expectNextCount(0) + .expectError(ResponseStatusException.class) + .verify(); + } + @Test void unresolvedViewName() { String returnValue = "account"; diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/tags/form/AbstractHtmlElementTag.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/tags/form/AbstractHtmlElementTag.java index f40ec6cd327..d0771ae9a66 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/tags/form/AbstractHtmlElementTag.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/tags/form/AbstractHtmlElementTag.java @@ -24,7 +24,6 @@ import jakarta.servlet.jsp.tagext.DynamicAttributes; import org.jspecify.annotations.Nullable; import org.springframework.util.CollectionUtils; -import org.springframework.util.ObjectUtils; import org.springframework.util.StringUtils; /** @@ -40,6 +39,7 @@ import org.springframework.util.StringUtils; * @author Rob Harrop * @author Jeremy Grelle * @author Rossen Stoyanchev + * @author Sebastien Deleuze * @since 2.0 */ @SuppressWarnings("serial") @@ -429,8 +429,7 @@ public abstract class AbstractHtmlElementTag extends AbstractDataBoundFormElemen */ protected void writeOptionalAttributes(TagWriter tagWriter) throws JspException { tagWriter.writeOptionalAttributeValue(CLASS_ATTRIBUTE, resolveCssClass()); - tagWriter.writeOptionalAttributeValue(STYLE_ATTRIBUTE, - ObjectUtils.getDisplayString(evaluate("cssStyle", getCssStyle()))); + writeOptionalAttribute(tagWriter, STYLE_ATTRIBUTE, getCssStyle()); writeOptionalAttribute(tagWriter, LANG_ATTRIBUTE, getLang()); writeOptionalAttribute(tagWriter, TITLE_ATTRIBUTE, getTitle()); writeOptionalAttribute(tagWriter, DIR_ATTRIBUTE, getDir()); @@ -459,10 +458,10 @@ public abstract class AbstractHtmlElementTag extends AbstractDataBoundFormElemen */ protected String resolveCssClass() throws JspException { if (getBindStatus().isError() && StringUtils.hasText(getCssErrorClass())) { - return ObjectUtils.getDisplayString(evaluate("cssErrorClass", getCssErrorClass())); + return getDisplayString(evaluate("cssErrorClass", getCssErrorClass())); } else { - return ObjectUtils.getDisplayString(evaluate("cssClass", getCssClass())); + return getDisplayString(evaluate("cssClass", getCssClass())); } } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/tags/form/FormTag.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/tags/form/FormTag.java index 8129620af08..2ebd7ea7b49 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/tags/form/FormTag.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/tags/form/FormTag.java @@ -32,7 +32,6 @@ import org.springframework.core.Conventions; import org.springframework.http.HttpMethod; import org.springframework.util.Assert; import org.springframework.util.CollectionUtils; -import org.springframework.util.ObjectUtils; import org.springframework.util.StringUtils; import org.springframework.web.servlet.support.RequestDataValueProcessor; import org.springframework.web.util.HtmlUtils; @@ -239,6 +238,7 @@ import org.springframework.web.util.UriUtils; * @author Juergen Hoeller * @author Scott Andrews * @author Rossen Stoyanchev + * @author Sebastien Deleuze * @since 2.0 */ @SuppressWarnings("serial") @@ -719,7 +719,7 @@ public class FormTag extends AbstractHtmlElementTag { */ @Override protected String resolveCssClass() throws JspException { - return ObjectUtils.getDisplayString(evaluate("cssClass", getCssClass())); + return getDisplayString(evaluate("cssClass", getCssClass())); } /** diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/view/DefaultRequestToViewNameTranslator.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/view/DefaultRequestToViewNameTranslator.java index 7d7161ce4aa..756f9da5ad6 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/view/DefaultRequestToViewNameTranslator.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/view/DefaultRequestToViewNameTranslator.java @@ -20,7 +20,9 @@ import jakarta.servlet.ServletRequest; import jakarta.servlet.http.HttpServletRequest; import org.jspecify.annotations.Nullable; +import org.springframework.http.HttpStatus; import org.springframework.util.StringUtils; +import org.springframework.web.server.ResponseStatusException; import org.springframework.web.servlet.RequestToViewNameTranslator; import org.springframework.web.util.ServletRequestPathUtils; @@ -50,6 +52,7 @@ import org.springframework.web.util.ServletRequestPathUtils; * * @author Rob Harrop * @author Juergen Hoeller + * @author Sebastien Deleuze * @since 2.0 * @see org.springframework.web.servlet.RequestToViewNameTranslator * @see org.springframework.web.servlet.ViewResolver @@ -127,13 +130,21 @@ public class DefaultRequestToViewNameTranslator implements RequestToViewNameTran * into the view name based on the configured parameters. * @throws IllegalArgumentException if neither a parsed RequestPath, nor a * String lookupPath have been resolved and cached as a request attribute. + * @throws ResponseStatusException with a 400 error code if the path contains a "redirect:" or a "forward:" prefix * @see ServletRequestPathUtils#getCachedPath(ServletRequest) * @see #transformPath */ @Override public String getViewName(HttpServletRequest request) { String path = ServletRequestPathUtils.getCachedPathValue(request); - return (this.prefix + transformPath(path) + this.suffix); + String viewName = this.prefix + transformPath(path) + this.suffix; + if (viewName.startsWith(UrlBasedViewResolver.REDIRECT_URL_PREFIX)) { + throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "Rejected path '" + path + "' with 'redirect:' prefix"); + } + if (viewName.startsWith(UrlBasedViewResolver.FORWARD_URL_PREFIX)) { + throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "Rejected path '" + path + "' with 'forward:' prefix"); + } + return viewName; } /** @@ -144,7 +155,7 @@ public class DefaultRequestToViewNameTranslator implements RequestToViewNameTran * @return the transformed path, with slashes and extensions stripped * if desired */ - protected @Nullable String transformPath(String lookupPath) { + protected String transformPath(String lookupPath) { String path = lookupPath; if (this.stripLeadingSlash && path.startsWith(SLASH)) { path = path.substring(1); diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/tags/form/FormTagTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/tags/form/FormTagTests.java index 48db96648e4..93885e2a834 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/tags/form/FormTagTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/tags/form/FormTagTests.java @@ -129,6 +129,20 @@ class FormTagTests extends AbstractHtmlElementTagTests { assertContainsAttribute(output, dynamicAttribute2, dynamicAttribute2); } + @Test + void writeFormWithHtmlEscaping() throws Exception { + this.tag.setCssClass("\"class\""); + this.tag.setCssStyle("\"style\""); + + this.tag.doStartTag(); + this.tag.doEndTag(); + this.tag.doFinally(); + + String output = getOutput(); + assertContainsAttribute(output, "class", ""class""); + assertContainsAttribute(output, "style", ""style""); + } + @Test void withActionFromRequest() throws Exception { String commandName = "myCommand"; diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/tags/form/InputTagTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/tags/form/InputTagTests.java index a8e35ccf9ae..effa54aeb4e 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/tags/form/InputTagTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/tags/form/InputTagTests.java @@ -314,6 +314,28 @@ class InputTagTests extends AbstractFormTagTests { assertContainsAttribute(output, "class", "bad"); } + @Test + void withErrorsAndHtmlEscaping() throws Exception { + this.tag.setPath("name"); + this.tag.setCssClass("\"good\""); + this.tag.setCssErrorClass("\"bad\""); + + BeanPropertyBindingResult errors = new BeanPropertyBindingResult(this.rob, COMMAND_NAME); + errors.rejectValue("name", "some.code", "Default Message"); + errors.rejectValue("name", "too.short", "Too Short"); + exposeBindingResult(errors); + + assertThat(this.tag.doStartTag()).isEqualTo(Tag.SKIP_BODY); + + String output = getOutput(); + assertTagOpened(output); + assertTagClosed(output); + + assertContainsAttribute(output, "type", getType()); + assertValueAttribute(output, "Rob"); + assertContainsAttribute(output, "class", ""bad""); + } + @Test void disabledFalse() throws Exception { this.tag.setPath("name"); diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/view/DefaultRequestToViewNameTranslatorTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/view/DefaultRequestToViewNameTranslatorTests.java index ec1d0fe4b48..a2e3c6e356d 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/view/DefaultRequestToViewNameTranslatorTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/view/DefaultRequestToViewNameTranslatorTests.java @@ -21,15 +21,19 @@ import java.util.stream.Stream; import org.junit.jupiter.api.Named; +import org.springframework.http.HttpStatus; +import org.springframework.web.server.ResponseStatusException; import org.springframework.web.servlet.handler.PathPatternsParameterizedTest; import org.springframework.web.servlet.handler.PathPatternsTestUtils; import org.springframework.web.testfixture.servlet.MockHttpServletRequest; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; /** * @author Rick Evans * @author Juergen Hoeller + * @author Sebastien Deleuze */ class DefaultRequestToViewNameTranslatorTests { @@ -121,6 +125,22 @@ class DefaultRequestToViewNameTranslatorTests { assertViewName(request, VIEW_NAME); } + @PathPatternsParameterizedTest + void getViewNameWithRedirectPrefixFails(Function requestFactory) { + MockHttpServletRequest request = requestFactory.apply(UrlBasedViewResolver.REDIRECT_URL_PREFIX + VIEW_NAME); + assertThatExceptionOfType(ResponseStatusException.class) + .isThrownBy(() -> this.translator.getViewName(request)) + .satisfies(ex -> assertThat(ex.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST)); + } + + @PathPatternsParameterizedTest + void getViewNameWithForwardPrefixFails(Function requestFactory) { + MockHttpServletRequest request = requestFactory.apply(UrlBasedViewResolver.FORWARD_URL_PREFIX + VIEW_NAME); + assertThatExceptionOfType(ResponseStatusException.class) + .isThrownBy(() -> this.translator.getViewName(request)) + .satisfies(ex -> assertThat(ex.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST)); + } + private void assertViewName(MockHttpServletRequest request, String expectedViewName) { String actualViewName = this.translator.getViewName(request);