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 b2a0457e05d..e7b66721972 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 @@ -136,37 +136,44 @@ public class ConstructorReference extends SpelNodeImpl { ConstructorExecutor executorToUse = this.cachedExecutor; if (executorToUse != null) { - try { - return executorToUse.execute(state.getEvaluationContext(), arguments); - } - catch (AccessException ex) { - // Two reasons this can occur: - // 1. the constructor invoked actually threw a real exception - // 2. the constructor invoked was not passed the arguments it expected and has become 'stale' - - // In the first case we should not retry, in the second case we should see if there is a - // better suited constructor. - - // To determine which situation it is, the AccessException will contain a cause. - // If the cause is an InvocationTargetException, a user exception was thrown inside the constructor. - // Otherwise, the constructor could not be invoked. - if (ex.getCause() instanceof InvocationTargetException cause) { - // User exception was the root cause - exit now - Throwable rootCause = cause.getCause(); - if (rootCause instanceof RuntimeException runtimeException) { - throw runtimeException; - } - else { - String typeName = (String) this.children[0].getValueInternal(state).getValue(); - throw new SpelEvaluationException(getStartPosition(), rootCause, - SpelMessage.CONSTRUCTOR_INVOCATION_PROBLEM, typeName, - FormatHelper.formatMethodForMessage("", argumentTypes)); - } - } - - // At this point we know it wasn't a user problem so worth a retry if a better candidate can be found + if (state.getEvaluationContext().getConstructorResolvers().isEmpty()) { + // Constructor resolution is not supported in the current context, + // so we discard the cached executor. this.cachedExecutor = null; } + else { + try { + return executorToUse.execute(state.getEvaluationContext(), arguments); + } + catch (AccessException ex) { + // Two reasons this can occur: + // 1. the constructor invoked actually threw a real exception + // 2. the constructor invoked was not passed the arguments it expected and has become 'stale' + + // In the first case we should not retry, in the second case we should see if there is a + // better suited constructor. + + // To determine which situation it is, the AccessException will contain a cause. + // If the cause is an InvocationTargetException, a user exception was thrown inside the constructor. + // Otherwise, the constructor could not be invoked. + if (ex.getCause() instanceof InvocationTargetException cause) { + // User exception was the root cause - exit now + Throwable rootCause = cause.getCause(); + if (rootCause instanceof RuntimeException runtimeException) { + throw runtimeException; + } + else { + String typeName = (String) this.children[0].getValueInternal(state).getValue(); + throw new SpelEvaluationException(getStartPosition(), rootCause, + SpelMessage.CONSTRUCTOR_INVOCATION_PROBLEM, typeName, + FormatHelper.formatMethodForMessage("", argumentTypes)); + } + } + + // At this point we know it wasn't a user problem so worth a retry if a better candidate can be found + this.cachedExecutor = null; + } + } } // Either there was no ConstructorExecutor or it no longer exists diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/ConstructorInvocationTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/ConstructorInvocationTests.java index 89dfe246372..4911acae28e 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/ConstructorInvocationTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/ConstructorInvocationTests.java @@ -19,6 +19,7 @@ package org.springframework.expression.spel; import java.util.ArrayList; import java.util.List; +import org.assertj.core.api.ThrowableTypeAssert; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -32,6 +33,7 @@ import org.springframework.expression.spel.testresources.PlaceOfBirth; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatException; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; /** * Tests invocation of constructors. @@ -142,6 +144,28 @@ class ConstructorInvocationTests extends AbstractExpressionTests { assertThat(ctx.getConstructorResolvers()).hasSize(2); } + @Test // gh-36985 + void cachedConstructorExecutorIsNotUsedWithoutRegisteredConstructorResolvers() { + ((StandardTypeLocator) super.context.getTypeLocator()).registerImport(Fruit.class.getPackageName()); + + // reflective constructor accessor is the only one by default + assertThat(super.context.getConstructorResolvers()).hasSize(1); + + String expression = "new Fruit('apple', T(java.awt.Color).RED, 'red').name"; + Expression expr = parser.parseExpression(expression); + assertThat(expr.getValue(super.context)).isEqualTo("apple"); + // Ensure the same expression can be evaluated again. + assertThat(expr.getValue(super.context)).isEqualTo("apple"); + + super.context.setConstructorResolvers(List.of()); + assertThat(super.context.getConstructorResolvers()).isEmpty(); + + // Evaluation of the same expression should no longer work. + assertThatSpelEvaluationException() + .isThrownBy(() -> expr.getValue(super.context)) + .extracting(SpelEvaluationException::getMessageCode).isEqualTo(SpelMessage.CONSTRUCTOR_NOT_FOUND); + } + @Test void varargsConstructors() { ((StandardTypeLocator) super.context.getTypeLocator()).registerImport(Fruit.class.getPackageName()); @@ -184,6 +208,10 @@ class ConstructorInvocationTests extends AbstractExpressionTests { evaluate("new String(3.0d)", "3.0", String.class); } + private ThrowableTypeAssert assertThatSpelEvaluationException() { + return assertThatExceptionOfType(SpelEvaluationException.class); + } + @SuppressWarnings("serial") static class TestException extends Exception {