Skip to content

Commit

Permalink
Fixed #6375 XmlConfiguration always checks for setter method
Browse files Browse the repository at this point in the history 
Also check for public fields

Signed-off-by: Greg Wilkins <gregw@webtide.com>
  • Loading branch information
@gregw
gregw committed Jun 11, 2021
1 parent 8e9a576 commit 43fadef
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 16 deletions.
37 changes: 21 additions & 16 deletions jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlConfiguration.java
View file
Expand Up @@ -515,8 +515,8 @@ public void configure(Object obj, XmlParser.Node cfg, int i) throws Exception
*/
private void set(Object obj, XmlParser.Node node) throws Exception
{
String attr = node.getAttribute("name");
String name = "set" + attr.substring(0, 1).toUpperCase(Locale.ENGLISH) + attr.substring(1);
String name = node.getAttribute("name");
String setter = "set" + name.substring(0, 1).toUpperCase(Locale.ENGLISH) + name.substring(1);
String id = node.getAttribute("id");
String property = node.getAttribute("property");
String propertyValue = null;
Expand All @@ -535,9 +535,14 @@ private void set(Object obj, XmlParser.Node node) throws Exception
// If no property value, then do not set
if (propertyValue == null)
{
// check that there is at least one setter that could have matched
if (Arrays.stream(oClass.getMethods()).noneMatch(m -> m.getName().equals(name)))
throw new NoSuchMethodException(String.format("No '%s' on %s", name, oClass.getName()));
// check that there is at least one setter or field that could have matched
if (Arrays.stream(oClass.getMethods()).noneMatch(m -> m.getName().equals(setter)) &&
Arrays.stream(oClass.getFields()).filter(f -> Modifier.isPublic(f.getModifiers())).noneMatch(f -> f.getName().equals(name)))
{
NoSuchMethodException e = new NoSuchMethodException(String.format("No method '%s' on %s", setter, oClass.getName()));
e.addSuppressed(new NoSuchFieldException(String.format("No field '%s' on %s", name, oClass.getName())));
throw e;
}
// otherwise it is a noop
return;
}
Expand All @@ -553,7 +558,7 @@ private void set(Object obj, XmlParser.Node node) throws Exception
vClass[0] = value.getClass();

if (LOG.isDebugEnabled())
LOG.debug("XML {}.{} ({})", (obj != null ? obj.toString() : oClass.getName()), name, value);
LOG.debug("XML {}.{} ({})", (obj != null ? obj.toString() : oClass.getName()), setter, value);

MultiException me = new MultiException();

Expand All @@ -564,7 +569,7 @@ private void set(Object obj, XmlParser.Node node) throws Exception
// Try for trivial match
try
{
Method set = oClass.getMethod(name, vClass);
Method set = oClass.getMethod(setter, vClass);
invokeMethod(set, obj, arg);
return;
}
Expand All @@ -579,7 +584,7 @@ private void set(Object obj, XmlParser.Node node) throws Exception
{
Field type = vClass[0].getField("TYPE");
vClass[0] = (Class<?>)type.get(null);
Method set = oClass.getMethod(name, vClass);
Method set = oClass.getMethod(setter, vClass);
invokeMethod(set, obj, arg);
return;
}
Expand All @@ -592,7 +597,7 @@ private void set(Object obj, XmlParser.Node node) throws Exception
// Try a field
try
{
Field field = oClass.getField(attr);
Field field = oClass.getField(name);
if (Modifier.isPublic(field.getModifiers()))
{
try
Expand Down Expand Up @@ -629,18 +634,18 @@ private void set(Object obj, XmlParser.Node node) throws Exception
// Search for a match by trying all the set methods
Method[] sets = oClass.getMethods();
Method set = null;
for (Method setter : sets)
for (Method s : sets)
{
if (setter.getParameterCount() != 1)
if (s.getParameterCount() != 1)
continue;
Class<?>[] paramTypes = setter.getParameterTypes();
if (name.equals(setter.getName()))
Class<?>[] paramTypes = s.getParameterTypes();
if (setter.equals(s.getName()))
{
types = types == null ? paramTypes[0].getName() : (types + "," + paramTypes[0].getName());
// lets try it
try
{
set = setter;
set = s;
invokeMethod(set, obj, arg);
return;
}
Expand All @@ -657,7 +662,7 @@ private void set(Object obj, XmlParser.Node node) throws Exception
if (paramTypes[0].isAssignableFrom(c))
{
setValue = convertArrayToCollection(value, c);
invokeMethod(setter, obj, setValue);
invokeMethod(s, obj, setValue);
return;
}
}
Expand Down Expand Up @@ -710,7 +715,7 @@ private void set(Object obj, XmlParser.Node node) throws Exception
}

// No Joy
String message = oClass + "." + name + "(" + vClass[0] + ")";
String message = oClass + "." + setter + "(" + vClass[0] + ")";
if (types != null)
message += ". Found setters for " + types;
NoSuchMethodException failure = new NoSuchMethodException(message);
Expand Down
25 changes: 25 additions & 0 deletions jetty-xml/src/test/java/org/eclipse/jetty/xml/XmlConfigurationTest.java
View file
Expand Up @@ -327,6 +327,30 @@ public void testSetWithProperty() throws Exception
assertEquals(configuration.getIdMap().get("test"), "This is a property value");
}

@Test
public void testSetFieldWithProperty() throws Exception
{
XmlConfiguration configuration = asXmlConfiguration("<Configure class=\"org.eclipse.jetty.xml.TestConfiguration\"><Set name=\"testField1\" property=\"prop\" id=\"test\"/></Configure>");
configuration.getProperties().put("prop", "42");
TestConfiguration tc = new TestConfiguration();
tc.testField1 = -1;
configuration.configure(tc);
assertEquals(42, tc.testField1);
assertEquals(configuration.getIdMap().get("test"), "42");
}

@Test
public void testNotSetFieldWithNullProperty() throws Exception
{
XmlConfiguration configuration = asXmlConfiguration("<Configure class=\"org.eclipse.jetty.xml.TestConfiguration\"><Set name=\"testField1\" property=\"prop\" id=\"test\"/></Configure>");
configuration.getProperties().remove("prop");
TestConfiguration tc = new TestConfiguration();
tc.testField1 = -1;
configuration.configure(tc);
assertEquals(-1, tc.testField1);
assertEquals(configuration.getIdMap().get("test"), null);
}

@Test
public void testSetWithNullProperty() throws Exception
{
Expand Down Expand Up @@ -386,6 +410,7 @@ public void testSetWithWrongNameAndNullProperty() throws Exception

NoSuchMethodException e = assertThrows(NoSuchMethodException.class, () -> configuration.configure(tc));
assertThat(e.getMessage(), containsString("setWrongName"));
assertThat(e.getSuppressed()[0], instanceOf(NoSuchFieldException.class));
assertEquals("default", tc.getTestString());
}

Expand Down

0 comments on commit 43fadef

Please sign in to comment.