root
/
spring-framework
mirror of https://github.com/spring-projects/spring-framework.git
1
0
Fork 0
Code Issues Actions Packages Projects Releases Wiki Activity

Correctly request primitive conversion in SpEL's Indexer 

Prior to this commit, SpEL's Indexer incorrectly requested conversion
to wrappers instead of primitives when setting an element in a
primitive array.

This commit addresses this by requesting primitive conversion -- for
example, conversion to `int.class` instead of `Integer.class` when
setting a value in an `int[]` array.

For greater clarity, this commit also switches from using `TYPE`
constants in wrapper classes to primitive class literals -- for
example, from `Integer.TYPE` to `int.class`.

Closes gh-32147
This commit is contained in:
Sam Brannen 2024-01-28 18:21:53 +01:00
parent 2e56361fe4
commit 62fa3f11c1
2 changed files with 68 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -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.
@ -336,45 +336,45 @@ public class Indexer extends SpelNodeImpl {
private void setArrayElement(TypeConverter converter, Object ctx, int idx, @Nullable Object newValue,
Class<?> arrayComponentType) throws EvaluationException {
if (arrayComponentType == Boolean.TYPE) {
if (arrayComponentType == boolean.class) {
boolean[] array = (boolean[]) ctx;
checkAccess(array.length, idx);
array[idx] = convertValue(converter, newValue, Boolean.class);
array[idx] = convertValue(converter, newValue, boolean.class);
}
else if (arrayComponentType == Byte.TYPE) {
else if (arrayComponentType == byte.class) {
byte[] array = (byte[]) ctx;
checkAccess(array.length, idx);
array[idx] = convertValue(converter, newValue, Byte.class);
array[idx] = convertValue(converter, newValue, byte.class);
}
else if (arrayComponentType == Character.TYPE) {
else if (arrayComponentType == char.class) {
char[] array = (char[]) ctx;
checkAccess(array.length, idx);
array[idx] = convertValue(converter, newValue, Character.class);
array[idx] = convertValue(converter, newValue, char.class);
}
else if (arrayComponentType == Double.TYPE) {
else if (arrayComponentType == double.class) {
double[] array = (double[]) ctx;
checkAccess(array.length, idx);
array[idx] = convertValue(converter, newValue, Double.class);
array[idx] = convertValue(converter, newValue, double.class);
}
else if (arrayComponentType == Float.TYPE) {
else if (arrayComponentType == float.class) {
float[] array = (float[]) ctx;
checkAccess(array.length, idx);
array[idx] = convertValue(converter, newValue, Float.class);
array[idx] = convertValue(converter, newValue, float.class);
}
else if (arrayComponentType == Integer.TYPE) {
else if (arrayComponentType == int.class) {
int[] array = (int[]) ctx;
checkAccess(array.length, idx);
array[idx] = convertValue(converter, newValue, Integer.class);
array[idx] = convertValue(converter, newValue, int.class);
}
else if (arrayComponentType == Long.TYPE) {
else if (arrayComponentType == long.class) {
long[] array = (long[]) ctx;
checkAccess(array.length, idx);
array[idx] = convertValue(converter, newValue, Long.class);
array[idx] = convertValue(converter, newValue, long.class);
}
else if (arrayComponentType == Short.TYPE) {
else if (arrayComponentType == short.class) {
short[] array = (short[]) ctx;
checkAccess(array.length, idx);
array[idx] = convertValue(converter, newValue, Short.class);
array[idx] = convertValue(converter, newValue, short.class);
}
else {
Object[] array = (Object[]) ctx;

67
spring-expression/src/test/java/org/springframework/expression/spel/SetValueTests.java
View File

@ -21,16 +21,19 @@ import java.util.List;
import java.util.Set;
import org.assertj.core.api.ThrowableTypeAssert;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
import org.springframework.expression.EvaluationException;
import org.springframework.expression.Expression;
import org.springframework.expression.ParseException;
import org.springframework.expression.spel.testresources.PlaceOfBirth;
import org.springframework.util.ObjectUtils;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.springframework.expression.spel.SpelMessage.TYPE_CONVERSION_ERROR;
/**
* Tests for assignment, setValue(), and isWritable() expressions.
@ -164,21 +167,19 @@ class SetValueTests extends AbstractExpressionTests {
setValue("arrayContainer.doubles[1]", List.of(42D), 42D);
}
@Disabled("Disabled due to bug in Indexer.setArrayElement() regarding primitive/wrapper types")
@Test
void setArrayElementToPrimitiveFromEmptyCollectionFails() {
List<Object> emptyList = List.of();
// TODO These fail because CollectionToObjectConverter returns null.
// It currently throws: java.lang.IllegalStateException: Null conversion result for index [[]].
// Whereas, it should throw a SpelEvaluationException.
setValueAndExpectError("arrayContainer.booleans[1]", emptyList);
setValueAndExpectError("arrayContainer.chars[1]", emptyList);
setValueAndExpectError("arrayContainer.shorts[1]", emptyList);
setValueAndExpectError("arrayContainer.bytes[1]", emptyList);
setValueAndExpectError("arrayContainer.ints[1]", emptyList);
setValueAndExpectError("arrayContainer.longs[1]", emptyList);
setValueAndExpectError("arrayContainer.floats[1]", emptyList);
setValueAndExpectError("arrayContainer.doubles[1]", emptyList);
@ParameterizedTest
@ValueSource(strings = {
"arrayContainer.booleans[1]",
"arrayContainer.chars[1]",
"arrayContainer.shorts[1]",
"arrayContainer.bytes[1]",
"arrayContainer.ints[1]",
"arrayContainer.longs[1]",
"arrayContainer.floats[1]",
"arrayContainer.doubles[1]"
})
void setArrayElementToPrimitiveFromEmptyCollectionFailsWithTypeConversionError(String expression) {
setValueAndExpectError(expression, List.of(), TYPE_CONVERSION_ERROR);
}
@Test
@ -272,6 +273,40 @@ class SetValueTests extends AbstractExpressionTests {
assertThatSpelEvaluationException().isThrownBy(() -> e.setValue(context, value));
}
/**
* Call setValue() but expect it to fail.
* @see #evaluateAndCheckError(org.springframework.expression.ExpressionParser, String, Class, SpelMessage, Object...)
*/
private void setValueAndExpectError(String expression, Object value, SpelMessage expectedMessage,
Object... otherProperties) {
Expression expr = parser.parseExpression(expression);
assertThat(expr).as("expression").isNotNull();
if (DEBUG) {
SpelUtilities.printAbstractSyntaxTree(System.out, expr);
}
assertThatSpelEvaluationException()
.isThrownBy(() -> expr.setValue(context, value))
.satisfies(ex -> {
assertThat(ex.getMessageCode()).isEqualTo(expectedMessage);
if (!ObjectUtils.isEmpty(otherProperties)) {
// first one is expected position of the error within the string
int pos = (Integer) otherProperties[0];
assertThat(ex.getPosition()).as("position").isEqualTo(pos);
if (otherProperties.length > 1) {
// Check inserts match
Object[] inserts = ex.getInserts();
assertThat(inserts).as("inserts").hasSizeGreaterThanOrEqualTo(otherProperties.length - 1);
Object[] expectedInserts = new Object[inserts.length];
System.arraycopy(otherProperties, 1, expectedInserts, 0, expectedInserts.length);
assertThat(inserts).as("inserts").containsExactly(expectedInserts);
}
}
});
}
private void setValue(String expression, Object value) {
Class<?> expectedType = value.getClass();
try {