8144966: Catalog API: Null handling and reference to Reader

Reviewed-by: mchung, rriggs

jaxp/src/java.xml/share/classes/javax/xml/catalog/CatalogImpl.java

@@ -32,7 +32,6 @@ import java.net.URI;
 import java.net.URISyntaxException;
 import java.net.URL;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
 import java.util.NoSuchElementException;
@@ -43,6 +42,7 @@ import java.util.stream.StreamSupport;
 import static javax.xml.catalog.BaseEntry.CatalogEntryType;
 import static javax.xml.catalog.CatalogFeatures.DEFER_TRUE;
 import javax.xml.catalog.CatalogFeatures.Feature;
+import static javax.xml.catalog.CatalogMessages.formatMessage;
 import javax.xml.parsers.ParserConfigurationException;
 import javax.xml.parsers.SAXParser;
 import javax.xml.parsers.SAXParserFactory;
@@ -109,25 +109,20 @@ class CatalogImpl extends GroupEntry implements Catalog {
      */
     public CatalogImpl(CatalogImpl parent, CatalogFeatures f, String... file) throws CatalogException {
         super(CatalogEntryType.CATALOG);
-        this.parent = parent;
-        if (parent == null) {
-            level = 0;
-        } else {
-            level = parent.level + 1;
-        }
         if (f == null) {
-            this.features = CatalogFeatures.defaults();
+            throw new NullPointerException(
-        } else {
+                    formatMessage(CatalogMessages.ERR_NULL_ARGUMENT, new Object[]{"CatalogFeatures"}));
-            this.features = f;
         }
-        setPrefer(features.get(Feature.PREFER));
+
-        setDeferred(features.get(Feature.DEFER));
+        if (file.length > 0) {
-        setResolve(features.get(Feature.RESOLVE));
+            CatalogMessages.reportNPEOnNull("The path to the catalog file", file[0]);
+        }
+
+        init(parent, f);
 
         //Path of catalog files
         String[] catalogFile = file;
-        if (level == 0
+        if (level == 0 && file.length == 0) {
-                && (file == null || (file.length == 0 || file[0] == null))) {
             String files = features.get(Feature.FILES);
             if (files != null) {
                 catalogFile = files.split(";[ ]*");
@@ -166,6 +161,23 @@ class CatalogImpl extends GroupEntry implements Catalog {
         }
     }
 
+    private void init(CatalogImpl parent, CatalogFeatures f) {
+        this.parent = parent;
+        if (parent == null) {
+            level = 0;
+        } else {
+            level = parent.level + 1;
+        }
+        if (f == null) {
+            this.features = CatalogFeatures.defaults();
+        } else {
+            this.features = f;
+        }
+        setPrefer(features.get(Feature.PREFER));
+        setDeferred(features.get(Feature.DEFER));
+        setResolve(features.get(Feature.RESOLVE));
+    }
+
     /**
      * Resets the Catalog instance to its initial state.
      */

jaxp/src/java.xml/share/classes/javax/xml/catalog/CatalogManager.java

@@ -38,33 +38,38 @@ public final class CatalogManager {
     }
 
     /**
-     * Creates a Catalog object using the specified feature settings and path to
+     * Creates a {@code Catalog} object using the specified feature settings and
-     * a catalog file. If the features is null, the default features will be used.
+     * path to one or more catalog files.
-     * If the path is empty, System property {@code javax.xml.catalog.files} will
-     * be read to locate the initial list of catalog files.
      * <p>
-     * If more than one catalog files are specified through the path argument or
+     * If {@code paths} is empty, system property {@code javax.xml.catalog.files}
+     * will be read to locate the initial list of catalog files.
+     * <p>
+     * If more than one catalog files are specified through the paths argument or
      * {@code javax.xml.catalog.files} property, the first entry is considered
      * the main catalog, while others are treated as alternative catalogs after
      * those referenced by the {@code nextCatalog} elements in the main catalog.
+     * <p>
+     * As specified in
+     * <a href="https://www.oasis-open.org/committees/download.php/14809/xml-catalogs.html#s.res.fail">
+     * XML Catalogs, OASIS Standard V1.1</a>, invalid path entries will be ignored.
+     * No error will be reported. In case all entries are invalid, the resolver
+     * will return as no mapping is found.
      *
      * @param features the catalog features
-     * @param path path(s) to one or more catalogs.
+     * @param paths path(s) to one or more catalogs.
      *
-     * @return a catalog instance
+     * @return an instance of a {@code Catalog}
-     * @throws CatalogException If no catalog can be found whether through the
+     * @throws CatalogException If an error occurs while parsing the catalog
-     * specified path or the System property {@code javax.xml.catalog.files}, or
-     * an error occurs while parsing the catalog
      */
-    public static Catalog catalog(CatalogFeatures features, String... path) {
+    public static Catalog catalog(CatalogFeatures features, String... paths) {
-        return new CatalogImpl(features, path);
+        return new CatalogImpl(features, paths);
     }
 
     /**
-     * Creates an instance of a CatalogResolver using the specified catalog.
+     * Creates an instance of a {@code CatalogResolver} using the specified catalog.
      *
      * @param catalog the catalog instance
-     * @return an instance of a CatalogResolver
+     * @return an instance of a {@code CatalogResolver}
      */
     public static CatalogResolver catalogResolver(Catalog catalog) {
         if (catalog == null) CatalogMessages.reportNPEOnNull("catalog", null);
@@ -72,10 +77,10 @@ public final class CatalogManager {
     }
 
     /**
-     * Creates an instance of a CatalogUriResolver using the specified catalog.
+     * Creates an instance of a {@code CatalogUriResolver} using the specified catalog.
      *
      * @param catalog the catalog instance
-     * @return an instance of a CatalogResolver
+     * @return an instance of a {@code CatalogResolver}
      */
     public static CatalogUriResolver catalogUriResolver(Catalog catalog) {
         if (catalog == null) CatalogMessages.reportNPEOnNull("catalog", null);
@@ -83,50 +88,60 @@ public final class CatalogManager {
     }
 
     /**
-     * Creates an instance of a CatalogResolver using the specified feature settings
+     * Creates an instance of a {@code CatalogResolver} using the specified feature
-     * and path to a catalog file. If the features is null, the default features will
+     * settings and path to one or more catalog files.
-     * be used. If the path is empty, System property {@code javax.xml.catalog.files}
+     * <p>
+     * If {@code paths} is empty, system property {@code javax.xml.catalog.files}
      * will be read to locate the initial list of catalog files.
      * <p>
-     * If more than one catalog files are specified through the path argument or
+     * If more than one catalog files are specified through the paths argument or
      * {@code javax.xml.catalog.files} property, the first entry is considered
      * the main catalog, while others are treated as alternative catalogs after
      * those referenced by the {@code nextCatalog} elements in the main catalog.
+     * <p>
+     * As specified in
+     * <a href="https://www.oasis-open.org/committees/download.php/14809/xml-catalogs.html#s.res.fail">
+     * XML Catalogs, OASIS Standard V1.1</a>, invalid path entries will be ignored.
+     * No error will be reported. In case all entries are invalid, the resolver
+     * will return as no mapping is found.
      *
      * @param features the catalog features
-     * @param path the path(s) to one or more catalogs
+     * @param paths the path(s) to one or more catalogs
      *
-     * @return an instance of a CatalogResolver
+     * @return an instance of a {@code CatalogResolver}
-     * @throws CatalogException If no catalog can be found whether through the
+     * @throws CatalogException If an error occurs while parsing the catalog
-     * specified path or the System property {@code javax.xml.catalog.files}, or
-     * an error occurs while parsing the catalog
      */
-    public static CatalogResolver catalogResolver(CatalogFeatures features, String... path) {
+    public static CatalogResolver catalogResolver(CatalogFeatures features, String... paths) {
-        Catalog catalog = catalog(features, path);
+        Catalog catalog = catalog(features, paths);
         return new CatalogResolverImpl(catalog);
     }
 
     /**
-     * Creates an instance of a CatalogUriResolver using the specified feature settings
+     * Creates an instance of a {@code CatalogUriResolver} using the specified
-     * and path to a catalog file. If the features is null, the default features will
+     * feature settings and path to one or more catalog files.
-     * be used. If the path is empty, System property {@code javax.xml.catalog.files}
+     * <p>
+     * If {@code paths} is empty, system property {@code javax.xml.catalog.files}
      * will be read to locate the initial list of catalog files.
      * <p>
-     * If more than one catalog files are specified through the path argument or
+     * If more than one catalog files are specified through the paths argument or
      * {@code javax.xml.catalog.files} property, the first entry is considered
      * the main catalog, while others are treated as alternative catalogs after
      * those referenced by the {@code nextCatalog} elements in the main catalog.
+     * <p>
+     * As specified in
+     * <a href="https://www.oasis-open.org/committees/download.php/14809/xml-catalogs.html#s.res.fail">
+     * XML Catalogs, OASIS Standard V1.1</a>, invalid path entries will be ignored.
+     * No error will be reported. In case all entries are invalid, the resolver
+     * will return as no mapping is found.
      *
      * @param features the catalog features
-     * @param path the path(s) to one or more catalogs
+     * @param paths the path(s) to one or more catalogs
      *
-     * @return an instance of a CatalogResolver
+     * @return an instance of a {@code CatalogUriResolver}
-     * @throws CatalogException If no catalog can be found whether through the
+     * @throws CatalogException If an error occurs while parsing the catalog
-     * specified path or the System property {@code javax.xml.catalog.files}, or
-     * an error occurs while parsing the catalog
      */
-    public static CatalogUriResolver catalogUriResolver(CatalogFeatures features, String... path) {
+    public static CatalogUriResolver catalogUriResolver(CatalogFeatures features, String... paths) {
-        Catalog catalog = catalog(features, path);
+        Catalog catalog = catalog(features, paths);
         return new CatalogUriResolverImpl(catalog);
     }
 }

jaxp/src/java.xml/share/classes/javax/xml/catalog/CatalogUriResolver.java

@@ -43,9 +43,9 @@ public interface CatalogUriResolver extends URIResolver {
      * absolute if the absolute URI is required
      *
      * @return a {@link javax.xml.transform.Source} object if a mapping is found.
-     * If no mapping is found, returns a {@link javax.xml.transform.Source} object
+     * If no mapping is found, returns an empty {@link javax.xml.transform.Source}
-     * containing an empty {@link java.io.Reader} if the
+     * object if the {@code javax.xml.catalog.resolve} property is set to
-     * {@code javax.xml.catalog.resolve} property is set to {@code ignore};
+     * {@code ignore};
      * returns a {@link javax.xml.transform.Source} object with the original URI
      * (href, or href resolved with base if base is not null) if the
      * {@code javax.xml.catalog.resolve} property is set to {@code continue}.

jaxp/test/javax/xml/jaxp/functional/catalog/DeferFeatureTest.java

@@ -57,21 +57,12 @@ public class DeferFeatureTest {
     @DataProvider(name = "catalog-countOfLoadedCatalogFile")
     private Object[][] data() {
         return new Object[][]{
-                // This catalog specifies null catalog explicitly,
+            // By default, alternative catalogs are not loaded.
-                // and the count of loaded catalogs should be 0.
-                { createCatalog(null), 0 },
-
-                // This catalog specifies null catalog implicitly,
-                // and the count of loaded catalogs should be 0.
             {createCatalog(CatalogFeatures.defaults()), 0},
-
+            // Alternative catalogs are not loaded when DEFER is set to true.
-                // This catalog loads null catalog with true DEFER,
-                // and the count of loaded catalogs should be 0.
             {createCatalog(createDeferFeature(DEFER_TRUE)), 0},
-
+            // The 3 alternative catalogs are not pre-loaded
-                // This catalog loads null catalog with false DEFER.
+            //when DEFER is set to false.
-                // It should load all of none-current catalogs and the
-                // count of loaded catalogs should be 3.
             {createCatalog(createDeferFeature(DEFER_FALSE)), 3}};
     }
 

jaxp/test/javax/xml/jaxp/libs/catalog/CatalogTestUtils.java

@@ -83,7 +83,7 @@ final class CatalogTestUtils {
      * Creates CatalogResolver with a set of catalogs.
      */
     static CatalogResolver catalogResolver(String... catalogName) {
-        return catalogResolver(null, catalogName);
+        return catalogResolver(CatalogFeatures.defaults(), catalogName);
     }
 
     /*
@@ -91,15 +91,16 @@ final class CatalogTestUtils {
      */
     static CatalogResolver catalogResolver(CatalogFeatures features,
             String... catalogName) {
-        return CatalogManager.catalogResolver(features,
+        return (catalogName == null) ?
-                getCatalogPaths(catalogName));
+                CatalogManager.catalogResolver(features) :
+                CatalogManager.catalogResolver(features, getCatalogPaths(catalogName));
     }
 
     /*
      * Creates catalogUriResolver with a set of catalogs.
      */
     static CatalogUriResolver catalogUriResolver(String... catalogName) {
-        return catalogUriResolver(null, catalogName);
+        return catalogUriResolver(CatalogFeatures.defaults(), catalogName);
     }
 
     /*
@@ -107,8 +108,9 @@ final class CatalogTestUtils {
      */
     static CatalogUriResolver catalogUriResolver(
             CatalogFeatures features, String... catalogName) {
-        return CatalogManager.catalogUriResolver(features,
+        return (catalogName == null) ?
-                getCatalogPaths(catalogName));
+                CatalogManager.catalogUriResolver(features) :
+                CatalogManager.catalogUriResolver(features, getCatalogPaths(catalogName));
     }
 
     // Gets the paths of the specified catalogs.

jaxp/test/javax/xml/jaxp/libs/jaxp/library/JAXPTestUtilities.java

@@ -89,7 +89,7 @@ public class JAXPTestUtilities {
     /**
      * BOM table for storing BOM header.
      */
-    private final static Map<String, byte[]> bom = new HashMap();
+    private final static Map<String, byte[]> bom = new HashMap<>();
 
     /**
      * Initialize all BOM headers.

jaxp/test/javax/xml/jaxp/unittest/catalog/CatalogTest.java

@@ -42,11 +42,31 @@ import org.xml.sax.XMLReader;
 import org.xml.sax.ext.DefaultHandler2;
 
 /*
- * @bug 8081248
+ * @bug 8081248, 8144966
  * @summary Tests basic Catalog functions.
  */
 
 public class CatalogTest {
+    /*
+       @bug 8144966
+       Verifies that passing null as CatalogFeatures will result in a NPE.
+    */
+    @Test(expectedExceptions = NullPointerException.class)
+    public void testFeatureNull() {
+        CatalogResolver resolver = CatalogManager.catalogResolver(null, "");
+
+    }
+
+    /*
+       @bug 8144966
+       Verifies that passing null as the path will result in a NPE.
+    */
+    @Test(expectedExceptions = NullPointerException.class)
+    public void testPathNull() {
+        String path = null;
+        CatalogResolver resolver = CatalogManager.catalogResolver(CatalogFeatures.defaults(), path);
+    }
+
     /*
        Tests basic catalog feature by using a CatalogResolver instance to
     resolve a DTD reference to a locally specified DTD file. If the resolution
@@ -61,7 +81,7 @@ public class CatalogTest {
         }
         String url = getClass().getResource(xml).getFile();
         try {
-            CatalogResolver cr = CatalogManager.catalogResolver(null, catalog);
+            CatalogResolver cr = CatalogManager.catalogResolver(CatalogFeatures.defaults(), catalog);
             XMLReader reader = saxParser.getXMLReader();
             reader.setEntityResolver(cr);
             MyHandler handler = new MyHandler(saxParser);
@@ -84,7 +104,7 @@ public class CatalogTest {
 
         String test = "testInvalidCatalog";
         try {
-            CatalogResolver resolver = CatalogManager.catalogResolver(null, catalog);
+            CatalogResolver resolver = CatalogManager.catalogResolver(CatalogFeatures.defaults(), catalog);
             String actualSystemId = resolver.resolveEntity(null, "http://remote/xml/dtd/sys/alice/docAlice.dtd").getSystemId();
         } catch (Exception e) {
             String msg = e.getMessage();