Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Merge pull request #3185 from mikecirioli/OSS-2590
[JENKINS-48463] Update jenkins core to use  XML v1.1
  • Loading branch information
batmat committed Feb 4, 2018
2 parents bff2d2a + 5f0f9f2 commit 0c17e82
Show file tree
Hide file tree
Showing 21 changed files with 226 additions and 27 deletions.
19 changes: 14 additions & 5 deletions core/pom.xml
Expand Up @@ -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>
Expand Down Expand Up @@ -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>
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/hudson/Main.java
Expand Up @@ -144,7 +144,7 @@ public static int remotePost(String[] args) throws Exception {
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();

Expand Down
10 changes: 5 additions & 5 deletions core/src/main/java/hudson/XmlFile.java
Expand Up @@ -26,7 +26,7 @@
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;
Expand All @@ -39,7 +39,6 @@
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;
Expand Down Expand Up @@ -187,7 +186,7 @@ public void write( Object o ) throws IOException {
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 {
Expand Down Expand Up @@ -352,13 +351,14 @@ private void attempt() throws Eureka {
/**
* {@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);
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/hudson/model/Fingerprint.java
Expand Up @@ -1256,7 +1256,7 @@ void save(File file) throws IOException {
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));
Expand Down
3 changes: 1 addition & 2 deletions core/src/main/java/hudson/model/View.java
Expand Up @@ -26,7 +26,6 @@

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;
Expand Down Expand Up @@ -1214,7 +1213,7 @@ public void updateByXml(Source source) throws IOException {
// 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
Expand Down
15 changes: 14 additions & 1 deletion core/src/main/java/hudson/util/XStream2.java
Expand Up @@ -26,6 +26,7 @@
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;
Expand Down Expand Up @@ -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;
}
Expand All @@ -109,6 +121,7 @@ public XStream2(HierarchicalStreamDriver hierarchicalStreamDriver) {
}

XStream2(ClassOwnership classOwnership) {
super(getDefaultDriver());
init();
this.classOwnership = classOwnership;
}
Expand Down Expand Up @@ -293,7 +306,7 @@ public Mapper getMapperInjectionPoint() {
*/
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);
}

Expand Down
10 changes: 5 additions & 5 deletions core/src/main/java/jenkins/util/xstream/XStreamDOM.java
Expand Up @@ -35,10 +35,10 @@
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;
Expand Down Expand Up @@ -241,11 +241,11 @@ public static WriterImpl newWriter() {
* 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) {
Expand All @@ -262,11 +262,11 @@ public static XStreamDOM from(XStream xs, Object obj) {
}

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) {
Expand Down
79 changes: 79 additions & 0 deletions 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"));
}
}
}
4 changes: 2 additions & 2 deletions core/src/test/java/hudson/util/XStream2Test.java
Expand Up @@ -29,7 +29,7 @@
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;
Expand Down Expand Up @@ -426,7 +426,7 @@ public void unmarshalToDefault_empty() {

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 {
Expand Down
8 changes: 8 additions & 0 deletions 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>
@@ -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>
8 changes: 8 additions & 0 deletions 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>
@@ -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>
2 changes: 1 addition & 1 deletion pom.xml
Expand Up @@ -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>
Expand Down
47 changes: 47 additions & 0 deletions 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();
}
}
}

0 comments on commit 0c17e82

Please sign in to comment.