Indexer enforces target descriptor only after non-null target check

Issue: SPR-16544
master
Juergen Hoeller 7 years ago
parent bfddbbe731
commit fa670dd07d
  1. 95
      spring-expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java
  2. 156
      spring-expression/src/test/java/org/springframework/expression/spel/SpelExceptionTests.java

@ -1,5 +1,5 @@
/*
* Copyright 2002-2017 the original author or authors.
* Copyright 2002-2018 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.
@ -16,6 +16,7 @@
package org.springframework.expression.spel.ast;
import java.lang.reflect.Constructor;
import java.lang.reflect.Field;
import java.lang.reflect.Member;
import java.lang.reflect.Method;
@ -113,22 +114,20 @@ public class Indexer extends SpelNodeImpl {
@Override
protected ValueRef getValueRef(ExpressionState state) throws EvaluationException {
TypedValue context = state.getActiveContextObject();
Object targetObject = context.getValue();
Object target = context.getValue();
TypeDescriptor targetDescriptor = context.getTypeDescriptor();
Assert.state(targetDescriptor != null, "No type descriptor");
TypedValue indexValue = null;
Object index = null;
TypedValue indexValue;
Object index;
// This first part of the if clause prevents a 'double dereference' of
// the property (SPR-5847)
if (targetObject instanceof Map && (this.children[0] instanceof PropertyOrFieldReference)) {
// This first part of the if clause prevents a 'double dereference' of the property (SPR-5847)
if (target instanceof Map && (this.children[0] instanceof PropertyOrFieldReference)) {
PropertyOrFieldReference reference = (PropertyOrFieldReference) this.children[0];
index = reference.getName();
indexValue = new TypedValue(index);
}
else {
// In case the map key is unqualified, we want it evaluated against
// the root object so temporarily push that on whilst evaluating the key
// In case the map key is unqualified, we want it evaluated against the root object
// so temporarily push that on whilst evaluating the key
try {
state.pushActiveContextObject(state.getRootContextObject());
indexValue = this.children[0].getValueInternal(state);
@ -140,48 +139,52 @@ public class Indexer extends SpelNodeImpl {
}
}
// Raise a proper exception in case of a null target
if (target == null) {
throw new SpelEvaluationException(getStartPosition(), SpelMessage.CANNOT_INDEX_INTO_NULL_VALUE);
}
// At this point, we need a TypeDescriptor for a non-null target object
Assert.state(targetDescriptor != null, "No type descriptor");
// Indexing into a Map
if (targetObject instanceof Map) {
if (target instanceof Map) {
Object key = index;
if (targetDescriptor.getMapKeyTypeDescriptor() != null) {
key = state.convertValue(key, targetDescriptor.getMapKeyTypeDescriptor());
}
this.indexedType = IndexedType.MAP;
return new MapIndexingValueRef(state.getTypeConverter(), (Map<?, ?>) targetObject, key, targetDescriptor);
}
if (targetObject == null) {
throw new SpelEvaluationException(getStartPosition(), SpelMessage.CANNOT_INDEX_INTO_NULL_VALUE);
return new MapIndexingValueRef(state.getTypeConverter(), (Map<?, ?>) target, key, targetDescriptor);
}
// if the object is something that looks indexable by an integer,
// If the object is something that looks indexable by an integer,
// attempt to treat the index value as a number
if (targetObject.getClass().isArray() || targetObject instanceof Collection || targetObject instanceof String) {
if (target.getClass().isArray() || target instanceof Collection || target instanceof String) {
int idx = (Integer) state.convertValue(index, TypeDescriptor.valueOf(Integer.class));
if (targetObject.getClass().isArray()) {
if (target.getClass().isArray()) {
this.indexedType = IndexedType.ARRAY;
return new ArrayIndexingValueRef(state.getTypeConverter(), targetObject, idx, targetDescriptor);
return new ArrayIndexingValueRef(state.getTypeConverter(), target, idx, targetDescriptor);
}
else if (targetObject instanceof Collection) {
if (targetObject instanceof List) {
else if (target instanceof Collection) {
if (target instanceof List) {
this.indexedType = IndexedType.LIST;
}
return new CollectionIndexingValueRef((Collection<?>) targetObject, idx, targetDescriptor,
return new CollectionIndexingValueRef((Collection<?>) target, idx, targetDescriptor,
state.getTypeConverter(), state.getConfiguration().isAutoGrowCollections(),
state.getConfiguration().getMaximumAutoGrowSize());
}
else {
this.indexedType = IndexedType.STRING;
return new StringIndexingLValue((String) targetObject, idx, targetDescriptor);
return new StringIndexingLValue((String) target, idx, targetDescriptor);
}
}
// Try and treat the index value as a property of the context object
// TODO could call the conversion service to convert the value to a String
// TODO: could call the conversion service to convert the value to a String
TypeDescriptor valueType = indexValue.getTypeDescriptor();
if (valueType != null && String.class == valueType.getType()) {
this.indexedType = IndexedType.OBJECT;
return new PropertyIndexingValueRef(targetObject, (String) index, state.getEvaluationContext(), targetDescriptor);
return new PropertyIndexingValueRef(
target, (String) index, state.getEvaluationContext(), targetDescriptor);
}
throw new SpelEvaluationException(
@ -200,12 +203,10 @@ public class Indexer extends SpelNodeImpl {
return (this.children[0] instanceof PropertyOrFieldReference || this.children[0].isCompilable());
}
else if (this.indexedType == IndexedType.OBJECT) {
// If the string name is changing the accessor is clearly going to change (so compilation is not possible)
if (this.cachedReadAccessor != null &&
(this.cachedReadAccessor instanceof ReflectivePropertyAccessor.OptimalPropertyAccessor) &&
(getChild(0) instanceof StringLiteral)) {
return true;
}
// If the string name is changing the accessor is clearly going to change (so no compilation possible)
return (this.cachedReadAccessor != null &&
this.cachedReadAccessor instanceof ReflectivePropertyAccessor.OptimalPropertyAccessor &&
getChild(0) instanceof StringLiteral);
}
return false;
}
@ -283,7 +284,8 @@ public class Indexer extends SpelNodeImpl {
this.children[0].generateCode(mv, cf);
cf.exitCompilationScope();
}
mv.visitMethodInsn(INVOKEINTERFACE, "java/util/Map", "get", "(Ljava/lang/Object;)Ljava/lang/Object;", true);
mv.visitMethodInsn(
INVOKEINTERFACE, "java/util/Map", "get", "(Ljava/lang/Object;)Ljava/lang/Object;", true);
}
else if (this.indexedType == IndexedType.OBJECT) {
@ -592,11 +594,11 @@ public class Indexer extends SpelNodeImpl {
}
}
catch (AccessException ex) {
throw new SpelEvaluationException(getStartPosition(), ex, SpelMessage.INDEXING_NOT_SUPPORTED_FOR_TYPE,
this.targetObjectTypeDescriptor.toString());
throw new SpelEvaluationException(getStartPosition(), ex,
SpelMessage.INDEXING_NOT_SUPPORTED_FOR_TYPE, this.targetObjectTypeDescriptor.toString());
}
throw new SpelEvaluationException(getStartPosition(), SpelMessage.INDEXING_NOT_SUPPORTED_FOR_TYPE,
this.targetObjectTypeDescriptor.toString());
throw new SpelEvaluationException(getStartPosition(),
SpelMessage.INDEXING_NOT_SUPPORTED_FOR_TYPE, this.targetObjectTypeDescriptor.toString());
}
@Override
@ -612,8 +614,8 @@ public class Indexer extends SpelNodeImpl {
accessor.write(this.evaluationContext, this.targetObject, this.name, newValue);
return;
}
List<PropertyAccessor> accessorsToTry =
AstUtils.getPropertyAccessorsToTry(contextObjectClass, this.evaluationContext.getPropertyAccessors());
List<PropertyAccessor> accessorsToTry = AstUtils.getPropertyAccessorsToTry(
contextObjectClass, this.evaluationContext.getPropertyAccessors());
for (PropertyAccessor accessor : accessorsToTry) {
if (accessor.canWrite(this.evaluationContext, this.targetObject, this.name)) {
Indexer.this.cachedWriteName = this.name;
@ -625,8 +627,8 @@ public class Indexer extends SpelNodeImpl {
}
}
catch (AccessException ex) {
throw new SpelEvaluationException(getStartPosition(), ex, SpelMessage.EXCEPTION_DURING_PROPERTY_WRITE,
this.name, ex.getMessage());
throw new SpelEvaluationException(getStartPosition(), ex,
SpelMessage.EXCEPTION_DURING_PROPERTY_WRITE, this.name, ex.getMessage());
}
}
@ -652,11 +654,12 @@ public class Indexer extends SpelNodeImpl {
private final int maximumSize;
public CollectionIndexingValueRef(Collection collection, int index, TypeDescriptor collectionEntryTypeDescriptor,
public CollectionIndexingValueRef(Collection collection, int index, TypeDescriptor collectionEntryDescriptor,
TypeConverter typeConverter, boolean growCollection, int maximumSize) {
this.collection = collection;
this.index = index;
this.collectionEntryDescriptor = collectionEntryTypeDescriptor;
this.collectionEntryDescriptor = collectionEntryDescriptor;
this.typeConverter = typeConverter;
this.growCollection = growCollection;
this.maximumSize = maximumSize;
@ -707,13 +710,15 @@ public class Indexer extends SpelNodeImpl {
throw new SpelEvaluationException(getStartPosition(), SpelMessage.UNABLE_TO_GROW_COLLECTION);
}
if (this.collectionEntryDescriptor.getElementTypeDescriptor() == null) {
throw new SpelEvaluationException(getStartPosition(), SpelMessage.UNABLE_TO_GROW_COLLECTION_UNKNOWN_ELEMENT_TYPE);
throw new SpelEvaluationException(
getStartPosition(), SpelMessage.UNABLE_TO_GROW_COLLECTION_UNKNOWN_ELEMENT_TYPE);
}
TypeDescriptor elementType = this.collectionEntryDescriptor.getElementTypeDescriptor();
try {
Constructor<?> ctor = ReflectionUtils.accessibleConstructor(elementType.getType());
int newElements = this.index - this.collection.size();
while (newElements >= 0) {
(this.collection).add(ReflectionUtils.accessibleConstructor(elementType.getType()).newInstance());
this.collection.add(ctor.newInstance());
newElements--;
}
}

@ -0,0 +1,156 @@
/*
* Copyright 2002-2018 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
*
* http://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.ArrayList;
import java.util.HashMap;
import org.junit.Test;
import org.springframework.expression.Expression;
import org.springframework.expression.ExpressionParser;
import org.springframework.expression.spel.standard.SpelExpressionParser;
import org.springframework.expression.spel.support.StandardEvaluationContext;
import static org.junit.Assert.*;
/**
* SpelEvaluationException tests (SPR-16544).
*
* @author Juergen Hoeller
* @author DJ Kulkarni
*/
public class SpelExceptionTests {
@Test(expected = SpelEvaluationException.class)
public void spelExpressionMapNullVariables() {
ExpressionParser parser = new SpelExpressionParser();
Expression spelExpression = parser.parseExpression("#aMap.containsKey('one')");
spelExpression.getValue();
}
@Test(expected = SpelEvaluationException.class)
public void spelExpressionMapIndexAccessNullVariables() {
ExpressionParser parser = new SpelExpressionParser();
Expression spelExpression = parser.parseExpression("#aMap['one'] eq 1");
spelExpression.getValue();
}
@Test
@SuppressWarnings("serial")
public void spelExpressionMapWithVariables() {
ExpressionParser parser = new SpelExpressionParser();
Expression spelExpression = parser.parseExpression("#aMap['one'] eq 1");
StandardEvaluationContext ctx = new StandardEvaluationContext();
ctx.setVariables(new HashMap<String, Object>() {
{
put("aMap", new HashMap<String, Integer>() {
{
put("one", 1);
put("two", 2);
put("three", 3);
}
});
}
});
boolean result = spelExpression.getValue(ctx, Boolean.class);
assertTrue(result);
}
@Test(expected = SpelEvaluationException.class)
public void spelExpressionListNullVariables() {
ExpressionParser parser = new SpelExpressionParser();
Expression spelExpression = parser.parseExpression("#aList.contains('one')");
spelExpression.getValue();
}
@Test(expected = SpelEvaluationException.class)
public void spelExpressionListIndexAccessNullVariables() {
ExpressionParser parser = new SpelExpressionParser();
Expression spelExpression = parser.parseExpression("#aList[0] eq 'one'");
spelExpression.getValue();
}
@Test
@SuppressWarnings("serial")
public void spelExpressionListWithVariables() {
ExpressionParser parser = new SpelExpressionParser();
Expression spelExpression = parser.parseExpression("#aList.contains('one')");
StandardEvaluationContext ctx = new StandardEvaluationContext();
ctx.setVariables(new HashMap<String, Object>() {
{
put("aList", new ArrayList<String>() {
{
add("one");
add("two");
add("three");
}
});
}
});
boolean result = spelExpression.getValue(ctx, Boolean.class);
assertTrue(result);
}
@Test
@SuppressWarnings("serial")
public void spelExpressionListIndexAccessWithVariables() {
ExpressionParser parser = new SpelExpressionParser();
Expression spelExpression = parser.parseExpression("#aList[0] eq 'one'");
StandardEvaluationContext ctx = new StandardEvaluationContext();
ctx.setVariables(new HashMap<String, Object>() {
{
put("aList", new ArrayList<String>() {
{
add("one");
add("two");
add("three");
}
});
}
});
boolean result = spelExpression.getValue(ctx, Boolean.class);
assertTrue(result);
}
@Test(expected = SpelEvaluationException.class)
public void spelExpressionArrayIndexAccessNullVariables() {
ExpressionParser parser = new SpelExpressionParser();
Expression spelExpression = parser.parseExpression("#anArray[0] eq 1");
spelExpression.getValue();
}
@Test
@SuppressWarnings("serial")
public void spelExpressionArrayWithVariables() {
ExpressionParser parser = new SpelExpressionParser();
Expression spelExpression = parser.parseExpression("#anArray[0] eq 1");
StandardEvaluationContext ctx = new StandardEvaluationContext();
ctx.setVariables(new HashMap<String, Object>() {
{
put("anArray", new int[] {1,2,3});
}
});
boolean result = spelExpression.getValue(ctx, Boolean.class);
assertTrue(result);
}
}
Loading…
Cancel
Save