8176405: Catalog circular reference check did not work in certain scenarios

Reviewed-by: rriggs, lancea
This commit is contained in:
Joe Wang 2017-03-23 21:28:13 -07:00
parent 6484016211
commit 6c2c3790e3
8 changed files with 157 additions and 121 deletions

@ -88,6 +88,7 @@ class CatalogImpl extends GroupEntry implements Catalog {
/**
* Construct a Catalog with specified URI.
*
* @param f the features object
* @param uris the uri(s) to one or more catalogs
* @throws CatalogException If an error happens while parsing the specified
* catalog file.
@ -100,6 +101,7 @@ class CatalogImpl extends GroupEntry implements Catalog {
* Construct a Catalog with specified URI.
*
* @param parent The parent catalog
* @param f the features object
* @param uris the uri(s) to one or more catalogs
* @throws CatalogException If an error happens while parsing the specified
* catalog file.
@ -137,7 +139,7 @@ class CatalogImpl extends GroupEntry implements Catalog {
for (String temp : catalogFile) {
uri = URI.create(temp);
start++;
if (verifyCatalogFile(uri)) {
if (verifyCatalogFile(null, uri)) {
systemId = temp;
try {
baseURI = new URL(systemId);
@ -169,12 +171,14 @@ class CatalogImpl extends GroupEntry implements Catalog {
parse(systemId);
}
setCatalog(this);
//save this catalog before loading the next
loadedCatalogs.put(systemId, this);
//Load delegate and alternative catalogs if defer is false.
if (!isDeferred()) {
loadDelegateCatalogs();
loadDelegateCatalogs(this);
loadNextCatalogs();
}
}
@ -365,14 +369,16 @@ class CatalogImpl extends GroupEntry implements Catalog {
//Check those specified in nextCatalogs
if (nextCatalogs != null) {
while (c == null && nextCatalogIndex < nextCatalogs.size()) {
c = getCatalog(nextCatalogs.get(nextCatalogIndex++).getCatalogURI());
c = getCatalog(catalog,
nextCatalogs.get(nextCatalogIndex++).getCatalogURI());
}
}
//Check the input list
if (c == null && inputFiles != null) {
while (c == null && inputFilesIndex < inputFiles.size()) {
c = getCatalog(URI.create(inputFiles.get(inputFilesIndex++)));
c = getCatalog(null,
URI.create(inputFiles.get(inputFilesIndex++)));
}
}
@ -408,14 +414,14 @@ class CatalogImpl extends GroupEntry implements Catalog {
//loads catalogs specified in nextCatalogs
if (nextCatalogs != null) {
nextCatalogs.stream().forEach((next) -> {
getCatalog(next.getCatalogURI());
getCatalog(this, next.getCatalogURI());
});
}
//loads catalogs from the input list
if (inputFiles != null) {
inputFiles.stream().forEach((uri) -> {
getCatalog(URI.create(uri));
getCatalog(null, URI.create(uri));
});
}
}
@ -423,17 +429,19 @@ class CatalogImpl extends GroupEntry implements Catalog {
/**
* Returns a Catalog object by the specified path.
*
* @param path the path to a catalog
* @param parent the parent catalog for the alternative catalogs to be loaded.
* It will be null if the ones to be loaded are from the input list.
* @param uri the path to a catalog
* @return a Catalog object
*/
Catalog getCatalog(URI uri) {
Catalog getCatalog(CatalogImpl parent, URI uri) {
if (uri == null) {
return null;
}
CatalogImpl c = null;
if (verifyCatalogFile(uri)) {
if (verifyCatalogFile(parent, uri)) {
c = getLoadedCatalog(uri.toASCIIString());
if (c == null) {
c = new CatalogImpl(this, features, uri);
@ -459,6 +467,6 @@ class CatalogImpl extends GroupEntry implements Catalog {
* @return a count of all loaded catalogs
*/
int loadedCatalogCount() {
return loadedCatalogs.size() + delegateCatalogs.size();
return loadedCatalogs.size();
}
}

@ -1,4 +1,4 @@
# Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved.
# Copyright (c) 2015, 2017, 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
@ -21,25 +21,32 @@
# or visit www.oracle.com if you need additional information or have any
# questions.
# Messages for message reporting
BadMessageKey = The error message corresponding to the message key can not be found.
FormatFailed = An internal error occurred while formatting the following message:\n
# General errors
BadMessageKey = JAXP09000001: The error message corresponding to the message key can not be found.
FormatFailed = JAXP09000002: An internal error occurred while formatting the following message:\n
OtherError = JAXP09000003: Unexpected error.
#invalid catalog file
InvalidCatalog = The document element of a catalog must be catalog.
InvalidEntryType = The entry type ''{0}'' is not valid.
CircularReference = Circular reference is not allowed: ''{0}''.
# Implementation restriction
CircularReference = JAXP09010001: Circular reference is not allowed: ''{0}''.
# Input or configuration errors
InvalidCatalog = JAXP09020001: The document element of a catalog must be catalog.
InvalidEntryType = JAXP09020002: The entry type ''{0}'' is not valid.
UriNotAbsolute = JAXP09020003: The specified URI ''{0}'' is not absolute.
UriNotValidUrl = JAXP09020004: The specified URI ''{0}'' is not a valid URL.
InvalidArgument = JAXP09020005: The specified argument ''{0}'' (case sensitive) for ''{1}'' is not valid.
NullArgument = JAXP09020006: The argument ''{0}'' can not be null.
InvalidPath = JAXP09020007: The path ''{0}'' is invalid.
# Parsing errors
ParserConf = JAXP09030001: Unexpected error while configuring a SAX parser.
ParsingFailed = JAXP09030002: Failed to parse the catalog file.
NoCatalogFound = JAXP09030003: No Catalog is specified.
# Resolving errors
NoMatchFound = JAXP09040001: No match found for publicId ''{0}'' and systemId ''{1}''.
NoMatchURIFound = JAXP09040002: No match found for href ''{0}'' and base ''{1}''.
FailedCreatingURI = JAXP09040003: Can not construct URI using href ''{0}'' and base ''{1}''.
#errors
UriNotAbsolute = The specified URI ''{0}'' is not absolute.
UriNotValidUrl = The specified URI ''{0}'' is not a valid URL.
InvalidArgument = The specified argument ''{0}'' (case sensitive) for ''{1}'' is not valid.
NullArgument = The argument ''{0}'' can not be null.
InvalidPath = The path ''{0}'' is invalid.
ParserConf = Unexpected error while configuring a SAX parser.
ParsingFailed = Failed to parse the catalog file.
NoCatalogFound = No Catalog is specified.
NoMatchFound = No match found for publicId ''{0}'' and systemId ''{1}''.
NoMatchURIFound = No match found for href ''{0}'' and base ''{1}''.
FailedCreatingURI = Can not construct URI using href ''{0}'' and base ''{1}''.
OtherError = Unexpected error.

@ -135,7 +135,8 @@ class GroupEntry extends BaseEntry {
/**
* Constructs a GroupEntry
*
* @param type The type of the entry
* @param type the type of the entry
* @param parent the parent Catalog
*/
public GroupEntry(CatalogEntryType type, CatalogImpl parent) {
super(type);
@ -165,9 +166,9 @@ class GroupEntry extends BaseEntry {
}
/**
* Constructs a group entry.
* @param catalog The catalog this GroupEntry belongs
* @param base The baseURI attribute
* @param attributes The attributes
* @param catalog the catalog this GroupEntry belongs to
* @param base the baseURI attribute
* @param attributes the attributes
*/
public GroupEntry(CatalogImpl catalog, String base, String... attributes) {
super(CatalogEntryType.GROUP, base);
@ -175,6 +176,15 @@ class GroupEntry extends BaseEntry {
this.catalog = catalog;
}
/**
* Sets the catalog for this GroupEntry.
*
* @param catalog the catalog this GroupEntry belongs to
*/
void setCatalog(CatalogImpl catalog) {
this.catalog = catalog;
}
/**
* Adds an entry.
*
@ -382,10 +392,9 @@ class GroupEntry extends BaseEntry {
/**
* Matches delegatePublic or delegateSystem against the specified id
*
* @param isSystem The flag to indicate whether the delegate is system or
* public
* @param id The system or public id to be matched
* @return The URI string if a mapping is found, or null otherwise.
* @param type the type of the Catalog entry
* @param id the system or public id to be matched
* @return the URI string if a mapping is found, or null otherwise.
*/
private String matchDelegate(CatalogEntryType type, String id) {
String match = null;
@ -412,7 +421,7 @@ class GroupEntry extends BaseEntry {
//Check delegate Catalogs
if (catalogId != null) {
Catalog delegateCatalog = loadCatalog(catalogId);
Catalog delegateCatalog = loadDelegateCatalog(catalog, catalogId);
if (delegateCatalog != null) {
if (type == CatalogEntryType.DELEGATESYSTEM) {
@ -430,30 +439,34 @@ class GroupEntry extends BaseEntry {
/**
* Loads all delegate catalogs.
*
* @param parent the parent catalog of the delegate catalogs
*/
void loadDelegateCatalogs() {
void loadDelegateCatalogs(CatalogImpl parent) {
entries.stream()
.filter((entry) -> (entry.type == CatalogEntryType.DELEGATESYSTEM ||
entry.type == CatalogEntryType.DELEGATEPUBLIC ||
entry.type == CatalogEntryType.DELEGATEURI))
.map((entry) -> (AltCatalog)entry)
.forEach((altCatalog) -> {
loadCatalog(altCatalog.getCatalogURI());
loadDelegateCatalog(parent, altCatalog.getCatalogURI());
});
}
/**
* Loads a delegate catalog by the catalogId specified.
* @param catalogId the catalog Id
*
* @param parent the parent catalog of the delegate catalog
* @param catalogURI the URI to the catalog
*/
Catalog loadCatalog(URI catalogURI) {
Catalog loadDelegateCatalog(CatalogImpl parent, URI catalogURI) {
CatalogImpl delegateCatalog = null;
if (catalogURI != null) {
String catalogId = catalogURI.toASCIIString();
delegateCatalog = getLoadedCatalog(catalogId);
if (delegateCatalog == null) {
if (verifyCatalogFile(catalogURI)) {
delegateCatalog = new CatalogImpl(catalog, features, catalogURI);
if (verifyCatalogFile(parent, catalogURI)) {
delegateCatalog = getLoadedCatalog(catalogId);
if (delegateCatalog == null) {
delegateCatalog = new CatalogImpl(parent, features, catalogURI);
delegateCatalog.load();
delegateCatalogs.put(catalogId, delegateCatalog);
}
@ -473,7 +486,7 @@ class GroupEntry extends BaseEntry {
CatalogImpl getLoadedCatalog(String catalogId) {
CatalogImpl c = null;
//checl delegate Catalogs
//check delegate Catalogs
c = delegateCatalogs.get(catalogId);
if (c == null) {
//check other loaded Catalogs
@ -492,11 +505,12 @@ class GroupEntry extends BaseEntry {
* Verifies that the catalog represented by the catalogId has not been
* searched or is not circularly referenced.
*
* @param catalogId The URI to a catalog
* @param parent the parent of the catalog to be loaded
* @param catalogURI the URI to the catalog
* @throws CatalogException if circular reference is found.
* @return true if the catalogId passed verification, false otherwise
*/
final boolean verifyCatalogFile(URI catalogURI) {
final boolean verifyCatalogFile(CatalogImpl parent, URI catalogURI) {
if (catalogURI == null) {
return false;
}
@ -508,7 +522,7 @@ class GroupEntry extends BaseEntry {
}
String catalogId = catalogURI.toASCIIString();
if (catalogsSearched.contains(catalogId) || isCircular(catalogId)) {
if (catalogsSearched.contains(catalogId) || isCircular(parent, catalogId)) {
CatalogMessages.reportRunTimeError(CatalogMessages.ERR_CIRCULAR_REFERENCE,
new Object[]{CatalogMessages.sanitize(catalogId)});
}
@ -518,10 +532,13 @@ class GroupEntry extends BaseEntry {
/**
* Checks whether the catalog is circularly referenced
*
* @param parent the parent of the catalog to be loaded
* @param systemId the system identifier of the catalog to be loaded
* @return true if is circular, false otherwise
*/
boolean isCircular(String systemId) {
boolean isCircular(CatalogImpl parent, String systemId) {
// first, check the parent of the catalog to be loaded
if (parent == null) {
return false;
}
@ -530,6 +547,7 @@ class GroupEntry extends BaseEntry {
return true;
}
return parent.isCircular(systemId);
// next, check parent's parent
return parent.isCircular(parent.parent, systemId);
}
}

@ -1,5 +1,5 @@
/*
* Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2015, 2017, 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
@ -27,14 +27,16 @@ import static catalog.CatalogTestUtils.DEFER_FALSE;
import static catalog.CatalogTestUtils.DEFER_TRUE;
import static catalog.CatalogTestUtils.getCatalogPath;
import static javax.xml.catalog.CatalogFeatures.Feature.DEFER;
import static javax.xml.catalog.CatalogManager.catalog;
import static jaxp.library.JAXPTestUtilities.runWithAllPerm;
import static jaxp.library.JAXPTestUtilities.tryRunWithAllPerm;
import java.lang.reflect.Method;
import javax.xml.catalog.Catalog;
import javax.xml.catalog.CatalogException;
import javax.xml.catalog.CatalogFeatures;
import javax.xml.catalog.CatalogManager;
import javax.xml.catalog.CatalogResolver;
import org.testng.Assert;
import org.testng.annotations.DataProvider;
@ -43,7 +45,7 @@ import org.testng.annotations.Test;
/*
* @test
* @bug 8077931
* @bug 8077931 8176405
* @library /javax/xml/jaxp/libs
* @modules java.xml/javax.xml.catalog:open
* @run testng/othervm -DrunSecMngr=true catalog.DeferFeatureTest
@ -61,6 +63,18 @@ public class DeferFeatureTest {
Assert.assertEquals(loadedCatalogCount(catalog), catalogCount);
}
@Test(dataProvider = "testDeferFeatureByResolve")
public void testDeferFeatureByResolve(Catalog catalog, int catalogCount)
throws Exception {
CatalogResolver cr = createResolver(catalog);
// trigger loading alternative catalogs
try {
cr.resolveEntity("-//REMOTE//DTD ALICE DOCALICE", "http://remote/dtd/alice/");
} catch (CatalogException ce) {}
Assert.assertEquals(loadedCatalogCount(catalog), catalogCount);
}
@DataProvider(name = "catalog-countOfLoadedCatalogFile")
public Object[][] data() {
return new Object[][]{
@ -73,12 +87,23 @@ public class DeferFeatureTest {
{createCatalog(createDeferFeature(DEFER_FALSE)), 4}};
}
@DataProvider(name = "testDeferFeatureByResolve")
public Object[][] getData() {
return new Object[][]{
{createCatalog(createDeferFeature(DEFER_TRUE)), 4}
};
}
private CatalogFeatures createDeferFeature(String defer) {
return CatalogFeatures.builder().with(DEFER, defer).build();
}
private Catalog createCatalog(CatalogFeatures feature) {
return catalog(feature, getCatalogPath("deferFeature.xml"));
return CatalogManager.catalog(feature, getCatalogPath("deferFeature.xml"));
}
private CatalogResolver createResolver(Catalog catalog) {
return CatalogManager.catalogResolver(catalog);
}
private int loadedCatalogCount(Catalog catalog) throws Exception {

@ -87,35 +87,6 @@ public class CatalogTest extends CatalogSupportBase {
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) throws Exception {
CatalogResolver catalogResolver = CatalogManager.catalogResolver(
cf,
getClass().getResource(xml).toURI());
catalogResolver.resolve("anyuri", "");
}
/*
DataProvider: used to verify circular reference
Data columns: CatalogFeatures, catalog
*/
@DataProvider(name = "getFeatures")
public Object[][] getFeatures() {
String self = "catalogReferCircle-itself.xml";
String left = "catalogReferCircle-left.xml";
return new Object[][]{
{CatalogFeatures.builder().with(CatalogFeatures.Feature.DEFER, "false").build(), self},
{CatalogFeatures.defaults(), self},
{CatalogFeatures.builder().with(CatalogFeatures.Feature.DEFER, "false").build(), left},
{CatalogFeatures.defaults(), left}
};
}
/*
* @bug 8163232
* Verifies that the CatalogResolver supports the following XML Resolvers:
@ -437,7 +408,10 @@ public class CatalogTest extends CatalogSupportBase {
public void resolveWithPrefer(String prefer, String cfile, String publicId,
String systemId, String expected) throws Exception {
URI catalogFile = getClass().getResource(cfile).toURI();
CatalogFeatures f = CatalogFeatures.builder().with(CatalogFeatures.Feature.PREFER, prefer).with(CatalogFeatures.Feature.RESOLVE, "ignore").build();
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);
@ -452,7 +426,9 @@ public class CatalogTest extends CatalogSupportBase {
@Test(dataProvider = "invalidAltCatalogs", expectedExceptions = CatalogException.class)
public void testDeferAltCatalogs(String file) throws Exception {
URI catalogFile = getClass().getResource(file).toURI();
CatalogFeatures features = CatalogFeatures.builder().with(CatalogFeatures.Feature.DEFER, "true").build();
CatalogFeatures features = CatalogFeatures.builder().
with(CatalogFeatures.Feature.DEFER, "true")
.build();
/*
Since the defer attribute is set to false in the specified catalog file,
the parent catalog will try to load the alt catalog, which will fail
@ -471,11 +447,17 @@ public class CatalogTest extends CatalogSupportBase {
URI catalogFile = getClass().getResource("JDK8146237_catalog.xml").toURI();
try {
CatalogFeatures features = CatalogFeatures.builder().with(CatalogFeatures.Feature.PREFER, "system").build();
CatalogFeatures features = CatalogFeatures.builder()
.with(CatalogFeatures.Feature.PREFER, "system")
.build();
Catalog catalog = CatalogManager.catalog(features, catalogFile);
CatalogResolver catalogResolver = CatalogManager.catalogResolver(catalog);
String actualSystemId = catalogResolver.resolveEntity("-//FOO//DTD XML Dummy V0.0//EN", "http://www.oracle.com/alt1sys.dtd").getSystemId();
Assert.assertTrue(actualSystemId.contains("dummy.dtd"), "Resulting id should contain dummy.dtd, indicating a match by publicId");
String actualSystemId = catalogResolver.resolveEntity(
"-//FOO//DTD XML Dummy V0.0//EN",
"http://www.oracle.com/alt1sys.dtd")
.getSystemId();
Assert.assertTrue(actualSystemId.contains("dummy.dtd"),
"Resulting id should contain dummy.dtd, indicating a match by publicId");
} catch (Exception e) {
Assert.fail(e.getMessage());
@ -572,20 +554,21 @@ public class CatalogTest extends CatalogSupportBase {
*/
@Test
public void testInvalidCatalog() throws Exception {
String expectedMsgId = "JAXP09040001";
URI catalog = getClass().getResource("catalog_invalid.xml").toURI();
String test = "testInvalidCatalog";
try {
CatalogResolver resolver = CatalogManager.catalogResolver(CatalogFeatures.defaults(), catalog);
String actualSystemId = resolver.resolveEntity(null, "http://remote/xml/dtd/sys/alice/docAlice.dtd").getSystemId();
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();
if (msg != null) {
if (msg.contains("No match found for publicId")) {
Assert.assertEquals(msg, "No match found for publicId 'null' and systemId 'http://remote/xml/dtd/sys/alice/docAlice.dtd'.");
System.out.println(test + ": expected [No match found for publicId 'null' and systemId 'http://remote/xml/dtd/sys/alice/docAlice.dtd'.]");
System.out.println("actual [" + msg + "]");
}
Assert.assertTrue(msg.contains(expectedMsgId),
"Message shall contain the corrent message ID " + expectedMsgId);
}
}
}
@ -607,7 +590,10 @@ public class CatalogTest extends CatalogSupportBase {
String test = "testInvalidCatalog";
try {
CatalogResolver resolver = CatalogManager.catalogResolver(f);
String actualSystemId = resolver.resolveEntity(null, "http://remote/xml/dtd/sys/alice/docAlice.dtd").getSystemId();
String actualSystemId = resolver.resolveEntity(
null,
"http://remote/xml/dtd/sys/alice/docAlice.dtd")
.getSystemId();
System.out.println("testIgnoreInvalidCatalog: expected [null]");
System.out.println("testIgnoreInvalidCatalog: expected [null]");
System.out.println("actual [" + actualSystemId + "]");
@ -628,7 +614,11 @@ public class CatalogTest extends CatalogSupportBase {
@DataProvider(name = "resolveUri")
public Object[][] getDataForUriResolver() {
return new Object[][]{
{"uri.xml", "urn:publicid:-:Acme,+Inc.:DTD+Book+Version+1.0", null, "http://local/base/dtd/book.dtd", "Uri in publicId namespace is incorrectly unwrapped"},
{"uri.xml",
"urn:publicid:-:Acme,+Inc.:DTD+Book+Version+1.0",
null,
"http://local/base/dtd/book.dtd",
"Uri in publicId namespace is incorrectly unwrapped"},
};
}
@ -654,7 +644,13 @@ public class CatalogTest extends CatalogSupportBase {
public Object[][] getDataForMatchingBothIds() {
String expected = "http://www.groupxmlbase.com/dtds/rewrite.dtd";
return new Object[][]{
{"rewriteSystem_id.xml", "system", "http://www.sys00test.com/rewrite.dtd", "PUB-404", expected, expected, "Relative rewriteSystem with xml:base at group level failed"},
{"rewriteSystem_id.xml",
"system",
"http://www.sys00test.com/rewrite.dtd",
"PUB-404",
expected,
expected,
"Relative rewriteSystem with xml:base at group level failed"},
};
}

@ -1,6 +0,0 @@
<?xml version="1.0" encoding="UTF-8"?>
<catalog xmlns="urn:oasis:names:tc:entity:xmlns:xml:catalog">
<nextCatalog catalog="catalogReferCircle-itself.xml" />
</catalog>

@ -1,6 +0,0 @@
<?xml version="1.0" encoding="UTF-8"?>
<catalog xmlns="urn:oasis:names:tc:entity:xmlns:xml:catalog">
<nextCatalog catalog="catalogReferCircle-right.xml" />
</catalog>

@ -1,6 +0,0 @@
<?xml version="1.0" encoding="UTF-8"?>
<catalog xmlns="urn:oasis:names:tc:entity:xmlns:xml:catalog">
<nextCatalog catalog="catalogReferCircle-left.xml" />
</catalog>