8326915: NPE when a validating parser is restricted

Reviewed-by: lancea, naoto

5 files changed, 116 insertions, 15 deletions

diff --git a/src/java.xml/share/classes/com/sun/org/apache/xerces/internal/impl/XMLEntityManager.java b/src/java.xml/share/classes/com/sun/org/apache/xerces/internal/impl/XMLEntityManager.java
index f922849d5fa..455e2b5446f 100644
--- a/src/java.xml/share/classes/com/sun/org/apache/xerces/internal/impl/XMLEntityManager.java
+++ b/src/java.xml/share/classes/com/sun/org/apache/xerces/internal/impl/XMLEntityManager.java
@@ -94,7 +94,7 @@ import org.xml.sax.InputSource;
  * @author K.Venugopal SUN Microsystems
  * @author Neeraj Bajaj SUN Microsystems
  * @author Sunitha Reddy SUN Microsystems
- * @LastModified: Jan 2024
+ * @LastModified: Feb 2024
  */
 public class XMLEntityManager implements XMLComponent, XMLEntityResolver {
 
@@ -1118,8 +1118,9 @@ public class XMLEntityManager implements XMLComponent, XMLEntityResolver {
      * this method attempts to resolve the resource as an EntityResolver first
      * and then URIResolver if no match is found.
      */
-    private XMLInputSource resolveEntityOrURI(CatalogResolver cr, String publicId, String systemId, String base) {
-        XMLInputSource xis = resolveEntity(cr, publicId, systemId, base);
+    private XMLInputSource resolveEntityOrURI(String catalogName, CatalogResolver cr,
+            String publicId, String systemId, String base) {
+        XMLInputSource xis = resolveEntity(catalogName, cr, publicId, systemId, base);
 
         if (xis != null) {
             return xis;
@@ -1137,13 +1138,21 @@ public class XMLEntityManager implements XMLComponent, XMLEntityResolver {
         return null;
     }
 
-    private XMLInputSource resolveEntity(CatalogResolver cr, String publicId, String systemId, String base) {
+    private XMLInputSource resolveEntity(String catalogName, CatalogResolver cr,
+            String publicId, String systemId, String base) {
         InputSource is = null;
         try {
             if (publicId != null || systemId != null) {
                 is = cr.resolveEntity(publicId, systemId);
             }
-        } catch (CatalogException e) {}
+        } catch (CatalogException e) {
+            //Note: XSDHandler does not set ErrorReporter on EntityManager
+            if (fErrorReporter != null) {
+                fErrorReporter.reportError(XMLMessageFormatter.XML_DOMAIN,"CatalogException",
+                    new Object[]{SecuritySupport.sanitizePath(catalogName)},
+                    XMLErrorReporter.SEVERITY_FATAL_ERROR, e );
+            }
+        }
 
         if (is != null && !is.isEmpty()) {
             return new XMLInputSource(is, true);
@@ -1216,7 +1225,7 @@ public class XMLEntityManager implements XMLComponent, XMLEntityResolver {
                 fCatalogResolver = CatalogManager.catalogResolver(fCatalogFeatures);
             }
             String pid = (publicId != null? publicId : resourceIdentifier.getNamespace());
-            xmlInputSource = resolveEntityOrURI(fCatalogResolver, pid, literalSystemId, baseSystemId);
+            xmlInputSource = resolveEntityOrURI(fCatalogFile, fCatalogResolver, pid, literalSystemId, baseSystemId);
         }
 
         // Step 3: use the default JDK Catalog Resolver if Step 2's resolve is continue
@@ -1225,7 +1234,7 @@ public class XMLEntityManager implements XMLComponent, XMLEntityResolver {
                 && JdkXmlUtils.isResolveContinue(fCatalogFeatures)) {
             initJdkCatalogResolver();
             // unlike a custom catalog, the JDK Catalog only contains entity references
-            xmlInputSource = resolveEntity(fDefCR, publicId, literalSystemId, baseSystemId);
+            xmlInputSource = resolveEntity("JDKCatalog", fDefCR, publicId, literalSystemId, baseSystemId);
         }
 
         // Step 4: default resolution if not resolved by a resolver and the RESOLVE
diff --git a/src/java.xml/share/classes/jdk/xml/internal/SecuritySupport.java b/src/java.xml/share/classes/jdk/xml/internal/SecuritySupport.java
index cfb2a264f7b..071676703e1 100644
--- a/src/java.xml/share/classes/jdk/xml/internal/SecuritySupport.java
+++ b/src/java.xml/share/classes/jdk/xml/internal/SecuritySupport.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2024, 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
@@ -387,20 +387,21 @@ public class SecuritySupport {
     }
 
     /**
-     * Strip off path from an URI
+     * Strips off path from a URI or file path.
      *
-     * @param uri an URI with full path
+     * @param input a URI or file path
      * @return the file name only
      */
-    public static String sanitizePath(String uri) {
-        if (uri == null) {
+    public static String sanitizePath(String input) {
+        if (input == null) {
             return "";
         }
-        int i = uri.lastIndexOf("/");
+        input = input.replace('\\', '/');
+        int i = input.lastIndexOf('/');
         if (i > 0) {
-            return uri.substring(i+1, uri.length());
+            return input.substring(i+1);
         }
-        return "";
+        return input;
     }
 
     /**
diff --git a/test/jaxp/javax/xml/jaxp/unittest/sbd/test/ExternalRefTest.java b/test/jaxp/javax/xml/jaxp/unittest/sbd/test/ExternalRefTest.java
new file mode 100644
index 00000000000..0b9daed0d83
--- /dev/null
+++ b/test/jaxp/javax/xml/jaxp/unittest/sbd/test/ExternalRefTest.java
@@ -0,0 +1,68 @@
+/*
+ * Copyright (c) 2024, 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 sbd.test;
+
+import java.io.File;
+import javax.xml.parsers.SAXParser;
+import javax.xml.parsers.SAXParserFactory;
+import org.testng.Assert;
+import org.testng.annotations.Test;
+import org.xml.sax.SAXParseException;
+import org.xml.sax.helpers.DefaultHandler;
+
+/*
+ * @test
+ * @bug 8326915
+ * @library /javax/xml/jaxp/libs /javax/xml/jaxp/unittest
+ * @run testng/othervm sbd.test.ExternalRefTest
+ * @summary Part of the Secure-By-Default (SBD) project. This test verifies issues
+ * and error message improvements related to external references.
+ */
+public class ExternalRefTest {
+    /**
+     * @bug 8326915
+     * Verifies that SAXParseException rather than NPE is thrown when a validating
+     * parser is restricted from processing external references.
+     * @throws Exception if the test fails
+     */
+    @Test
+    public void testValidatingParser() throws Exception {
+        Assert.assertThrows(SAXParseException.class, () -> validateWithParser());
+    }
+
+    private void validateWithParser() throws Exception {
+            SAXParserFactory spf = SAXParserFactory.newInstance();
+
+            spf.setNamespaceAware(true);
+            spf.setValidating(true);
+
+            SAXParser parser = spf.newSAXParser();
+            parser.setProperty("http://java.sun.com/xml/jaxp/properties/schemaLanguage",
+                    "http://www.w3.org/2001/XMLSchema");
+
+            parser.setProperty("jdk.xml.jdkcatalog.resolve", "strict");
+            File xmlFile = new File(getClass().getResource("ExternalRefTest.xml").getPath());
+
+            parser.parse(xmlFile, new DefaultHandler());
+    }
+}
diff --git a/test/jaxp/javax/xml/jaxp/unittest/sbd/test/ExternalRefTest.xml b/test/jaxp/javax/xml/jaxp/unittest/sbd/test/ExternalRefTest.xml
new file mode 100644
index 00000000000..0e691f67446
--- /dev/null
+++ b/test/jaxp/javax/xml/jaxp/unittest/sbd/test/ExternalRefTest.xml
@@ -0,0 +1,8 @@
+<?xml version="1.0"?>
+<test:a
+    xmlns:test="test"
+    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xsi:schemaLocation="test ExternalRefTest.xsd">
+<b>1</b>
+<b>2</b>
+</test:a>
diff --git a/test/jaxp/javax/xml/jaxp/unittest/sbd/test/ExternalRefTest.xsd b/test/jaxp/javax/xml/jaxp/unittest/sbd/test/ExternalRefTest.xsd
new file mode 100644
index 00000000000..09a434fe792
--- /dev/null
+++ b/test/jaxp/javax/xml/jaxp/unittest/sbd/test/ExternalRefTest.xsd
@@ -0,0 +1,15 @@
+<?xml version="1.0"?>
+
+<xsd:schema
+  xmlns:xsd="http://www.w3.org/2001/XMLSchema"
+  xmlns="test"
+  targetNamespace="test">
+
+     <xsd:element name="a" type="A"/>
+     <xsd:complexType name="A">
+          <xsd:sequence>
+	       <xsd:element name="b" type="xsd:string" maxOccurs="100"/>
+	  </xsd:sequence>
+     </xsd:complexType>
+
+</xsd:schema>