8162431: CatalogUriResolver with circular/self referencing catalog file is not throwing CatalogException as expected
Reviewed-by: lancea
--- a/jaxp/src/java.xml/share/classes/javax/xml/catalog/CatalogImpl.java Tue Aug 30 17:49:43 2016 -0700
+++ b/jaxp/src/java.xml/share/classes/javax/xml/catalog/CatalogImpl.java Wed Aug 31 14:33:23 2016 -0700
@@ -31,6 +31,7 @@
import java.net.URISyntaxException;
import java.net.URL;
import java.util.ArrayList;
+import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.NoSuchElementException;
@@ -72,9 +73,6 @@
//System Id for this catalog
String systemId;
- //The parent
- CatalogImpl parent = null;
-
/*
A list of catalog entry files from the input, excluding the current catalog.
Paths in the List are normalized.
@@ -107,7 +105,7 @@
* catalog file.
*/
public CatalogImpl(CatalogImpl parent, CatalogFeatures f, String... file) throws CatalogException {
- super(CatalogEntryType.CATALOG);
+ super(CatalogEntryType.CATALOG, parent);
if (f == null) {
throw new NullPointerException(
formatMessage(CatalogMessages.ERR_NULL_ARGUMENT, new Object[]{"CatalogFeatures"}));
@@ -117,7 +115,7 @@
CatalogMessages.reportNPEOnNull("The path to the catalog file", file[0]);
}
- init(parent, f);
+ init(f);
//Path of catalog files
String[] catalogFile = file;
@@ -159,19 +157,34 @@
}
}
}
-
- if (systemId != null) {
- parse(systemId);
- }
}
}
- private void init(CatalogImpl parent, CatalogFeatures f) {
- this.parent = parent;
+ /**
+ * Loads the catalog
+ */
+ void load() {
+ if (systemId != null) {
+ parse(systemId);
+ }
+
+ //save this catalog before loading the next
+ loadedCatalogs.put(systemId, this);
+
+ //Load delegate and alternative catalogs if defer is false.
+ if (!isDeferred()) {
+ loadDelegateCatalogs();
+ loadNextCatalogs();
+ }
+ }
+
+ private void init(CatalogFeatures f) {
if (parent == null) {
level = 0;
} else {
level = parent.level + 1;
+ this.loadedCatalogs = parent.loadedCatalogs;
+ this.catalogsSearched = parent.catalogsSearched;
}
if (f == null) {
this.features = CatalogFeatures.defaults();
@@ -196,11 +209,6 @@
entries.stream().filter((entry) -> (entry.type == CatalogEntryType.GROUP)).forEach((entry) -> {
((GroupEntry) entry).reset();
});
-
- if (parent != null) {
- this.loadedCatalogs = parent.loadedCatalogs;
- this.catalogsSearched = parent.catalogsSearched;
- }
}
/**
@@ -421,16 +429,16 @@
void loadNextCatalogs() {
//loads catalogs specified in nextCatalogs
if (nextCatalogs != null) {
- for (NextCatalog next : nextCatalogs) {
+ nextCatalogs.stream().forEach((next) -> {
getCatalog(next.getCatalogURI());
- }
+ });
}
//loads catalogs from the input list
if (inputFiles != null) {
- for (String file : inputFiles) {
+ inputFiles.stream().forEach((file) -> {
getCatalog(getSystemId(file));
- }
+ });
}
}
@@ -445,14 +453,14 @@
return null;
}
- Catalog c = null;
+ CatalogImpl c = null;
String path = uri.toASCIIString();
if (verifyCatalogFile(uri)) {
c = getLoadedCatalog(path);
if (c == null) {
c = new CatalogImpl(this, features, path);
- saveLoadedCatalog(path, c);
+ c.load();
}
}
return c;
@@ -464,7 +472,7 @@
* @param catalogId the catalogId associated with the Catalog object
* @param c the Catalog to be saved
*/
- void saveLoadedCatalog(String catalogId, Catalog c) {
+ void saveLoadedCatalog(String catalogId, CatalogImpl c) {
loadedCatalogs.put(catalogId, c);
}
--- a/jaxp/src/java.xml/share/classes/javax/xml/catalog/CatalogManager.java Tue Aug 30 17:49:43 2016 -0700
+++ b/jaxp/src/java.xml/share/classes/javax/xml/catalog/CatalogManager.java Wed Aug 31 14:33:23 2016 -0700
@@ -62,7 +62,9 @@
* @throws CatalogException If an error occurs while parsing the catalog
*/
public static Catalog catalog(CatalogFeatures features, String... paths) {
- return new CatalogImpl(features, paths);
+ CatalogImpl catalog = new CatalogImpl(features, paths);
+ catalog.load();
+ return catalog;
}
/**
--- a/jaxp/src/java.xml/share/classes/javax/xml/catalog/CatalogReader.java Tue Aug 30 17:49:43 2016 -0700
+++ b/jaxp/src/java.xml/share/classes/javax/xml/catalog/CatalogReader.java Wed Aug 31 14:33:23 2016 -0700
@@ -259,15 +259,6 @@
CatalogEntryType type = CatalogEntryType.getType(localName);
if (type == CatalogEntryType.GROUP) {
inGroup = false;
- } else if (type == CatalogEntryType.CATALOGENTRY) {
- /*
- Done reading the catalog file.
- Load delegate and alternative catalogs if defer is false.
- */
- if (!catalog.isDeferred()) {
- catalog.loadDelegateCatalogs();
- catalog.loadNextCatalogs();
- }
}
}
--- a/jaxp/src/java.xml/share/classes/javax/xml/catalog/CatalogResolverImpl.java Tue Aug 30 17:49:43 2016 -0700
+++ b/jaxp/src/java.xml/share/classes/javax/xml/catalog/CatalogResolverImpl.java Wed Aug 31 14:33:23 2016 -0700
@@ -119,6 +119,10 @@
String result = null;
CatalogImpl c = (CatalogImpl)catalog;
String uri = Normalizer.normalizeURI(href);
+ if (uri == null) {
+ return null;
+ }
+
//check whether uri is an urn
if (uri != null && uri.startsWith(Util.URN)) {
String publicId = Normalizer.decodeURN(uri);
--- a/jaxp/src/java.xml/share/classes/javax/xml/catalog/GroupEntry.java Tue Aug 30 17:49:43 2016 -0700
+++ b/jaxp/src/java.xml/share/classes/javax/xml/catalog/GroupEntry.java Wed Aug 31 14:33:23 2016 -0700
@@ -48,6 +48,9 @@
//Value of the prefer attribute
boolean isPreferPublic = true;
+ //The parent of the catalog instance
+ CatalogImpl parent = null;
+
//The catalog instance this group belongs to
CatalogImpl catalog;
@@ -55,10 +58,10 @@
List<BaseEntry> entries = new ArrayList<>();
//loaded delegated catalog by system id
- Map<String, Catalog> delegateCatalogs = new HashMap<>();
+ Map<String, CatalogImpl> delegateCatalogs = new HashMap<>();
//A list of all loaded Catalogs, including this, and next catalogs
- Map<String, Catalog> loadedCatalogs = new HashMap<>();
+ Map<String, CatalogImpl> loadedCatalogs = new HashMap<>();
/*
A list of Catalog Ids that have already been searched in a matching
@@ -136,8 +139,9 @@
*
* @param type The type of the entry
*/
- public GroupEntry(CatalogEntryType type) {
+ public GroupEntry(CatalogEntryType type, CatalogImpl parent) {
super(type);
+ this.parent = parent;
}
/**
@@ -163,7 +167,7 @@
}
/**
* Constructs a group entry.
- * @param catalog The parent catalog
+ * @param catalog The catalog this GroupEntry belongs
* @param base The baseURI attribute
* @param attributes The attributes
*/
@@ -445,13 +449,14 @@
* @param catalogId the catalog Id
*/
Catalog loadCatalog(URI catalogURI) {
- Catalog delegateCatalog = null;
+ CatalogImpl delegateCatalog = null;
if (catalogURI != null) {
String catalogId = catalogURI.toASCIIString();
delegateCatalog = getLoadedCatalog(catalogId);
if (delegateCatalog == null) {
if (verifyCatalogFile(catalogURI)) {
delegateCatalog = new CatalogImpl(catalog, features, catalogId);
+ delegateCatalog.load();
delegateCatalogs.put(catalogId, delegateCatalog);
}
}
@@ -467,8 +472,8 @@
* @return a Catalog object previously loaded, or null if none in the saved
* list
*/
- Catalog getLoadedCatalog(String catalogId) {
- Catalog c = null;
+ CatalogImpl getLoadedCatalog(String catalogId) {
+ CatalogImpl c = null;
//checl delegate Catalogs
c = delegateCatalogs.get(catalogId);
@@ -504,7 +509,7 @@
}
String catalogId = catalogURI.toASCIIString();
- if (catalogsSearched.contains(catalogId)) {
+ if (catalogsSearched.contains(catalogId) || isCircular(catalogId)) {
CatalogMessages.reportRunTimeError(CatalogMessages.ERR_CIRCULAR_REFERENCE,
new Object[]{CatalogMessages.sanitize(catalogId)});
}
@@ -512,4 +517,20 @@
return true;
}
+ /**
+ * Checks whether the catalog is circularly referenced
+ * @param systemId the system identifier of the catalog to be loaded
+ * @return true if is circular, false otherwise
+ */
+ boolean isCircular(String systemId) {
+ if (parent == null) {
+ return false;
+ }
+
+ if (parent.systemId.equals(systemId)) {
+ return true;
+ }
+
+ return parent.isCircular(systemId);
+ }
}
--- a/jaxp/test/javax/xml/jaxp/functional/catalog/DeferFeatureTest.java Tue Aug 30 17:49:43 2016 -0700
+++ b/jaxp/test/javax/xml/jaxp/functional/catalog/DeferFeatureTest.java Wed Aug 31 14:33:23 2016 -0700
@@ -64,12 +64,12 @@
public Object[][] data() {
return new Object[][]{
// By default, alternative catalogs are not loaded.
- {createCatalog(CatalogFeatures.defaults()), 0},
+ {createCatalog(CatalogFeatures.defaults()), 1},
// Alternative catalogs are not loaded when DEFER is set to true.
- {createCatalog(createDeferFeature(DEFER_TRUE)), 0},
- // The 3 alternative catalogs are not pre-loaded
+ {createCatalog(createDeferFeature(DEFER_TRUE)), 1},
+ // The 3 alternative catalogs are pre-loaded along with the parent
//when DEFER is set to false.
- {createCatalog(createDeferFeature(DEFER_FALSE)), 3}};
+ {createCatalog(createDeferFeature(DEFER_FALSE)), 4}};
}
private CatalogFeatures createDeferFeature(String defer) {
--- a/jaxp/test/javax/xml/jaxp/unittest/catalog/CatalogTest.java Tue Aug 30 17:49:43 2016 -0700
+++ b/jaxp/test/javax/xml/jaxp/unittest/catalog/CatalogTest.java Wed Aug 31 14:33:23 2016 -0700
@@ -92,6 +92,34 @@
super.setUp();
}
+ /*
+ * @bug 8162431
+ * Verifies that circular references are caught and
+ * CatalogException is thrown.
+ */
+ @Test(dataProvider = "getFeatures", expectedExceptions = CatalogException.class)
+ public void testCircularRef(CatalogFeatures cf, String xml) {
+ CatalogResolver catalogResolver = CatalogManager.catalogResolver(
+ cf,
+ getClass().getResource(xml).getFile());
+ catalogResolver.resolve("anyuri", "");
+ }
+
+ /*
+ DataProvider: used to verify circular reference
+ Data columns: CatalogFeatures, catalog
+ */
+ @DataProvider(name = "getFeatures")
+ public Object[][] getFeatures() {
+
+ return new Object[][]{
+ {CatalogFeatures.builder().with(CatalogFeatures.Feature.DEFER, "false").build(),
+ "catalogReferCircle-itself.xml"},
+ {CatalogFeatures.defaults(), "catalogReferCircle-itself.xml"},
+ {CatalogFeatures.builder().with(CatalogFeatures.Feature.DEFER, "false").build(),
+ "catalogReferCircle-left.xml"},
+ {CatalogFeatures.defaults(), "catalogReferCircle-left.xml"},};
+ }
/*
* @bug 8163232
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/jaxp/test/javax/xml/jaxp/unittest/catalog/catalogReferCircle-itself.xml Wed Aug 31 14:33:23 2016 -0700
@@ -0,0 +1,6 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<catalog xmlns="urn:oasis:names:tc:entity:xmlns:xml:catalog">
+ <nextCatalog catalog="catalogReferCircle-itself.xml" />
+</catalog>
+
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/jaxp/test/javax/xml/jaxp/unittest/catalog/catalogReferCircle-left.xml Wed Aug 31 14:33:23 2016 -0700
@@ -0,0 +1,6 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<catalog xmlns="urn:oasis:names:tc:entity:xmlns:xml:catalog">
+ <nextCatalog catalog="catalogReferCircle-right.xml" />
+</catalog>
+
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/jaxp/test/javax/xml/jaxp/unittest/catalog/catalogReferCircle-right.xml Wed Aug 31 14:33:23 2016 -0700
@@ -0,0 +1,6 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<catalog xmlns="urn:oasis:names:tc:entity:xmlns:xml:catalog">
+ <nextCatalog catalog="catalogReferCircle-left.xml" />
+</catalog>
+