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 2236868a6f98..033817bd944f 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 @@ -22,6 +22,7 @@ import java.util.Map; import java.util.function.Supplier; +import org.springframework.asm.Label; import org.springframework.asm.MethodVisitor; import org.springframework.core.convert.TypeDescriptor; import org.springframework.expression.AccessException; @@ -73,6 +74,9 @@ private enum IndexedType {ARRAY, LIST, MAP, STRING, OBJECT} @Nullable private IndexedType indexedType; + @Nullable + private String originalPrimitiveExitTypeDescriptor; + @Nullable private volatile String arrayTypeDescriptor; @@ -271,6 +275,17 @@ public void generateCode(MethodVisitor mv, CodeFlow cf) { cf.loadTarget(mv); } + Label skipIfNull = null; + if (this.nullSafe) { + mv.visitInsn(DUP); + skipIfNull = new Label(); + Label continueLabel = new Label(); + mv.visitJumpInsn(IFNONNULL, continueLabel); + CodeFlow.insertCheckCast(mv, exitTypeDescriptor); + mv.visitJumpInsn(GOTO, skipIfNull); + mv.visitLabel(continueLabel); + } + SpelNodeImpl index = this.children[0]; if (this.indexedType == IndexedType.ARRAY) { @@ -333,6 +348,16 @@ else if (this.indexedType == IndexedType.OBJECT) { } cf.pushDescriptor(exitTypeDescriptor); + + if (skipIfNull != null) { + if (this.originalPrimitiveExitTypeDescriptor != null) { + // The output of the indexer is a primitive, but from the logic above it + // might be null. So, to have a common stack element type at the skipIfNull + // target, it is necessary to box the primitive. + CodeFlow.insertBoxIfNecessary(mv, this.originalPrimitiveExitTypeDescriptor); + } + mv.visitLabel(skipIfNull); + } } @Override @@ -396,56 +421,56 @@ private Object accessArrayElement(Object ctx, int idx) throws SpelEvaluationExce if (arrayComponentType == boolean.class) { boolean[] array = (boolean[]) ctx; checkAccess(array.length, idx); - this.exitTypeDescriptor = "Z"; + setExitTypeDescriptor("Z"); this.arrayTypeDescriptor = "[Z"; return array[idx]; } else if (arrayComponentType == byte.class) { byte[] array = (byte[]) ctx; checkAccess(array.length, idx); - this.exitTypeDescriptor = "B"; + setExitTypeDescriptor("B"); this.arrayTypeDescriptor = "[B"; return array[idx]; } else if (arrayComponentType == char.class) { char[] array = (char[]) ctx; checkAccess(array.length, idx); - this.exitTypeDescriptor = "C"; + setExitTypeDescriptor("C"); this.arrayTypeDescriptor = "[C"; return array[idx]; } else if (arrayComponentType == double.class) { double[] array = (double[]) ctx; checkAccess(array.length, idx); - this.exitTypeDescriptor = "D"; + setExitTypeDescriptor("D"); this.arrayTypeDescriptor = "[D"; return array[idx]; } else if (arrayComponentType == float.class) { float[] array = (float[]) ctx; checkAccess(array.length, idx); - this.exitTypeDescriptor = "F"; + setExitTypeDescriptor("F"); this.arrayTypeDescriptor = "[F"; return array[idx]; } else if (arrayComponentType == int.class) { int[] array = (int[]) ctx; checkAccess(array.length, idx); - this.exitTypeDescriptor = "I"; + setExitTypeDescriptor("I"); this.arrayTypeDescriptor = "[I"; return array[idx]; } else if (arrayComponentType == long.class) { long[] array = (long[]) ctx; checkAccess(array.length, idx); - this.exitTypeDescriptor = "J"; + setExitTypeDescriptor("J"); this.arrayTypeDescriptor = "[J"; return array[idx]; } else if (arrayComponentType == short.class) { short[] array = (short[]) ctx; checkAccess(array.length, idx); - this.exitTypeDescriptor = "S"; + setExitTypeDescriptor("S"); this.arrayTypeDescriptor = "[S"; return array[idx]; } @@ -453,7 +478,7 @@ else if (arrayComponentType == short.class) { Object[] array = (Object[]) ctx; checkAccess(array.length, idx); Object retValue = array[idx]; - this.exitTypeDescriptor = CodeFlow.toDescriptor(arrayComponentType); + setExitTypeDescriptor(CodeFlow.toDescriptor(arrayComponentType)); this.arrayTypeDescriptor = CodeFlow.toDescriptor(array.getClass()); return retValue; } @@ -466,6 +491,19 @@ private void checkAccess(int arrayLength, int index) throws SpelEvaluationExcept } } + private void setExitTypeDescriptor(String descriptor) { + // If this indexer would return a primitive - and yet it is also marked + // null-safe - then the exit type descriptor must be promoted to the box + // type to allow a null value to be passed on. + if (this.nullSafe && CodeFlow.isPrimitive(descriptor)) { + this.originalPrimitiveExitTypeDescriptor = descriptor; + this.exitTypeDescriptor = CodeFlow.toBoxedDescriptor(descriptor); + } + else { + this.exitTypeDescriptor = descriptor; + } + } + @SuppressWarnings("unchecked") private T convertValue(TypeConverter converter, @Nullable Object value, Class targetType) { T result = (T) converter.convertValue( @@ -602,7 +640,7 @@ public TypedValue getValue() { Indexer.this.cachedReadName = this.name; Indexer.this.cachedReadTargetType = targetObjectRuntimeClass; if (accessor instanceof CompilablePropertyAccessor compilablePropertyAccessor) { - Indexer.this.exitTypeDescriptor = CodeFlow.toDescriptor(compilablePropertyAccessor.getPropertyType()); + setExitTypeDescriptor(CodeFlow.toDescriptor(compilablePropertyAccessor.getPropertyType())); } return accessor.read(this.evaluationContext, this.targetObject, this.name); } diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java index 72edfb425425..77da11dea144 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java @@ -725,6 +725,198 @@ else if (object instanceof int[] ints) { } + @Nested + class NullSafeIndexTests { // gh-29847 + + private final RootContextWithIndexedProperties rootContext = new RootContextWithIndexedProperties(); + + private final StandardEvaluationContext context = new StandardEvaluationContext(rootContext); + + @Test + void nullSafeIndexIntoPrimitiveIntArray() { + expression = parser.parseExpression("intArray?.[0]"); + + // Cannot compile before the array type is known. + assertThat(expression.getValue(context)).isNull(); + assertCannotCompile(expression); + assertThat(expression.getValue(context)).isNull(); + assertThat(getAst().getExitDescriptor()).isNull(); + + rootContext.intArray = new int[] { 8, 9, 10 }; + assertThat(expression.getValue(context)).isEqualTo(8); + assertCanCompile(expression); + assertThat(expression.getValue(context)).isEqualTo(8); + // Normally we would expect the exit type descriptor to be "I" for an + // element of an int[]. However, with null-safe indexing support the + // only way for it to evaluate to null is to box the 'int' to an 'Integer'. + assertThat(getAst().getExitDescriptor()).isEqualTo("Ljava/lang/Integer"); + + // Null-safe support should have been compiled once the array type is known. + rootContext.intArray = null; + assertThat(expression.getValue(context)).isNull(); + assertCanCompile(expression); + assertThat(expression.getValue(context)).isNull(); + assertThat(getAst().getExitDescriptor()).isEqualTo("Ljava/lang/Integer"); + } + + @Test + void nullSafeIndexIntoNumberArray() { + expression = parser.parseExpression("numberArray?.[0]"); + + // Cannot compile before the array type is known. + assertThat(expression.getValue(context)).isNull(); + assertCannotCompile(expression); + assertThat(expression.getValue(context)).isNull(); + assertThat(getAst().getExitDescriptor()).isNull(); + + rootContext.numberArray = new Number[] { 8, 9, 10 }; + assertThat(expression.getValue(context)).isEqualTo(8); + assertCanCompile(expression); + assertThat(expression.getValue(context)).isEqualTo(8); + assertThat(getAst().getExitDescriptor()).isEqualTo("Ljava/lang/Number"); + + // Null-safe support should have been compiled once the array type is known. + rootContext.numberArray = null; + assertThat(expression.getValue(context)).isNull(); + assertCanCompile(expression); + assertThat(expression.getValue(context)).isNull(); + assertThat(getAst().getExitDescriptor()).isEqualTo("Ljava/lang/Number"); + } + + @Test + void nullSafeIndexIntoList() { + expression = parser.parseExpression("list?.[0]"); + + // Cannot compile before the list type is known. + assertThat(expression.getValue(context)).isNull(); + assertCannotCompile(expression); + assertThat(expression.getValue(context)).isNull(); + assertThat(getAst().getExitDescriptor()).isNull(); + + rootContext.list = List.of(42); + assertThat(expression.getValue(context)).isEqualTo(42); + assertCanCompile(expression); + assertThat(expression.getValue(context)).isEqualTo(42); + assertThat(getAst().getExitDescriptor()).isEqualTo("Ljava/lang/Object"); + + // Null-safe support should have been compiled once the list type is known. + rootContext.list = null; + assertThat(expression.getValue(context)).isNull(); + assertCanCompile(expression); + assertThat(expression.getValue(context)).isNull(); + assertThat(getAst().getExitDescriptor()).isEqualTo("Ljava/lang/Object"); + } + + @Test + void nullSafeIndexIntoSetCannotBeCompiled() { + expression = parser.parseExpression("set?.[0]"); + + assertThat(expression.getValue(context)).isNull(); + assertCannotCompile(expression); + assertThat(expression.getValue(context)).isNull(); + assertThat(getAst().getExitDescriptor()).isNull(); + + rootContext.set = Set.of(42); + assertThat(expression.getValue(context)).isEqualTo(42); + assertCannotCompile(expression); + assertThat(expression.getValue(context)).isEqualTo(42); + assertThat(getAst().getExitDescriptor()).isNull(); + } + + @Test + void nullSafeIndexIntoStringCannotBeCompiled() { + expression = parser.parseExpression("string?.[0]"); + + assertThat(expression.getValue(context)).isNull(); + assertCannotCompile(expression); + assertThat(expression.getValue(context)).isNull(); + assertThat(getAst().getExitDescriptor()).isNull(); + + rootContext.string = "XYZ"; + assertThat(expression.getValue(context)).isEqualTo("X"); + assertCannotCompile(expression); + assertThat(expression.getValue(context)).isEqualTo("X"); + assertThat(getAst().getExitDescriptor()).isNull(); + } + + @Test + void nullSafeIndexIntoMap() { + expression = parser.parseExpression("map?.['enigma']"); + + // Cannot compile before the map type is known. + assertThat(expression.getValue(context)).isNull(); + assertCannotCompile(expression); + assertThat(expression.getValue(context)).isNull(); + assertThat(getAst().getExitDescriptor()).isNull(); + + rootContext.map = Map.of("enigma", 42); + assertThat(expression.getValue(context)).isEqualTo(42); + assertCanCompile(expression); + assertThat(expression.getValue(context)).isEqualTo(42); + assertThat(getAst().getExitDescriptor()).isEqualTo("Ljava/lang/Object"); + + // Null-safe support should have been compiled once the map type is known. + rootContext.map = null; + assertThat(expression.getValue(context)).isNull(); + assertCanCompile(expression); + assertThat(expression.getValue(context)).isNull(); + assertThat(getAst().getExitDescriptor()).isEqualTo("Ljava/lang/Object"); + } + + @Test + void nullSafeIndexIntoObjectViaPrimitiveProperty() { + expression = parser.parseExpression("person?.['age']"); + + // Cannot compile before the Person type is known. + assertThat(expression.getValue(context)).isNull(); + assertCannotCompile(expression); + assertThat(expression.getValue(context)).isNull(); + assertThat(getAst().getExitDescriptor()).isNull(); + + rootContext.person = new Person("Jane"); + rootContext.person.setAge(42); + assertThat(expression.getValue(context)).isEqualTo(42); + assertCanCompile(expression); + assertThat(expression.getValue(context)).isEqualTo(42); + // Normally we would expect the exit type descriptor to be "I" for + // an int. However, with null-safe indexing support the only way + // for it to evaluate to null is to box the 'int' to an 'Integer'. + assertThat(getAst().getExitDescriptor()).isEqualTo("Ljava/lang/Integer"); + + // Null-safe support should have been compiled once the Person type is known. + rootContext.person = null; + assertThat(expression.getValue(context)).isNull(); + assertCanCompile(expression); + assertThat(expression.getValue(context)).isNull(); + assertThat(getAst().getExitDescriptor()).isEqualTo("Ljava/lang/Integer"); + } + + @Test + void nullSafeIndexIntoObjectViaStringProperty() { + expression = parser.parseExpression("person?.['name']"); + + // Cannot compile before the Person type is known. + assertThat(expression.getValue(context)).isNull(); + assertCannotCompile(expression); + assertThat(expression.getValue(context)).isNull(); + assertThat(getAst().getExitDescriptor()).isNull(); + + rootContext.person = new Person("Jane"); + assertThat(expression.getValue(context)).isEqualTo("Jane"); + assertCanCompile(expression); + assertThat(expression.getValue(context)).isEqualTo("Jane"); + assertThat(getAst().getExitDescriptor()).isEqualTo("Ljava/lang/String"); + + // Null-safe support should have been compiled once the Person type is known. + rootContext.person = null; + assertThat(expression.getValue(context)).isNull(); + assertCanCompile(expression); + assertThat(expression.getValue(context)).isNull(); + assertThat(getAst().getExitDescriptor()).isEqualTo("Ljava/lang/String"); + } + + } + @Nested class PropertyVisibilityTests { @@ -6736,4 +6928,15 @@ public void setValue2(Integer value) { } } + // Must be public with public fields/properties. + public static class RootContextWithIndexedProperties { + public int[] intArray; + public Number[] numberArray; + public List list; + public Set set; + public String string; + public Map map; + public Person person; + } + }