From f4fd53d0aee67319bf2c7bcaa671c2e97e66383f Mon Sep 17 00:00:00 2001
From: Joe Wang <joehw@openjdk.org>
Date: Fri, 25 Mar 2022 18:10:45 +0000
Subject: [PATCH] 8273370: Preferences.exportSubtree() generates invalid XML if
 value contains control char

Reviewed-by: lancea, naoto
---
 .../xml/internal/serializer/ToStream.java     |  23 ++-
 .../unittest/transform/OpenJDK100017Test.java |  14 +-
 .../unittest/transform/SerializationTest.java | 160 ++++++++++++++++++
 .../jaxp/unittest/transform/preferences.xml   |  12 ++
 4 files changed, 194 insertions(+), 15 deletions(-)
 create mode 100644 test/jaxp/javax/xml/jaxp/unittest/transform/SerializationTest.java
 create mode 100644 test/jaxp/javax/xml/jaxp/unittest/transform/preferences.xml

diff --git a/src/java.xml/share/classes/com/sun/org/apache/xml/internal/serializer/ToStream.java b/src/java.xml/share/classes/com/sun/org/apache/xml/internal/serializer/ToStream.java
index 93a4dd8d301..126f5d15255 100644
--- a/src/java.xml/share/classes/com/sun/org/apache/xml/internal/serializer/ToStream.java
+++ b/src/java.xml/share/classes/com/sun/org/apache/xml/internal/serializer/ToStream.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2006, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2006, 2022, Oracle and/or its affiliates. All rights reserved.
  */
 /*
  * Licensed to the Apache Software Foundation (ASF) under one or more
@@ -20,6 +20,7 @@
 
 package com.sun.org.apache.xml.internal.serializer;
 
+import com.sun.org.apache.xerces.internal.util.XMLChar;
 import com.sun.org.apache.xml.internal.serializer.dom3.DOMConstants;
 import com.sun.org.apache.xml.internal.serializer.utils.MsgKey;
 import com.sun.org.apache.xml.internal.serializer.utils.Utils;
@@ -54,7 +55,7 @@ import org.xml.sax.SAXException;
  * serializers (xml, html, text ...) that write output to a stream.
  *
  * @xsl.usage internal
- * @LastModified: June 2021
+ * @LastModified: Mar 2022
  */
 abstract public class ToStream extends SerializerBase {
 
@@ -1745,13 +1746,19 @@ abstract public class ToStream extends SerializerBase {
             }
             else
             {
-                /*  This if check is added to support control characters in XML 1.1.
-                 *  If a character is a Control Character within C0 and C1 range, it is desirable
-                 *  to write it out as Numeric Character Reference(NCR) regardless of XML Version
-                 *  being used for output document.
+                /*
+                 *  The check was added to support control characters in XML 1.1.
+                 *  It previously wrote Control Characters within C0 and C1 range
+                 *  as Numeric Character Reference(NCR) regardless of XML Version,
+                 *  which was incorrect as Control Characters are invalid in XML 1.0.
                  */
-                if (isCharacterInC0orC1Range(ch) ||
-                        (XMLVERSION11.equals(getVersion()) && isNELorLSEPCharacter(ch)))
+                boolean isVer11 = XMLVERSION11.equals(getVersion());
+                if (!isVer11 && XMLChar.isInvalid(ch)) {
+                    throw new org.xml.sax.SAXException(Utils.messages.createMessage(
+                            MsgKey.ER_WF_INVALID_CHARACTER_IN_TEXT,
+                            new Object[]{Integer.toHexString(ch)}));
+                }
+                if (isCharacterInC0orC1Range(ch) || (isVer11 && isNELorLSEPCharacter(ch)))
                 {
                     writeCharRef(writer, ch);
                 }
diff --git a/test/jaxp/javax/xml/jaxp/unittest/transform/OpenJDK100017Test.java b/test/jaxp/javax/xml/jaxp/unittest/transform/OpenJDK100017Test.java
index 425468ef26a..f3802ec5c46 100644
--- a/test/jaxp/javax/xml/jaxp/unittest/transform/OpenJDK100017Test.java
+++ b/test/jaxp/javax/xml/jaxp/unittest/transform/OpenJDK100017Test.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2022, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -23,6 +23,7 @@
 
 package transform;
 
+import com.sun.org.apache.xerces.internal.util.XMLChar;
 import java.io.IOException;
 
 import javax.xml.transform.TransformerConfigurationException;
@@ -32,19 +33,16 @@ import javax.xml.transform.sax.TransformerHandler;
 import javax.xml.transform.stream.StreamResult;
 
 import org.testng.Assert;
-import org.testng.annotations.Listeners;
 import org.testng.annotations.Test;
 import org.xml.sax.SAXException;
 
 /*
  * @test
- * @bug 6883209
- * @library /javax/xml/jaxp/libs /javax/xml/jaxp/unittest
- * @run testng/othervm -DrunSecMngr=true -Djava.security.manager=allow transform.OpenJDK100017Test
+ * @bug 6883209 8273370
+ * @modules java.xml/com.sun.org.apache.xerces.internal.util
  * @run testng/othervm transform.OpenJDK100017Test
  * @summary Test XSLT won't cause StackOverflow when it handle many characters.
  */
-@Listeners({jaxp.library.BasePolicy.class})
 public class OpenJDK100017Test {
 
     @Test
@@ -56,7 +54,9 @@ public class OpenJDK100017Test {
 
             StringBuilder sb = new StringBuilder(4096);
             for (int x = 4096; x > 0; x--) {
-                sb.append((char) x);
+                if (XMLChar.isValid(x)) {
+                    sb.append((char)x);
+                }
             }
             ser.characters(sb.toString().toCharArray(), 0, sb.toString().toCharArray().length);
             ser.endDocument();
diff --git a/test/jaxp/javax/xml/jaxp/unittest/transform/SerializationTest.java b/test/jaxp/javax/xml/jaxp/unittest/transform/SerializationTest.java
new file mode 100644
index 00000000000..b13a9a2c93b
--- /dev/null
+++ b/test/jaxp/javax/xml/jaxp/unittest/transform/SerializationTest.java
@@ -0,0 +1,160 @@
+/*
+ * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+package transform;
+
+import java.io.BufferedWriter;
+import java.io.ByteArrayOutputStream;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.io.OutputStreamWriter;
+import java.util.prefs.InvalidPreferencesFormatException;
+import java.util.prefs.Preferences;
+import javax.xml.parsers.DocumentBuilderFactory;
+import javax.xml.parsers.ParserConfigurationException;
+import javax.xml.transform.OutputKeys;
+import javax.xml.transform.Transformer;
+import javax.xml.transform.TransformerException;
+import javax.xml.transform.TransformerFactory;
+import javax.xml.transform.dom.DOMSource;
+import javax.xml.transform.stream.StreamResult;
+import org.testng.Assert;
+import org.testng.annotations.Test;
+import org.w3c.dom.DOMImplementation;
+import org.w3c.dom.Document;
+import org.w3c.dom.DocumentType;
+import org.w3c.dom.Element;
+
+/*
+ * @test
+ * @bug 8273370
+ * @run testng transform.SerializationTest
+ * @summary Verifies that the characters are written correctly during serialization.
+ */
+public class SerializationTest {
+
+    private static final String PREFS_DTD_URI
+            = "http://java.sun.com/dtd/preferences.dtd";
+    private static String CLS_DIR = System.getProperty("test.classes", ".");
+    private static String SRC_DIR = System.getProperty("test.src");
+
+    /**
+     * Verifies that the XMLSupport for exportSubtree handles control characters
+     * correctly by reporting en error.
+     *
+     * Note: exportSubtree currently throws AssertionError. It would be more
+     * appropriate to throw InvalidPreferencesFormatException as the import
+     * method does. Since this is an edge case however, we'll keep it as is to
+     * avoid signature change.
+     *
+     * The following was the original test:
+            Preferences p = Preferences.userRoot().node("test");
+            p.put("key", "[\u0018\u0019]");
+            p.exportSubtree(new ByteArrayOutputStream());
+     *
+     * The code however, hanged when running in JTReg. This test therefore replaced
+     * the above code with the process extracted from the exportSubtree routine.
+     *
+     * @throws Exception if the test fails
+     */
+    @Test
+    public void testTrasformer() throws Exception {
+        Assert.assertThrows(AssertionError.class,
+                () -> export(new ByteArrayOutputStream()));
+    }
+
+    private void export(OutputStream os) throws IOException {
+        Document doc = createPrefsDoc("preferences");
+        Element preferences = doc.getDocumentElement();
+        preferences.setAttribute("EXTERNAL_XML_VERSION", "1.0");
+        Element xmlRoot = (Element) preferences.appendChild(doc.createElement("root"));
+        xmlRoot.setAttribute("type", "user");
+
+        Element e = xmlRoot;
+
+        e.appendChild(doc.createElement("map"));
+        e = (Element) e.appendChild(doc.createElement("node"));
+        e.setAttribute("name", "test");
+
+        putPreferencesInXml(e, doc);
+
+        writeDoc(doc, os);
+    }
+
+    private static Document createPrefsDoc(String qname) {
+        try {
+            DOMImplementation di = DocumentBuilderFactory.newInstance().
+                    newDocumentBuilder().getDOMImplementation();
+            DocumentType dt = di.createDocumentType(qname, null, PREFS_DTD_URI);
+            return di.createDocument(null, qname, dt);
+        } catch (ParserConfigurationException e) {
+            throw new AssertionError(e);
+        }
+    }
+
+    private static void putPreferencesInXml(Element elt, Document doc) {
+        Element map = (Element) elt.appendChild(doc.createElement("map"));
+        Element entry = (Element) map.appendChild(doc.createElement("entry"));
+        entry.setAttribute("key", "key");
+        entry.setAttribute("value", "[\u0018\u0019]");
+    }
+
+    private void writeDoc(Document doc, OutputStream out)
+            throws IOException {
+        try {
+            TransformerFactory tf = TransformerFactory.newInstance();
+            tf.setAttribute("indent-number", 2);
+            Transformer t = tf.newTransformer();
+            t.setOutputProperty(OutputKeys.DOCTYPE_SYSTEM, doc.getDoctype().getSystemId());
+            t.setOutputProperty(OutputKeys.INDENT, "yes");
+            //Transformer resets the "indent" info if the "result" is a StreamResult with
+            //an OutputStream object embedded, creating a Writer object on top of that
+            //OutputStream object however works.
+            t.transform(new DOMSource(doc),
+                    new StreamResult(new BufferedWriter(new OutputStreamWriter(out, "UTF-8"))));
+        } catch (TransformerException e) {
+            throw new AssertionError(e);
+        }
+    }
+
+    /**
+     * Verifies that the XMLSupport for importPreferences handles control
+     * characters correctly by reporting en error.
+     *
+     * Note: this is the existing behavior. This test is here to match with the
+     * export method.
+     *
+     * "preferences.xml" was generated by calling the exportSubtree method
+     * before the patch.
+     *
+     * @throws Exception if the test fails
+     */
+    @Test
+    public void testParser() throws Exception {
+        Assert.assertThrows(InvalidPreferencesFormatException.class, () -> {
+            Preferences.importPreferences(
+                    new FileInputStream(new File(SRC_DIR + "/preferences.xml")));
+        });
+    }
+}
diff --git a/test/jaxp/javax/xml/jaxp/unittest/transform/preferences.xml b/test/jaxp/javax/xml/jaxp/unittest/transform/preferences.xml
new file mode 100644
index 00000000000..f2ac3cf4860
--- /dev/null
+++ b/test/jaxp/javax/xml/jaxp/unittest/transform/preferences.xml
@@ -0,0 +1,12 @@
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<!DOCTYPE preferences SYSTEM "http://java.sun.com/dtd/preferences.dtd">
+<preferences EXTERNAL_XML_VERSION="1.0">
+  <root type="user">
+    <map/>
+    <node name="test">
+      <map>
+        <entry key="key" value="[&#24;&#25;]"/>
+      </map>
+    </node>
+  </root>
+</preferences>