From 4c75054f9041279423f31754aa23f9ca781e9d43 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Sun, 3 Jul 2011 19:26:49 +0000 Subject: [PATCH] DataBinder uses a default limit of 256 for array/collection auto-growing (SPR-7842) --- .../springframework/beans/BeanWrapper.java | 13 ++++- .../beans/BeanWrapperImpl.java | 37 +++++++++--- .../beans/BeanWrapperAutoGrowingTests.java | 24 +++++++- .../validation/BeanPropertyBindingResult.java | 11 +++- .../validation/DataBinder.java | 26 ++++++++- .../validation/DataBinderTests.java | 58 ++++++++++++++++++- 6 files changed, 152 insertions(+), 17 deletions(-) diff --git a/org.springframework.beans/src/main/java/org/springframework/beans/BeanWrapper.java b/org.springframework.beans/src/main/java/org/springframework/beans/BeanWrapper.java index 3469d8506a..aea348d6fd 100644 --- a/org.springframework.beans/src/main/java/org/springframework/beans/BeanWrapper.java +++ b/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. + *

Default is unlimited on a plain BeanWrapper. + */ + void setAutoGrowCollectionLimit(int autoGrowCollectionLimit); + + /** + * Return the limit for array and collection auto-growing. + */ + int getAutoGrowCollectionLimit(); + } diff --git a/org.springframework.beans/src/main/java/org/springframework/beans/BeanWrapperImpl.java b/org.springframework.beans/src/main/java/org/springframework/beans/BeanWrapperImpl.java index 26f7390f7b..ea4ee8ea2c 100644 --- a/org.springframework.beans/src/main/java/org/springframework/beans/BeanWrapperImpl.java +++ b/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. + *

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. + *

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. + *

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++) { diff --git a/org.springframework.beans/src/test/java/org/springframework/beans/BeanWrapperAutoGrowingTests.java b/org.springframework.beans/src/test/java/org/springframework/beans/BeanWrapperAutoGrowingTests.java index 691b89c78d..469b22cdaf 100644 --- a/org.springframework.beans/src/test/java/org/springframework/beans/BeanWrapperAutoGrowingTests.java +++ b/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 { diff --git a/org.springframework.context/src/main/java/org/springframework/validation/BeanPropertyBindingResult.java b/org.springframework.context/src/main/java/org/springframework/validation/BeanPropertyBindingResult.java index b974d6ca75..a2a82a1226 100644 --- a/org.springframework.context/src/main/java/org/springframework/validation/BeanPropertyBindingResult.java +++ b/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; } diff --git a/org.springframework.context/src/main/java/org/springframework/validation/DataBinder.java b/org.springframework.context/src/main/java/org/springframework/validation/DataBinder.java index c44833c30b..0d39ff6eb7 100644 --- a/org.springframework.context/src/main/java/org/springframework/validation/DataBinder.java +++ b/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. + *

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. *

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); } diff --git a/org.springframework.context/src/test/java/org/springframework/validation/DataBinderTests.java b/org.springframework.context/src/test/java/org/springframework/validation/DataBinderTests.java index 753748ff7d..c1436825f7 100644 --- a/org.springframework.context/src/test/java/org/springframework/validation/DataBinderTests.java +++ b/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 { -- GitLab