Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixed #6375 XmlConfiguration always checks for setter method #6398

Merged
merged 4 commits into from Jun 15, 2021
Merged

Fixed #6375 XmlConfiguration always checks for setter method #6398

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8e9a576
Fixed #6375 XmlConfiguration always checks for setter method
gregw Jun 11, 2021
43fadef
Fixed #6375 XmlConfiguration always checks for setter method
gregw Jun 11, 2021
5f157bc
Update jetty-xml/src/test/java/org/eclipse/jetty/xml/XmlConfiguration…
gregw Jun 12, 2021
69199c6
Merge branch 'jetty-10.0.x-6375-xmlSetWithWrongName' of github.com:ec…
gregw Jun 14, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
50 changes: 31 additions & 19 deletions jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlConfiguration.java
View file
Open in desktop
Expand Up @@ -515,38 +515,50 @@ 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 = 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;

Class<?> oClass = nodeClass(node);
if (oClass != null)
obj = null;
else
oClass = obj.getClass();

// Look for a property value
if (property != null)
{
Map<String, String> properties = _configuration.getProperties();
propertyValue = properties.get(property);
// If no property value, then do not set
if (propertyValue == null)
{
// 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
gregw marked this conversation as resolved.
Show resolved Hide resolved
return;
}
}

String name = "set" + attr.substring(0, 1).toUpperCase(Locale.ENGLISH) + attr.substring(1);
Object value = value(obj, node);
if (value == null)
value = propertyValue;
Object[] arg = {value};

Class<?> oClass = nodeClass(node);
if (oClass != null)
obj = null;
else
oClass = obj.getClass();

Class<?>[] vClass = {Object.class};
if (value != null)
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 @@ -557,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 @@ -572,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 @@ -585,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 @@ -622,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 @@ -650,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 @@ -703,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
51 changes: 51 additions & 0 deletions jetty-xml/src/test/java/org/eclipse/jetty/xml/XmlConfigurationTest.java
View file
Open in desktop
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 @@ -363,6 +387,33 @@ public void testSetWithNullPropertyAndValue() throws Exception
assertNull(configuration.getIdMap().get("test"));
}

@Test
public void testSetWithWrongNameAndProperty() throws Exception
{
XmlConfiguration configuration = asXmlConfiguration("<Configure class=\"org.eclipse.jetty.xml.TestConfiguration\"><Set name=\"WrongName\" property=\"prop\" id=\"test\"/></Configure>");
configuration.getProperties().put("prop", "This is a property value");
TestConfiguration tc = new TestConfiguration();
tc.setTestString("default");

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

@Test
public void testSetWithWrongNameAndNullProperty() throws Exception
{
XmlConfiguration configuration = asXmlConfiguration("<Configure class=\"org.eclipse.jetty.xml.TestConfiguration\"><Set name=\"WrongName\" property=\"prop\" id=\"test\"/></Configure>");
configuration.getProperties().remove("prop");
TestConfiguration tc = new TestConfiguration();
tc.setTestString("default");

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

@Test
public void testMeaningfullSetException() throws Exception
{
Expand Down