From 9585f94cfc13cbc0289d6d7f4bfc9baf60f1333f Mon Sep 17 00:00:00 2001 From: mike cirioli Date: Fri, 8 Dec 2017 14:33:12 -0500 Subject: [PATCH 01/11] [oss-2590] updated dependencies to support move to xml 1.1 --- core/pom.xml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/core/pom.xml b/core/pom.xml index a4b053eee5..cdd4e104ff 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -240,6 +240,11 @@ THE SOFTWARE. + + net.sf.kxml + kxml2 + 2.3.0 + jfree jfreechart @@ -446,11 +451,6 @@ THE SOFTWARE. spring-aop ${spring.version} - - xpp3 - xpp3 - 1.1.4c - junit junit From 9a4a913e387c49d0c4cec602280e25c37196f83c Mon Sep 17 00:00:00 2001 From: mike cirioli Date: Fri, 8 Dec 2017 14:45:22 -0500 Subject: [PATCH 02/11] [JENKINS-48463] - update jenkins core to use xml v1.1 - switched to KxmlDriver (needed for xml 1.1 support) - updated to create xml v1.1 headers - updated test to use KXml2Driver explicitly - updated unit tests to support xml v1.1 --- core/src/main/java/hudson/Main.java | 2 +- core/src/main/java/hudson/XmlFile.java | 7 ++-- .../main/java/hudson/model/Fingerprint.java | 2 +- core/src/main/java/hudson/util/XStream2.java | 2 +- .../test/java/hudson/PluginManagerTest.java | 2 +- .../hudson/util/XStream2EncodingTest.java | 2 +- .../java/jenkins/model/RunIdMigratorTest.java | 36 +++++++++---------- .../jenkins/util/xstream/XStreamDOMTest.java | 3 +- .../test/java/jenkins/xml/XMLUtilsTest.java | 6 ++-- .../java/hudson/cli/GetNodeCommandTest.java | 2 +- .../java/hudson/cli/GetViewCommandTest.java | 2 +- .../model/AbstractItemSecurityTest.java | 4 +-- .../model/ComputerConfigDotXmlTest.java | 2 +- .../java/hudson/model/ItemGroupMixInTest.java | 2 +- .../util/RobustReflectionConverterTest.java | 2 +- 15 files changed, 38 insertions(+), 38 deletions(-) diff --git a/core/src/main/java/hudson/Main.java b/core/src/main/java/hudson/Main.java index 896ae4dd53..11cb516cee 100644 --- a/core/src/main/java/hudson/Main.java +++ b/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(""); + w.write(""); w.write(""); w.flush(); diff --git a/core/src/main/java/hudson/XmlFile.java b/core/src/main/java/hudson/XmlFile.java index 16644262b9..ae65b265d5 100644 --- a/core/src/main/java/hudson/XmlFile.java +++ b/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.xml.KXml2Driver; 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; @@ -172,7 +171,7 @@ public final class XmlFile { mkdirs(); AtomicFileWriter w = new AtomicFileWriter(file); try { - w.write("\n"); + w.write("\n"); beingWritten.put(o, null); writing.set(file); try { @@ -343,7 +342,7 @@ public final class XmlFile { private static final SAXParserFactory JAXP = SAXParserFactory.newInstance(); - private static final Xpp3Driver DEFAULT_DRIVER = new Xpp3Driver(); + private static final KXml2Driver DEFAULT_DRIVER = new KXml2Driver(); static { JAXP.setNamespaceAware(true); diff --git a/core/src/main/java/hudson/model/Fingerprint.java b/core/src/main/java/hudson/model/Fingerprint.java index b2fcc6c5d6..2e0a7ce166 100644 --- a/core/src/main/java/hudson/model/Fingerprint.java +++ b/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(""); + w.println(""); w.println(""); w.print(" "); w.print(DATE_CONVERTER.toString(timestamp)); diff --git a/core/src/main/java/hudson/util/XStream2.java b/core/src/main/java/hudson/util/XStream2.java index 6b17761cb6..285dd6b2db 100644 --- a/core/src/main/java/hudson/util/XStream2.java +++ b/core/src/main/java/hudson/util/XStream2.java @@ -215,7 +215,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("\n"); + w.write("\n"); toXML(obj, w); } diff --git a/core/src/test/java/hudson/PluginManagerTest.java b/core/src/test/java/hudson/PluginManagerTest.java index 843e15ba1e..1e89e5c5c8 100644 --- a/core/src/test/java/hudson/PluginManagerTest.java +++ b/core/src/test/java/hudson/PluginManagerTest.java @@ -64,7 +64,7 @@ public class PluginManagerTest { @Issue("SECURITY-167") @Test public void parseInvalidRequestedPlugins() throws Exception { - String evilXML = "\n" + + String evilXML = "\n" + "]>\n" + "\n" + " \n" + diff --git a/core/src/test/java/hudson/util/XStream2EncodingTest.java b/core/src/test/java/hudson/util/XStream2EncodingTest.java index ea23797dea..ea10414ebc 100644 --- a/core/src/test/java/hudson/util/XStream2EncodingTest.java +++ b/core/src/test/java/hudson/util/XStream2EncodingTest.java @@ -70,7 +70,7 @@ public class XStream2EncodingTest { Thing t = (Thing) xs.fromXML(new ByteArrayInputStream(ambiguousXml)); assertThat(t.field, not(msg)); ByteArrayOutputStream baos2 = new ByteArrayOutputStream(); - baos2.write("\n".getBytes("UTF-8")); + baos2.write("\n".getBytes("UTF-8")); baos2.write(ambiguousXml); t = (Thing) xs.fromXML(new ByteArrayInputStream(ambiguousXml)); assertThat(t.field, not(msg)); diff --git a/core/src/test/java/jenkins/model/RunIdMigratorTest.java b/core/src/test/java/jenkins/model/RunIdMigratorTest.java index 71a3e47832..fda491eda6 100644 --- a/core/src/test/java/jenkins/model/RunIdMigratorTest.java +++ b/core/src/test/java/jenkins/model/RunIdMigratorTest.java @@ -83,13 +83,13 @@ public class RunIdMigratorTest { @Test public void legacy() throws Exception { assumeFalse("Symlinks don't work well on Windows", Functions.isWindows()); - write("2014-01-02_03-04-05/build.xml", "\n\n ok\n 99\n ok\n"); + write("2014-01-02_03-04-05/build.xml", "\n\n ok\n 99\n ok\n"); link("99", "2014-01-02_03-04-05"); link("lastFailedBuild", "-1"); link("lastSuccessfulBuild", "99"); - assertEquals("{2014-01-02_03-04-05={build.xml='\n\n ok\n 99\n ok\n'}, 99=→2014-01-02_03-04-05, lastFailedBuild=→-1, lastSuccessfulBuild=→99}", summarize()); + assertEquals("{2014-01-02_03-04-05={build.xml='\n\n ok\n 99\n ok\n'}, 99=→2014-01-02_03-04-05, lastFailedBuild=→-1, lastSuccessfulBuild=→99}", summarize()); assertTrue(migrator.migrate(dir, null)); - assertEquals("{99={build.xml='\n\n ok\n 2014-01-02_03-04-05\n 1388649845000\n ok\n'}, lastFailedBuild=→-1, lastSuccessfulBuild=→99, legacyIds='2014-01-02_03-04-05 99\n'}", summarize()); + assertEquals("{99={build.xml='\n\n ok\n 2014-01-02_03-04-05\n 1388649845000\n ok\n'}, lastFailedBuild=→-1, lastSuccessfulBuild=→99, legacyIds='2014-01-02_03-04-05 99\n'}", summarize()); assertEquals(99, migrator.findNumber("2014-01-02_03-04-05")); migrator = new RunIdMigrator(); assertFalse(migrator.migrate(dir, null)); @@ -104,58 +104,58 @@ public class RunIdMigratorTest { assumeFalse("Symlinks don't work well on Windows", Functions.isWindows()); write("2014-01-02_03-04-04/build.xml", "\n 98\n"); link("98", "2014-01-02_03-04-04"); - write("99/build.xml", "\n\n ok\n 1388649845000\n ok\n"); + write("99/build.xml", "\n\n ok\n 1388649845000\n ok\n"); link("lastFailedBuild", "-1"); link("lastSuccessfulBuild", "99"); - assertEquals("{2014-01-02_03-04-04={build.xml='\n 98\n'}, 98=→2014-01-02_03-04-04, 99={build.xml='\n\n ok\n 1388649845000\n ok\n'}, lastFailedBuild=→-1, lastSuccessfulBuild=→99}", summarize()); + assertEquals("{2014-01-02_03-04-04={build.xml='\n 98\n'}, 98=→2014-01-02_03-04-04, 99={build.xml='\n\n ok\n 1388649845000\n ok\n'}, lastFailedBuild=→-1, lastSuccessfulBuild=→99}", summarize()); assertTrue(migrator.migrate(dir, null)); - assertEquals("{98={build.xml='\n 2014-01-02_03-04-04\n 1388649844000\n'}, 99={build.xml='\n\n ok\n 1388649845000\n ok\n'}, lastFailedBuild=→-1, lastSuccessfulBuild=→99, legacyIds='2014-01-02_03-04-04 98\n'}", summarize()); + assertEquals("{98={build.xml='\n 2014-01-02_03-04-04\n 1388649844000\n'}, 99={build.xml='\n\n ok\n 1388649845000\n ok\n'}, lastFailedBuild=→-1, lastSuccessfulBuild=→99, legacyIds='2014-01-02_03-04-04 98\n'}", summarize()); } @Test public void reverseImmediately() throws Exception { assumeFalse("Symlinks don't work well on Windows", Functions.isWindows()); File root = dir; dir = new File(dir, "jobs/somefolder/jobs/someproject/promotions/OK/builds"); - write("99/build.xml", "\n\n ok\n 2014-01-02_03-04-05\n 1388649845000\n ok\n"); + write("99/build.xml", "\n\n ok\n 2014-01-02_03-04-05\n 1388649845000\n ok\n"); link("lastFailedBuild", "-1"); link("lastSuccessfulBuild", "99"); write("legacyIds", "2014-01-02_03-04-05 99\n"); - assertEquals("{99={build.xml='\n\n ok\n 2014-01-02_03-04-05\n 1388649845000\n ok\n'}, lastFailedBuild=→-1, lastSuccessfulBuild=→99, legacyIds='2014-01-02_03-04-05 99\n'}", summarize()); + assertEquals("{99={build.xml='\n\n ok\n 2014-01-02_03-04-05\n 1388649845000\n ok\n'}, lastFailedBuild=→-1, lastSuccessfulBuild=→99, legacyIds='2014-01-02_03-04-05 99\n'}", summarize()); RunIdMigrator.main(root.getAbsolutePath()); - assertEquals("{2014-01-02_03-04-05={build.xml='\n\n ok\n 99\n ok\n'}, 99=→2014-01-02_03-04-05, lastFailedBuild=→-1, lastSuccessfulBuild=→99}", summarize()); + assertEquals("{2014-01-02_03-04-05={build.xml='\n\n ok\n 99\n ok\n'}, 99=→2014-01-02_03-04-05, lastFailedBuild=→-1, lastSuccessfulBuild=→99}", summarize()); } @Test public void reverseAfterNewBuilds() throws Exception { assumeFalse("Symlinks don't work well on Windows", Functions.isWindows()); File root = dir; dir = new File(dir, "jobs/someproject/modules/test$test/builds"); - write("1/build.xml", "\n\n ok\n 1388649845000\n ok\n"); + write("1/build.xml", "\n\n ok\n 1388649845000\n ok\n"); write("legacyIds", ""); - assertEquals("{1={build.xml='\n\n ok\n 1388649845000\n ok\n'}, legacyIds=''}", summarize()); + assertEquals("{1={build.xml='\n\n ok\n 1388649845000\n ok\n'}, legacyIds=''}", summarize()); RunIdMigrator.main(root.getAbsolutePath()); - assertEquals("{1=→2014-01-02_03-04-05, 2014-01-02_03-04-05={build.xml='\n\n ok\n 1\n ok\n'}}", summarize()); + assertEquals("{1=→2014-01-02_03-04-05, 2014-01-02_03-04-05={build.xml='\n\n ok\n 1\n ok\n'}}", summarize()); } @Test public void reverseMatrixAfterNewBuilds() throws Exception { assumeFalse("Symlinks don't work well on Windows", Functions.isWindows()); File root = dir; dir = new File(dir, "jobs/someproject/Environment=prod/builds"); - write("1/build.xml", "\n\n ok\n 1388649845000\n ok\n"); + write("1/build.xml", "\n\n ok\n 1388649845000\n ok\n"); write("legacyIds", ""); - assertEquals("{1={build.xml='\n\n ok\n 1388649845000\n ok\n'}, legacyIds=''}", summarize()); + assertEquals("{1={build.xml='\n\n ok\n 1388649845000\n ok\n'}, legacyIds=''}", summarize()); RunIdMigrator.main(root.getAbsolutePath()); - assertEquals("{1=→2014-01-02_03-04-05, 2014-01-02_03-04-05={build.xml='\n\n ok\n 1\n ok\n'}}", summarize()); + assertEquals("{1=→2014-01-02_03-04-05, 2014-01-02_03-04-05={build.xml='\n\n ok\n 1\n ok\n'}}", summarize()); } @Test public void reverseMavenAfterNewBuilds() throws Exception { assumeFalse("Symlinks don't work well on Windows", Functions.isWindows()); File root = dir; dir = new File(dir, "jobs/someproject/test$test/builds"); - write("1/build.xml", "\n\n ok\n 1388649845000\n ok\n"); + write("1/build.xml", "\n\n ok\n 1388649845000\n ok\n"); write("legacyIds", ""); - assertEquals("{1={build.xml='\n\n ok\n 1388649845000\n ok\n'}, legacyIds=''}", summarize()); + assertEquals("{1={build.xml='\n\n ok\n 1388649845000\n ok\n'}, legacyIds=''}", summarize()); RunIdMigrator.main(root.getAbsolutePath()); - assertEquals("{1=→2014-01-02_03-04-05, 2014-01-02_03-04-05={build.xml='\n\n ok\n 1\n ok\n'}}", summarize()); + assertEquals("{1=→2014-01-02_03-04-05, 2014-01-02_03-04-05={build.xml='\n\n ok\n 1\n ok\n'}}", summarize()); } // TODO test sane recovery from various error conditions diff --git a/core/src/test/java/jenkins/util/xstream/XStreamDOMTest.java b/core/src/test/java/jenkins/util/xstream/XStreamDOMTest.java index b32ed5a40f..69a1fbe7ef 100644 --- a/core/src/test/java/jenkins/util/xstream/XStreamDOMTest.java +++ b/core/src/test/java/jenkins/util/xstream/XStreamDOMTest.java @@ -23,6 +23,7 @@ */ package jenkins.util.xstream; +import com.thoughtworks.xstream.io.xml.KXml2Driver; import hudson.util.XStream2; import org.apache.commons.io.IOUtils; import org.junit.Before; @@ -53,7 +54,7 @@ public class XStreamDOMTest { @Before public void setUp() throws Exception { - xs = new XStream2(); + xs = new XStream2(new KXml2Driver()); xs.alias("foo", Foo.class); } diff --git a/core/src/test/java/jenkins/xml/XMLUtilsTest.java b/core/src/test/java/jenkins/xml/XMLUtilsTest.java index 55b0d806bb..5b998b1ca1 100644 --- a/core/src/test/java/jenkins/xml/XMLUtilsTest.java +++ b/core/src/test/java/jenkins/xml/XMLUtilsTest.java @@ -49,7 +49,7 @@ public class XMLUtilsTest { @Issue("SECURITY-167") @Test() public void testSafeTransformDoesNotProcessForeignResources() throws Exception { - final String xml = "\n" + + final String xml = "\n" + "\n" + "]>\n" + @@ -82,7 +82,7 @@ public class XMLUtilsTest { @Issue("SECURITY-167") @Test() public void testUpdateByXmlIDoesNotFail() throws Exception { - final String xml = "\n" + + final String xml = "\n" + "\n" + " \n" + " &\n" + @@ -119,7 +119,7 @@ public class XMLUtilsTest { @Test public void testParse_with_XXE() throws IOException, XPathExpressionException { try { - final String xml = "\n" + + final String xml = "\n" + "\n" + " ]> " + diff --git a/test/src/test/java/hudson/cli/GetNodeCommandTest.java b/test/src/test/java/hudson/cli/GetNodeCommandTest.java index b2792924aa..71cc405915 100644 --- a/test/src/test/java/hudson/cli/GetNodeCommandTest.java +++ b/test/src/test/java/hudson/cli/GetNodeCommandTest.java @@ -74,7 +74,7 @@ public class GetNodeCommandTest { .invokeWithArgs("MySlave") ; - assertThat(result.stdout(), startsWith("")); + assertThat(result.stdout(), startsWith("")); assertThat(result.stdout(), containsString("MySlave")); assertThat(result, hasNoErrorOutput()); assertThat(result, succeeded()); diff --git a/test/src/test/java/hudson/cli/GetViewCommandTest.java b/test/src/test/java/hudson/cli/GetViewCommandTest.java index d3a6fd9ef7..c08af750d2 100644 --- a/test/src/test/java/hudson/cli/GetViewCommandTest.java +++ b/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("")); + assertThat(result.stdout(), startsWith("")); assertThat(result.stdout(), containsString("aView")); } diff --git a/test/src/test/java/hudson/model/AbstractItemSecurityTest.java b/test/src/test/java/hudson/model/AbstractItemSecurityTest.java index 653097d295..7ed8b79557 100644 --- a/test/src/test/java/hudson/model/AbstractItemSecurityTest.java +++ b/test/src/test/java/hudson/model/AbstractItemSecurityTest.java @@ -47,7 +47,7 @@ public class AbstractItemSecurityTest { @Issue("SECURITY-167") @Test() public void testUpdateByXmlDoesNotProcessForeignResources() throws Exception { - final String xml = "\n" + + final String xml = "\n" + "\n" + "]>\n" + @@ -73,7 +73,7 @@ public class AbstractItemSecurityTest { @Issue("SECURITY-167") @Test() public void testUpdateByXmlDoesNotFail() throws Exception { - final String xml = "\n" + + final String xml = "\n" + "\n" + " &\n" + " \n" + diff --git a/test/src/test/java/hudson/model/ComputerConfigDotXmlTest.java b/test/src/test/java/hudson/model/ComputerConfigDotXmlTest.java index 737e7b3228..6661d55780 100644 --- a/test/src/test/java/hudson/model/ComputerConfigDotXmlTest.java +++ b/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("")); + assertThat(out, startsWith("")); assertThat(out, containsString("slave0")); assertThat(out, containsString("dummy")); } diff --git a/test/src/test/java/hudson/model/ItemGroupMixInTest.java b/test/src/test/java/hudson/model/ItemGroupMixInTest.java index 79ea318b8b..0bf83e5970 100644 --- a/test/src/test/java/hudson/model/ItemGroupMixInTest.java +++ b/test/src/test/java/hudson/model/ItemGroupMixInTest.java @@ -185,7 +185,7 @@ public class ItemGroupMixInTest { @Test public void createProjectFromXMLShouldNoCreateEntities() throws IOException { - final String xml = "\n" + + final String xml = "\n" + "\n" + "]>\n" + diff --git a/test/src/test/java/hudson/util/RobustReflectionConverterTest.java b/test/src/test/java/hudson/util/RobustReflectionConverterTest.java index 0bff3490cf..bdbc887b88 100644 --- a/test/src/test/java/hudson/util/RobustReflectionConverterTest.java +++ b/test/src/test/java/hudson/util/RobustReflectionConverterTest.java @@ -168,7 +168,7 @@ public class RobustReflectionConverterTest { } private static final String CONFIGURATION_TEMPLATE = - "" + "" + "" + "" + "" From 37365eeca7e8683920dd0d13f039e36380f19e24 Mon Sep 17 00:00:00 2001 From: mike cirioli Date: Fri, 8 Dec 2017 17:30:55 -0500 Subject: [PATCH 03/11] [JENKINS-48463] update hard coded references of XPP3Driver to KXml2Driver in order to support XMLv1.1 --- core/src/main/java/hudson/model/View.java | 4 ++-- .../src/main/java/jenkins/util/xstream/XStreamDOM.java | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/hudson/model/View.java b/core/src/main/java/hudson/model/View.java index fa3914af44..fe181c327d 100644 --- a/core/src/main/java/hudson/model/View.java +++ b/core/src/main/java/hudson/model/View.java @@ -26,7 +26,7 @@ package hudson.model; import com.thoughtworks.xstream.converters.ConversionException; import com.thoughtworks.xstream.io.StreamException; -import com.thoughtworks.xstream.io.xml.Xpp3Driver; +import com.thoughtworks.xstream.io.xml.KXml2Driver; import hudson.DescriptorExtensionList; import hudson.Extension; import hudson.ExtensionPoint; @@ -1213,7 +1213,7 @@ public abstract class View extends AbstractModelObject implements AccessControll // Do not allow overwriting view name as it might collide with another // view in same ViewGroup and might not satisfy Jenkins.checkGoodName. String oldname = name; - Object o = Jenkins.XSTREAM.unmarshal(new Xpp3Driver().createReader(in), this); + Object o = Jenkins.XSTREAM.unmarshal(new KXml2Driver().createReader(in), this); 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 diff --git a/core/src/main/java/jenkins/util/xstream/XStreamDOM.java b/core/src/main/java/jenkins/util/xstream/XStreamDOM.java index d98f346540..f0def484da 100644 --- a/core/src/main/java/jenkins/util/xstream/XStreamDOM.java +++ b/core/src/main/java/jenkins/util/xstream/XStreamDOM.java @@ -34,8 +34,8 @@ import com.thoughtworks.xstream.io.HierarchicalStreamWriter; 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.KXml2Driver; import com.thoughtworks.xstream.io.xml.XmlFriendlyReplacer; -import com.thoughtworks.xstream.io.xml.Xpp3Driver; import hudson.Util; import hudson.util.VariableResolver; @@ -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(new KXml2Driver().createWriter(os)); } public void writeTo(Writer w) { - writeTo(new Xpp3Driver().createWriter(w)); + writeTo(new KXml2Driver().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(new KXml2Driver().createReader(in)); } public static XStreamDOM from(Reader in) { - return from(new Xpp3Driver().createReader(in)); + return from(new KXml2Driver().createReader(in)); } public static XStreamDOM from(HierarchicalStreamReader in) { From 570b52dbcc4e6b2555bb8080994d30cc7228f9ca Mon Sep 17 00:00:00 2001 From: mike cirioli Date: Wed, 10 Jan 2018 08:31:34 -0500 Subject: [PATCH 04/11] [JENKINS-48463] refactored XStream2 to default to KXml2Driver by default. Changed all explicit references of the hierarchical driver to use a static convinience method XStream2.getDefaultDriver() to ensure all XML operations are using the same driver. --- core/src/main/java/hudson/XmlFile.java | 7 ++-- core/src/main/java/hudson/model/View.java | 3 +- core/src/main/java/hudson/util/XStream2.java | 13 +++++++ .../java/jenkins/util/xstream/XStreamDOM.java | 10 +++--- .../test/java/hudson/PluginManagerTest.java | 2 +- .../hudson/util/XStream2EncodingTest.java | 2 +- .../java/jenkins/model/RunIdMigratorTest.java | 36 +++++++++---------- .../jenkins/util/xstream/XStreamDOMTest.java | 3 +- .../test/java/jenkins/xml/XMLUtilsTest.java | 6 ++-- .../model/AbstractItemSecurityTest.java | 4 +-- .../java/hudson/model/ItemGroupMixInTest.java | 2 +- 11 files changed, 50 insertions(+), 38 deletions(-) diff --git a/core/src/main/java/hudson/XmlFile.java b/core/src/main/java/hudson/XmlFile.java index ae65b265d5..f63ed33881 100644 --- a/core/src/main/java/hudson/XmlFile.java +++ b/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.KXml2Driver; +import com.thoughtworks.xstream.io.HierarchicalStreamDriver; import hudson.diagnosis.OldDataMonitor; import hudson.model.Descriptor; import hudson.util.AtomicFileWriter; @@ -336,13 +336,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 KXml2Driver DEFAULT_DRIVER = new KXml2Driver(); + private static final HierarchicalStreamDriver DEFAULT_DRIVER = XStream2.getDefaultDriver(); + + private static final XStream DEFAULT_XSTREAM = new XStream2(DEFAULT_DRIVER); static { JAXP.setNamespaceAware(true); diff --git a/core/src/main/java/hudson/model/View.java b/core/src/main/java/hudson/model/View.java index fe181c327d..159f0befc3 100644 --- a/core/src/main/java/hudson/model/View.java +++ b/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.KXml2Driver; import hudson.DescriptorExtensionList; import hudson.Extension; import hudson.ExtensionPoint; @@ -1213,7 +1212,7 @@ public abstract class View extends AbstractModelObject implements AccessControll // Do not allow overwriting view name as it might collide with another // view in same ViewGroup and might not satisfy Jenkins.checkGoodName. String oldname = name; - Object o = Jenkins.XSTREAM.unmarshal(new KXml2Driver().createReader(in), this); + Object o = Jenkins.XSTREAM.unmarshal(XStream2.getDefaultDriver().createReader(in), this); 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 diff --git a/core/src/main/java/hudson/util/XStream2.java b/core/src/main/java/hudson/util/XStream2.java index 285dd6b2db..b4a9520d28 100644 --- a/core/src/main/java/hudson/util/XStream2.java +++ b/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; @@ -86,7 +87,18 @@ public class XStream2 extends XStream { */ private MapperInjectionPoint mapperInjectionPoint; + /** + * Convinience 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; } @@ -98,6 +110,7 @@ public class XStream2 extends XStream { } XStream2(ClassOwnership classOwnership) { + super(getDefaultDriver()); init(); this.classOwnership = classOwnership; } diff --git a/core/src/main/java/jenkins/util/xstream/XStreamDOM.java b/core/src/main/java/jenkins/util/xstream/XStreamDOM.java index f0def484da..2b2ed1621b 100644 --- a/core/src/main/java/jenkins/util/xstream/XStreamDOM.java +++ b/core/src/main/java/jenkins/util/xstream/XStreamDOM.java @@ -34,11 +34,11 @@ import com.thoughtworks.xstream.io.HierarchicalStreamWriter; 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.KXml2Driver; import com.thoughtworks.xstream.io.xml.XmlFriendlyReplacer; 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 KXml2Driver().createWriter(os)); + writeTo(XStream2.getDefaultDriver().createWriter(os)); } public void writeTo(Writer w) { - writeTo(new KXml2Driver().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 KXml2Driver().createReader(in)); + return from(XStream2.getDefaultDriver().createReader(in)); } public static XStreamDOM from(Reader in) { - return from(new KXml2Driver().createReader(in)); + return from(XStream2.getDefaultDriver().createReader(in)); } public static XStreamDOM from(HierarchicalStreamReader in) { diff --git a/core/src/test/java/hudson/PluginManagerTest.java b/core/src/test/java/hudson/PluginManagerTest.java index 1e89e5c5c8..843e15ba1e 100644 --- a/core/src/test/java/hudson/PluginManagerTest.java +++ b/core/src/test/java/hudson/PluginManagerTest.java @@ -64,7 +64,7 @@ public class PluginManagerTest { @Issue("SECURITY-167") @Test public void parseInvalidRequestedPlugins() throws Exception { - String evilXML = "\n" + + String evilXML = "\n" + "]>\n" + "\n" + " \n" + diff --git a/core/src/test/java/hudson/util/XStream2EncodingTest.java b/core/src/test/java/hudson/util/XStream2EncodingTest.java index ea10414ebc..ea23797dea 100644 --- a/core/src/test/java/hudson/util/XStream2EncodingTest.java +++ b/core/src/test/java/hudson/util/XStream2EncodingTest.java @@ -70,7 +70,7 @@ public class XStream2EncodingTest { Thing t = (Thing) xs.fromXML(new ByteArrayInputStream(ambiguousXml)); assertThat(t.field, not(msg)); ByteArrayOutputStream baos2 = new ByteArrayOutputStream(); - baos2.write("\n".getBytes("UTF-8")); + baos2.write("\n".getBytes("UTF-8")); baos2.write(ambiguousXml); t = (Thing) xs.fromXML(new ByteArrayInputStream(ambiguousXml)); assertThat(t.field, not(msg)); diff --git a/core/src/test/java/jenkins/model/RunIdMigratorTest.java b/core/src/test/java/jenkins/model/RunIdMigratorTest.java index fda491eda6..71a3e47832 100644 --- a/core/src/test/java/jenkins/model/RunIdMigratorTest.java +++ b/core/src/test/java/jenkins/model/RunIdMigratorTest.java @@ -83,13 +83,13 @@ public class RunIdMigratorTest { @Test public void legacy() throws Exception { assumeFalse("Symlinks don't work well on Windows", Functions.isWindows()); - write("2014-01-02_03-04-05/build.xml", "\n\n ok\n 99\n ok\n"); + write("2014-01-02_03-04-05/build.xml", "\n\n ok\n 99\n ok\n"); link("99", "2014-01-02_03-04-05"); link("lastFailedBuild", "-1"); link("lastSuccessfulBuild", "99"); - assertEquals("{2014-01-02_03-04-05={build.xml='\n\n ok\n 99\n ok\n'}, 99=→2014-01-02_03-04-05, lastFailedBuild=→-1, lastSuccessfulBuild=→99}", summarize()); + assertEquals("{2014-01-02_03-04-05={build.xml='\n\n ok\n 99\n ok\n'}, 99=→2014-01-02_03-04-05, lastFailedBuild=→-1, lastSuccessfulBuild=→99}", summarize()); assertTrue(migrator.migrate(dir, null)); - assertEquals("{99={build.xml='\n\n ok\n 2014-01-02_03-04-05\n 1388649845000\n ok\n'}, lastFailedBuild=→-1, lastSuccessfulBuild=→99, legacyIds='2014-01-02_03-04-05 99\n'}", summarize()); + assertEquals("{99={build.xml='\n\n ok\n 2014-01-02_03-04-05\n 1388649845000\n ok\n'}, lastFailedBuild=→-1, lastSuccessfulBuild=→99, legacyIds='2014-01-02_03-04-05 99\n'}", summarize()); assertEquals(99, migrator.findNumber("2014-01-02_03-04-05")); migrator = new RunIdMigrator(); assertFalse(migrator.migrate(dir, null)); @@ -104,58 +104,58 @@ public class RunIdMigratorTest { assumeFalse("Symlinks don't work well on Windows", Functions.isWindows()); write("2014-01-02_03-04-04/build.xml", "\n 98\n"); link("98", "2014-01-02_03-04-04"); - write("99/build.xml", "\n\n ok\n 1388649845000\n ok\n"); + write("99/build.xml", "\n\n ok\n 1388649845000\n ok\n"); link("lastFailedBuild", "-1"); link("lastSuccessfulBuild", "99"); - assertEquals("{2014-01-02_03-04-04={build.xml='\n 98\n'}, 98=→2014-01-02_03-04-04, 99={build.xml='\n\n ok\n 1388649845000\n ok\n'}, lastFailedBuild=→-1, lastSuccessfulBuild=→99}", summarize()); + assertEquals("{2014-01-02_03-04-04={build.xml='\n 98\n'}, 98=→2014-01-02_03-04-04, 99={build.xml='\n\n ok\n 1388649845000\n ok\n'}, lastFailedBuild=→-1, lastSuccessfulBuild=→99}", summarize()); assertTrue(migrator.migrate(dir, null)); - assertEquals("{98={build.xml='\n 2014-01-02_03-04-04\n 1388649844000\n'}, 99={build.xml='\n\n ok\n 1388649845000\n ok\n'}, lastFailedBuild=→-1, lastSuccessfulBuild=→99, legacyIds='2014-01-02_03-04-04 98\n'}", summarize()); + assertEquals("{98={build.xml='\n 2014-01-02_03-04-04\n 1388649844000\n'}, 99={build.xml='\n\n ok\n 1388649845000\n ok\n'}, lastFailedBuild=→-1, lastSuccessfulBuild=→99, legacyIds='2014-01-02_03-04-04 98\n'}", summarize()); } @Test public void reverseImmediately() throws Exception { assumeFalse("Symlinks don't work well on Windows", Functions.isWindows()); File root = dir; dir = new File(dir, "jobs/somefolder/jobs/someproject/promotions/OK/builds"); - write("99/build.xml", "\n\n ok\n 2014-01-02_03-04-05\n 1388649845000\n ok\n"); + write("99/build.xml", "\n\n ok\n 2014-01-02_03-04-05\n 1388649845000\n ok\n"); link("lastFailedBuild", "-1"); link("lastSuccessfulBuild", "99"); write("legacyIds", "2014-01-02_03-04-05 99\n"); - assertEquals("{99={build.xml='\n\n ok\n 2014-01-02_03-04-05\n 1388649845000\n ok\n'}, lastFailedBuild=→-1, lastSuccessfulBuild=→99, legacyIds='2014-01-02_03-04-05 99\n'}", summarize()); + assertEquals("{99={build.xml='\n\n ok\n 2014-01-02_03-04-05\n 1388649845000\n ok\n'}, lastFailedBuild=→-1, lastSuccessfulBuild=→99, legacyIds='2014-01-02_03-04-05 99\n'}", summarize()); RunIdMigrator.main(root.getAbsolutePath()); - assertEquals("{2014-01-02_03-04-05={build.xml='\n\n ok\n 99\n ok\n'}, 99=→2014-01-02_03-04-05, lastFailedBuild=→-1, lastSuccessfulBuild=→99}", summarize()); + assertEquals("{2014-01-02_03-04-05={build.xml='\n\n ok\n 99\n ok\n'}, 99=→2014-01-02_03-04-05, lastFailedBuild=→-1, lastSuccessfulBuild=→99}", summarize()); } @Test public void reverseAfterNewBuilds() throws Exception { assumeFalse("Symlinks don't work well on Windows", Functions.isWindows()); File root = dir; dir = new File(dir, "jobs/someproject/modules/test$test/builds"); - write("1/build.xml", "\n\n ok\n 1388649845000\n ok\n"); + write("1/build.xml", "\n\n ok\n 1388649845000\n ok\n"); write("legacyIds", ""); - assertEquals("{1={build.xml='\n\n ok\n 1388649845000\n ok\n'}, legacyIds=''}", summarize()); + assertEquals("{1={build.xml='\n\n ok\n 1388649845000\n ok\n'}, legacyIds=''}", summarize()); RunIdMigrator.main(root.getAbsolutePath()); - assertEquals("{1=→2014-01-02_03-04-05, 2014-01-02_03-04-05={build.xml='\n\n ok\n 1\n ok\n'}}", summarize()); + assertEquals("{1=→2014-01-02_03-04-05, 2014-01-02_03-04-05={build.xml='\n\n ok\n 1\n ok\n'}}", summarize()); } @Test public void reverseMatrixAfterNewBuilds() throws Exception { assumeFalse("Symlinks don't work well on Windows", Functions.isWindows()); File root = dir; dir = new File(dir, "jobs/someproject/Environment=prod/builds"); - write("1/build.xml", "\n\n ok\n 1388649845000\n ok\n"); + write("1/build.xml", "\n\n ok\n 1388649845000\n ok\n"); write("legacyIds", ""); - assertEquals("{1={build.xml='\n\n ok\n 1388649845000\n ok\n'}, legacyIds=''}", summarize()); + assertEquals("{1={build.xml='\n\n ok\n 1388649845000\n ok\n'}, legacyIds=''}", summarize()); RunIdMigrator.main(root.getAbsolutePath()); - assertEquals("{1=→2014-01-02_03-04-05, 2014-01-02_03-04-05={build.xml='\n\n ok\n 1\n ok\n'}}", summarize()); + assertEquals("{1=→2014-01-02_03-04-05, 2014-01-02_03-04-05={build.xml='\n\n ok\n 1\n ok\n'}}", summarize()); } @Test public void reverseMavenAfterNewBuilds() throws Exception { assumeFalse("Symlinks don't work well on Windows", Functions.isWindows()); File root = dir; dir = new File(dir, "jobs/someproject/test$test/builds"); - write("1/build.xml", "\n\n ok\n 1388649845000\n ok\n"); + write("1/build.xml", "\n\n ok\n 1388649845000\n ok\n"); write("legacyIds", ""); - assertEquals("{1={build.xml='\n\n ok\n 1388649845000\n ok\n'}, legacyIds=''}", summarize()); + assertEquals("{1={build.xml='\n\n ok\n 1388649845000\n ok\n'}, legacyIds=''}", summarize()); RunIdMigrator.main(root.getAbsolutePath()); - assertEquals("{1=→2014-01-02_03-04-05, 2014-01-02_03-04-05={build.xml='\n\n ok\n 1\n ok\n'}}", summarize()); + assertEquals("{1=→2014-01-02_03-04-05, 2014-01-02_03-04-05={build.xml='\n\n ok\n 1\n ok\n'}}", summarize()); } // TODO test sane recovery from various error conditions diff --git a/core/src/test/java/jenkins/util/xstream/XStreamDOMTest.java b/core/src/test/java/jenkins/util/xstream/XStreamDOMTest.java index 69a1fbe7ef..b32ed5a40f 100644 --- a/core/src/test/java/jenkins/util/xstream/XStreamDOMTest.java +++ b/core/src/test/java/jenkins/util/xstream/XStreamDOMTest.java @@ -23,7 +23,6 @@ */ package jenkins.util.xstream; -import com.thoughtworks.xstream.io.xml.KXml2Driver; import hudson.util.XStream2; import org.apache.commons.io.IOUtils; import org.junit.Before; @@ -54,7 +53,7 @@ public class XStreamDOMTest { @Before public void setUp() throws Exception { - xs = new XStream2(new KXml2Driver()); + xs = new XStream2(); xs.alias("foo", Foo.class); } diff --git a/core/src/test/java/jenkins/xml/XMLUtilsTest.java b/core/src/test/java/jenkins/xml/XMLUtilsTest.java index 5b998b1ca1..55b0d806bb 100644 --- a/core/src/test/java/jenkins/xml/XMLUtilsTest.java +++ b/core/src/test/java/jenkins/xml/XMLUtilsTest.java @@ -49,7 +49,7 @@ public class XMLUtilsTest { @Issue("SECURITY-167") @Test() public void testSafeTransformDoesNotProcessForeignResources() throws Exception { - final String xml = "\n" + + final String xml = "\n" + "\n" + "]>\n" + @@ -82,7 +82,7 @@ public class XMLUtilsTest { @Issue("SECURITY-167") @Test() public void testUpdateByXmlIDoesNotFail() throws Exception { - final String xml = "\n" + + final String xml = "\n" + "\n" + " \n" + " &\n" + @@ -119,7 +119,7 @@ public class XMLUtilsTest { @Test public void testParse_with_XXE() throws IOException, XPathExpressionException { try { - final String xml = "\n" + + final String xml = "\n" + "\n" + " ]> " + diff --git a/test/src/test/java/hudson/model/AbstractItemSecurityTest.java b/test/src/test/java/hudson/model/AbstractItemSecurityTest.java index 7ed8b79557..653097d295 100644 --- a/test/src/test/java/hudson/model/AbstractItemSecurityTest.java +++ b/test/src/test/java/hudson/model/AbstractItemSecurityTest.java @@ -47,7 +47,7 @@ public class AbstractItemSecurityTest { @Issue("SECURITY-167") @Test() public void testUpdateByXmlDoesNotProcessForeignResources() throws Exception { - final String xml = "\n" + + final String xml = "\n" + "\n" + "]>\n" + @@ -73,7 +73,7 @@ public class AbstractItemSecurityTest { @Issue("SECURITY-167") @Test() public void testUpdateByXmlDoesNotFail() throws Exception { - final String xml = "\n" + + final String xml = "\n" + "\n" + " &\n" + " \n" + diff --git a/test/src/test/java/hudson/model/ItemGroupMixInTest.java b/test/src/test/java/hudson/model/ItemGroupMixInTest.java index 0bf83e5970..79ea318b8b 100644 --- a/test/src/test/java/hudson/model/ItemGroupMixInTest.java +++ b/test/src/test/java/hudson/model/ItemGroupMixInTest.java @@ -185,7 +185,7 @@ public class ItemGroupMixInTest { @Test public void createProjectFromXMLShouldNoCreateEntities() throws IOException { - final String xml = "\n" + + final String xml = "\n" + "\n" + "]>\n" + From 24a1852fc2c9a74c59a25c3e0ed022e315894344 Mon Sep 17 00:00:00 2001 From: mike cirioli Date: Thu, 11 Jan 2018 11:04:46 -0500 Subject: [PATCH 05/11] [JENKINS-48463] Added additional unit tests for verifying xml v1.1 compatibility - verify can still read xml 1.0 - verify can read xml 1.1 - verify can read xml 1.1 with 'special' characters - verify that jenkins can start with a config.xml having and special characters --- core/src/test/java/hudson/XmlFileTest.java | 84 +++++++++++++++++++ .../hudson/confg_1_1_with_special_chars.xml | 9 ++ core/src/test/resources/hudson/config_1_0.xml | 8 ++ core/src/test/resources/hudson/config_1_1.xml | 8 ++ pom.xml | 2 +- test/src/test/java/hudson/XMLFileTest.java | 23 +++++ .../config.xml | 9 ++ 7 files changed, 142 insertions(+), 1 deletion(-) create mode 100644 core/src/test/java/hudson/XmlFileTest.java create mode 100644 core/src/test/resources/hudson/confg_1_1_with_special_chars.xml create mode 100644 core/src/test/resources/hudson/config_1_0.xml create mode 100644 core/src/test/resources/hudson/config_1_1.xml create mode 100644 test/src/test/java/hudson/XMLFileTest.java create mode 100644 test/src/test/resources/hudson/XMLFileTest/canStartWithXml_1_1_ConfigsTest/config.xml diff --git a/core/src/test/java/hudson/XmlFileTest.java b/core/src/test/java/hudson/XmlFileTest.java new file mode 100644 index 0000000000..0583e46e99 --- /dev/null +++ b/core/src/test/java/hudson/XmlFileTest.java @@ -0,0 +1,84 @@ +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 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"); + File configFile = new File(configUrl.getFile()); + XStream2 xs = new XStream2(); + xs.alias("hudson", Jenkins.class); + + XmlFile xmlFile = new XmlFile(xs, configFile); + 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"); + File configFile = new File(configUrl.getFile()); + XStream2 xs = new XStream2(); + xs.alias("hudson", Jenkins.class); + + XmlFile xmlFile = new XmlFile(xs, configFile); + if (xmlFile.exists()) { + Node n = (Node) xmlFile.read(); + assertThat(n.getNumExecutors(), is(2)); + assertThat(n.getMode().toString(), is("NORMAL")); + } + } + + @Test + @Ignore + //TODO: find a way to complete this test + public void xml1_0ConfigMigrateTo1_1Test() throws IOException { + URL configUrl = getClass().getResource("/hudson/config_1_0.xml"); + File configFile = new File(configUrl.getFile()); + XStream2 xs = new XStream2(); + xs.alias("hudson", Jenkins.class); + + XmlFile xmlFile = new XmlFile(xs, configFile); + if (xmlFile.exists()) { + Node n = (Node) xmlFile.read(); + assertThat(n.getNumExecutors(), is(2)); + assertThat(n.getMode().toString(), is("NORMAL")); + // this fails, because configFile.getParent() returns null....how do i fix? + n.save(); + // Now verify that the node is showing tag + } + + } + + @Test + public void canReadXmlWithControlCharsTest() throws IOException { + URL configUrl = getClass().getResource("/hudson/confg_1_1_with_special_chars.xml"); + File configFile = new File(configUrl.getFile()); + XStream2 xs = new XStream2(); + xs.alias("hudson", Jenkins.class); + + XmlFile xmlFile = new XmlFile(xs, configFile); + 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")); + } + } +} diff --git a/core/src/test/resources/hudson/confg_1_1_with_special_chars.xml b/core/src/test/resources/hudson/confg_1_1_with_special_chars.xml new file mode 100644 index 0000000000..cb072b5ebb --- /dev/null +++ b/core/src/test/resources/hudson/confg_1_1_with_special_chars.xml @@ -0,0 +1,9 @@ + + + + 1.480.1 + 2 + NORMAL + + + \ No newline at end of file diff --git a/core/src/test/resources/hudson/config_1_0.xml b/core/src/test/resources/hudson/config_1_0.xml new file mode 100644 index 0000000000..c3a76a6eef --- /dev/null +++ b/core/src/test/resources/hudson/config_1_0.xml @@ -0,0 +1,8 @@ + + + + 1.480.1 + 2 + NORMAL + + \ No newline at end of file diff --git a/core/src/test/resources/hudson/config_1_1.xml b/core/src/test/resources/hudson/config_1_1.xml new file mode 100644 index 0000000000..c3fd8c0603 --- /dev/null +++ b/core/src/test/resources/hudson/config_1_1.xml @@ -0,0 +1,8 @@ + + + + 1.480.1 + 2 + NORMAL + + \ No newline at end of file diff --git a/pom.xml b/pom.xml index c798586c2b..7b64d217d7 100644 --- a/pom.xml +++ b/pom.xml @@ -129,7 +129,7 @@ THE SOFTWARE. org.apache.ant ant - 1.8.4 + 1.9.2 diff --git a/test/src/test/java/hudson/XMLFileTest.java b/test/src/test/java/hudson/XMLFileTest.java new file mode 100644 index 0000000000..dde05c3423 --- /dev/null +++ b/test/src/test/java/hudson/XMLFileTest.java @@ -0,0 +1,23 @@ +package hudson; + +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")); + + } +} diff --git a/test/src/test/resources/hudson/XMLFileTest/canStartWithXml_1_1_ConfigsTest/config.xml b/test/src/test/resources/hudson/XMLFileTest/canStartWithXml_1_1_ConfigsTest/config.xml new file mode 100644 index 0000000000..cb072b5ebb --- /dev/null +++ b/test/src/test/resources/hudson/XMLFileTest/canStartWithXml_1_1_ConfigsTest/config.xml @@ -0,0 +1,9 @@ + + + + 1.480.1 + 2 + NORMAL + + + \ No newline at end of file From b9c2148d6e53ef4c7f0f01fab591b0333b4c69f1 Mon Sep 17 00:00:00 2001 From: mike cirioli Date: Fri, 12 Jan 2018 08:55:34 -0500 Subject: [PATCH 06/11] [JENKINS-48463] Added a unit test that validates jenkins xml v1.0 configs are silently migrated to xml v1.1 as they are persisted --- core/src/test/java/hudson/XmlFileTest.java | 24 +------------------ test/src/test/java/hudson/XMLFileTest.java | 22 +++++++++++++++++ .../silentlyMigrateConfigsTest/config.xml | 9 +++++++ 3 files changed, 32 insertions(+), 23 deletions(-) create mode 100644 test/src/test/resources/hudson/XMLFileTest/silentlyMigrateConfigsTest/config.xml diff --git a/core/src/test/java/hudson/XmlFileTest.java b/core/src/test/java/hudson/XmlFileTest.java index 0583e46e99..3493b4335f 100644 --- a/core/src/test/java/hudson/XmlFileTest.java +++ b/core/src/test/java/hudson/XmlFileTest.java @@ -7,7 +7,6 @@ import java.io.IOException; import java.net.URL; import jenkins.model.Jenkins; -import org.junit.Ignore; import org.junit.Test; import static org.hamcrest.CoreMatchers.is; @@ -44,28 +43,7 @@ public class XmlFileTest { assertThat(n.getMode().toString(), is("NORMAL")); } } - - @Test - @Ignore - //TODO: find a way to complete this test - public void xml1_0ConfigMigrateTo1_1Test() throws IOException { - URL configUrl = getClass().getResource("/hudson/config_1_0.xml"); - File configFile = new File(configUrl.getFile()); - XStream2 xs = new XStream2(); - xs.alias("hudson", Jenkins.class); - - XmlFile xmlFile = new XmlFile(xs, configFile); - if (xmlFile.exists()) { - Node n = (Node) xmlFile.read(); - assertThat(n.getNumExecutors(), is(2)); - assertThat(n.getMode().toString(), is("NORMAL")); - // this fails, because configFile.getParent() returns null....how do i fix? - n.save(); - // Now verify that the node is showing tag - } - - } - + @Test public void canReadXmlWithControlCharsTest() throws IOException { URL configUrl = getClass().getResource("/hudson/confg_1_1_with_special_chars.xml"); diff --git a/test/src/test/java/hudson/XMLFileTest.java b/test/src/test/java/hudson/XMLFileTest.java index dde05c3423..0fa2c8af08 100644 --- a/test/src/test/java/hudson/XMLFileTest.java +++ b/test/src/test/java/hudson/XMLFileTest.java @@ -1,5 +1,8 @@ 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; @@ -20,4 +23,23 @@ public class XMLFileTest { 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("LESS_TERMCAP_mb=\u001B[01;31m")); + //verify that the persisted top level config.xml is v1.1 + File configFile = new File(j.jenkins.getRootPath().getRemote() + File.separator + "config.xml"); + assertThat(configFile.exists(), is(true)); + BufferedReader config = new BufferedReader(new FileReader(configFile)); + assertThat(config.readLine(), is("")); + config.close(); + } } diff --git a/test/src/test/resources/hudson/XMLFileTest/silentlyMigrateConfigsTest/config.xml b/test/src/test/resources/hudson/XMLFileTest/silentlyMigrateConfigsTest/config.xml new file mode 100644 index 0000000000..f26e502947 --- /dev/null +++ b/test/src/test/resources/hudson/XMLFileTest/silentlyMigrateConfigsTest/config.xml @@ -0,0 +1,9 @@ + + + + 1.480.1 + 2 + NORMAL + + + \ No newline at end of file From a0f419120b72b3e645f5beb7b5213ae01cc58364 Mon Sep 17 00:00:00 2001 From: mike cirioli Date: Fri, 12 Jan 2018 10:16:57 -0500 Subject: [PATCH 07/11] [JENKINS-48463] It appears that the KXml2Driver is more tolerant of technically illegal xml (ie. it doesn't complain if an Xml v1.0 file contains special characters). Added a unit test that proves this, as well as fixing silentlyMigrateConfigsTest so that it starts with technically valid Xml 1.0 content --- core/src/test/java/hudson/XmlFileTest.java | 19 ++++++++++++++++++- .../hudson/config_1_0_with_special_chars.xml | 9 +++++++++ ....xml => config_1_1_with_special_chars.xml} | 0 test/src/test/java/hudson/XMLFileTest.java | 2 +- .../silentlyMigrateConfigsTest/config.xml | 2 +- 5 files changed, 29 insertions(+), 3 deletions(-) create mode 100644 core/src/test/resources/hudson/config_1_0_with_special_chars.xml rename core/src/test/resources/hudson/{confg_1_1_with_special_chars.xml => config_1_1_with_special_chars.xml} (100%) diff --git a/core/src/test/java/hudson/XmlFileTest.java b/core/src/test/java/hudson/XmlFileTest.java index 3493b4335f..b34d7340b4 100644 --- a/core/src/test/java/hudson/XmlFileTest.java +++ b/core/src/test/java/hudson/XmlFileTest.java @@ -8,6 +8,8 @@ import java.net.URL; import jenkins.model.Jenkins; 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; @@ -29,6 +31,21 @@ public class XmlFileTest { } } + @Test(expected = SAXParseException.class) + public void xml1_0_withSpecialCharsShouldFail() throws IOException { + URL configUrl = getClass().getResource("/hudson/config_1_0_with_special_chars.xml"); + File configFile = new File(configUrl.getFile()); + XStream2 xs = new XStream2(); + xs.alias("hudson", Jenkins.class); + + XmlFile xmlFile = new XmlFile(xs, configFile); + 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"); @@ -46,7 +63,7 @@ public class XmlFileTest { @Test public void canReadXmlWithControlCharsTest() throws IOException { - URL configUrl = getClass().getResource("/hudson/confg_1_1_with_special_chars.xml"); + URL configUrl = getClass().getResource("/hudson/config_1_1_with_special_chars.xml"); File configFile = new File(configUrl.getFile()); XStream2 xs = new XStream2(); xs.alias("hudson", Jenkins.class); diff --git a/core/src/test/resources/hudson/config_1_0_with_special_chars.xml b/core/src/test/resources/hudson/config_1_0_with_special_chars.xml new file mode 100644 index 0000000000..f26e502947 --- /dev/null +++ b/core/src/test/resources/hudson/config_1_0_with_special_chars.xml @@ -0,0 +1,9 @@ + + + + 1.480.1 + 2 + NORMAL + + + \ No newline at end of file diff --git a/core/src/test/resources/hudson/confg_1_1_with_special_chars.xml b/core/src/test/resources/hudson/config_1_1_with_special_chars.xml similarity index 100% rename from core/src/test/resources/hudson/confg_1_1_with_special_chars.xml rename to core/src/test/resources/hudson/config_1_1_with_special_chars.xml diff --git a/test/src/test/java/hudson/XMLFileTest.java b/test/src/test/java/hudson/XMLFileTest.java index 0fa2c8af08..b384ee7dfb 100644 --- a/test/src/test/java/hudson/XMLFileTest.java +++ b/test/src/test/java/hudson/XMLFileTest.java @@ -34,7 +34,7 @@ public class XMLFileTest { public void silentlyMigrateConfigsTest() throws Exception { j.jenkins.save(); // verify that we did indeed load our test config.xml - assertThat(j.jenkins.getLabelString(), is("LESS_TERMCAP_mb=\u001B[01;31m")); + 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.getRootPath().getRemote() + File.separator + "config.xml"); assertThat(configFile.exists(), is(true)); diff --git a/test/src/test/resources/hudson/XMLFileTest/silentlyMigrateConfigsTest/config.xml b/test/src/test/resources/hudson/XMLFileTest/silentlyMigrateConfigsTest/config.xml index f26e502947..1cc5aa6799 100644 --- a/test/src/test/resources/hudson/XMLFileTest/silentlyMigrateConfigsTest/config.xml +++ b/test/src/test/resources/hudson/XMLFileTest/silentlyMigrateConfigsTest/config.xml @@ -4,6 +4,6 @@ 1.480.1 2 NORMAL - + \ No newline at end of file From 0431cc75fd2745335a8e06c75697890ef049df79 Mon Sep 17 00:00:00 2001 From: mike cirioli Date: Fri, 12 Jan 2018 11:10:34 -0500 Subject: [PATCH 08/11] [JENKINS-48463] - Ignoring test xml1_0_withSpecialCharsShouldFail due to KXml2Driver being tolerant of control characters which should be illegal in an XML v1.0 content. This test should be revisted if we switch to another XML driver --- core/src/test/java/hudson/XmlFileTest.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/core/src/test/java/hudson/XmlFileTest.java b/core/src/test/java/hudson/XmlFileTest.java index b34d7340b4..f4c3c42e02 100644 --- a/core/src/test/java/hudson/XmlFileTest.java +++ b/core/src/test/java/hudson/XmlFileTest.java @@ -7,6 +7,7 @@ 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; @@ -31,6 +32,9 @@ public class XmlFileTest { } } + // 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"); From 992100ac4223b3d5eab11e8dccae5e55c2a18484 Mon Sep 17 00:00:00 2001 From: mike cirioli Date: Tue, 23 Jan 2018 13:13:13 -0500 Subject: [PATCH 09/11] [JENKINS-48463] - fixed reference to deprecated Xpp3Driver and replaced with new default driver --- core/src/main/java/hudson/model/View.java | 2 +- core/src/test/java/hudson/util/XStream2Test.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/hudson/model/View.java b/core/src/main/java/hudson/model/View.java index 3cb1a61d1f..721c87476a 100644 --- a/core/src/main/java/hudson/model/View.java +++ b/core/src/main/java/hudson/model/View.java @@ -1213,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.XSTREAM.unmarshal(XStream2.getDefaultDriver().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 diff --git a/core/src/test/java/hudson/util/XStream2Test.java b/core/src/test/java/hudson/util/XStream2Test.java index f5a038ae79..59192f02a4 100644 --- a/core/src/test/java/hudson/util/XStream2Test.java +++ b/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 { From d994e65a9a380908cfd6ac3e4c41c9f2040d9a73 Mon Sep 17 00:00:00 2001 From: mike cirioli Date: Fri, 26 Jan 2018 10:16:34 -0500 Subject: [PATCH 10/11] [JENKINS-48463] Erring on the side of caution and incuding xpp3 libs back in, just in case some plugin might be depending on them --- core/pom.xml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/core/pom.xml b/core/pom.xml index 7ef2c3227d..9c4faab34d 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -246,6 +246,11 @@ THE SOFTWARE. + + xpp3 + xpp3 + 1.1.4c + net.sf.kxml kxml2 From 5f0f9f2b87be0a2d2c1dfb34df59db117f9ea90f Mon Sep 17 00:00:00 2001 From: mike cirioli Date: Tue, 30 Jan 2018 09:31:15 -0500 Subject: [PATCH 11/11] [JENKINS-48463] added try-with-resources to XmlFileTest to ensure proper file handle cleanup in the event of failure Fixed small nits regarding typo's and code cleanup Added comment explaining why Xpp3 is still included as a dep --- core/pom.xml | 10 +++++++--- core/src/main/java/hudson/util/XStream2.java | 2 +- core/src/test/java/hudson/XmlFileTest.java | 12 ++++-------- test/src/test/java/hudson/XMLFileTest.java | 10 ++++++---- 4 files changed, 18 insertions(+), 16 deletions(-) diff --git a/core/pom.xml b/core/pom.xml index 9c4faab34d..a0752203b3 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -246,15 +246,19 @@ THE SOFTWARE. + xpp3 xpp3 1.1.4c - net.sf.kxml - kxml2 - 2.3.0 + net.sf.kxml + kxml2 + 2.3.0 jfree diff --git a/core/src/main/java/hudson/util/XStream2.java b/core/src/main/java/hudson/util/XStream2.java index 13d0b359c9..4f90342ea9 100644 --- a/core/src/main/java/hudson/util/XStream2.java +++ b/core/src/main/java/hudson/util/XStream2.java @@ -99,7 +99,7 @@ public class XStream2 extends XStream { private MapperInjectionPoint mapperInjectionPoint; /** - * Convinience method so we only have to change the driver in one place + * 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 diff --git a/core/src/test/java/hudson/XmlFileTest.java b/core/src/test/java/hudson/XmlFileTest.java index f4c3c42e02..69c85a93f7 100644 --- a/core/src/test/java/hudson/XmlFileTest.java +++ b/core/src/test/java/hudson/XmlFileTest.java @@ -20,11 +20,10 @@ public class XmlFileTest { @Test public void canReadXml1_0Test() throws IOException { URL configUrl = getClass().getResource("/hudson/config_1_0.xml"); - File configFile = new File(configUrl.getFile()); XStream2 xs = new XStream2(); xs.alias("hudson", Jenkins.class); - XmlFile xmlFile = new XmlFile(xs, configFile); + XmlFile xmlFile = new XmlFile(xs, new File(configUrl.getFile())); if (xmlFile.exists()) { Node n = (Node) xmlFile.read(); assertThat(n.getNumExecutors(), is(2)); @@ -38,11 +37,10 @@ public class XmlFileTest { @Test(expected = SAXParseException.class) public void xml1_0_withSpecialCharsShouldFail() throws IOException { URL configUrl = getClass().getResource("/hudson/config_1_0_with_special_chars.xml"); - File configFile = new File(configUrl.getFile()); XStream2 xs = new XStream2(); xs.alias("hudson", Jenkins.class); - XmlFile xmlFile = new XmlFile(xs, configFile); + XmlFile xmlFile = new XmlFile(xs, new File(configUrl.getFile())); if (xmlFile.exists()) { Node n = (Node) xmlFile.read(); assertThat(n.getNumExecutors(), is(2)); @@ -53,11 +51,10 @@ public class XmlFileTest { @Test public void canReadXml1_1Test() throws IOException { URL configUrl = getClass().getResource("/hudson/config_1_1.xml"); - File configFile = new File(configUrl.getFile()); XStream2 xs = new XStream2(); xs.alias("hudson", Jenkins.class); - XmlFile xmlFile = new XmlFile(xs, configFile); + XmlFile xmlFile = new XmlFile(xs, new File(configUrl.getFile())); if (xmlFile.exists()) { Node n = (Node) xmlFile.read(); assertThat(n.getNumExecutors(), is(2)); @@ -68,11 +65,10 @@ public class XmlFileTest { @Test public void canReadXmlWithControlCharsTest() throws IOException { URL configUrl = getClass().getResource("/hudson/config_1_1_with_special_chars.xml"); - File configFile = new File(configUrl.getFile()); XStream2 xs = new XStream2(); xs.alias("hudson", Jenkins.class); - XmlFile xmlFile = new XmlFile(xs, configFile); + XmlFile xmlFile = new XmlFile(xs, new File(configUrl.getFile())); if (xmlFile.exists()) { Node n = (Node) xmlFile.read(); assertThat(n.getNumExecutors(), is(2)); diff --git a/test/src/test/java/hudson/XMLFileTest.java b/test/src/test/java/hudson/XMLFileTest.java index b384ee7dfb..1b3a9ff3ea 100644 --- a/test/src/test/java/hudson/XMLFileTest.java +++ b/test/src/test/java/hudson/XMLFileTest.java @@ -36,10 +36,12 @@ public class XMLFileTest { // 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.getRootPath().getRemote() + File.separator + "config.xml"); + File configFile = new File(j.jenkins.getRootDir(), "config.xml"); assertThat(configFile.exists(), is(true)); - BufferedReader config = new BufferedReader(new FileReader(configFile)); - assertThat(config.readLine(), is("")); - config.close(); + + try (BufferedReader config = new BufferedReader(new FileReader(configFile))) { + assertThat(config.readLine(), is("")); + config.close(); + } } }