[JENKINS-69129] Support escaped emoji characters in XML files (jenkin…

…sci#7778) * [JENKINS-69129] Support escaped emoji characters in XML files * Java 17 * Matching https://github.com/x-stream/xstream/blob/49ed8eb97c8551db0c8c5fa466f02d068862c272/xstream/src/java/com/thoughtworks/xstream/io/xml/SimpleStaxDriver.java#L55
krisstern · Apr 3, 2023 · ce3f6a8 · ce3f6a8
1 parent 40d56a7
commit ce3f6a8
Show file tree

Hide file tree

Showing 7 changed files with 87 additions and 8 deletions.
diff --git a/bom/pom.xml b/bom/pom.xml
@@ -175,6 +175,7 @@ THE SOFTWARE.
         <artifactId>jcip-annotations</artifactId>
         <version>1.0</version>
       </dependency>
+      <!-- TODO remove after determining this will not break plugins -->
       <dependency>
         <groupId>net.sf.kxml</groupId>
         <artifactId>kxml2</artifactId>

diff --git a/core/pom.xml b/core/pom.xml
@@ -265,6 +265,7 @@ THE SOFTWARE.
       <groupId>net.jcip</groupId>
       <artifactId>jcip-annotations</artifactId>
     </dependency>
+    <!-- TODO remove after determining this will not break plugins -->
     <dependency>
       <groupId>net.sf.kxml</groupId>
       <artifactId>kxml2</artifactId>

diff --git a/core/src/main/java/hudson/util/XStream2.java b/core/src/main/java/hudson/util/XStream2.java
@@ -46,7 +46,8 @@
 import com.thoughtworks.xstream.io.HierarchicalStreamReader;
 import com.thoughtworks.xstream.io.HierarchicalStreamWriter;
 import com.thoughtworks.xstream.io.ReaderWrapper;
-import com.thoughtworks.xstream.io.xml.KXml2Driver;
+import com.thoughtworks.xstream.io.xml.PrettyPrintWriter;
+import com.thoughtworks.xstream.io.xml.StandardStaxDriver;
 import com.thoughtworks.xstream.mapper.CannotResolveClassException;
 import com.thoughtworks.xstream.mapper.Mapper;
 import com.thoughtworks.xstream.mapper.MapperWrapper;
@@ -74,13 +75,16 @@
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Field;
 import java.lang.reflect.InvocationTargetException;
+import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import javax.xml.stream.XMLInputFactory;
+import javax.xml.stream.XMLOutputFactory;
 import jenkins.model.Jenkins;
 import jenkins.util.SystemProperties;
 import jenkins.util.xstream.SafeURLConverter;
@@ -123,7 +127,45 @@ public class XStream2 extends XStream {
      * @return a new instance of the HierarchicalStreamDriver we want to use
      */
     public static HierarchicalStreamDriver getDefaultDriver() {
-        return new KXml2Driver();
+        return new StaxDriver();
+    }
+
+    private static class StaxDriver extends StandardStaxDriver {
+        /*
+         * The below two methods are copied from com.thoughtworks.xstream.io.xml.AbstractXppDriver to preserve
+         * compatibility.
+         */
+
+        @Override
+        public HierarchicalStreamWriter createWriter(Writer out) {
+            return new PrettyPrintWriter(out, getNameCoder());
+        }
+
+        @Override
+        public HierarchicalStreamWriter createWriter(OutputStream out) {
+            /*
+             * While it is tempting to use StandardCharsets.UTF_8 here, this would break
+             * hudson.util.XStream2EncodingTest#toXMLUnspecifiedEncoding.
+             */
+            return createWriter(new OutputStreamWriter(out, Charset.defaultCharset()));
+        }
+
+        /*
+         * The below two methods are copied from com.thoughtworks.xstream.io.xml.StaxDriver for Java 17 compatibility.
+         */
+
+        @Override
+        protected XMLInputFactory createInputFactory() {
+            final XMLInputFactory instance = XMLInputFactory.newInstance();
+            instance.setProperty(XMLInputFactory.IS_NAMESPACE_AWARE, false);
+            instance.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false);
+            return instance;
+        }
+
+        @Override
+        protected XMLOutputFactory createOutputFactory() {
+            return XMLOutputFactory.newInstance();
+        }
     }
 
     public XStream2() {

diff --git a/core/src/test/java/hudson/XmlFileTest.java b/core/src/test/java/hudson/XmlFileTest.java
@@ -2,17 +2,19 @@
 
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.instanceOf;
 import static org.junit.Assert.assertThrows;
 
+import com.thoughtworks.xstream.converters.ConversionException;
+import com.thoughtworks.xstream.io.StreamException;
 import hudson.model.Node;
+import hudson.util.RobustReflectionConverter;
 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.xml.sax.SAXParseException;
 
 public class XmlFileTest {
 
@@ -30,9 +32,6 @@ public void canReadXml1_0Test() throws IOException {
         }
     }
 
-    // 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
     public void xml1_0_withSpecialCharsShouldFail() {
         URL configUrl = getClass().getResource("/hudson/config_1_0_with_special_chars.xml");
@@ -41,7 +40,15 @@ public void xml1_0_withSpecialCharsShouldFail() {
 
         XmlFile xmlFile =  new XmlFile(xs, new File(configUrl.getFile()));
         if (xmlFile.exists()) {
-            assertThrows(SAXParseException.class, xmlFile::read);
+            IOException e = assertThrows(IOException.class, xmlFile::read);
+            assertThat(e.getCause(), instanceOf(ConversionException.class));
+            ConversionException ce = (ConversionException) e.getCause();
+            assertThat(ce.get("cause-exception"), is(StreamException.class.getName()));
+            assertThat(ce.get("class"), is(Jenkins.class.getName()));
+            assertThat(ce.get("required-type"), is(Jenkins.class.getName()));
+            assertThat(ce.get("converter-type"), is(RobustReflectionConverter.class.getName()));
+            assertThat(ce.get("path"), is("/hudson/label"));
+            assertThat(ce.get("line number"), is("7"));
         }
     }
 

diff --git a/core/src/test/java/hudson/util/XStream2Test.java b/core/src/test/java/hudson/util/XStream2Test.java
@@ -40,6 +40,7 @@
 import com.thoughtworks.xstream.mapper.CannotResolveClassException;
 import hudson.model.Result;
 import hudson.model.Run;
+import java.io.InputStream;
 import java.io.StringReader;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -570,4 +571,23 @@ public static final class C4 {}
     @XStreamAlias("C-5")
     public static final class C5 {}
 
+    @Issue("JENKINS-69129")
+    @Test
+    public void testEmoji() throws Exception {
+        Bar bar;
+        try (InputStream is = getClass().getResource("XStream2Emoji.xml").openStream()) {
+            bar = (Bar) new XStream2().fromXML(is);
+        }
+        assertEquals("Fox 🦊", bar.s);
+    }
+
+    @Issue("JENKINS-69129")
+    @Test
+    public void testEmojiEscaped() throws Exception {
+        Bar bar;
+        try (InputStream is = getClass().getResource("XStream2EmojiEscaped.xml").openStream()) {
+            bar = (Bar) new XStream2().fromXML(is);
+        }
+        assertEquals("Fox 🦊", bar.s);
+    }
 }
diff --git a/core/src/test/resources/hudson/util/XStream2Emoji.xml b/core/src/test/resources/hudson/util/XStream2Emoji.xml
@@ -0,0 +1,4 @@
+<?xml version='1.1' encoding='UTF-8'?>
+<hudson.util.XStream2Test_-Bar>
+  <s>Fox 🦊</s>
+</hudson.util.XStream2Test_-Bar>
diff --git a/core/src/test/resources/hudson/util/XStream2EmojiEscaped.xml b/core/src/test/resources/hudson/util/XStream2EmojiEscaped.xml
@@ -0,0 +1,4 @@
+<?xml version='1.1' encoding='UTF-8'?>
+<hudson.util.XStream2Test_-Bar>
+  <s>Fox &#129418;</s>
+</hudson.util.XStream2Test_-Bar>