Reject competing SpEL accessors

spring-expression/src/main/java/org/springframework/expression/spel/SpelMessage.java

@@ -303,7 +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 7.1 */
+	COMPETING_ACCESSORS(Kind.ERROR, 1085,
+			"Found competing read and write accessors for ''{0}'' in ''{1}''");
 
 
 

spring-expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java

@@ -256,11 +256,19 @@ private ValueRef getValueRef(ExpressionState state, AccessMode accessMode) throw
 			try {
 				for (IndexAccessor indexAccessor : accessorsToTry) {
 					if (indexAccessor.canRead(evalContext, target, index)) {
+						if (accessMode.supportsWrites && !indexAccessor.canWrite(evalContext, target, index) &&
+								hasCompetingWriteAccessor(accessorsToTry, indexAccessor, evalContext, target, index)) {
+							throw new SpelEvaluationException(getStartPosition(), SpelMessage.COMPETING_ACCESSORS,
+									index, target.getClass().getTypeName());
+						}
 						this.indexedType = IndexedType.CUSTOM;
 						return new IndexAccessorValueRef(target, index, evalContext, targetDescriptor);
 					}
 				}
 			}
+			catch (SpelEvaluationException ex) {
+				throw ex;
+			}
 			catch (Exception ex) {
 				throw new SpelEvaluationException(
 						getStartPosition(), ex, SpelMessage.EXCEPTION_DURING_INDEX_READ,
@@ -472,6 +480,17 @@ private static Class<?> getObjectType(Object obj) {
 		return (obj instanceof Class<?> clazz ? clazz : obj.getClass());
 	}
 
+	private static boolean hasCompetingWriteAccessor(List<IndexAccessor> accessors, IndexAccessor readAccessor,
+			EvaluationContext evalContext, Object target, Object index) throws AccessException {
+
+		for (IndexAccessor accessor : accessors) {
+			if (accessor != readAccessor && accessor.canWrite(evalContext, target, index)) {
+				return true;
+			}
+		}
+		return false;
+	}
+
 
 	/**
 	 * Tracks state when the {@code Indexer} is being used as a {@link PropertyAccessor}.

spring-expression/src/main/java/org/springframework/expression/spel/ast/PropertyOrFieldReference.java

@@ -128,7 +128,13 @@ public TypedValue getValueInternal(ExpressionState state) throws EvaluationExcep
 	private TypedValue getValueInternal(TypedValue contextObject, EvaluationContext evalContext,
 			boolean isAutoGrowNullReferences) throws EvaluationException {
 
-		TypedValue result = readProperty(contextObject, evalContext, this.name);
+		return getValueInternal(contextObject, evalContext, isAutoGrowNullReferences, false);
+	}
+
+	private TypedValue getValueInternal(TypedValue contextObject, EvaluationContext evalContext,
+			boolean isAutoGrowNullReferences, boolean readWriteRequired) throws EvaluationException {
+
+		TypedValue result = readProperty(contextObject, evalContext, this.name, readWriteRequired);
 
 		// Dynamically create the objects if the user has requested that optional behavior
 		if (result.getValue() == null && isAutoGrowNullReferences &&
@@ -140,14 +146,14 @@ private TypedValue getValueInternal(TypedValue contextObject, EvaluationContext
 				if (isWritableProperty(this.name, contextObject, evalContext)) {
 					List<?> newList = new ArrayList<>();
 					writeProperty(contextObject, evalContext, this.name, newList);
-					result = readProperty(contextObject, evalContext, this.name);
+					result = readProperty(contextObject, evalContext, this.name, false);
 				}
 			}
 			else if (Map.class == resultDescriptor.getType()) {
 				if (isWritableProperty(this.name,contextObject, evalContext)) {
 					Map<?,?> newMap = new HashMap<>();
 					writeProperty(contextObject, evalContext, this.name, newMap);
-					result = readProperty(contextObject, evalContext, this.name);
+					result = readProperty(contextObject, evalContext, this.name, false);
 				}
 			}
 			else {
@@ -157,7 +163,7 @@ else if (Map.class == resultDescriptor.getType()) {
 						Class<?> clazz = resultDescriptor.getType();
 						Object newObject = ReflectionUtils.accessibleConstructor(clazz).newInstance();
 						writeProperty(contextObject, evalContext, this.name, newObject);
-						result = readProperty(contextObject, evalContext, this.name);
+						result = readProperty(contextObject, evalContext, this.name, false);
 					}
 				}
 				catch (InvocationTargetException ex) {
@@ -197,7 +203,8 @@ public String toStringAST() {
 	 * @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)
+	private TypedValue readProperty(TypedValue contextObject, EvaluationContext evalContext, String name,
+			boolean readWriteRequired)
 			throws EvaluationException {
 
 		final Object originalTarget = contextObject.getValue();
@@ -245,6 +252,11 @@ private TypedValue readProperty(TypedValue contextObject, EvaluationContext eval
 			for (PropertyAccessor accessor : accessorsToTry) {
 				// First, attempt to find the property on the target object.
 				if (accessor.canRead(evalContext, target, name)) {
+					if (readWriteRequired && !accessor.canWrite(evalContext, target, name) &&
+							hasCompetingWriteAccessor(accessorsToTry, accessor, evalContext, target, name)) {
+						throw new SpelEvaluationException(getStartPosition(), SpelMessage.COMPETING_ACCESSORS,
+								name, FormatHelper.formatClassNameForMessage(getObjectClass(target)));
+					}
 					if (accessor instanceof ReflectivePropertyAccessor reflectivePropertyAccessor) {
 						accessor = reflectivePropertyAccessor.createOptimalAccessor(
 								evalContext, target, name);
@@ -254,6 +266,12 @@ private TypedValue readProperty(TypedValue contextObject, EvaluationContext eval
 				}
 				// Second, attempt to find the property on the original Optional instance.
 				else if (fallbackOptionalTarget != null && accessor.canRead(evalContext, fallbackOptionalTarget, name)) {
+					if (readWriteRequired && !accessor.canWrite(evalContext, fallbackOptionalTarget, name) &&
+							hasCompetingWriteAccessor(
+									accessorsToTry, accessor, evalContext, fallbackOptionalTarget, name)) {
+						throw new SpelEvaluationException(getStartPosition(), SpelMessage.COMPETING_ACCESSORS,
+								name, FormatHelper.formatClassNameForMessage(getObjectClass(fallbackOptionalTarget)));
+					}
 					if (accessor instanceof ReflectivePropertyAccessor reflectivePropertyAccessor) {
 						accessor = reflectivePropertyAccessor.createOptimalAccessor(
 								evalContext, fallbackOptionalTarget, name);
@@ -266,6 +284,9 @@ else if (fallbackOptionalTarget != null && accessor.canRead(evalContext, fallbac
 				}
 			}
 		}
+		catch (SpelEvaluationException ex) {
+			throw ex;
+		}
 		catch (Exception ex) {
 			throw new SpelEvaluationException(ex, SpelMessage.EXCEPTION_DURING_PROPERTY_READ, name, ex.getMessage());
 		}
@@ -286,6 +307,17 @@ else if (fallbackOptionalTarget != null && accessor.canRead(evalContext, fallbac
 		}
 	}
 
+	private static boolean hasCompetingWriteAccessor(List<PropertyAccessor> accessors, PropertyAccessor readAccessor,
+			EvaluationContext evalContext, @Nullable Object target, String name) throws AccessException {
+
+		for (PropertyAccessor accessor : accessors) {
+			if (accessor != readAccessor && accessor.canWrite(evalContext, target, name)) {
+				return true;
+			}
+		}
+		return false;
+	}
+
 	private void writeProperty(
 			TypedValue contextObject, EvaluationContext evalContext, String name, @Nullable Object newValue)
 			throws EvaluationException {
@@ -432,7 +464,7 @@ public AccessorValueRef(PropertyOrFieldReference propertyOrFieldReference, Typed
 		@Override
 		public TypedValue getValue() {
 			TypedValue value =
-					this.ref.getValueInternal(this.contextObject, this.evalContext, this.autoGrowNullReferences);
+					this.ref.getValueInternal(this.contextObject, this.evalContext, this.autoGrowNullReferences, true);
 			if (this.ref.cachedReadAccessor instanceof CompilablePropertyAccessor compilablePropertyAccessor) {
 				this.ref.setExitTypeDescriptor(CodeFlow.toDescriptor(compilablePropertyAccessor.getPropertyType()));
 			}

spring-expression/src/test/java/org/springframework/expression/spel/IndexingTests.java

@@ -63,6 +63,7 @@
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoMoreInteractions;
+import static org.springframework.expression.spel.SpelMessage.COMPETING_ACCESSORS;
 import static org.springframework.expression.spel.SpelMessage.EXCEPTION_DURING_INDEX_READ;
 import static org.springframework.expression.spel.SpelMessage.EXCEPTION_DURING_INDEX_WRITE;
 import static org.springframework.expression.spel.SpelMessage.INDEXING_NOT_SUPPORTED_FOR_TYPE;
@@ -716,6 +717,18 @@ void readAndWriteIndexWithSimpleEvaluationContext() {
 			assertThat(expr.getValue(context, arrayNode)).isSameAs(node1);
 		}
 
+		@Test  // gh-32737
+		void competingReadAndWriteIndexAccessors() {
+			context.addIndexAccessor(new ReadOnlyIndexAccessor());
+			context.addIndexAccessor(new WriteOnlyIndexAccessor());
+
+			Expression expr = parser.parseExpression("[0]++");
+
+			assertThatExceptionOfType(SpelEvaluationException.class)
+					.isThrownBy(() -> expr.getValue(context, this))
+					.extracting(SpelEvaluationException::getMessageCode).isEqualTo(COMPETING_ACCESSORS);
+		}
+
 		@Test  // gh-32706
 		void readIndexWithStringIndexType() {
 			BirdNameToColorMappings birdNameMappings = new BirdNameToColorMappings();
@@ -847,6 +860,62 @@ public void write(EvaluationContext context, Object target, Object index, @Nulla
 				arrayNode.set(intIndex, this.objectMapper.convertValue(newValue, JsonNode.class));
 			}
 		}
+
+
+		private static class ReadOnlyIndexAccessor implements IndexAccessor {
+
+			@Override
+			public Class<?>[] getSpecificTargetClasses() {
+				return null;
+			}
+
+			@Override
+			public boolean canRead(EvaluationContext context, Object target, Object index) {
+				return true;
+			}
+
+			@Override
+			public TypedValue read(EvaluationContext context, Object target, Object index) {
+				return new TypedValue(1);
+			}
+
+			@Override
+			public boolean canWrite(EvaluationContext context, Object target, Object index) {
+				return false;
+			}
+
+			@Override
+			public void write(EvaluationContext context, Object target, Object index, @Nullable Object newValue) {
+			}
+		}
+
+
+		private static class WriteOnlyIndexAccessor implements IndexAccessor {
+
+			@Override
+			public Class<?>[] getSpecificTargetClasses() {
+				return null;
+			}
+
+			@Override
+			public boolean canRead(EvaluationContext context, Object target, Object index) {
+				return false;
+			}
+
+			@Override
+			public TypedValue read(EvaluationContext context, Object target, Object index) {
+				return TypedValue.NULL;
+			}
+
+			@Override
+			public boolean canWrite(EvaluationContext context, Object target, Object index) {
+				return true;
+			}
+
+			@Override
+			public void write(EvaluationContext context, Object target, Object index, @Nullable Object newValue) {
+			}
+		}
 	}
 
 

spring-expression/src/test/java/org/springframework/expression/spel/PropertyAccessTests.java

@@ -300,6 +300,19 @@ void propertyAccessWithArrayIndexOutOfBounds() {
 			.extracting(SpelEvaluationException::getMessageCode).isEqualTo(SpelMessage.ARRAY_INDEX_OUT_OF_BOUNDS);
 	}
 
+	@Test  // gh-32737
+	void competingReadAndWritePropertyAccessors() {
+		StandardEvaluationContext context = new StandardEvaluationContext(new Object());
+		context.addPropertyAccessor(new ConfigurablePropertyAccessor(Map.of("counter", 1)));
+		context.addPropertyAccessor(new WriteOnlyPropertyAccessor());
+
+		Expression expression = parser.parseExpression("counter++");
+
+		assertThatSpelEvaluationException()
+				.isThrownBy(() -> expression.getValue(context))
+				.extracting(SpelEvaluationException::getMessageCode).isEqualTo(SpelMessage.COMPETING_ACCESSORS);
+	}
+
 
 	private ThrowableTypeAssert<SpelEvaluationException> assertThatSpelEvaluationException() {
 		return assertThatExceptionOfType(SpelEvaluationException.class);
@@ -389,4 +402,32 @@ public void write(EvaluationContext context, Object target, String name, Object
 		}
 	}
 
+
+	private static class WriteOnlyPropertyAccessor implements PropertyAccessor {
+
+		@Override
+		public Class<?>[] getSpecificTargetClasses() {
+			return null;
+		}
+
+		@Override
+		public boolean canRead(EvaluationContext context, Object target, String name) {
+			return false;
+		}
+
+		@Override
+		public TypedValue read(EvaluationContext context, Object target, String name) {
+			return TypedValue.NULL;
+		}
+
+		@Override
+		public boolean canWrite(EvaluationContext context, Object target, String name) {
+			return "counter".equals(name);
+		}
+
+		@Override
+		public void write(EvaluationContext context, Object target, String name, Object newValue) {
+		}
+	}
+
 }