Limit auto grow collection size when using SpEL

Provide an additional constructor on SpelParserConfiguration that can
be used to limit the maximum size that a collection will auto grow when
being accessed via a SpEL expression.

This constraint is particularly useful when SpEL is used with data
binding as it prevents a malicious user from crafting a request that
causes OutOfMemory exceptions.

Issue: SPR-10229

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2009 the original author or authors.
+ * Copyright 2002-2013 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.
@@ -20,6 +20,7 @@ package org.springframework.expression.spel;
  * Configuration object for the SpEL expression parser.
  *
  * @author Juergen Hoeller
+ * @author Phillip Webb
  * @since 3.0
  * @see org.springframework.expression.spel.standard.SpelExpressionParser#SpelExpressionParser(SpelParserConfiguration)
  */
@@ -29,19 +30,52 @@ public class SpelParserConfiguration {
 
 	private final boolean autoGrowCollections;
 
+	private int maximumAutoGrowSize;
 
+
+	/**
+	 * Create a new {@link SpelParserConfiguration} instance.
+	 * @param autoGrowNullReferences if null references should automatically grow
+	 * @param autoGrowCollections if collections should automatically grow
+	 * @see #SpelParserConfiguration(boolean, boolean, int)
+	 */
 	public SpelParserConfiguration(boolean autoGrowNullReferences, boolean autoGrowCollections) {
+		this(autoGrowNullReferences, autoGrowCollections, Integer.MAX_VALUE);
+	}
+
+	/**
+	 * Create a new {@link SpelParserConfiguration} instance.
+	 * @param autoGrowNullReferences if null references should automatically grow
+	 * @param autoGrowCollections if collections should automatically grow
+	 * @param maximumAutoGrowSize the maximum size that the collection can auto grow
+	 */
+	public SpelParserConfiguration(boolean autoGrowNullReferences,
+			boolean autoGrowCollections, int maximumAutoGrowSize) {
 		this.autoGrowNullReferences = autoGrowNullReferences;
 		this.autoGrowCollections = autoGrowCollections;
+		this.maximumAutoGrowSize = maximumAutoGrowSize;
 	}
 
 
+	/**
+	 * @return {@code true} if {@code null} references should be automatically grown
+	 */
 	public boolean isAutoGrowNullReferences() {
 		return this.autoGrowNullReferences;
 	}
 
+	/**
+	 * @return {@code true} if collections should be automatically grown
+	 */
 	public boolean isAutoGrowCollections() {
 		return this.autoGrowCollections;
 	}
 
+	/**
+	 * @return the maximum size that a collection can auto grow
+	 */
+	public int getMaximumAutoGrowSize() {
+		return this.maximumAutoGrowSize;
+	}
+
 }

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2013 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.
@@ -38,6 +38,7 @@ import org.springframework.expression.spel.support.ReflectivePropertyAccessor;
  * (lists/sets)/arrays
  *
  * @author Andy Clement
+ * @author Phillip Webb
  * @since 3.0
  */
 // TODO support multidimensional arrays
@@ -257,25 +258,20 @@ public class Indexer extends SpelNodeImpl {
 
 		private final boolean growCollection;
 
+		private int maximumSize;
+
 		CollectionIndexingValueRef(Collection collection, int index, TypeDescriptor collectionEntryTypeDescriptor,
-				TypeConverter typeConverter, boolean growCollection) {
+				TypeConverter typeConverter, boolean growCollection, int maximumSize) {
 			this.collection = collection;
 			this.index = index;
 			this.collectionEntryTypeDescriptor = collectionEntryTypeDescriptor;
 			this.typeConverter = typeConverter;
 			this.growCollection = growCollection;
+			this.maximumSize = maximumSize;
 		}
 
 		public TypedValue getValue() {
-			if (this.index >= this.collection.size()) {
-				if (this.growCollection) {
-					growCollection(this.collectionEntryTypeDescriptor, this.index, this.collection);
-				}
-				else {
-					throw new SpelEvaluationException(getStartPosition(), SpelMessage.COLLECTION_INDEX_OUT_OF_BOUNDS,
-							this.collection.size(), this.index);
-				}
-			}
+			growCollectionIfNecessary();
 			if (this.collection instanceof List) {
 				Object o = ((List) this.collection).get(this.index);
 				return new TypedValue(o, this.collectionEntryTypeDescriptor.elementTypeDescriptor(o));
@@ -291,15 +287,7 @@ public class Indexer extends SpelNodeImpl {
 		}
 
 		public void setValue(Object newValue) {
-			if (this.index >= this.collection.size()) {
-				if (this.growCollection) {
-					growCollection(this.collectionEntryTypeDescriptor, this.index, this.collection);
-				}
-				else {
-					throw new SpelEvaluationException(getStartPosition(), SpelMessage.COLLECTION_INDEX_OUT_OF_BOUNDS,
-							this.collection.size(), this.index);
-				}
-			}
+			growCollectionIfNecessary();
 			if (this.collection instanceof List) {
 				List list = (List) this.collection;
 				if (this.collectionEntryTypeDescriptor.getElementTypeDescriptor() != null) {
@@ -314,6 +302,36 @@ public class Indexer extends SpelNodeImpl {
 			}
 		}
 
+		private void growCollectionIfNecessary() {
+			if (this.index >= this.collection.size()) {
+
+				if (!this.growCollection) {
+					throw new SpelEvaluationException(getStartPosition(), SpelMessage.COLLECTION_INDEX_OUT_OF_BOUNDS,
+							this.collection.size(), this.index);
+				}
+
+				if(this.index >= this.maximumSize) {
+					throw new SpelEvaluationException(getStartPosition(), SpelMessage.UNABLE_TO_GROW_COLLECTION);
+				}
+
+				if (this.collectionEntryTypeDescriptor.getElementTypeDescriptor() == null) {
+					throw new SpelEvaluationException(getStartPosition(), SpelMessage.UNABLE_TO_GROW_COLLECTION_UNKNOWN_ELEMENT_TYPE);
+				}
+
+				TypeDescriptor elementType = this.collectionEntryTypeDescriptor.getElementTypeDescriptor();
+				try {
+					int newElements = this.index - this.collection.size();
+					while (newElements >= 0) {
+						(this.collection).add(elementType.getType().newInstance());
+						newElements--;
+					}
+				}
+				catch (Exception ex) {
+					throw new SpelEvaluationException(getStartPosition(), ex, SpelMessage.UNABLE_TO_GROW_COLLECTION);
+				}
+			}
+		}
+
 		public boolean isWritable() {
 			return true;
 		}
@@ -403,7 +421,8 @@ public class Indexer extends SpelNodeImpl {
 			}
 			else if (targetObject instanceof Collection) {
 				return new CollectionIndexingValueRef((Collection<?>) targetObject, idx, targetObjectTypeDescriptor,
-						state.getTypeConverter(), state.getConfiguration().isAutoGrowCollections());
+						state.getTypeConverter(), state.getConfiguration().isAutoGrowCollections(),
+						state.getConfiguration().getMaximumAutoGrowSize());
 			}
 			else if (targetObject instanceof String) {
 				return new StringIndexingLValue((String) targetObject, idx, targetObjectTypeDescriptor);
@@ -421,32 +440,6 @@ public class Indexer extends SpelNodeImpl {
 				targetObjectTypeDescriptor.toString());
 	}
 
-	/**
-	 * Attempt to grow the specified collection so that the specified index is valid.
-	 * @param targetType the type of the elements in the collection
-	 * @param index the index into the collection that needs to be valid
-	 * @param collection the collection to grow with elements
-	 */
-	private void growCollection(TypeDescriptor targetType, int index, Collection<Object> collection) {
-		if (targetType.getElementTypeDescriptor() == null) {
-			throw new SpelEvaluationException(getStartPosition(), SpelMessage.UNABLE_TO_GROW_COLLECTION_UNKNOWN_ELEMENT_TYPE);
-		}
-		TypeDescriptor elementType = targetType.getElementTypeDescriptor();
-		Object newCollectionElement;
-		try {
-			int newElements = index - collection.size();
-			while (newElements > 0) {
-				collection.add(elementType.getType().newInstance());
-				newElements--;
-			}
-			newCollectionElement = elementType.getType().newInstance();
-		}
-		catch (Exception ex) {
-			throw new SpelEvaluationException(getStartPosition(), ex, SpelMessage.UNABLE_TO_GROW_COLLECTION);
-		}
-		collection.add(newCollectionElement);
-	}
-
 	@Override
 	public String toStringAST() {
 		StringBuilder sb = new StringBuilder();

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2013 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,7 +16,14 @@
 
 package org.springframework.expression.spel;
 
-import static org.junit.Assert.*;
+import static org.hamcrest.Matchers.equalTo;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 import java.lang.reflect.Method;
 import java.util.ArrayList;
@@ -24,7 +31,6 @@ import java.util.List;
 import java.util.Map;
 
 import org.junit.Test;
-
 import org.springframework.core.convert.TypeDescriptor;
 import org.springframework.expression.AccessException;
 import org.springframework.expression.BeanResolver;
@@ -48,6 +54,7 @@ import org.springframework.expression.spel.testresources.TestPerson;
  * @author Andy Clement
  * @author Mark Fisher
  * @author Sam Brannen
+ * @author Phillip Webb
  * @since 3.0
  */
 public class EvaluationTests extends ExpressionTestCase {
@@ -708,6 +715,23 @@ public class EvaluationTests extends ExpressionTestCase {
 		}
 	}
 
+	@Test
+	public void limitCollectionGrowing() throws Exception {
+		TestClass instance = new TestClass();
+		StandardEvaluationContext ctx = new StandardEvaluationContext(instance);
+		SpelExpressionParser parser = new SpelExpressionParser( new SpelParserConfiguration(true, true, 3));
+		Expression expression = parser.parseExpression("foo[2]");
+		expression.setValue(ctx, "2");
+		assertThat(instance.getFoo().size(), equalTo(3));
+		expression = parser.parseExpression("foo[3]");
+		try {
+			expression.setValue(ctx, "3");
+		} catch(SpelEvaluationException see) {
+			assertEquals(SpelMessage.UNABLE_TO_GROW_COLLECTION, see.getMessageCode());
+			assertThat(instance.getFoo().size(), equalTo(3));
+		}
+	}
+
 	// For now I am making #this not assignable
 	@Test
 	public void increment01root() {