DataBinder uses a default limit of 256 for array/collection auto-growing (SPR-7842)

git-svn-id: https://src.springframework.org/svn/spring-framework/trunk@4652 50f2f4bb-b051-0410-bef5-90022cba6387

org.springframework.beans/src/main/java/org/springframework/beans/BeanWrapper.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2009 the original author or authors.
+ * Copyright 2002-2011 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.
@@ -92,4 +92,15 @@ public interface BeanWrapper extends ConfigurablePropertyAccessor {
 	 */
 	boolean isAutoGrowNestedPaths();
 
+	/**
+	 * Specify a limit for array and collection auto-growing.
+	 * <p>Default is unlimited on a plain BeanWrapper.
+	 */
+	void setAutoGrowCollectionLimit(int autoGrowCollectionLimit);
+
+	/**
+	 * Return the limit for array and collection auto-growing.
+	 */
+	int getAutoGrowCollectionLimit();
+
 }

org.springframework.beans/src/main/java/org/springframework/beans/BeanWrapperImpl.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2010 the original author or authors.
+ * Copyright 2002-2011 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.
@@ -37,6 +37,7 @@ import java.util.Set;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+
 import org.springframework.core.CollectionFactory;
 import org.springframework.core.GenericCollectionTypeResolver;
 import org.springframework.core.MethodParameter;
@@ -119,6 +120,8 @@ public class BeanWrapperImpl extends AbstractPropertyAccessor implements BeanWra
 
 	private boolean autoGrowNestedPaths = false;
 
+	private int autoGrowCollectionLimit = Integer.MAX_VALUE;
+
 
 	/**
 	 * Create new empty BeanWrapperImpl. Wrapped instance needs to be set afterwards.
@@ -183,6 +186,7 @@ public class BeanWrapperImpl extends AbstractPropertyAccessor implements BeanWra
 		setWrappedInstance(object, nestedPath, superBw.getWrappedInstance());
 		setExtractOldValueForEditor(superBw.isExtractOldValueForEditor());
 		setAutoGrowNestedPaths(superBw.isAutoGrowNestedPaths());
+		setAutoGrowCollectionLimit(superBw.getAutoGrowCollectionLimit());
 		setConversionService(superBw.getConversionService());
 		setSecurityContext(superBw.acc);
 	}
@@ -250,22 +254,38 @@ public class BeanWrapperImpl extends AbstractPropertyAccessor implements BeanWra
 	}
 
 	/**
-	 * If this BeanWrapper should "auto grow" nested paths.
-	 * When true, auto growth is triggered on nested paths when null values are encountered.
-	 * When true, auto growth is triggered on collection properties when out of bounds indexes are accessed.
-	 * Default is false.
+	 * Set whether this BeanWrapper should attempt to "auto-grow" a nested path that contains a null value.
+	 * <p>If "true", a null path location will be populated with a default object value and traversed
+	 * instead of resulting in a {@link NullValueInNestedPathException}. Turning this flag on also
+	 * enables auto-growth of collection elements when accessing an out-of-bounds index.
+	 * <p>Default is "false" on a plain BeanWrapper.
 	 */
 	public void setAutoGrowNestedPaths(boolean autoGrowNestedPaths) {
 		this.autoGrowNestedPaths = autoGrowNestedPaths;
 	}
 
 	/**
-	 * If this BeanWrapper should "auto grow" nested paths.
+	 * Return whether "auto-growing" of nested paths has been activated.
 	 */
 	public boolean isAutoGrowNestedPaths() {
 		return this.autoGrowNestedPaths;
 	}
 
+	/**
+	 * Specify a limit for array and collection auto-growing.
+	 * <p>Default is unlimited on a plain BeanWrapper.
+	 */
+	public void setAutoGrowCollectionLimit(int autoGrowCollectionLimit) {
+		this.autoGrowCollectionLimit = autoGrowCollectionLimit;
+	}
+
+	/**
+	 * Return the limit for array and collection auto-growing.
+	 */
+	public int getAutoGrowCollectionLimit() {
+		return this.autoGrowCollectionLimit;
+	}
+
 	/**
 	 * Set the security context used during the invocation of the wrapped instance methods.
 	 * Can be null.
@@ -832,7 +852,7 @@ public class BeanWrapperImpl extends AbstractPropertyAccessor implements BeanWra
 			return array;
 		}
 		int length = Array.getLength(array);
-		if (index >= length) {
+		if (index >= length && index < this.autoGrowCollectionLimit) {
 			Class<?> componentType = array.getClass().getComponentType();
 			Object newArray = Array.newInstance(componentType, index + 1);
 			System.arraycopy(array, 0, newArray, 0, length);
@@ -855,7 +875,8 @@ public class BeanWrapperImpl extends AbstractPropertyAccessor implements BeanWra
 		if (!this.autoGrowNestedPaths) {
 			return;
 		}
-		if (index >= collection.size()) {
+		int size = collection.size();
+		if (index >= size && index < this.autoGrowCollectionLimit) {
 			Class elementType = GenericCollectionTypeResolver.getCollectionReturnType(pd.getReadMethod(), nestingLevel);
 			if (elementType != null) {
 				for (int i = collection.size(); i < index + 1; i++) {

org.springframework.beans/src/test/java/org/springframework/beans/BeanWrapperAutoGrowingTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2010 the original author or authors.
+ * Copyright 2002-2011 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.
@@ -19,10 +19,11 @@ package org.springframework.beans;
 import java.util.List;
 import java.util.Map;
 
-import static org.junit.Assert.*;
 import org.junit.Before;
 import org.junit.Test;
 
+import static org.junit.Assert.*;
+
 /**
  * @author Keith Donald
  * @author Juergen Hoeller
@@ -117,6 +118,19 @@ public class BeanWrapperAutoGrowingTests {
 		assertNotNull(wrapper.getPropertyValue("list[3]"));
 	}
 
+	@Test
+	public void getPropertyValueAutoGrowListFailsAgainstLimit() {
+		wrapper.setAutoGrowCollectionLimit(2);
+		try {
+			assertNotNull(wrapper.getPropertyValue("list[4]"));
+			fail("Should have thrown InvalidPropertyException");
+		}
+		catch (InvalidPropertyException ex) {
+			// expected
+			assertTrue(ex.getRootCause() instanceof IndexOutOfBoundsException);
+		}
+	}
+
 	@Test
 	public void getPropertyValueAutoGrowMultiDimensionalList() {
 		assertNotNull(wrapper.getPropertyValue("multiList[0][0]"));
@@ -135,6 +149,12 @@ public class BeanWrapperAutoGrowingTests {
 		assertTrue(bean.getMap().get("A") instanceof Bean);
 	}
 
+	@Test
+	public void setNestedPropertyValueAutoGrowMap() {
+		wrapper.setPropertyValue("map[A].nested", new Bean());
+		assertTrue(bean.getMap().get("A").getNested() instanceof Bean);
+	}
+
 
 	public static class Bean {
 

org.springframework.context/src/main/java/org/springframework/validation/BeanPropertyBindingResult.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2009 the original author or authors.
+ * Copyright 2002-2011 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.
@@ -47,6 +47,8 @@ public class BeanPropertyBindingResult extends AbstractPropertyBindingResult imp
 
 	private final boolean autoGrowNestedPaths;
 
+	private final int autoGrowCollectionLimit;
+
 	private transient BeanWrapper beanWrapper;
 
 
@@ -56,7 +58,7 @@ public class BeanPropertyBindingResult extends AbstractPropertyBindingResult imp
 	 * @param objectName the name of the target object
 	 */
 	public BeanPropertyBindingResult(Object target, String objectName) {
-		this(target, objectName, true);
+		this(target, objectName, true, Integer.MAX_VALUE);
 	}
 
 	/**
@@ -64,11 +66,13 @@ public class BeanPropertyBindingResult extends AbstractPropertyBindingResult imp
 	 * @param target the target bean to bind onto
 	 * @param objectName the name of the target object
 	 * @param autoGrowNestedPaths whether to "auto-grow" a nested path that contains a null value
+	 * @param autoGrowCollectionLimit the limit for array and collection auto-growing
 	 */
-	public BeanPropertyBindingResult(Object target, String objectName, boolean autoGrowNestedPaths) {
+	public BeanPropertyBindingResult(Object target, String objectName, boolean autoGrowNestedPaths, int autoGrowCollectionLimit) {
 		super(objectName);
 		this.target = target;
 		this.autoGrowNestedPaths = autoGrowNestedPaths;
+		this.autoGrowCollectionLimit = autoGrowCollectionLimit;
 	}
 
 
@@ -88,6 +92,7 @@ public class BeanPropertyBindingResult extends AbstractPropertyBindingResult imp
 			this.beanWrapper = createBeanWrapper();
 			this.beanWrapper.setExtractOldValueForEditor(true);
 			this.beanWrapper.setAutoGrowNestedPaths(this.autoGrowNestedPaths);
+			this.beanWrapper.setAutoGrowCollectionLimit(this.autoGrowCollectionLimit);
 		}
 		return this.beanWrapper;
 	}

org.springframework.context/src/main/java/org/springframework/validation/DataBinder.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2010 the original author or authors.
+ * Copyright 2002-2011 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.
@@ -105,6 +105,9 @@ public class DataBinder implements PropertyEditorRegistry, TypeConverter {
 	/** Default object name used for binding: "target" */
 	public static final String DEFAULT_OBJECT_NAME = "target";
 
+	/** Default limit for array and collection growing: 256 */
+	public static final int DEFAULT_AUTO_GROW_COLLECTION_LIMIT = 256;
+
 
 	/**
 	 * We'll create a lot of DataBinder instances: Let's use a static logger.
@@ -127,6 +130,8 @@ public class DataBinder implements PropertyEditorRegistry, TypeConverter {
 
 	private boolean autoGrowNestedPaths = true;
 
+	private int autoGrowCollectionLimit = DEFAULT_AUTO_GROW_COLLECTION_LIMIT;
+
 	private String[] allowedFields;
 
 	private String[] disallowedFields;
@@ -199,6 +204,22 @@ public class DataBinder implements PropertyEditorRegistry, TypeConverter {
 		return this.autoGrowNestedPaths;
 	}
 
+	/**
+	 * Specify the limit for array and collection auto-growing.
+	 * <p>Default is 256, preventing OutOfMemoryErrors in case of large indexes.
+	 * Raise this limit if your auto-growing needs are unusually high.
+	 */
+	public void setAutoGrowCollectionLimit(int autoGrowCollectionLimit) {
+		this.autoGrowCollectionLimit = autoGrowCollectionLimit;
+	}
+
+	/**
+	 * Return the current limit for array and collection auto-growing.
+	 */
+	public int getAutoGrowCollectionLimit() {
+		return this.autoGrowCollectionLimit;
+	}
+
 	/**
 	 * Initialize standard JavaBean property access for this DataBinder.
 	 * <p>This is the default; an explicit call just leads to eager initialization.
@@ -207,7 +228,8 @@ public class DataBinder implements PropertyEditorRegistry, TypeConverter {
 	public void initBeanPropertyAccess() {
 		Assert.state(this.bindingResult == null,
 				"DataBinder is already initialized - call initBeanPropertyAccess before other configuration methods");
-		this.bindingResult = new BeanPropertyBindingResult(getTarget(), getObjectName(), isAutoGrowNestedPaths());
+		this.bindingResult = new BeanPropertyBindingResult(
+				getTarget(), getObjectName(), isAutoGrowNestedPaths(), getAutoGrowCollectionLimit());
 		if (this.conversionService != null) {
 			this.bindingResult.initConversion(this.conversionService);
 		}

org.springframework.context/src/test/java/org/springframework/validation/DataBinderTests.java

@@ -34,6 +34,7 @@ import org.springframework.beans.BeanWithObjectProperty;
 import org.springframework.beans.DerivedTestBean;
 import org.springframework.beans.ITestBean;
 import org.springframework.beans.IndexedTestBean;
+import org.springframework.beans.InvalidPropertyException;
 import org.springframework.beans.MutablePropertyValues;
 import org.springframework.beans.NotWritablePropertyException;
 import org.springframework.beans.NullValueInNestedPathException;
@@ -1487,7 +1488,6 @@ public class DataBinderTests extends TestCase {
 		MutablePropertyValues mpvs = new MutablePropertyValues();
 		mpvs.add("name", name);
 		mpvs.add("beanName", beanName);
-
 		binder.bind(mpvs);
 
 		assertEquals(name, testBean.getName());
@@ -1496,6 +1496,62 @@ public class DataBinderTests extends TestCase {
 		assertEquals("beanName", disallowedFields[0]);
 	}
 
+	public void testAutoGrowWithinDefaultLimit() throws Exception {
+		TestBean testBean = new TestBean();
+		DataBinder binder = new DataBinder(testBean, "testBean");
+
+		MutablePropertyValues mpvs = new MutablePropertyValues();
+		mpvs.add("stringArray[4]", "");
+		binder.bind(mpvs);
+
+		assertEquals(5, testBean.getStringArray().length);
+	}
+
+	public void testAutoGrowBeyondDefaultLimit() throws Exception {
+		TestBean testBean = new TestBean();
+		DataBinder binder = new DataBinder(testBean, "testBean");
+
+		MutablePropertyValues mpvs = new MutablePropertyValues();
+		mpvs.add("stringArray[256]", "");
+		try {
+			binder.bind(mpvs);
+			fail("Should have thrown InvalidPropertyException");
+		}
+		catch (InvalidPropertyException ex) {
+			// expected
+			assertTrue(ex.getRootCause() instanceof IndexOutOfBoundsException);
+		}
+	}
+
+	public void testAutoGrowWithinCustomLimit() throws Exception {
+		TestBean testBean = new TestBean();
+		DataBinder binder = new DataBinder(testBean, "testBean");
+		binder.setAutoGrowCollectionLimit(10);
+
+		MutablePropertyValues mpvs = new MutablePropertyValues();
+		mpvs.add("stringArray[4]", "");
+		binder.bind(mpvs);
+
+		assertEquals(5, testBean.getStringArray().length);
+	}
+
+	public void testAutoGrowBeyondCustomLimit() throws Exception {
+		TestBean testBean = new TestBean();
+		DataBinder binder = new DataBinder(testBean, "testBean");
+		binder.setAutoGrowCollectionLimit(10);
+
+		MutablePropertyValues mpvs = new MutablePropertyValues();
+		mpvs.add("stringArray[16]", "");
+		try {
+			binder.bind(mpvs);
+			fail("Should have thrown InvalidPropertyException");
+		}
+		catch (InvalidPropertyException ex) {
+			// expected
+			assertTrue(ex.getRootCause() instanceof IndexOutOfBoundsException);
+		}
+	}
+
 
 	private static class BeanWithIntegerList {