Skip to content

Commit 045ee52

Browse files
sbrannensbrannen
committed
Invoke target.toString() safely in ReflectionTestUtils
ReflectionTestUtils invokes toString() on target objects in order to build exception and logging messages, and prior to this commit problems could occur if the invocation of toString() threw an exception. This commit addresses this issue by ensuring that all invocations of toString() on target objects within ReflectionTestUtils are performed safely within try-catch blocks. Issue: SPR-14363
1 parent 2fd4462 commit 045ee52

File tree

5 files changed

+128
-33
lines changed

5 files changed

+128
-33
lines changed

‎spring-test/src/main/java/org/springframework/test/util/ReflectionTestUtils.java

Lines changed: 29 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -173,14 +173,14 @@ public static void setField(Object targetObject, Class<?> targetClass, String na
173173
Field field = ReflectionUtils.findField(targetClass, name, type);
174174
if (field == null) {
175175
throw new IllegalArgumentException(String.format(
176-
"Could not find field '%s' of type [%s] on target object [%s] or target class [%s]", name, type,
177-
ultimateTarget, targetClass));
176+
"Could not find field '%s' of type [%s] on %s or target class [%s]", name, type,
177+
safeToString(ultimateTarget), targetClass));
178178
}
179179

180180
if (logger.isDebugEnabled()) {
181181
logger.debug(String.format(
182-
"Setting field '%s' of type [%s] on target object [%s] or target class [%s] to value [%s]", name, type,
183-
ultimateTarget, targetClass, value));
182+
"Setting field '%s' of type [%s] on %s or target class [%s] to value [%s]", name, type,
183+
safeToString(ultimateTarget), targetClass, value));
184184
}
185185
ReflectionUtils.makeAccessible(field);
186186
ReflectionUtils.setField(field, ultimateTarget, value);
@@ -253,14 +253,13 @@ public static Object getField(Object targetObject, Class<?> targetClass, String
253253

254254
Field field = ReflectionUtils.findField(targetClass, name);
255255
if (field == null) {
256-
throw new IllegalArgumentException(
257-
String.format("Could not find field '%s' on target object [%s] or target class [%s]", name,
258-
ultimateTarget, targetClass));
256+
throw new IllegalArgumentException(String.format("Could not find field '%s' on %s or target class [%s]",
257+
name, safeToString(ultimateTarget), targetClass));
259258
}
260259

261260
if (logger.isDebugEnabled()) {
262-
logger.debug(String.format("Getting field '%s' from target object [%s] or target class [%s]", name,
263-
ultimateTarget, targetClass));
261+
logger.debug(String.format("Getting field '%s' from %s or target class [%s]", name,
262+
safeToString(ultimateTarget), targetClass));
264263
}
265264
ReflectionUtils.makeAccessible(field);
266265
return ReflectionUtils.getField(field, ultimateTarget);
@@ -327,13 +326,16 @@ public static void invokeSetterMethod(Object target, String name, Object value,
327326
method = ReflectionUtils.findMethod(target.getClass(), setterMethodName, paramTypes);
328327
}
329328
if (method == null) {
330-
throw new IllegalArgumentException("Could not find setter method '" + setterMethodName +
331-
"' on target [" + target + "] with parameter type [" + type + "]");
329+
throw new IllegalArgumentException(String.format(
330+
"Could not find setter method '%s' on %s with parameter type [%s]", setterMethodName,
331+
safeToString(target), type));
332332
}
333333

334334
if (logger.isDebugEnabled()) {
335-
logger.debug("Invoking setter method '" + setterMethodName + "' on target [" + target + "]");
335+
logger.debug(String.format("Invoking setter method '%s' on %s with value [%s]", setterMethodName,
336+
safeToString(target), value));
336337
}
338+
337339
ReflectionUtils.makeAccessible(method);
338340
ReflectionUtils.invokeMethod(method, target, value);
339341
}
@@ -372,12 +374,12 @@ public static Object invokeGetterMethod(Object target, String name) {
372374
method = ReflectionUtils.findMethod(target.getClass(), getterMethodName);
373375
}
374376
if (method == null) {
375-
throw new IllegalArgumentException("Could not find getter method '" + getterMethodName +
376-
"' on target [" + target + "]");
377+
throw new IllegalArgumentException(String.format(
378+
"Could not find getter method '%s' on %s", getterMethodName, safeToString(target)));
377379
}
378380

379381
if (logger.isDebugEnabled()) {
380-
logger.debug("Invoking getter method '" + getterMethodName + "' on target [" + target + "]");
382+
logger.debug(String.format("Invoking getter method '%s' on %s", getterMethodName, safeToString(target)));
381383
}
382384
ReflectionUtils.makeAccessible(method);
383385
return ReflectionUtils.invokeMethod(method, target);
@@ -412,8 +414,8 @@ public static <T> T invokeMethod(Object target, String name, Object... args) {
412414
methodInvoker.prepare();
413415

414416
if (logger.isDebugEnabled()) {
415-
logger.debug("Invoking method '" + name + "' on target [" + target + "] with arguments [" +
416-
ObjectUtils.nullSafeToString(args) + "]");
417+
logger.debug(String.format("Invoking method '%s' on %s with arguments %s", name, safeToString(target),
418+
ObjectUtils.nullSafeToString(args)));
417419
}
418420

419421
return (T) methodInvoker.invoke();
@@ -424,4 +426,14 @@ public static <T> T invokeMethod(Object target, String name, Object... args) {
424426
}
425427
}
426428

429+
private static String safeToString(Object target) {
430+
try {
431+
return String.format("target object [%s]", target);
432+
}
433+
catch (Exception ex) {
434+
return String.format("target of type [%s] whose toString() method threw [%s]",
435+
(target != null ? target.getClass().getName() : "unknown"), ex);
436+
}
437+
}
438+
427439
}

‎spring-test/src/test/java/org/springframework/test/util/ReflectionTestUtilsTests.java

Lines changed: 35 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,8 @@ public class ReflectionTestUtilsTests {
4848

4949
private final Component component = new Component();
5050

51+
private final LegacyEntity entity = new LegacyEntity();
52+
5153
@Rule
5254
public final ExpectedException exception = ExpectedException.none();
5355

@@ -202,17 +204,6 @@ public void setFieldWithNullValueForPrimitiveBoolean() throws Exception {
202204
setField(person, "likesPets", null, boolean.class);
203205
}
204206

205-
/**
206-
* Verifies behavior requested in <a href="https://jira.spring.io/browse/SPR-9571">SPR-9571</a>.
207-
*/
208-
@Test
209-
public void setFieldOnLegacyEntityWithSideEffectsInToString() {
210-
String testCollaborator = "test collaborator";
211-
LegacyEntity entity = new LegacyEntity();
212-
setField(entity, "collaborator", testCollaborator, Object.class);
213-
assertTrue(entity.toString().contains(testCollaborator));
214-
}
215-
216207
@Test
217208
public void setStaticFieldViaClass() throws Exception {
218209
setField(StaticFields.class, "publicField", "xxx");
@@ -398,4 +389,37 @@ public void invokeMethodWithTooManyArguments() {
398389
invokeMethod(component, "configure", new Integer(42), "enigma", "baz", "quux");
399390
}
400391

392+
@Test // SPR-14363
393+
public void getFieldOnLegacyEntityWithSideEffectsInToString() {
394+
Object collaborator = getField(entity, "collaborator");
395+
assertNotNull(collaborator);
396+
}
397+
398+
@Test // SPR-9571 and SPR-14363
399+
public void setFieldOnLegacyEntityWithSideEffectsInToString() {
400+
String testCollaborator = "test collaborator";
401+
setField(entity, "collaborator", testCollaborator, Object.class);
402+
assertTrue(entity.toString().contains(testCollaborator));
403+
}
404+
405+
@Test // SPR-14363
406+
public void invokeMethodOnLegacyEntityWithSideEffectsInToString() {
407+
invokeMethod(entity, "configure", new Integer(42), "enigma");
408+
assertEquals("number should have been configured", new Integer(42), entity.getNumber());
409+
assertEquals("text should have been configured", "enigma", entity.getText());
410+
}
411+
412+
@Test // SPR-14363
413+
public void invokeGetterMethodOnLegacyEntityWithSideEffectsInToString() {
414+
Object collaborator = invokeGetterMethod(entity, "collaborator");
415+
assertNotNull(collaborator);
416+
}
417+
418+
@Test // SPR-14363
419+
public void invokeSetterMethodOnLegacyEntityWithSideEffectsInToString() {
420+
String testCollaborator = "test collaborator";
421+
invokeSetterMethod(entity, "collaborator", testCollaborator);
422+
assertTrue(entity.toString().contains(testCollaborator));
423+
}
424+
401425
}

‎spring-test/src/test/java/org/springframework/test/util/subpackage/LegacyEntity.java

Lines changed: 29 additions & 5 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-2016 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.
@@ -31,17 +31,41 @@ public class LegacyEntity {
3131

3232
@Override
3333
public String toString() {
34-
throw new RuntimeException(
34+
throw new LegacyEntityException(
3535
"Invoking toString() on the default collaborator causes an undesirable side effect");
36-
};
36+
}
3737
};
3838

39+
private Integer number;
40+
private String text;
41+
42+
43+
public void configure(Integer number, String text) {
44+
this.number = number;
45+
this.text = text;
46+
}
47+
48+
public Integer getNumber() {
49+
return this.number;
50+
}
51+
52+
public String getText() {
53+
return this.text;
54+
}
55+
56+
public Object getCollaborator() {
57+
return this.collaborator;
58+
}
59+
60+
public void setCollaborator(Object collaborator) {
61+
this.collaborator = collaborator;
62+
}
3963

4064
@Override
4165
public String toString() {
4266
return new ToStringCreator(this)//
43-
.append("collaborator", this.collaborator)//
44-
.toString();
67+
.append("collaborator", this.collaborator)//
68+
.toString();
4569
}
4670

4771
}

‎spring-test/src/test/java/org/springframework/test/util/subpackage/LegacyEntityException.java

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
/*
2+
* Copyright 2002-2016 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.test.util.subpackage;
18+
19+
/**
20+
* Exception thrown by a {@link LegacyEntity}.
21+
*
22+
* @author Sam Brannen
23+
* @since 4.3.1
24+
*/
25+
@SuppressWarnings("serial")
26+
public class LegacyEntityException extends RuntimeException {
27+
28+
public LegacyEntityException(String message) {
29+
super(message);
30+
}
31+
32+
}

‎spring-test/src/test/resources/log4j.properties

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@ log4j.logger.org.springframework.test.context.web=WARN
2525
#log4j.logger.org.springframework.test.context.support.AbstractGenericContextLoader=INFO
2626
#log4j.logger.org.springframework.test.context.support.AnnotationConfigContextLoader=INFO
2727

28+
# The following must be kept at DEBUG in order to test SPR-14363.
29+
log4j.logger.org.springframework.test.util=DEBUG
30+
2831
log4j.logger.org.springframework.test.web.servlet.result=DEBUG
2932

3033
#log4j.logger.org.springframework.test=TRACE

0 commit comments

Comments
 (0)