Skip to content

Commit bc5affa

Browse files
jhoellerjhoeller
committed
Made BeanUtils.copyProperties defensive about property type mismatches
Issue: SPR-11209
1 parent 92dad18 commit bc5affa

File tree

2 files changed

+69
-29
lines changed

2 files changed

+69
-29
lines changed

‎spring-beans/src/main/java/org/springframework/beans/BeanUtils.java‎

Lines changed: 25 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2012 the original author or authors.
2+
* Copyright 2002-2013 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -199,7 +199,7 @@ public static Method findMethod(Class<?> clazz, String methodName, Class<?>... p
199199
* @return the Method object, or {@code null} if not found
200200
* @see Class#getDeclaredMethod
201201
*/
202-
public static Method findDeclaredMethod(Class<?> clazz, String methodName, Class<?>[] paramTypes) {
202+
public static Method findDeclaredMethod(Class<?> clazz, String methodName, Class<?>... paramTypes) {
203203
try {
204204
return clazz.getDeclaredMethod(methodName, paramTypes);
205205
}
@@ -455,7 +455,7 @@ public static PropertyEditor findEditorByConvention(Class<?> targetType) {
455455
* @param beanClasses the classes to check against
456456
* @return the property type, or {@code Object.class} as fallback
457457
*/
458-
public static Class<?> findPropertyType(String propertyName, Class<?>[] beanClasses) {
458+
public static Class<?> findPropertyType(String propertyName, Class<?>... beanClasses) {
459459
if (beanClasses != null) {
460460
for (Class<?> beanClass : beanClasses) {
461461
PropertyDescriptor pd = getPropertyDescriptor(beanClass, propertyName);
@@ -528,7 +528,7 @@ public static boolean isSimpleValueType(Class<?> clazz) {
528528
* @see BeanWrapper
529529
*/
530530
public static void copyProperties(Object source, Object target) throws BeansException {
531-
copyProperties(source, target, null, null);
531+
copyProperties(source, target, null, (String[]) null);
532532
}
533533

534534
/**
@@ -548,7 +548,7 @@ public static void copyProperties(Object source, Object target) throws BeansExce
548548
public static void copyProperties(Object source, Object target, Class<?> editable)
549549
throws BeansException {
550550

551-
copyProperties(source, target, editable, null);
551+
copyProperties(source, target, editable, (String[]) null);
552552
}
553553

554554
/**
@@ -565,7 +565,7 @@ public static void copyProperties(Object source, Object target, Class<?> editabl
565565
* @throws BeansException if the copying failed
566566
* @see BeanWrapper
567567
*/
568-
public static void copyProperties(Object source, Object target, String[] ignoreProperties)
568+
public static void copyProperties(Object source, Object target, String... ignoreProperties)
569569
throws BeansException {
570570

571571
copyProperties(source, target, null, ignoreProperties);
@@ -583,7 +583,7 @@ public static void copyProperties(Object source, Object target, String[] ignoreP
583583
* @throws BeansException if the copying failed
584584
* @see BeanWrapper
585585
*/
586-
private static void copyProperties(Object source, Object target, Class<?> editable, String[] ignoreProperties)
586+
private static void copyProperties(Object source, Object target, Class<?> editable, String... ignoreProperties)
587587
throws BeansException {
588588

589589
Assert.notNull(source, "Source must not be null");
@@ -601,24 +601,27 @@ private static void copyProperties(Object source, Object target, Class<?> editab
601601
List<String> ignoreList = (ignoreProperties != null) ? Arrays.asList(ignoreProperties) : null;
602602

603603
for (PropertyDescriptor targetPd : targetPds) {
604-
if (targetPd.getWriteMethod() != null &&
605-
(ignoreProperties == null || (!ignoreList.contains(targetPd.getName())))) {
604+
Method writeMethod = targetPd.getWriteMethod();
605+
if (writeMethod != null && (ignoreProperties == null || (!ignoreList.contains(targetPd.getName())))) {
606606
PropertyDescriptor sourcePd = getPropertyDescriptor(source.getClass(), targetPd.getName());
607-
if (sourcePd != null && sourcePd.getReadMethod() != null) {
608-
try {
609-
Method readMethod = sourcePd.getReadMethod();
610-
if (!Modifier.isPublic(readMethod.getDeclaringClass().getModifiers())) {
611-
readMethod.setAccessible(true);
607+
if (sourcePd != null) {
608+
Method readMethod = sourcePd.getReadMethod();
609+
if (readMethod != null &&
610+
writeMethod.getParameterTypes()[0].isAssignableFrom(readMethod.getReturnType())) {
611+
try {
612+
if (!Modifier.isPublic(readMethod.getDeclaringClass().getModifiers())) {
613+
readMethod.setAccessible(true);
614+
}
615+
Object value = readMethod.invoke(source);
616+
if (!Modifier.isPublic(writeMethod.getDeclaringClass().getModifiers())) {
617+
writeMethod.setAccessible(true);
618+
}
619+
writeMethod.invoke(target, value);
612620
}
613-
Object value = readMethod.invoke(source);
614-
Method writeMethod = targetPd.getWriteMethod();
615-
if (!Modifier.isPublic(writeMethod.getDeclaringClass().getModifiers())) {
616-
writeMethod.setAccessible(true);
621+
catch (Throwable ex) {
622+
throw new FatalBeanException(
623+
"Could not copy property '" + targetPd.getName() + "' from source to target", ex);
617624
}
618-
writeMethod.invoke(target, value);
619-
}
620-
catch (Throwable ex) {
621-
throw new FatalBeanException("Could not copy properties from source to target", ex);
622625
}
623626
}
624627
}

‎spring-beans/src/test/java/org/springframework/beans/BeanUtilsTests.java‎

Lines changed: 44 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,14 @@
1616

1717
package org.springframework.beans;
1818

19-
import static org.junit.Assert.*;
20-
2119
import java.beans.Introspector;
2220
import java.beans.PropertyDescriptor;
2321
import java.lang.reflect.Method;
2422
import java.util.ArrayList;
2523
import java.util.List;
2624

2725
import org.junit.Test;
26+
2827
import org.springframework.beans.factory.BeanFactory;
2928
import org.springframework.beans.propertyeditors.CustomDateEditor;
3029
import org.springframework.core.io.Resource;
@@ -33,6 +32,8 @@
3332
import org.springframework.tests.sample.beans.ITestBean;
3433
import org.springframework.tests.sample.beans.TestBean;
3534

35+
import static org.junit.Assert.*;
36+
3637
/**
3738
* Unit tests for {@link BeanUtils}.
3839
*
@@ -78,8 +79,7 @@ public void testGetPropertyDescriptors() throws Exception {
7879
@Test
7980
public void testBeanPropertyIsArray() {
8081
PropertyDescriptor[] descriptors = BeanUtils.getPropertyDescriptors(ContainerBean.class);
81-
for (int i = 0; i < descriptors.length; i++) {
82-
PropertyDescriptor descriptor = descriptors[i];
82+
for (PropertyDescriptor descriptor : descriptors) {
8383
if ("containedBeans".equals(descriptor.getName())) {
8484
assertTrue("Property should be an array", descriptor.getPropertyType().isArray());
8585
assertEquals(descriptor.getPropertyType().getComponentType(), ContainedBean.class);
@@ -170,7 +170,7 @@ public void testCopyPropertiesWithIgnore() throws Exception {
170170
assertTrue("Touchy empty", tb2.getTouchy() == null);
171171

172172
// "spouse", "touchy", "age" should not be copied
173-
BeanUtils.copyProperties(tb, tb2, new String[]{"spouse", "touchy", "age"});
173+
BeanUtils.copyProperties(tb, tb2, "spouse", "touchy", "age");
174174
assertTrue("Name copied", tb2.getName() == null);
175175
assertTrue("Age still empty", tb2.getAge() == 0);
176176
assertTrue("Touchy still empty", tb2.getTouchy() == null);
@@ -181,7 +181,16 @@ public void testCopyPropertiesWithIgnoredNonExistingProperty() {
181181
NameAndSpecialProperty source = new NameAndSpecialProperty();
182182
source.setName("name");
183183
TestBean target = new TestBean();
184-
BeanUtils.copyProperties(source, target, new String[]{"specialProperty"});
184+
BeanUtils.copyProperties(source, target, "specialProperty");
185+
assertEquals(target.getName(), "name");
186+
}
187+
188+
@Test
189+
public void testCopyPropertiesWithInvalidProperty() {
190+
InvalidProperty source = new InvalidProperty();
191+
source.setName("name");
192+
InvalidProperty target = new InvalidProperty();
193+
BeanUtils.copyProperties(source, target);
185194
assertEquals(target.getName(), "name");
186195
}
187196

@@ -256,7 +265,8 @@ public void testSPR6063() {
256265
assertEquals(String.class, keyDescr.getPropertyType());
257266
for (PropertyDescriptor propertyDescriptor : descrs) {
258267
if (propertyDescriptor.getName().equals(keyDescr.getName())) {
259-
assertEquals(propertyDescriptor.getName() + " has unexpected type", keyDescr.getPropertyType(), propertyDescriptor.getPropertyType());
268+
assertEquals(propertyDescriptor.getName() + " has unexpected type",
269+
keyDescr.getPropertyType(), propertyDescriptor.getPropertyType());
260270
}
261271
}
262272
}
@@ -291,6 +301,31 @@ public int getSpecialProperty() {
291301
}
292302

293303

304+
@SuppressWarnings("unused")
305+
private static class InvalidProperty {
306+
307+
private String name;
308+
309+
private String value;
310+
311+
public void setName(String name) {
312+
this.name = name;
313+
}
314+
315+
public String getName() {
316+
return this.name;
317+
}
318+
319+
public void setValue(int value) {
320+
this.value = Integer.toString(value);
321+
}
322+
323+
public String getValue() {
324+
return this.value;
325+
}
326+
}
327+
328+
294329
@SuppressWarnings("unused")
295330
private static class ContainerBean {
296331

@@ -346,6 +381,7 @@ public void doSomethingWithAMultiDimensionalArray(String[][] strings) {
346381
}
347382
}
348383

384+
349385
private interface MapEntry<K, V> {
350386

351387
K getKey();
@@ -357,6 +393,7 @@ private interface MapEntry<K, V> {
357393
void setValue(V value);
358394
}
359395

396+
360397
private static class Bean implements MapEntry<String, String> {
361398

362399
private String key;

0 commit comments

Comments
 (0)