From e1ab306506d4e350b119331387f504b726fbc776 Mon Sep 17 00:00:00 2001
From: Sam Brannen <104798+sbrannen@users.noreply.github.com>
Date: Tue, 6 Aug 2024 11:33:03 +0300
Subject: [PATCH] Enforce read-only semantics in SpEL's SimpleEvaluationContext
SimpleEvaluationContext.forReadOnlyDataBinding() documents that it
creates a SimpleEvaluationContext for read-only access to public
properties; however, prior to this commit write access was not disabled
for indexed structures when using the assignment operator, the
increment operator, or the decrement operator.
In order to better align with the documented contract for
forReadOnlyDataBinding(), this commit makes it possible to disable
assignment in general in order to enforce read-only semantics for
SpEL's SimpleEvaluationContext when created via the
forReadOnlyDataBinding() factory method. Specifically:
- This commit introduces a new isAssignmentEnabled() "default" method
in the EvaluationContext API, which returns true by default.
- SimpleEvaluationContext overrides isAssignmentEnabled(), returning
false if the context was created via the forReadOnlyDataBinding()
factory method.
- The Assign, OpDec, and OpInc AST nodes -- representing the assignment
(=), increment (++), and decrement (--) operators, respectively --
now throw a SpelEvaluationException if assignment is disabled for the
current EvaluationContext.
See gh-33319
Closes gh-33321
(cherry picked from commit 0127de5a7a80319a9a7973ad125002309a591a77)
---
.../expression/EvaluationContext.java | 16 +-
.../expression/spel/ast/Assign.java | 7 +-
.../expression/spel/ast/OpDec.java | 6 +-
.../expression/spel/ast/OpInc.java | 8 +-
.../spel/support/SimpleEvaluationContext.java | 77 ++-
.../spel/CompilableMapAccessor.java | 117 +++++
.../expression/spel/PropertyAccessTests.java | 8 +-
.../spel/SpelCompilationCoverageTests.java | 78 ---
.../support/SimpleEvaluationContextTests.java | 477 ++++++++++++++++++
9 files changed, 685 insertions(+), 109 deletions(-)
create mode 100644 spring-expression/src/test/java/org/springframework/expression/spel/CompilableMapAccessor.java
create mode 100644 spring-expression/src/test/java/org/springframework/expression/spel/support/SimpleEvaluationContextTests.java
diff --git a/spring-expression/src/main/java/org/springframework/expression/EvaluationContext.java b/spring-expression/src/main/java/org/springframework/expression/EvaluationContext.java
index 0c393a86dbe..5bb6c46e0d6 100644
--- a/spring-expression/src/main/java/org/springframework/expression/EvaluationContext.java
+++ b/spring-expression/src/main/java/org/springframework/expression/EvaluationContext.java
@@ -1,5 +1,5 @@
/*
- * Copyright 2002-2023 the original author or authors.
+ * Copyright 2002-2024 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -132,4 +132,18 @@ public interface EvaluationContext {
@Nullable
Object lookupVariable(String name);
+ /**
+ * Determine if assignment is enabled within expressions evaluated by this evaluation
+ * context.
+ *
If this method returns {@code false}, the assignment ({@code =}), increment
+ * ({@code ++}), and decrement ({@code --}) operators are disabled.
+ *
By default, this method returns {@code true}. Concrete implementations may override
+ * this default method to disable assignment.
+ * @return {@code true} if assignment is enabled; {@code false} otherwise
+ * @since 5.3.38
+ */
+ default boolean isAssignmentEnabled() {
+ return true;
+ }
+
}
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 55e5d2e4ff0..1b47ead1607 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
@@ -1,5 +1,5 @@
/*
- * Copyright 2002-2023 the original author or authors.
+ * Copyright 2002-2024 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -19,6 +19,8 @@ package org.springframework.expression.spel.ast;
import org.springframework.expression.EvaluationException;
import org.springframework.expression.TypedValue;
import org.springframework.expression.spel.ExpressionState;
+import org.springframework.expression.spel.SpelEvaluationException;
+import org.springframework.expression.spel.SpelMessage;
/**
* Represents assignment. An alternative to calling {@code setValue}
@@ -39,6 +41,9 @@ public class Assign extends SpelNodeImpl {
@Override
public TypedValue getValueInternal(ExpressionState state) throws EvaluationException {
+ if (!state.getEvaluationContext().isAssignmentEnabled()) {
+ throw new SpelEvaluationException(getStartPosition(), SpelMessage.NOT_ASSIGNABLE, toStringAST());
+ }
return this.children[0].setValueInternal(state, () -> this.children[1].getValueInternal(state));
}
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 d61e8d64106..ce15fdc6072 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
@@ -1,5 +1,5 @@
/*
- * Copyright 2002-2021 the original author or authors.
+ * Copyright 2002-2024 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -51,6 +51,10 @@ public class OpDec extends Operator {
@Override
public TypedValue getValueInternal(ExpressionState state) throws EvaluationException {
+ if (!state.getEvaluationContext().isAssignmentEnabled()) {
+ throw new SpelEvaluationException(getStartPosition(), SpelMessage.OPERAND_NOT_DECREMENTABLE, toStringAST());
+ }
+
SpelNodeImpl operand = getLeftOperand();
// The operand is going to be read and then assigned to, we don't want to evaluate it twice.
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 f6dc184c0f5..59640a1858a 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
@@ -1,5 +1,5 @@
/*
- * Copyright 2002-2021 the original author or authors.
+ * Copyright 2002-2024 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -51,6 +51,10 @@ public class OpInc extends Operator {
@Override
public TypedValue getValueInternal(ExpressionState state) throws EvaluationException {
+ if (!state.getEvaluationContext().isAssignmentEnabled()) {
+ throw new SpelEvaluationException(getStartPosition(), SpelMessage.OPERAND_NOT_INCREMENTABLE, toStringAST());
+ }
+
SpelNodeImpl operand = getLeftOperand();
ValueRef valueRef = operand.getValueRef(state);
@@ -104,7 +108,7 @@ public class OpInc extends Operator {
}
}
- // set the name value
+ // set the new value
try {
valueRef.setValue(newValue.getValue());
}
diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/support/SimpleEvaluationContext.java b/spring-expression/src/main/java/org/springframework/expression/spel/support/SimpleEvaluationContext.java
index 1168c9c91a2..8e0f11be50f 100644
--- a/spring-expression/src/main/java/org/springframework/expression/spel/support/SimpleEvaluationContext.java
+++ b/spring-expression/src/main/java/org/springframework/expression/spel/support/SimpleEvaluationContext.java
@@ -1,5 +1,5 @@
/*
- * Copyright 2002-2023 the original author or authors.
+ * Copyright 2002-2024 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -51,25 +51,25 @@ import org.springframework.lang.Nullable;
* SpEL language syntax, e.g. excluding references to Java types, constructors,
* and bean references.
*
- *
When creating a {@code SimpleEvaluationContext} you need to choose the
- * level of support that you need for property access in SpEL expressions:
+ *
When creating a {@code SimpleEvaluationContext} you need to choose the level of
+ * support that you need for data binding in SpEL expressions:
*
- *
A custom {@code PropertyAccessor} (typically not reflection-based),
- * potentially combined with a {@link DataBindingPropertyAccessor}
- *
Data binding properties for read-only access
- *
Data binding properties for read and write
+ *
Data binding for read-only access
+ *
Data binding for read and write access
+ *
A custom {@code PropertyAccessor} (typically not reflection-based), potentially
+ * combined with a {@link DataBindingPropertyAccessor}
*
*
- *
Conveniently, {@link SimpleEvaluationContext#forReadOnlyDataBinding()}
- * enables read access to properties via {@link DataBindingPropertyAccessor};
- * same for {@link SimpleEvaluationContext#forReadWriteDataBinding()} when
- * write access is needed as well. Alternatively, configure custom accessors
- * via {@link SimpleEvaluationContext#forPropertyAccessors}, and potentially
- * activate method resolution and/or a type converter through the builder.
+ *
Conveniently, {@link SimpleEvaluationContext#forReadOnlyDataBinding()} enables
+ * read-only access to properties via {@link DataBindingPropertyAccessor}. Similarly,
+ * {@link SimpleEvaluationContext#forReadWriteDataBinding()} enables read and write access
+ * to properties. Alternatively, configure custom accessors via
+ * {@link SimpleEvaluationContext#forPropertyAccessors} and potentially activate method
+ * resolution and/or a type converter through the builder.
*
*
Note that {@code SimpleEvaluationContext} is typically not configured
* with a default root object. Instead it is meant to be created once and
- * used repeatedly through {@code getValue} calls on a pre-compiled
+ * used repeatedly through {@code getValue} calls on a predefined
* {@link org.springframework.expression.Expression} with both an
* {@code EvaluationContext} and a root object as arguments:
* {@link org.springframework.expression.Expression#getValue(EvaluationContext, Object)}.
@@ -81,9 +81,9 @@ import org.springframework.lang.Nullable;
* @author Juergen Hoeller
* @author Sam Brannen
* @since 4.3.15
- * @see #forPropertyAccessors
* @see #forReadOnlyDataBinding()
* @see #forReadWriteDataBinding()
+ * @see #forPropertyAccessors
* @see StandardEvaluationContext
* @see StandardTypeConverter
* @see DataBindingPropertyAccessor
@@ -109,14 +109,17 @@ public final class SimpleEvaluationContext implements EvaluationContext {
private final Map variables = new HashMap<>();
+ private final boolean assignmentEnabled;
+
private SimpleEvaluationContext(List accessors, List resolvers,
- @Nullable TypeConverter converter, @Nullable TypedValue rootObject) {
+ @Nullable TypeConverter converter, @Nullable TypedValue rootObject, boolean assignmentEnabled) {
this.propertyAccessors = accessors;
this.methodResolvers = resolvers;
this.typeConverter = (converter != null ? converter : new StandardTypeConverter());
this.rootObject = (rootObject != null ? rootObject : TypedValue.NULL);
+ this.assignmentEnabled = assignmentEnabled;
}
@@ -224,15 +227,33 @@ public final class SimpleEvaluationContext implements EvaluationContext {
return this.variables.get(name);
}
+ /**
+ * Determine if assignment is enabled within expressions evaluated by this evaluation
+ * context.
+ *
If this method returns {@code false}, the assignment ({@code =}), increment
+ * ({@code ++}), and decrement ({@code --}) operators are disabled.
+ * @return {@code true} if assignment is enabled; {@code false} otherwise
+ * @since 5.3.38
+ * @see #forPropertyAccessors(PropertyAccessor...)
+ * @see #forReadOnlyDataBinding()
+ * @see #forReadWriteDataBinding()
+ */
+ @Override
+ public boolean isAssignmentEnabled() {
+ return this.assignmentEnabled;
+ }
/**
* Create a {@code SimpleEvaluationContext} for the specified {@link PropertyAccessor}
* delegates: typically a custom {@code PropertyAccessor} specific to a use case
* (e.g. attribute resolution in a custom data structure), potentially combined with
* a {@link DataBindingPropertyAccessor} if property dereferences are needed as well.
+ *
Assignment is enabled within expressions evaluated by the context created via
+ * this factory method.
* @param accessors the accessor delegates to use
* @see DataBindingPropertyAccessor#forReadOnlyAccess()
* @see DataBindingPropertyAccessor#forReadWriteAccess()
+ * @see #isAssignmentEnabled()
*/
public static Builder forPropertyAccessors(PropertyAccessor... accessors) {
for (PropertyAccessor accessor : accessors) {
@@ -241,34 +262,40 @@ public final class SimpleEvaluationContext implements EvaluationContext {
"ReflectivePropertyAccessor. Consider using DataBindingPropertyAccessor or a custom subclass.");
}
}
- return new Builder(accessors);
+ return new Builder(true, accessors);
}
/**
* Create a {@code SimpleEvaluationContext} for read-only access to
* public properties via {@link DataBindingPropertyAccessor}.
+ *
Assignment is disabled within expressions evaluated by the context created via
+ * this factory method.
* @see DataBindingPropertyAccessor#forReadOnlyAccess()
* @see #forPropertyAccessors
+ * @see #isAssignmentEnabled()
*/
public static Builder forReadOnlyDataBinding() {
- return new Builder(DataBindingPropertyAccessor.forReadOnlyAccess());
+ return new Builder(false, DataBindingPropertyAccessor.forReadOnlyAccess());
}
/**
* Create a {@code SimpleEvaluationContext} for read-write access to
* public properties via {@link DataBindingPropertyAccessor}.
+ *
Assignment is enabled within expressions evaluated by the context created via
+ * this factory method.
* @see DataBindingPropertyAccessor#forReadWriteAccess()
* @see #forPropertyAccessors
+ * @see #isAssignmentEnabled()
*/
public static Builder forReadWriteDataBinding() {
- return new Builder(DataBindingPropertyAccessor.forReadWriteAccess());
+ return new Builder(true, DataBindingPropertyAccessor.forReadWriteAccess());
}
/**
* Builder for {@code SimpleEvaluationContext}.
*/
- public static class Builder {
+ public static final class Builder {
private final List accessors;
@@ -280,10 +307,15 @@ public final class SimpleEvaluationContext implements EvaluationContext {
@Nullable
private TypedValue rootObject;
- public Builder(PropertyAccessor... accessors) {
+ private final boolean assignmentEnabled;
+
+
+ private Builder(boolean assignmentEnabled, PropertyAccessor... accessors) {
+ this.assignmentEnabled = assignmentEnabled;
this.accessors = Arrays.asList(accessors);
}
+
/**
* Register the specified {@link MethodResolver} delegates for
* a combination of property access and method resolution.
@@ -362,7 +394,8 @@ public final class SimpleEvaluationContext implements EvaluationContext {
}
public SimpleEvaluationContext build() {
- return new SimpleEvaluationContext(this.accessors, this.resolvers, this.typeConverter, this.rootObject);
+ return new SimpleEvaluationContext(this.accessors, this.resolvers, this.typeConverter, this.rootObject,
+ this.assignmentEnabled);
}
}
diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/CompilableMapAccessor.java b/spring-expression/src/test/java/org/springframework/expression/spel/CompilableMapAccessor.java
new file mode 100644
index 00000000000..0d065f5bb29
--- /dev/null
+++ b/spring-expression/src/test/java/org/springframework/expression/spel/CompilableMapAccessor.java
@@ -0,0 +1,117 @@
+/*
+ * Copyright 2002-2024 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.expression.spel;
+
+import java.util.Map;
+
+import org.springframework.asm.MethodVisitor;
+import org.springframework.expression.AccessException;
+import org.springframework.expression.EvaluationContext;
+import org.springframework.expression.TypedValue;
+import org.springframework.lang.Nullable;
+import org.springframework.util.Assert;
+
+/**
+ * This is a local COPY of {@link org.springframework.context.expression.MapAccessor}.
+ *
+ * @author Juergen Hoeller
+ * @author Andy Clement
+ * @since 4.1
+ */
+public class CompilableMapAccessor implements CompilablePropertyAccessor {
+
+ @Override
+ public Class>[] getSpecificTargetClasses() {
+ return new Class>[] {Map.class};
+ }
+
+ @Override
+ public boolean canRead(EvaluationContext context, @Nullable Object target, String name) throws AccessException {
+ return (target instanceof Map, ?> map && map.containsKey(name));
+ }
+
+ @Override
+ public TypedValue read(EvaluationContext context, @Nullable Object target, String name) throws AccessException {
+ Assert.state(target instanceof Map, "Target must be of type Map");
+ Map, ?> map = (Map, ?>) target;
+ Object value = map.get(name);
+ if (value == null && !map.containsKey(name)) {
+ throw new MapAccessException(name);
+ }
+ return new TypedValue(value);
+ }
+
+ @Override
+ public boolean canWrite(EvaluationContext context, @Nullable Object target, String name) throws AccessException {
+ return true;
+ }
+
+ @Override
+ @SuppressWarnings("unchecked")
+ public void write(EvaluationContext context, @Nullable Object target, String name, @Nullable Object newValue)
+ throws AccessException {
+
+ Assert.state(target instanceof Map, "Target must be a Map");
+ Map