From bc5affa79a67b28c858730dd69156ebf6161164a Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 11 Dec 2013 20:49:18 +0100 Subject: [PATCH] Made BeanUtils.copyProperties defensive about property type mismatches Issue: SPR-11209 --- .../org/springframework/beans/BeanUtils.java | 47 +++++++++-------- .../springframework/beans/BeanUtilsTests.java | 51 ++++++++++++++++--- 2 files changed, 69 insertions(+), 29 deletions(-) diff --git a/spring-beans/src/main/java/org/springframework/beans/BeanUtils.java b/spring-beans/src/main/java/org/springframework/beans/BeanUtils.java index 018cf240d3..6efe51880f 100644 --- a/spring-beans/src/main/java/org/springframework/beans/BeanUtils.java +++ b/spring-beans/src/main/java/org/springframework/beans/BeanUtils.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. @@ -199,7 +199,7 @@ public abstract class BeanUtils { * @return the Method object, or {@code null} if not found * @see Class#getDeclaredMethod */ - public static Method findDeclaredMethod(Class clazz, String methodName, Class[] paramTypes) { + public static Method findDeclaredMethod(Class clazz, String methodName, Class... paramTypes) { try { return clazz.getDeclaredMethod(methodName, paramTypes); } @@ -455,7 +455,7 @@ public abstract class BeanUtils { * @param beanClasses the classes to check against * @return the property type, or {@code Object.class} as fallback */ - public static Class findPropertyType(String propertyName, Class[] beanClasses) { + public static Class findPropertyType(String propertyName, Class... beanClasses) { if (beanClasses != null) { for (Class beanClass : beanClasses) { PropertyDescriptor pd = getPropertyDescriptor(beanClass, propertyName); @@ -528,7 +528,7 @@ public abstract class BeanUtils { * @see BeanWrapper */ public static void copyProperties(Object source, Object target) throws BeansException { - copyProperties(source, target, null, null); + copyProperties(source, target, null, (String[]) null); } /** @@ -548,7 +548,7 @@ public abstract class BeanUtils { public static void copyProperties(Object source, Object target, Class editable) throws BeansException { - copyProperties(source, target, editable, null); + copyProperties(source, target, editable, (String[]) null); } /** @@ -565,7 +565,7 @@ public abstract class BeanUtils { * @throws BeansException if the copying failed * @see BeanWrapper */ - public static void copyProperties(Object source, Object target, String[] ignoreProperties) + public static void copyProperties(Object source, Object target, String... ignoreProperties) throws BeansException { copyProperties(source, target, null, ignoreProperties); @@ -583,7 +583,7 @@ public abstract class BeanUtils { * @throws BeansException if the copying failed * @see BeanWrapper */ - private static void copyProperties(Object source, Object target, Class editable, String[] ignoreProperties) + private static void copyProperties(Object source, Object target, Class editable, String... ignoreProperties) throws BeansException { Assert.notNull(source, "Source must not be null"); @@ -601,24 +601,27 @@ public abstract class BeanUtils { List ignoreList = (ignoreProperties != null) ? Arrays.asList(ignoreProperties) : null; for (PropertyDescriptor targetPd : targetPds) { - if (targetPd.getWriteMethod() != null && - (ignoreProperties == null || (!ignoreList.contains(targetPd.getName())))) { + Method writeMethod = targetPd.getWriteMethod(); + if (writeMethod != null && (ignoreProperties == null || (!ignoreList.contains(targetPd.getName())))) { PropertyDescriptor sourcePd = getPropertyDescriptor(source.getClass(), targetPd.getName()); - if (sourcePd != null && sourcePd.getReadMethod() != null) { - try { - Method readMethod = sourcePd.getReadMethod(); - if (!Modifier.isPublic(readMethod.getDeclaringClass().getModifiers())) { - readMethod.setAccessible(true); + if (sourcePd != null) { + Method readMethod = sourcePd.getReadMethod(); + if (readMethod != null && + writeMethod.getParameterTypes()[0].isAssignableFrom(readMethod.getReturnType())) { + try { + if (!Modifier.isPublic(readMethod.getDeclaringClass().getModifiers())) { + readMethod.setAccessible(true); + } + Object value = readMethod.invoke(source); + if (!Modifier.isPublic(writeMethod.getDeclaringClass().getModifiers())) { + writeMethod.setAccessible(true); + } + writeMethod.invoke(target, value); } - Object value = readMethod.invoke(source); - Method writeMethod = targetPd.getWriteMethod(); - if (!Modifier.isPublic(writeMethod.getDeclaringClass().getModifiers())) { - writeMethod.setAccessible(true); + catch (Throwable ex) { + throw new FatalBeanException( + "Could not copy property '" + targetPd.getName() + "' from source to target", ex); } - writeMethod.invoke(target, value); - } - catch (Throwable ex) { - throw new FatalBeanException("Could not copy properties from source to target", ex); } } } diff --git a/spring-beans/src/test/java/org/springframework/beans/BeanUtilsTests.java b/spring-beans/src/test/java/org/springframework/beans/BeanUtilsTests.java index e0149909e7..685fbcf764 100644 --- a/spring-beans/src/test/java/org/springframework/beans/BeanUtilsTests.java +++ b/spring-beans/src/test/java/org/springframework/beans/BeanUtilsTests.java @@ -16,8 +16,6 @@ package org.springframework.beans; -import static org.junit.Assert.*; - import java.beans.Introspector; import java.beans.PropertyDescriptor; import java.lang.reflect.Method; @@ -25,6 +23,7 @@ import java.util.ArrayList; import java.util.List; import org.junit.Test; + import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.propertyeditors.CustomDateEditor; import org.springframework.core.io.Resource; @@ -33,6 +32,8 @@ import org.springframework.tests.sample.beans.DerivedTestBean; import org.springframework.tests.sample.beans.ITestBean; import org.springframework.tests.sample.beans.TestBean; +import static org.junit.Assert.*; + /** * Unit tests for {@link BeanUtils}. * @@ -78,8 +79,7 @@ public final class BeanUtilsTests { @Test public void testBeanPropertyIsArray() { PropertyDescriptor[] descriptors = BeanUtils.getPropertyDescriptors(ContainerBean.class); - for (int i = 0; i < descriptors.length; i++) { - PropertyDescriptor descriptor = descriptors[i]; + for (PropertyDescriptor descriptor : descriptors) { if ("containedBeans".equals(descriptor.getName())) { assertTrue("Property should be an array", descriptor.getPropertyType().isArray()); assertEquals(descriptor.getPropertyType().getComponentType(), ContainedBean.class); @@ -170,7 +170,7 @@ public final class BeanUtilsTests { assertTrue("Touchy empty", tb2.getTouchy() == null); // "spouse", "touchy", "age" should not be copied - BeanUtils.copyProperties(tb, tb2, new String[]{"spouse", "touchy", "age"}); + BeanUtils.copyProperties(tb, tb2, "spouse", "touchy", "age"); assertTrue("Name copied", tb2.getName() == null); assertTrue("Age still empty", tb2.getAge() == 0); assertTrue("Touchy still empty", tb2.getTouchy() == null); @@ -181,7 +181,16 @@ public final class BeanUtilsTests { NameAndSpecialProperty source = new NameAndSpecialProperty(); source.setName("name"); TestBean target = new TestBean(); - BeanUtils.copyProperties(source, target, new String[]{"specialProperty"}); + BeanUtils.copyProperties(source, target, "specialProperty"); + assertEquals(target.getName(), "name"); + } + + @Test + public void testCopyPropertiesWithInvalidProperty() { + InvalidProperty source = new InvalidProperty(); + source.setName("name"); + InvalidProperty target = new InvalidProperty(); + BeanUtils.copyProperties(source, target); assertEquals(target.getName(), "name"); } @@ -256,7 +265,8 @@ public final class BeanUtilsTests { assertEquals(String.class, keyDescr.getPropertyType()); for (PropertyDescriptor propertyDescriptor : descrs) { if (propertyDescriptor.getName().equals(keyDescr.getName())) { - assertEquals(propertyDescriptor.getName() + " has unexpected type", keyDescr.getPropertyType(), propertyDescriptor.getPropertyType()); + assertEquals(propertyDescriptor.getName() + " has unexpected type", + keyDescr.getPropertyType(), propertyDescriptor.getPropertyType()); } } } @@ -291,6 +301,31 @@ public final class BeanUtilsTests { } + @SuppressWarnings("unused") + private static class InvalidProperty { + + private String name; + + private String value; + + public void setName(String name) { + this.name = name; + } + + public String getName() { + return this.name; + } + + public void setValue(int value) { + this.value = Integer.toString(value); + } + + public String getValue() { + return this.value; + } + } + + @SuppressWarnings("unused") private static class ContainerBean { @@ -346,6 +381,7 @@ public final class BeanUtilsTests { } } + private interface MapEntry { K getKey(); @@ -357,6 +393,7 @@ public final class BeanUtilsTests { void setValue(V value); } + private static class Bean implements MapEntry { private String key;