Issue #4656 generalise XmlConfiguration elements (#4661)

* Issue #4656 generalise XmlConfiguration elements

Extended the use of Id, Class elements

Signed-off-by: Greg Wilkins <gregw@webtide.com>

* Issue #4657 Validation in XmlConfiguration

Default to validate if xerces is in the name
Fixed test harness to prepend DTD.

Signed-off-by: Greg Wilkins <gregw@webtide.com>

* Issue #4657 Validation in XmlConfiguration

WIP on adding validation to XMLs

Signed-off-by: Greg Wilkins <gregw@webtide.com>

* Issue #4656 - Fix bad xml id attribute usage in jetty-maven-plugin

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>

Co-authored-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>

jetty-deploy/src/test/resources/context-binding-test-1.xml

@@ -1,7 +1,9 @@
+<?xml version="1.0"?>
+<!DOCTYPE Configure PUBLIC "-//Jetty//Configure//EN" "https://www.eclipse.org/jetty/configure_10_0.dtd">
 <Configure class="org.eclipse.jetty.webapp.WebAppContext">
   <Call name="getServerClassMatcher">
     <Call name="include">
       <Arg>org.eclipse.foo.</Arg>
-    </Call>`
+    </Call>
   </Call>
 </Configure>

jetty-maven-plugin/src/it/jetty-start-gwt-it/beer-server/src/main/jettyconf/context.xml

@@ -1,3 +1,5 @@
+<?xml version="1.0"?>
+<!DOCTYPE Configure PUBLIC "-//Jetty//Configure//EN" "https://www.eclipse.org/jetty/configure_10_0.dtd">
 <Configure class="org.eclipse.jetty.webapp.WebAppContext">
   <Call name="setInitParameter">
     <Arg>org.eclipse.jetty.servlet.Default.useFileMappedBuffer</Arg>

jetty-maven-plugin/src/main/resources/maven.xml

@@ -4,9 +4,13 @@
 <Configure id="wac" class="org.eclipse.jetty.maven.plugin.MavenWebAppContext">
 
   <Call class="org.eclipse.jetty.maven.plugin.WebAppPropertyConverter" name="fromProperties">
-    <Arg><Ref id="wac"/></Arg>
+    <Arg>
+      <Ref refid="wac" />
+    </Arg>
     <Arg><Property name="jetty.base" default="."/>/etc/maven.props</Arg>
-    <Arg><Ref id="Server"/></Arg>
+    <Arg>
+      <Ref refid="Server" />
+    </Arg>
     <Arg></Arg>
   </Call>
 

jetty-start/src/test/resources/bogus.xml

@@ -1,2 +1,3 @@
 <?xml version="1.0"?>
+<!DOCTYPE Configure PUBLIC "-//Jetty//Configure//EN" "https://www.eclipse.org/jetty/configure_10_0.dtd">
 <Configure />

jetty-websocket/websocket-javax-server/src/test/resources/jetty-websocket-httpclient.xml

@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8"?>
-
+<!DOCTYPE Configure PUBLIC "-//Jetty//Configure//EN" "https://www.eclipse.org/jetty/configure_10_0.dtd">
 <Configure class="org.eclipse.jetty.client.HttpClient">
   <Call name="getAuthenticationStore">
     <Call name="addAuthentication">

jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlConfiguration.java

@@ -388,7 +388,7 @@ public class XmlConfiguration
             if (id != null)
                 _configuration.getIdMap().put(id, obj);
 
-            AttrOrElementNode aoeNode = new AttrOrElementNode(obj, _root, "Arg");
+            AttrOrElementNode aoeNode = new AttrOrElementNode(obj, _root, "Id", "Class", "Arg");
             // The Object already existed, if it has <Arg> nodes, warn about them not being used.
             aoeNode.getNodes("Arg")
                 .forEach((node) -> LOG.warn("Ignored arg {} in {}", node, this._configuration._location));
@@ -399,19 +399,22 @@ public class XmlConfiguration
         @Override
         public Object configure() throws Exception
         {
-            Class<?> oClass = nodeClass(_root);
+            AttrOrElementNode aoeNode = new AttrOrElementNode(_root, "Id", "Class", "Arg");
-
+            String id = aoeNode.getString("Id");
-            String id = _root.getAttribute("id");
+            String clazz = aoeNode.getString("Class");
             Object obj = id == null ? null : _configuration.getIdMap().get(id);
+            Class<?> oClass = clazz != null ? Loader.loadClass(clazz) : obj == null ? null : obj.getClass();
 
-            AttrOrElementNode aoeNode;
+            if (LOG.isDebugEnabled())
+                LOG.debug("Configure {} {}", oClass, obj);
 
             if (obj == null && oClass != null)
             {
-                aoeNode = new AttrOrElementNode(_root, "Arg");
                 try
                 {
                     obj = construct(oClass, new Args(null, oClass, aoeNode.getNodes("Arg")));
+                    if (id != null)
+                        _configuration.getIdMap().put(id, obj);
                 }
                 catch (NoSuchMethodException x)
                 {
@@ -420,13 +423,10 @@ public class XmlConfiguration
             }
             else
             {
-                aoeNode = new AttrOrElementNode(obj, _root, "Arg");
                 // The Object already existed, if it has <Arg> nodes, warn about them not being used.
                 aoeNode.getNodes("Arg")
                     .forEach((node) -> LOG.warn("Ignored arg {} in {}", node, this._configuration._location));
             }
-            if (id != null)
-                _configuration.getIdMap().put(id, obj);
 
             _configuration.initializeDefaults(obj);
             configure(obj, _root, aoeNode.getNext());
@@ -1021,13 +1021,14 @@ public class XmlConfiguration
          */
         private Object refObj(XmlParser.Node node) throws Exception
         {
-            String refid = node.getAttribute("refid");
+            AttrOrElementNode aoeNode = new AttrOrElementNode(node, "Id");
+            String refid = aoeNode.getString("Id");
             if (refid == null)
-                refid = node.getAttribute("id");
+                refid = node.getAttribute("refid");
             Object obj = _configuration.getIdMap().get(refid);
             if (obj == null && node.size() > 0)
                 throw new IllegalStateException("No object for refid=" + refid);
-            configure(obj, node, 0);
+            configure(obj, node, aoeNode.getNext());
             return obj;
         }
 

jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlParser.java

@@ -70,7 +70,7 @@ public class XmlParser
     public XmlParser()
     {
         SAXParserFactory factory = SAXParserFactory.newInstance();
-        boolean validatingDefault = factory.getClass().toString().startsWith("org.apache.xerces.");
+        boolean validatingDefault = factory.getClass().toString().contains("org.apache.xerces.");
         String validatingProp = System.getProperty("org.eclipse.jetty.xml.XmlParser.Validating", validatingDefault ? "true" : "false");
         boolean validating = Boolean.valueOf(validatingProp).booleanValue();
         setValidating(validating);

jetty-xml/src/main/resources/org/eclipse/jetty/xml/configure_10_0.dtd

@@ -19,21 +19,18 @@ my be specified if a match is not achieved.
 
 <!ENTITY % CONFIG "Set|Get|Put|Call|New|Ref|Array|Map|Property">
 <!ENTITY % VALUE "#PCDATA|Get|Call|New|Ref|Array|Map|SystemProperty|Env|Property">
-
+<!ENTITY % TYPE_ATTR "type CDATA #IMPLIED " >
-<!ENTITY % TYPEATTR "type CDATA #IMPLIED " > <!-- String|Character|Short|Byte|Integer|Long|Boolean|Float|Double|char|short|byte|int|long|boolean|float|double|URL|InetAddress|InetAddrPort| #classname -->
+<!ENTITY % CLASS_ATTR "class CDATA #IMPLIED" >
-<!ENTITY % IMPLIEDCLASSATTR "class CDATA #IMPLIED" >
+<!ENTITY % NAME_ATTR_REQUIRED "name CDATA #REQUIRED" >
-<!ENTITY % CLASSATTR "class CDATA #REQUIRED" >
+<!ENTITY % NAME_ATTR "name CDATA #IMPLIED" >
-<!ENTITY % NAMEATTR "name CDATA #REQUIRED" >
+<!ENTITY % PROPERTY_ATTR "property CDATA #IMPLIED" >
-<!ENTITY % IMPLIEDNAMEATTR "name CDATA #IMPLIED" >
+<!ENTITY % DEPRECATED_ATTR "deprecated CDATA #IMPLIED" >
-<!ENTITY % PROPERTYATTR "property CDATA #IMPLIED" >
+<!ENTITY % DEFAULT_ATTR "default CDATA #IMPLIED" >
-<!ENTITY % DEPRECATEDATTR "deprecated CDATA #IMPLIED" >
+<!ENTITY % ID_ATTR "id ID #IMPLIED" >
-<!ENTITY % DEFAULTATTR "default CDATA #IMPLIED" >
+<!ENTITY % ARG_ATTR "arg CDATA #IMPLIED" >
-<!ENTITY % IDATTR "id ID #IMPLIED" >
+<!ENTITY % ITEM_ATTR "item CDATA #IMPLIED" >
-<!ENTITY % ARGATTR "arg CDATA #IMPLIED" >
+<!ENTITY % REF_ATTR "refid CDATA #IMPLIED" >
-<!ENTITY % ITEMATTR "item CDATA #IMPLIED" >
+<!ENTITY % ID_ATTR_REQUIRED "id ID #REQUIRED" >
-<!ENTITY % REFATTR "refid CDATA #IMPLIED" >
-<!ENTITY % REQUIREDIDATTR "id ID #REQUIRED" >
-
 
 <!--
 Configure Element.
@@ -42,9 +39,8 @@ can be configured:
 
     <Configure class="com.acme.MyClass"> ... </Configure>
 -->
-<!ELEMENT Configure (Arg*,(%CONFIG;)*) >
+<!ELEMENT Configure (Id?,Class?,Arg*,(%CONFIG;)*) >
-<!ATTLIST Configure %IMPLIEDCLASSATTR; %IDATTR; >
+<!ATTLIST Configure %CLASS_ATTR; %ID_ATTR; >
-
 
 <!--
 Set Element.
@@ -68,7 +64,7 @@ value.
 A Set with a class attribute is treated as a static set method invocation.
 -->
 <!ELEMENT Set (%VALUE;)* >
-<!ATTLIST Set  %IDATTR; %NAMEATTR; %TYPEATTR; %IMPLIEDCLASSATTR; %PROPERTYATTR;>
+<!ATTLIST Set  %ID_ATTR; %NAME_ATTR_REQUIRED; %TYPE_ATTR; %CLASS_ATTR; %PROPERTY_ATTR;>
 
 <!--
 Get Element.
@@ -82,7 +78,7 @@ which act on the object returned by the get call.
 A Get with a class attribute is treated as a static get method or field.
 -->
 <!ELEMENT Get (%CONFIG;)* >
-<!ATTLIST Get %NAMEATTR; %IMPLIEDCLASSATTR; %IDATTR; >
+<!ATTLIST Get %NAME_ATTR_REQUIRED; %CLASS_ATTR; %ID_ATTR; >
 
 
 <!--
@@ -99,7 +95,7 @@ elements they are added as strings before being converted to any
 specified type.
 -->
 <!ELEMENT Put (%VALUE;)* >
-<!ATTLIST Put %NAMEATTR; %TYPEATTR; >
+<!ATTLIST Put %NAME_ATTR_REQUIRED; %TYPE_ATTR; >
 
 
 <!--
@@ -166,7 +162,7 @@ This is equivalent to:
 A Call with a class attribute is treated as a static call.
 -->
 <!ELEMENT Call (Id?,Name?,Class?,Arg*,(%CONFIG;)*) >
-<!ATTLIST Call %ARGATTR; %IMPLIEDNAMEATTR; %IMPLIEDCLASSATTR; %IDATTR; >
+<!ATTLIST Call %ARG_ATTR; %NAME_ATTR; %CLASS_ATTR; %ID_ATTR; >
 
 
 <!--
@@ -182,7 +178,7 @@ elements they are added as strings before being converted to any
 specified type.
 -->
 <!ELEMENT Arg (%VALUE;)* >
-<!ATTLIST Arg %TYPEATTR; %IMPLIEDNAMEATTR; >
+<!ATTLIST Arg %TYPE_ATTR; %NAME_ATTR; >
 
 
 <!--
@@ -207,7 +203,7 @@ This is equivalent to:
  o.setTest("Value2");
 -->
 <!ELEMENT New (Id?,Name?,Class?,Arg*,(%CONFIG;)*) >
-<!ATTLIST New %IDATTR; %IMPLIEDCLASSATTR; %ARGATTR; >
+<!ATTLIST New %ID_ATTR; %CLASS_ATTR; %ARG_ATTR; >
 
 
 <!--
@@ -222,8 +218,8 @@ which act on the referenced object.
    <Set name="Test">Value2</Set>
  </New>
 -->
-<!ELEMENT Ref (%CONFIG;)* >
+<!ELEMENT Ref (Id?,(%CONFIG;)*) >
-<!ATTLIST Ref %IDATTR; %REFATTR;>
+<!ATTLIST Ref %ID_ATTR; %REF_ATTR;>
 
 
 <!--
@@ -242,7 +238,7 @@ This is equivalent to:
  String[] a = new String[] { "value0", new String("value1") };
 -->
 <!ELEMENT Array (Id?,Type?,Item*) >
-<!ATTLIST Array %IDATTR;%TYPEATTR;%ITEMATTR; >
+<!ATTLIST Array %ID_ATTR;%TYPE_ATTR;%ITEM_ATTR; >
 
 
 <!--
@@ -264,7 +260,7 @@ This is equivalent to:
  m.put("keyName", new String("value1"));
 -->
 <!ELEMENT Map (Id?,Entry*) >
-<!ATTLIST Map %IDATTR; >
+<!ATTLIST Map %ID_ATTR; >
 <!ELEMENT Entry (Item,Item) >
 
 
@@ -280,7 +276,7 @@ If it contains multiple value elements they are added as strings
 before being converted to any specified type.
 -->
 <!ELEMENT Item (%VALUE;)* >
-<!ATTLIST Item %TYPEATTR; %IDATTR; >
+<!ATTLIST Item %TYPE_ATTR; %ID_ATTR; >
 
 
 <!--
@@ -297,7 +293,7 @@ This is equivalent to:
  System.getProperty("Test","value");
 -->
 <!ELEMENT SystemProperty (Id?,Name?,Deprecated*,Default?) >
-<!ATTLIST SystemProperty %IMPLIEDNAMEATTR; %DEFAULTATTR; %DEPRECATEDATTR; %IDATTR; >
+<!ATTLIST SystemProperty %NAME_ATTR; %DEFAULT_ATTR; %DEPRECATED_ATTR; %ID_ATTR; >
 
 
 <!--
@@ -316,7 +312,7 @@ This is equivalent to:
 
 -->
 <!ELEMENT Env (Id?,Name?,Deprecated*,Default?) >
-<!ATTLIST Env %IMPLIEDNAMEATTR; %DEFAULTATTR; %DEPRECATEDATTR; %IDATTR; >
+<!ATTLIST Env %NAME_ATTR; %DEFAULT_ATTR; %DEPRECATED_ATTR; %ID_ATTR; >
 
 
 <!--
@@ -326,4 +322,4 @@ The name attribute specifies the property name and the optional
 default argument provides a default value.
 -->
 <!ELEMENT Property (Id?,Name?,Deprecated*,Default?) >
-<!ATTLIST Property %IMPLIEDNAMEATTR; %DEFAULTATTR; %DEPRECATEDATTR; %IDATTR; >
+<!ATTLIST Property %NAME_ATTR; %DEFAULT_ATTR; %DEPRECATED_ATTR; %ID_ATTR; >

jetty-xml/src/test/java/org/eclipse/jetty/xml/XmlConfigurationTest.java

@@ -265,11 +265,19 @@ public class XmlConfigurationTest
 
     public XmlConfiguration asXmlConfiguration(String rawXml) throws IOException, SAXException
     {
+        if (rawXml.indexOf("!DOCTYPE") < 0)
+            rawXml = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n" +
+                "<!DOCTYPE Configure PUBLIC \"-//Jetty//Configure//EN\" \"https://www.eclipse.org/jetty/configure_10_0.dtd\">\n" +
+                rawXml;
         return asXmlConfiguration("raw.xml", rawXml);
     }
 
     public XmlConfiguration asXmlConfiguration(String filename, String rawXml) throws IOException, SAXException
     {
+        if (rawXml.indexOf("!DOCTYPE") < 0)
+            rawXml = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n" +
+                "<!DOCTYPE Configure PUBLIC \"-//Jetty//Configure//EN\" \"https://www.eclipse.org/jetty/configure_10_0.dtd\">\n" +
+                rawXml;
         Path testFile = workDir.getEmptyPathDir().resolve(filename);
         try (BufferedWriter writer = Files.newBufferedWriter(testFile, UTF_8))
         {

jetty-xml/src/test/resources/org/eclipse/jetty/xml/configureWithElements.xml

@@ -1,7 +1,8 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <!DOCTYPE Configure PUBLIC "-//Jetty//Configure//EN" "https://www.eclipse.org/jetty/configure_10_0.dtd">
 
-<Configure class="org.eclipse.jetty.xml.TestConfiguration">
+<Configure>
+  <Class>org.eclipse.jetty.xml.TestConfiguration</Class>
   <Arg name="name">name</Arg>
 
   <Set name="Test">SetValue</Set>
@@ -69,7 +70,6 @@
       <Arg>String</Arg>
   </New></Put>
 
-
   <Put name="ObjectsWhiteString">
     <New>
       <Class>java.lang.Integer</Class>
@@ -87,25 +87,33 @@
   <Put name="Float" type="Float">2.3</Put>
   <Put name="Env"><Env name="HOME"/></Put>
 
-  <Set name="nested">
   <New>
+    <Id>testId</Id>
     <Class>org.eclipse.jetty.xml.TestConfiguration</Class>
     <Set name="testString">nested</Set>
+  </New>
+
+  <Set name="nested">
+    <Ref>
+      <Id>testId</Id>
       <Set name="nested">
         <New class="org.eclipse.jetty.xml.TestConfiguration">
         </New>
       </Set>
-    </New>
+    </Ref>
   </Set>
 
-  <Call name="call">
+  <Call>
+    <Name>call</Name>
   </Call>
 
-  <Call name="call">
+  <Call>
+    <Name>call</Name>
     <Arg type="boolean">false</Arg>
   </Call>
 
-  <Call name="call">
+  <Call>
+    <Name>call</Name>
     <Arg type="boolean">true</Arg>
     <Put name="nested">put</Put>
     <Set name="Test">Call1</Set>

tests/test-integration/src/test/resources/ssl.xml

@@ -1,3 +1,5 @@
+<?xml version="1.0"?>
+<!DOCTYPE Configure PUBLIC "-//Jetty//Configure//EN" "https://www.eclipse.org/jetty/configure_10_0.dtd">
 <Configure id="sslContextFactory" class="org.eclipse.jetty.util.ssl.SslContextFactory$Server">
   <Set name="KeyStorePath"><Property name="jetty.home" default="." />/<Property name="jetty.sslContext.keyStorePath" default="keystore.p12"/></Set>
   <Set name="KeyStorePassword"><Property name="jetty.sslContext.keyStorePassword"/></Set>