Merge pull request #3185 from mikecirioli/OSS-2590

[JENKINS-48463] Update jenkins core to use  XML v1.1

core/pom.xml

@@ -246,6 +246,20 @@ THE SOFTWARE.
         </exclusion>
       </exclusions>
     </dependency>
+    <!--
+    still including the xpp3 driver to ensure backwards compatibilty
+    for other plugins that may be depending on it
+    -->
+    <dependency>
+      <groupId>xpp3</groupId>
+      <artifactId>xpp3</artifactId>
+      <version>1.1.4c</version>
+    </dependency>
+    <dependency>
+      <groupId>net.sf.kxml</groupId>
+      <artifactId>kxml2</artifactId>
+      <version>2.3.0</version>
+    </dependency>
     <dependency>
       <groupId>jfree</groupId>
       <artifactId>jfreechart</artifactId>
@@ -452,11 +466,6 @@ THE SOFTWARE.
       <artifactId>spring-aop</artifactId>
       <version>${spring.version}</version>
     </dependency>
-    <dependency>
-      <groupId>xpp3</groupId>
-      <artifactId>xpp3</artifactId>
-      <version>1.1.4c</version>
-    </dependency>
     <dependency>
       <groupId>junit</groupId>
       <artifactId>junit</artifactId>

core/src/main/java/hudson/Main.java

@@ -144,7 +144,7 @@ public class Main {
             int ret;
             try (OutputStream os = Files.newOutputStream(tmpFile.toPath());
                  Writer w = new OutputStreamWriter(os,"UTF-8")) {
-                w.write("<?xml version='1.0' encoding='UTF-8'?>");
+                w.write("<?xml version='1.1' encoding='UTF-8'?>");
                 w.write("<run><log encoding='hexBinary' content-encoding='"+Charset.defaultCharset().name()+"'>");
                 w.flush();
 

core/src/main/java/hudson/XmlFile.java

@@ -26,7 +26,7 @@ package hudson;
 import com.thoughtworks.xstream.XStream;
 import com.thoughtworks.xstream.converters.Converter;
 import com.thoughtworks.xstream.converters.UnmarshallingContext;
-import com.thoughtworks.xstream.io.xml.Xpp3Driver;
+import com.thoughtworks.xstream.io.HierarchicalStreamDriver;
 import hudson.diagnosis.OldDataMonitor;
 import hudson.model.Descriptor;
 import hudson.util.AtomicFileWriter;
@@ -39,7 +39,6 @@ import org.xml.sax.Locator;
 import org.xml.sax.SAXException;
 import org.xml.sax.ext.Locator2;
 import org.xml.sax.helpers.DefaultHandler;
-
 import javax.xml.parsers.ParserConfigurationException;
 import javax.xml.parsers.SAXParserFactory;
 import java.io.BufferedInputStream;
@@ -187,7 +186,7 @@ public final class XmlFile {
         mkdirs();
         AtomicFileWriter w = new AtomicFileWriter(file);
         try {
-            w.write("<?xml version='1.0' encoding='UTF-8'?>\n");
+            w.write("<?xml version='1.1' encoding='UTF-8'?>\n");
             beingWritten.put(o, null);
             writing.set(file);
             try {
@@ -352,13 +351,14 @@ public final class XmlFile {
     /**
      * {@link XStream} instance is supposed to be thread-safe.
      */
-    private static final XStream DEFAULT_XSTREAM = new XStream2();
 
     private static final Logger LOGGER = Logger.getLogger(XmlFile.class.getName());
 
     private static final SAXParserFactory JAXP = SAXParserFactory.newInstance();
 
-    private static final Xpp3Driver DEFAULT_DRIVER = new Xpp3Driver();
+    private static final HierarchicalStreamDriver DEFAULT_DRIVER = XStream2.getDefaultDriver();
+
+    private static final XStream DEFAULT_XSTREAM = new XStream2(DEFAULT_DRIVER);
 
     static {
         JAXP.setNamespaceAware(true);

core/src/main/java/hudson/model/Fingerprint.java

@@ -1256,7 +1256,7 @@ public class Fingerprint implements ModelObject, Saveable {
             AtomicFileWriter afw = new AtomicFileWriter(file);
             try {
                 PrintWriter w = new PrintWriter(afw);
-                w.println("<?xml version='1.0' encoding='UTF-8'?>");
+                w.println("<?xml version='1.1' encoding='UTF-8'?>");
                 w.println("<fingerprint>");
                 w.print("  <timestamp>");
                 w.print(DATE_CONVERTER.toString(timestamp));

core/src/main/java/hudson/model/View.java

@@ -26,7 +26,6 @@ package hudson.model;
 
 import com.thoughtworks.xstream.converters.ConversionException;
 import com.thoughtworks.xstream.io.StreamException;
-import com.thoughtworks.xstream.io.xml.Xpp3Driver;
 import hudson.DescriptorExtensionList;
 import hudson.Extension;
 import hudson.ExtensionPoint;
@@ -1214,7 +1213,7 @@ public abstract class View extends AbstractModelObject implements AccessControll
             // view in same ViewGroup and might not satisfy Jenkins.checkGoodName.
             String oldname = name;
             ViewGroup oldOwner = owner; // oddly, this field is not transient
-            Object o = Jenkins.XSTREAM2.unmarshal(new Xpp3Driver().createReader(in), this, null, true);
+            Object o = Jenkins.XSTREAM2.unmarshal(XStream2.getDefaultDriver().createReader(in), this, null, true);
             if (!o.getClass().equals(getClass())) {
                 // ensure that we've got the same view type. extending this code to support updating
                 // to different view type requires destroying & creating a new view type

core/src/main/java/hudson/util/XStream2.java

@@ -26,6 +26,7 @@ package hudson.util;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.thoughtworks.xstream.XStream;
+import com.thoughtworks.xstream.io.xml.KXml2Driver;
 import com.thoughtworks.xstream.mapper.AnnotationMapper;
 import com.thoughtworks.xstream.mapper.Mapper;
 import com.thoughtworks.xstream.mapper.MapperWrapper;
@@ -97,7 +98,18 @@ public class XStream2 extends XStream {
      */
     private MapperInjectionPoint mapperInjectionPoint;
 
+    /**
+     * Convenience method so we only have to change the driver in one place
+     * if we switch to something new in the future
+     *
+     * @return a new instance of the HierarchicalStreamDriver we want to use
+     */
+    public static HierarchicalStreamDriver getDefaultDriver() {
+        return new KXml2Driver();
+    }
+
     public XStream2() {
+        super(getDefaultDriver());
         init();
         classOwnership = null;
     }
@@ -109,6 +121,7 @@ public class XStream2 extends XStream {
     }
 
     XStream2(ClassOwnership classOwnership) {
+        super(getDefaultDriver());
         init();
         this.classOwnership = classOwnership;
     }
@@ -293,7 +306,7 @@ public class XStream2 extends XStream {
      */
     public void toXMLUTF8(Object obj, OutputStream out) throws IOException {
         Writer w = new OutputStreamWriter(out, Charset.forName("UTF-8"));
-        w.write("<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n");
+        w.write("<?xml version=\"1.1\" encoding=\"UTF-8\"?>\n");
         toXML(obj, w);
     }
 

core/src/main/java/jenkins/util/xstream/XStreamDOM.java

@@ -35,10 +35,10 @@ import com.thoughtworks.xstream.io.xml.AbstractXmlReader;
 import com.thoughtworks.xstream.io.xml.AbstractXmlWriter;
 import com.thoughtworks.xstream.io.xml.DocumentReader;
 import com.thoughtworks.xstream.io.xml.XmlFriendlyReplacer;
-import com.thoughtworks.xstream.io.xml.Xpp3Driver;
 import hudson.Util;
 import hudson.util.VariableResolver;
 
+import hudson.util.XStream2;
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.io.Reader;
@@ -241,11 +241,11 @@ public class XStreamDOM {
      * Writes this {@link XStreamDOM} into {@link OutputStream}.
      */
     public void writeTo(OutputStream os) {
-        writeTo(new Xpp3Driver().createWriter(os));
+        writeTo(XStream2.getDefaultDriver().createWriter(os));
     }
 
     public void writeTo(Writer w) {
-        writeTo(new Xpp3Driver().createWriter(w));
+        writeTo(XStream2.getDefaultDriver().createWriter(w));
     }
 
     public void writeTo(HierarchicalStreamWriter w) {
@@ -262,11 +262,11 @@ public class XStreamDOM {
     }
 
     public static XStreamDOM from(InputStream in) {
-        return from(new Xpp3Driver().createReader(in));
+        return from(XStream2.getDefaultDriver().createReader(in));
     }
 
     public static XStreamDOM from(Reader in) {
-        return from(new Xpp3Driver().createReader(in));
+        return from(XStream2.getDefaultDriver().createReader(in));
     }
 
     public static XStreamDOM from(HierarchicalStreamReader in) {

core/src/test/java/hudson/XmlFileTest.java

@@ -0,0 +1,79 @@
+package hudson;
+
+import hudson.model.Node;
+import hudson.util.XStream2;
+import java.io.File;
+import java.io.IOException;
+import java.net.URL;
+
+import jenkins.model.Jenkins;
+import org.junit.Ignore;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.xml.sax.SAXParseException;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertThat;
+
+public class XmlFileTest {
+
+    @Test
+    public void canReadXml1_0Test() throws IOException {
+        URL configUrl = getClass().getResource("/hudson/config_1_0.xml");
+        XStream2  xs = new XStream2();
+        xs.alias("hudson", Jenkins.class);
+
+        XmlFile xmlFile =  new XmlFile(xs, new File(configUrl.getFile()));
+        if (xmlFile.exists()) {
+            Node n = (Node) xmlFile.read();
+            assertThat(n.getNumExecutors(), is(2));
+            assertThat(n.getMode().toString(), is("NORMAL"));
+        }
+    }
+
+    // KXml2Driver is able to parse XML 1.0 even if it has control characters which
+    // should be illegal.  Ignoring this test until we switch to a more compliant driver
+    @Ignore
+    @Test(expected = SAXParseException.class)
+    public void xml1_0_withSpecialCharsShouldFail() throws IOException {
+        URL configUrl = getClass().getResource("/hudson/config_1_0_with_special_chars.xml");
+        XStream2  xs = new XStream2();
+        xs.alias("hudson", Jenkins.class);
+
+        XmlFile xmlFile =  new XmlFile(xs, new File(configUrl.getFile()));
+        if (xmlFile.exists()) {
+            Node n = (Node) xmlFile.read();
+            assertThat(n.getNumExecutors(), is(2));
+            assertThat(n.getMode().toString(), is("NORMAL"));
+        }
+    }
+
+    @Test
+    public void canReadXml1_1Test() throws IOException {
+        URL configUrl = getClass().getResource("/hudson/config_1_1.xml");
+        XStream2  xs = new XStream2();
+        xs.alias("hudson", Jenkins.class);
+
+        XmlFile xmlFile =  new XmlFile(xs, new File(configUrl.getFile()));
+        if (xmlFile.exists()) {
+            Node n = (Node) xmlFile.read();
+            assertThat(n.getNumExecutors(), is(2));
+            assertThat(n.getMode().toString(), is("NORMAL"));
+        }
+    }
+    
+    @Test
+    public void canReadXmlWithControlCharsTest() throws IOException {
+        URL configUrl = getClass().getResource("/hudson/config_1_1_with_special_chars.xml");
+        XStream2  xs = new XStream2();
+        xs.alias("hudson", Jenkins.class);
+
+        XmlFile xmlFile =  new XmlFile(xs, new File(configUrl.getFile()));
+        if (xmlFile.exists()) {
+            Node n = (Node) xmlFile.read();
+            assertThat(n.getNumExecutors(), is(2));
+            assertThat(n.getMode().toString(), is("NORMAL"));
+            assertThat(n.getLabelString(), is("LESS_TERMCAP_mb=\u001B[01;31m"));
+        }
+    }
+}

core/src/test/java/hudson/util/XStream2Test.java

@@ -29,7 +29,7 @@ import static org.junit.Assert.*;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.thoughtworks.xstream.XStreamException;
-import com.thoughtworks.xstream.io.xml.Xpp3Driver;
+import com.thoughtworks.xstream.io.xml.KXml2Driver;
 import com.thoughtworks.xstream.security.ForbiddenClassException;
 import hudson.model.Result;
 import hudson.model.Run;
@@ -426,7 +426,7 @@ public class XStream2Test {
 
     private Object fromXMLNullingOut(String xml, Object root) {
         // Currently not offering a public convenience API for this:
-        return Jenkins.XSTREAM2.unmarshal(new Xpp3Driver().createReader(new StringReader(xml)), root, null, true);
+        return Jenkins.XSTREAM2.unmarshal(XStream2.getDefaultDriver().createReader(new StringReader(xml)), root, null, true);
     }
 
     public static class WithDefaults {

core/src/test/resources/hudson/config_1_0.xml

@@ -0,0 +1,8 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<hudson>
+    <disabledAdministrativeMonitors/>
+    <version>1.480.1</version>
+    <numExecutors>2</numExecutors>
+    <mode>NORMAL</mode>
+    <!-- Yeah, we deleted a lot of this ... fine for testing. -->
+</hudson>

core/src/test/resources/hudson/config_1_0_with_special_chars.xml

@@ -0,0 +1,9 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<hudson>
+    <disabledAdministrativeMonitors/>
+    <version>1.480.1</version>
+    <numExecutors>2</numExecutors>
+    <mode>NORMAL</mode>
+    <label>LESS_TERMCAP_mb=&#x1b;[01;31m</label>
+    <!-- Yeah, we deleted a lot of this ... fine for testing. -->
+</hudson>

core/src/test/resources/hudson/config_1_1.xml

@@ -0,0 +1,8 @@
+<?xml version='1.1' encoding='UTF-8'?>
+<hudson>
+    <disabledAdministrativeMonitors/>
+    <version>1.480.1</version>
+    <numExecutors>2</numExecutors>
+    <mode>NORMAL</mode>
+    <!-- Yeah, we deleted a lot of this ... fine for testing. -->
+</hudson>

core/src/test/resources/hudson/config_1_1_with_special_chars.xml

@@ -0,0 +1,9 @@
+<?xml version='1.1' encoding='UTF-8'?>
+<hudson>
+    <disabledAdministrativeMonitors/>
+    <version>1.480.1</version>
+    <numExecutors>2</numExecutors>
+    <mode>NORMAL</mode>
+    <label>LESS_TERMCAP_mb=&#x1b;[01;31m</label>
+    <!-- Yeah, we deleted a lot of this ... fine for testing. -->
+</hudson>

pom.xml

@@ -133,7 +133,7 @@ THE SOFTWARE.
       <dependency>
         <groupId>org.apache.ant</groupId>
         <artifactId>ant</artifactId>
-        <version>1.8.4</version>
+        <version>1.9.2</version>
       </dependency>
 
       <dependency>

test/src/test/java/hudson/XMLFileTest.java

@@ -0,0 +1,47 @@
+package hudson;
+
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.FileReader;
+import org.junit.Rule;
+import org.junit.Test;
+import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.recipes.LocalData;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertThat;
+
+public class XMLFileTest {
+
+    @Rule
+    public JenkinsRule j = new JenkinsRule();
+
+    @Test
+    @LocalData
+    public void canStartWithXml_1_1_ConfigsTest() {
+
+        assertThat(j.jenkins.getLabelString(),is("LESS_TERMCAP_mb=\u001B[01;31m"));
+
+    }
+
+    /**
+     *
+     * This test validates that xml v1.0 configs silently get migrated to xml v1.1 when they are persisted
+     *
+     */
+    @Test
+    @LocalData
+    public void silentlyMigrateConfigsTest() throws Exception {
+        j.jenkins.save();
+        // verify that we did indeed load our test config.xml
+        assertThat(j.jenkins.getLabelString(), is("I am a label"));
+        //verify that the persisted top level config.xml is v1.1
+        File configFile = new File(j.jenkins.getRootDir(), "config.xml");
+        assertThat(configFile.exists(), is(true));
+
+        try (BufferedReader config = new BufferedReader(new FileReader(configFile))) {
+            assertThat(config.readLine(), is("<?xml version='1.1' encoding='UTF-8'?>"));
+            config.close();
+        }
+    }
+}

test/src/test/java/hudson/cli/GetNodeCommandTest.java

@@ -74,7 +74,7 @@ public class GetNodeCommandTest {
                 .invokeWithArgs("MySlave")
         ;
 
-        assertThat(result.stdout(), startsWith("<?xml version=\"1.0\" encoding=\"UTF-8\"?>"));
+        assertThat(result.stdout(), startsWith("<?xml version=\"1.1\" encoding=\"UTF-8\"?>"));
         assertThat(result.stdout(), containsString("<name>MySlave</name>"));
         assertThat(result, hasNoErrorOutput());
         assertThat(result, succeeded());

test/src/test/java/hudson/cli/GetViewCommandTest.java

@@ -79,7 +79,7 @@ public class GetViewCommandTest {
 
         assertThat(result, succeeded());
         assertThat(result, hasNoErrorOutput());
-        assertThat(result.stdout(), startsWith("<?xml version=\"1.0\" encoding=\"UTF-8\"?>"));
+        assertThat(result.stdout(), startsWith("<?xml version=\"1.1\" encoding=\"UTF-8\"?>"));
         assertThat(result.stdout(), containsString("<name>aView</name>"));
     }
 

test/src/test/java/hudson/model/ComputerConfigDotXmlTest.java

@@ -121,7 +121,7 @@ public class ComputerConfigDotXmlTest {
         computer.doConfigDotXml(req, rsp);
 
         final String out = outputStream.toString();
-        assertThat(out, startsWith("<?xml version=\"1.0\" encoding=\"UTF-8\"?>"));
+        assertThat(out, startsWith("<?xml version=\"1.1\" encoding=\"UTF-8\"?>"));
         assertThat(out, containsString("<name>slave0</name>"));
         assertThat(out, containsString("<description>dummy</description>"));
     }

test/src/test/java/hudson/util/RobustReflectionConverterTest.java

@@ -170,7 +170,7 @@ public class RobustReflectionConverterTest {
     }
     
     private static final String CONFIGURATION_TEMPLATE =
-            "<?xml version='1.0' encoding='UTF-8'?>"
+            "<?xml version='1.1' encoding='UTF-8'?>"
             + "<project>"
             + "<properties>"
             +     "<hudson.util.RobustReflectionConverterTest_-KeywordProperty>"

test/src/test/resources/hudson/XMLFileTest/canStartWithXml_1_1_ConfigsTest/config.xml

@@ -0,0 +1,9 @@
+<?xml version='1.1' encoding='UTF-8'?>
+<hudson>
+    <disabledAdministrativeMonitors/>
+    <version>1.480.1</version>
+    <numExecutors>2</numExecutors>
+    <mode>NORMAL</mode>
+    <label>LESS_TERMCAP_mb=&#x1b;[01;31m</label>
+    <!-- Yeah, we deleted a lot of this ... fine for testing. -->
+</hudson>

test/src/test/resources/hudson/XMLFileTest/silentlyMigrateConfigsTest/config.xml

@@ -0,0 +1,9 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<hudson>
+    <disabledAdministrativeMonitors/>
+    <version>1.480.1</version>
+    <numExecutors>2</numExecutors>
+    <mode>NORMAL</mode>
+    <label>I am a label</label>
+    <!-- Yeah, we deleted a lot of this ... fine for testing. -->
+</hudson>