From 8e9a576e0d81f9638ad7b59fc4d41d07fe3e1fbd Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Fri, 11 Jun 2021 15:28:56 +1000 Subject: [PATCH 1/3] Fixed #6375 XmlConfiguration always checks for setter method Fix #6375 by making XmlConfiguration Set handling always check for a matching setter, even if the property attribute is given but not set. Signed-off-by: Greg Wilkins --- .../eclipse/jetty/xml/XmlConfiguration.java | 21 ++++++++++----- .../jetty/xml/XmlConfigurationTest.java | 26 +++++++++++++++++++ 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlConfiguration.java b/jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlConfiguration.java index 17f5ae4d2617..b92a08e012dc 100644 --- a/jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlConfiguration.java +++ b/jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlConfiguration.java @@ -516,9 +516,17 @@ 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 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) { @@ -526,21 +534,20 @@ private void set(Object obj, XmlParser.Node node) throws Exception propertyValue = properties.get(property); // 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())); + // otherwise it is a noop 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(); diff --git a/jetty-xml/src/test/java/org/eclipse/jetty/xml/XmlConfigurationTest.java b/jetty-xml/src/test/java/org/eclipse/jetty/xml/XmlConfigurationTest.java index 81d00277499c..6d528959d66f 100644 --- a/jetty-xml/src/test/java/org/eclipse/jetty/xml/XmlConfigurationTest.java +++ b/jetty-xml/src/test/java/org/eclipse/jetty/xml/XmlConfigurationTest.java @@ -363,6 +363,32 @@ public void testSetWithNullPropertyAndValue() throws Exception assertNull(configuration.getIdMap().get("test")); } + @Test + public void testSetWithWrongNameAndProperty() throws Exception + { + XmlConfiguration configuration = asXmlConfiguration(""); + 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(""); + configuration.getProperties().remove("prop"); + 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 testMeaningfullSetException() throws Exception { From 43fadef2822ef3e358c239c519aafd86f8c6b336 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Fri, 11 Jun 2021 17:33:05 +1000 Subject: [PATCH 2/3] Fixed #6375 XmlConfiguration always checks for setter method Also check for public fields Signed-off-by: Greg Wilkins --- .../eclipse/jetty/xml/XmlConfiguration.java | 37 +++++++++++-------- .../jetty/xml/XmlConfigurationTest.java | 25 +++++++++++++ 2 files changed, 46 insertions(+), 16 deletions(-) diff --git a/jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlConfiguration.java b/jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlConfiguration.java index b92a08e012dc..4cd59c3a8795 100644 --- a/jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlConfiguration.java +++ b/jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlConfiguration.java @@ -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; @@ -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; } @@ -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(); @@ -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; } @@ -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; } @@ -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 @@ -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; } @@ -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; } } @@ -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); diff --git a/jetty-xml/src/test/java/org/eclipse/jetty/xml/XmlConfigurationTest.java b/jetty-xml/src/test/java/org/eclipse/jetty/xml/XmlConfigurationTest.java index 6d528959d66f..5275ba2c23e1 100644 --- a/jetty-xml/src/test/java/org/eclipse/jetty/xml/XmlConfigurationTest.java +++ b/jetty-xml/src/test/java/org/eclipse/jetty/xml/XmlConfigurationTest.java @@ -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(""); + 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(""); + 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 { @@ -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()); } From 5f157bcab79a2acf3a7b5ac98704fe36c49f5219 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Sat, 12 Jun 2021 15:36:42 +1000 Subject: [PATCH 3/3] Update jetty-xml/src/test/java/org/eclipse/jetty/xml/XmlConfigurationTest.java fixed formatting --- .../test/java/org/eclipse/jetty/xml/XmlConfigurationTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jetty-xml/src/test/java/org/eclipse/jetty/xml/XmlConfigurationTest.java b/jetty-xml/src/test/java/org/eclipse/jetty/xml/XmlConfigurationTest.java index 6d528959d66f..410da7ddf068 100644 --- a/jetty-xml/src/test/java/org/eclipse/jetty/xml/XmlConfigurationTest.java +++ b/jetty-xml/src/test/java/org/eclipse/jetty/xml/XmlConfigurationTest.java @@ -375,8 +375,8 @@ public void testSetWithWrongNameAndProperty() throws Exception assertThat(e.getMessage(), containsString("setWrongName")); assertEquals("default", tc.getTestString()); } + @Test - public void testSetWithWrongNameAndNullProperty() throws Exception { XmlConfiguration configuration = asXmlConfiguration("");