8316383: NullPointerException in AbstractSAXParser after JDK-8306632

Reviewed-by: lancea, naoto
openjdk · Sep 21, 2023 · 4e57177 · 4e57177 · openjdk-notifier · Sep 21, 2023
1 parent 3b397c8
commit 4e57177
Show file tree

Hide file tree

Showing 3 changed files with 68 additions and 13 deletions.
diff --git a/src/java.xml/share/classes/com/sun/org/apache/xerces/internal/parsers/SAXParser.java b/src/java.xml/share/classes/com/sun/org/apache/xerces/internal/parsers/SAXParser.java
@@ -27,7 +27,9 @@
 import com.sun.org.apache.xerces.internal.xni.parser.XMLParserConfiguration;
 import jdk.xml.internal.JdkConstants;
 import jdk.xml.internal.JdkProperty;
+import jdk.xml.internal.Utils;
 import jdk.xml.internal.XMLSecurityManager;
+import org.xml.sax.SAXException;
 import org.xml.sax.SAXNotRecognizedException;
 import org.xml.sax.SAXNotSupportedException;
 
@@ -39,7 +41,7 @@
  * @author Arnaud  Le Hors, IBM
  * @author Andy Clark, IBM
  *
- * @LastModified: July 2023
+ * @LastModified: Sep 2023
  */
 public class SAXParser
     extends AbstractSAXParser {
@@ -89,6 +91,7 @@ public class SAXParser
      */
     public SAXParser(XMLParserConfiguration config) {
         super(config);
+        initSecurityManager();
     } // <init>(XMLParserConfiguration)
 
     /**
@@ -125,6 +128,7 @@ public SAXParser(SymbolTable symbolTable, XMLGrammarPool grammarPool) {
             fConfiguration.setProperty(XMLGRAMMAR_POOL, grammarPool);
         }
 
+        initSecurityManager();
     } // <init>(SymbolTable,XMLGrammarPool)
 
     /**
@@ -152,16 +156,6 @@ public void setProperty(String name, Object value)
             return;
         }
 
-        if (securityManager == null) {
-            securityManager = new XMLSecurityManager(true);
-            super.setProperty(Constants.SECURITY_MANAGER, securityManager);
-        }
-
-        if (securityPropertyManager == null) {
-            securityPropertyManager = new XMLSecurityPropertyManager();
-            super.setProperty(JdkConstants.XML_SECURITY_PROPERTY_MANAGER, securityPropertyManager);
-        }
-
         int index = securityPropertyManager.getIndex(name);
         if (index > -1) {
             /**
@@ -178,4 +172,25 @@ public void setProperty(String name, Object value)
             }
         }
     }
+
+    /**
+     * Initiates the SecurityManager. This becomes necessary when the SAXParser
+     * is constructed directly by, for example, XMLReaderFactory rather than
+     * through SAXParserFactory.
+     */
+    private void initSecurityManager() {
+        try {
+            if (securityManager == null) {
+                securityManager = new XMLSecurityManager(true);
+                super.setProperty(Constants.SECURITY_MANAGER, securityManager);
+            }
+
+            if (securityPropertyManager == null) {
+                securityPropertyManager = new XMLSecurityPropertyManager();
+                super.setProperty(JdkConstants.XML_SECURITY_PROPERTY_MANAGER, securityPropertyManager);
+            }
+        } catch (SAXException e) {
+            Utils.dPrint(() -> e.getMessage());
+        }
+    }
 } // class SAXParser
diff --git a/src/java.xml/share/classes/jdk/xml/internal/Utils.java b/src/java.xml/share/classes/jdk/xml/internal/Utils.java
@@ -26,11 +26,38 @@
 package jdk.xml.internal;
 
 import java.util.Arrays;
+import java.util.function.Supplier;
 
 /**
  * General utility. Use JdkXmlUtils for XML processing related functions.
  */
 public class Utils {
+    // The debug flag
+    private static boolean debug = false;
+
+    /*
+     * The {@systemProperty jaxp.debug} property is supported by JAXP factories
+     * and used to print out information related to the configuration of factories
+     * and processors
+     */
+    static {
+        try {
+            String val = SecuritySupport.getSystemProperty("jaxp.debug");
+            // Allow simply setting the prop to turn on debug
+            debug = val != null && !"false".equals(val);
+        }
+        catch (SecurityException se) {
+            debug = false;
+        }
+    }
+
+    // print out debug information if jaxp.debug is enabled
+    public static void dPrint(Supplier<String> msgGen) {
+        if (debug) {
+            System.err.println("JAXP: " + msgGen.get());
+        }
+    }
+
     /**
      * Creates a new array with copies of the original array and additional items
      * appended to the end of it.

diff --git a/test/jaxp/javax/xml/jaxp/unittest/sax/XMLReaderTest.java b/test/jaxp/javax/xml/jaxp/unittest/sax/XMLReaderTest.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2023, 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
@@ -33,11 +33,13 @@
 import org.testng.annotations.Listeners;
 import org.testng.annotations.Test;
 import org.xml.sax.SAXException;
+import org.xml.sax.XMLReader;
 import org.xml.sax.helpers.XMLReaderAdapter;
+import org.xml.sax.helpers.XMLReaderFactory;
 
 /*
  * @test
- * @bug 8158246
+ * @bug 8158246 8316383
  * @library /javax/xml/jaxp/libs /javax/xml/jaxp/unittest
  * @run testng/othervm -DrunSecMngr=true -Djava.security.manager=allow sax.XMLReaderTest
  * @run testng/othervm sax.XMLReaderTest
@@ -69,4 +71,15 @@ public void testcreateXMLReader() throws SAXException, ParserConfigurationExcept
         setSystemProperty(SAX_PROPNAME, className + "nosuch");
         XMLReaderAdapter adapter = new XMLReaderAdapter();
     }
+
+    /*
+     * @bug 8316383
+     * Verifies that the XMLReader is initialized properly when it's created
+     * with XMLReaderFactory.
+     */
+    @Test
+    public void testCreateXMLReaderWithXMLReaderFactory() throws SAXException, ParserConfigurationException {
+        XMLReader reader = XMLReaderFactory.createXMLReader();
+        reader.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
+    }
 }