8151162: Public entries not searched when prefer='system'

Reviewed-by: lancea

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

@@ -52,8 +52,8 @@ final class CatalogResolverImpl implements CatalogResolver {
     @Override
     public InputSource resolveEntity(String publicId, String systemId) {
         //Normalize publicId and systemId
-        systemId = Normalizer.normalizeURI(systemId);
+        systemId = Normalizer.normalizeURI(Util.getNotNullOrEmpty(systemId));
-        publicId = Normalizer.normalizePublicId(Normalizer.decodeURN(publicId));
+        publicId = Normalizer.normalizePublicId(Normalizer.decodeURN(Util.getNotNullOrEmpty(publicId)));
 
         //check whether systemId is an urn
         if (systemId != null && systemId.startsWith("urn:publicid:")) {
@@ -87,7 +87,17 @@ final class CatalogResolverImpl implements CatalogResolver {
     }
 
     /**
-     * Resolves the publicId or systemId to one specified in the catalog.
+     * Resolves the publicId or systemId using public or system entries in the catalog.
+     *
+     * The resolution follows the following rules determined by the prefer setting:
+     *
+     * prefer "system": attempts to resolve with a system entry;
+     *                  attempts to resolve with a public entry when only
+     *                  publicId is specified.
+     *
+     * prefer "public": attempts to resolve with a system entry;
+     *                  attempts to resolve with a public entry if no matching
+     *                  system entry is found.
      * @param catalog the catalog
      * @param publicId the publicId
      * @param systemId the systemId
@@ -99,9 +109,14 @@ final class CatalogResolverImpl implements CatalogResolver {
         //search the current catalog
         catalog.reset();
         if (systemId != null) {
+            /*
+               If a system identifier is specified, it is used no matter how
+            prefer is set.
+            */
             resolvedSystemId = catalog.matchSystem(systemId);
         }
-        if (resolvedSystemId == null) {
+
+        if (resolvedSystemId == null && publicId != null) {
             resolvedSystemId = catalog.matchPublic(publicId);
         }
 

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

@@ -60,6 +60,9 @@ final class CatalogUriResolverImpl implements CatalogUriResolver {
 
     @Override
     public Source resolve(String href, String base) {
+        href = Util.getNotNullOrEmpty(href);
+        base = Util.getNotNullOrEmpty(base);
+
         if (href == null) return null;
 
         CatalogImpl c = (CatalogImpl)catalog;

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

@@ -82,6 +82,9 @@ class GroupEntry extends BaseEntry {
     //The length of the longest match of a suffix type
     int longestSuffixMatch = 0;
 
+    //Indicate whether a system entry has been searched
+    boolean systemEntrySearched = false;
+
     /**
      * PreferType represents possible values of the prefer property
      */
@@ -156,6 +159,7 @@ class GroupEntry extends BaseEntry {
         longestRewriteMatch = 0;
         suffixMatch = null;
         longestSuffixMatch = 0;
+        systemEntrySearched = false;
     }
     /**
      * Constructs a group entry.
@@ -212,6 +216,7 @@ class GroupEntry extends BaseEntry {
      * @return An URI string if a mapping is found, or null otherwise.
      */
     public String matchSystem(String systemId) {
+        systemEntrySearched = true;
         String match = null;
         for (BaseEntry entry : entries) {
             switch (entry.type) {
@@ -277,11 +282,13 @@ class GroupEntry extends BaseEntry {
      * @return An URI string if a mapping is found, or null otherwise.
      */
     public String matchPublic(String publicId) {
-        //as the specification required
+        /*
-        if (!isPreferPublic) {
+           When both public and system identifiers are specified, and prefer is
+        not public (that is, system), only system entry will be used.
+        */
+        if (!isPreferPublic && systemEntrySearched) {
             return null;
         }
-
         //match public entries
         String match = null;
         for (BaseEntry entry : entries) {

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

@@ -122,4 +122,25 @@ class Util {
         }
         return null;
     }
+
+    /**
+     * Checks whether the specified string is null or empty, returns the original
+     * string with leading and trailing spaces removed if not.
+     * @param test the string to be tested
+     * @return the original string with leading and trailing spaces removed,
+     * or null if it is null or empty
+     *
+     */
+    static String getNotNullOrEmpty(String test) {
+        if (test == null) {
+            return test;
+        } else {
+            String temp = test.trim();
+            if (temp.length() == 0) {
+                return null;
+            } else {
+                return temp;
+            }
+        }
+    }
 }

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

@@ -43,10 +43,48 @@ import org.xml.sax.XMLReader;
 import org.xml.sax.ext.DefaultHandler2;
 
 /*
- * @bug 8081248, 8144966, 8146606, 8146237, 8151154, 8150969
+ * @bug 8081248, 8144966, 8146606, 8146237, 8151154, 8150969, 8151162
  * @summary Tests basic Catalog functions.
  */
 public class CatalogTest {
+    /*
+     * @bug 8151162
+     * Verifies that the Catalog matches specified publicId or systemId and returns
+     * results as expected.
+     */
+    @Test(dataProvider = "matchWithPrefer")
+    public void matchWithPrefer(String prefer, String cfile, String publicId, String systemId, String expected) {
+        String catalogFile = getClass().getResource(cfile).getFile();
+        Catalog c = CatalogManager.catalog(CatalogFeatures.builder().with(CatalogFeatures.Feature.PREFER, prefer).build(), catalogFile);
+        String result;
+        if (publicId != null && publicId.length() > 0) {
+            result = c.matchPublic(publicId);
+        } else {
+            result = c.matchSystem(systemId);
+        }
+        Assert.assertEquals(expected, result);
+    }
+
+    /*
+     * @bug 8151162
+     * Verifies that the CatalogResolver resolves specified publicId or systemId
+     * in accordance with the prefer setting.
+     * prefer "system": resolves with a system entry.
+     *                  Exception: use the public entry when the catalog contains
+     *                  only public entry and only publicId is specified.
+     * prefer "public": attempts to resolve with a system entry;
+     *                  attempts to resolve with a public entry if no matching
+     *                  system entry is found.
+     */
+    @Test(dataProvider = "resolveWithPrefer")
+    public void resolveWithPrefer(String prefer, String cfile, String publicId, String systemId, String expected) {
+        String catalogFile = getClass().getResource(cfile).getFile();
+        CatalogFeatures f = CatalogFeatures.builder().with(CatalogFeatures.Feature.PREFER, prefer).with(CatalogFeatures.Feature.RESOLVE, "ignore").build();
+        CatalogResolver catalogResolver = CatalogManager.catalogResolver(f, catalogFile);
+        String result = catalogResolver.resolveEntity(publicId, systemId).getSystemId();
+        Assert.assertEquals(expected, result);
+    }
+
     /**
      * @bug 8150969
      * Verifies that the defer attribute set in the catalog file takes precedence
@@ -232,6 +270,60 @@ public class CatalogTest {
         }
     }
 
+    static String id = "http://openjdk.java.net/xml/catalog/dtd/system.dtd";
+    /*
+       DataProvider: used to verify how prefer settings affect the result of the
+        Catalog's matching operation.
+        Data columns:
+        prefer, catalog, publicId, systemId, expected result
+     */
+    @DataProvider(name = "matchWithPrefer")
+    Object[][] getDataForMatch() {
+        return new Object[][]{
+            {"public", "pubOnly.xml", id, "", "http://local/base/dtd/public.dtd"},
+            {"public", "sysOnly.xml", id, "", null},
+            {"public", "sysAndPub.xml", id, "", "http://local/base/dtd/public.dtd"},
+            {"system", "pubOnly.xml", id, "", "http://local/base/dtd/public.dtd"},
+            {"system", "sysOnly.xml", id, "", null},
+            {"system", "sysAndPub.xml", id, "", "http://local/base/dtd/public.dtd"},
+            {"public", "pubOnly.xml", "", id, null},
+            {"public", "sysOnly.xml", "", id, "http://local/base/dtd/system.dtd"},
+            {"public", "sysAndPub.xml", "", id, "http://local/base/dtd/system.dtd"},
+            {"system", "pubOnly.xml", "", id, null},
+            {"system", "sysOnly.xml", "", id, "http://local/base/dtd/system.dtd"},
+            {"system", "sysAndPub.xml", "", id, "http://local/base/dtd/system.dtd"},
+        };
+    }
+
+    /*
+       DataProvider: used to verify how prefer settings affect the result of the
+        CatalogResolver's resolution operation.
+        Data columns:
+        prefer, catalog, publicId, systemId, expected result
+     */
+    @DataProvider(name = "resolveWithPrefer")
+    Object[][] getDataForResolve() {
+        return new Object[][]{
+            {"system", "pubOnly.xml", id, "", "http://local/base/dtd/public.dtd"},
+            {"system", "pubOnly.xml", "", id, null},
+            {"system", "pubOnly.xml", id, id, null},
+            {"public", "pubOnly.xml", id, "", "http://local/base/dtd/public.dtd"},
+            {"public", "pubOnly.xml", "", id, null},
+            {"public", "pubOnly.xml", id, id, "http://local/base/dtd/public.dtd"},
+            {"system", "sysOnly.xml", id, "", null},
+            {"system", "sysOnly.xml", "", id, "http://local/base/dtd/system.dtd"},
+            {"system", "sysOnly.xml", id, id, "http://local/base/dtd/system.dtd"},
+            {"public", "sysOnly.xml", id, "", null},
+            {"public", "sysOnly.xml", "", id, "http://local/base/dtd/system.dtd"},
+            {"public", "sysOnly.xml", id, id, "http://local/base/dtd/system.dtd"},
+            {"system", "sysAndPub.xml", id, "", "http://local/base/dtd/public.dtd"},
+            {"system", "sysAndPub.xml", "", id, "http://local/base/dtd/system.dtd"},
+            {"system", "sysAndPub.xml", id, id, "http://local/base/dtd/system.dtd"},
+            {"public", "sysAndPub.xml", id, "", "http://local/base/dtd/public.dtd"},
+            {"public", "sysAndPub.xml", "", id, "http://local/base/dtd/system.dtd"},
+            {"public", "sysAndPub.xml", id, id, "http://local/base/dtd/system.dtd"},
+        };
+    }
     /*
        DataProvider: catalogs that contain invalid next or delegate catalogs.
                      The defer attribute is set to false.

jaxp/test/javax/xml/jaxp/unittest/catalog/pubOnly.xml

@@ -0,0 +1,6 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<catalog xmlns="urn:oasis:names:tc:entity:xmlns:xml:catalog" xml:base="http://local/base/dtd/">
+
+    <public publicId="http://openjdk.java.net/xml/catalog/dtd/system.dtd" uri="public.dtd"/>
+
+</catalog>

jaxp/test/javax/xml/jaxp/unittest/catalog/sysAndPub.xml

@@ -0,0 +1,7 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<catalog xmlns="urn:oasis:names:tc:entity:xmlns:xml:catalog" xml:base="http://local/base/dtd/">
+
+    <system systemId="http://openjdk.java.net/xml/catalog/dtd/system.dtd" uri="system.dtd"/>
+    <public publicId="http://openjdk.java.net/xml/catalog/dtd/system.dtd" uri="public.dtd"/>
+
+</catalog>

jaxp/test/javax/xml/jaxp/unittest/catalog/sysOnly.xml

@@ -0,0 +1,6 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<catalog xmlns="urn:oasis:names:tc:entity:xmlns:xml:catalog" xml:base="http://local/base/dtd/">
+
+    <system systemId="http://openjdk.java.net/xml/catalog/dtd/system.dtd" uri="system.dtd"/>
+
+</catalog>