Skip to content

Commit 161060b

Browse files
committed
Reject write methods not starting with "set" in Property
Property.resolveName() located the "set" prefix of a write method with String.indexOf, which matches the token anywhere in the method name. A write method that merely contains "set" (for example offsetX or upset) was silently accepted and resolved to a meaningless property name derived from whatever follows the token, while only names with no "set" token at all were rejected. Match the prefix only at the start of the method name via startsWith, so that write methods that are not setters consistently throw the existing IllegalArgumentException. Extracted from the change originally bundled into gh-36911. Signed-off-by: junhyeong9812 <pickjog@gmail.com>
1 parent c771205 commit 161060b

2 files changed

Lines changed: 85 additions & 4 deletions

File tree

spring-core/src/main/java/org/springframework/core/convert/Property.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -150,12 +150,11 @@ private String resolveName() {
150150
return StringUtils.uncapitalize(this.readMethod.getName().substring(index));
151151
}
152152
else if (this.writeMethod != null) {
153-
int index = this.writeMethod.getName().indexOf("set");
154-
if (index == -1) {
153+
String methodName = this.writeMethod.getName();
154+
if (!methodName.startsWith("set")) {
155155
throw new IllegalArgumentException("Not a setter method");
156156
}
157-
index += 3;
158-
return StringUtils.uncapitalize(this.writeMethod.getName().substring(index));
157+
return StringUtils.uncapitalize(methodName.substring(3));
159158
}
160159
else {
161160
throw new IllegalStateException("Property is neither readable nor writable");
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
/*
2+
* Copyright 2002-present 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+
* https://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.core.convert;
18+
19+
import java.lang.reflect.Method;
20+
21+
import org.junit.jupiter.api.Test;
22+
23+
import static org.assertj.core.api.Assertions.assertThat;
24+
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
25+
26+
/**
27+
* Tests for {@link Property} setter name resolution.
28+
*
29+
* @author Junhyeong Kim
30+
*/
31+
class PropertyTests {
32+
33+
@Test
34+
void resolveNameForSetter() throws Exception {
35+
assertThat(writeProperty("setName").getName()).isEqualTo("name");
36+
}
37+
38+
@Test // no "set" token at all: rejected before and after this change
39+
void rejectNonSetterWriteMethod() {
40+
assertThatIllegalArgumentException()
41+
.isThrownBy(() -> writeProperty("updateName"))
42+
.withMessage("Not a setter method");
43+
}
44+
45+
@Test // "set" embedded mid-name: formerly accepted and resolved to "x"
46+
void rejectWriteMethodEmbeddingSetInName() {
47+
assertThatIllegalArgumentException()
48+
.isThrownBy(() -> writeProperty("offsetX"))
49+
.withMessage("Not a setter method");
50+
}
51+
52+
@Test // "set" at the end of the name: formerly accepted and resolved to ""
53+
void rejectWriteMethodEndingWithSetToken() {
54+
assertThatIllegalArgumentException()
55+
.isThrownBy(() -> writeProperty("upset"))
56+
.withMessage("Not a setter method");
57+
}
58+
59+
60+
private static Property writeProperty(String writeMethodName) throws Exception {
61+
Method writeMethod = TestBean.class.getMethod(writeMethodName, String.class);
62+
return new Property(TestBean.class, null, writeMethod);
63+
}
64+
65+
66+
@SuppressWarnings("unused")
67+
static class TestBean {
68+
69+
public void setName(String name) {
70+
}
71+
72+
public void updateName(String name) {
73+
}
74+
75+
public void offsetX(String value) {
76+
}
77+
78+
public void upset(String value) {
79+
}
80+
}
81+
82+
}

0 commit comments

Comments
 (0)