DTD hyperlink with XML catalog and PUBLIC declaration doesn't work

Fixes #850 Signed-off-by: azerr <[email protected]>
eclipse-lemminx · Aug 24, 2020 · 1e3f131 · 1e3f131
1 parent f3ab019
commit 1e3f131
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 27 deletions.
diff --git a/...lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/ContentModelPlugin.java b/...lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/ContentModelPlugin.java
@@ -53,7 +53,7 @@ public class ContentModelPlugin implements IXMLExtension {
 
 	private final ICodeActionParticipant codeActionParticipant;
 
-	private final IDocumentLinkParticipant documentLinkParticipant;
+	private IDocumentLinkParticipant documentLinkParticipant;
 
 	private final ITypeDefinitionParticipant typeDefinitionParticipant;
 
@@ -66,7 +66,6 @@ public ContentModelPlugin() {
 		hoverParticipant = new ContentModelHoverParticipant();
 		diagnosticsParticipant = new ContentModelDiagnosticsParticipant(this);
 		codeActionParticipant = new ContentModelCodeActionParticipant();
-		documentLinkParticipant = new ContentModelDocumentLinkParticipant();
 		typeDefinitionParticipant = new ContentModelTypeDefinitionParticipant();
 	}
 
@@ -144,6 +143,7 @@ public void start(InitializeParams params, XMLExtensionsRegistry registry) {
 		if (params != null) {
 			contentModelManager.setRootURI(params.getRootUri());
 		}
+		documentLinkParticipant = new ContentModelDocumentLinkParticipant(resolverManager);
 		registry.registerCompletionParticipant(completionParticipant);
 		registry.registerHoverParticipant(hoverParticipant);
 		registry.registerDiagnosticsParticipant(diagnosticsParticipant);

diff --git a/...pse/lemminx/extensions/contentmodel/participants/ContentModelDocumentLinkParticipant.java b/...pse/lemminx/extensions/contentmodel/participants/ContentModelDocumentLinkParticipant.java
@@ -17,8 +17,6 @@
 import java.util.Collection;
 import java.util.List;
 
-import org.apache.xerces.impl.XMLEntityManager;
-import org.apache.xerces.util.URI.MalformedURIException;
 import org.eclipse.lemminx.commons.BadLocationException;
 import org.eclipse.lemminx.dom.DOMDocument;
 import org.eclipse.lemminx.dom.DOMDocumentType;
@@ -28,6 +26,7 @@
 import org.eclipse.lemminx.dom.SchemaLocationHint;
 import org.eclipse.lemminx.dom.XMLModel;
 import org.eclipse.lemminx.services.extensions.IDocumentLinkParticipant;
+import org.eclipse.lemminx.uriresolver.URIResolverExtensionManager;
 import org.eclipse.lsp4j.DocumentLink;
 
 /**
@@ -44,13 +43,19 @@
  */
 public class ContentModelDocumentLinkParticipant implements IDocumentLinkParticipant {
 
+	private final URIResolverExtensionManager resolverManager;
+
+	public ContentModelDocumentLinkParticipant(URIResolverExtensionManager resolverManager) {
+		this.resolverManager = resolverManager;
+	}
+
 	@Override
 	public void findDocumentLinks(DOMDocument document, List<DocumentLink> links) {
 		// Document link for xsi:noNamespaceSchemaLocation
 		NoNamespaceSchemaLocation noNamespaceSchemaLocation = document.getNoNamespaceSchemaLocation();
 		if (noNamespaceSchemaLocation != null) {
 			try {
-				String location = getResolvedLocation(document.getDocumentURI(),
+				String location = resolverManager.resolve(document.getDocumentURI(), null,
 						noNamespaceSchemaLocation.getLocation());
 				if (location != null) {
 					DOMRange attrValue = noNamespaceSchemaLocation.getAttr().getNodeAttrValue();
@@ -65,7 +70,8 @@ public void findDocumentLinks(DOMDocument document, List<DocumentLink> links) {
 		// Document link for DTD
 		DOMDocumentType docType = document.getDoctype();
 		if (docType != null) {
-			String location = getResolvedLocation(document.getDocumentURI(), docType.getSystemIdWithoutQuotes());
+			String location = resolverManager.resolve(document.getDocumentURI(), docType.getPublicIdWithoutQuotes(),
+					docType.getSystemIdWithoutQuotes());
 			if (location != null) {
 				try {
 					DOMRange systemIdRange = docType.getSystemIdNode();
@@ -80,7 +86,7 @@ public void findDocumentLinks(DOMDocument document, List<DocumentLink> links) {
 		// Document link for xml-model/href
 		List<XMLModel> xmlModels = document.getXMLModels();
 		for (XMLModel xmlModel : xmlModels) {
-			String location = getResolvedLocation(document.getDocumentURI(), xmlModel.getHref());
+			String location = resolverManager.resolve(document.getDocumentURI(), null, xmlModel.getHref());
 			if (location != null) {
 				try {
 					DOMRange hrefRange = xmlModel.getHrefNode();
@@ -99,7 +105,7 @@ public void findDocumentLinks(DOMDocument document, List<DocumentLink> links) {
 				Collection<SchemaLocationHint> schemaLocationHints = schemaLocation.getSchemaLocationHints();
 				String location;
 				for (SchemaLocationHint schemaLocationHint : schemaLocationHints) {
-					location = getResolvedLocation(document.getDocumentURI(), schemaLocationHint.getHint());
+					location = resolverManager.resolve(document.getDocumentURI(), null, schemaLocationHint.getHint());
 					if (location != null) {
 						links.add(createDocumentLink(schemaLocationHint, location, false));
 					}
@@ -110,20 +116,4 @@ public void findDocumentLinks(DOMDocument document, List<DocumentLink> links) {
 		}
 	}
 
-	/**
-	 * Returns the expanded system location
-	 *
-	 * @return the expanded system location
-	 */
-	private static String getResolvedLocation(String documentURI, String location) {
-		if (location == null) {
-			return null;
-		}
-		try {
-			return XMLEntityManager.expandSystemId(location, documentURI, false);
-		} catch (MalformedURIException e) {
-			return location;
-		}
-	}
-
 }
diff --git a/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/XMLAssert.java b/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/XMLAssert.java
@@ -680,12 +680,20 @@ private static String getHoverLabel(Hover hover) {
 	// ------------------- Links assert
 
 	public static void testDocumentLinkFor(String xml, String fileURI, DocumentLink... expected) {
+		testDocumentLinkFor(xml, fileURI, null, expected);
+	}
+
+	public static void testDocumentLinkFor(String xml, String fileURI, String catalogPath, DocumentLink... expected) {
 		TextDocument document = new TextDocument(xml, fileURI != null ? fileURI : "test.xml");
 
 		XMLLanguageService xmlLanguageService = new XMLLanguageService();
 
 		ContentModelSettings settings = new ContentModelSettings();
 		settings.setUseCache(false);
+		// Configure XML catalog for XML schema
+		if (catalogPath != null) {
+			settings.setCatalogs(new String[] { catalogPath });
+		}
 		xmlLanguageService.doSave(new SettingsSaveContext(settings));
 
 		DOMDocument xmlDocument = DOMParser.getInstance().parse(document,
@@ -705,8 +713,8 @@ public static void assertDocumentLinks(List<DocumentLink> actual, DocumentLink..
 		assertEquals(expected.length, actual.size());
 		for (int i = 0; i < expected.length; i++) {
 			assertEquals(expected[i].getRange(), actual.get(i).getRange(), " Range test '" + i + "' link");
-			assertEquals(Paths.get(expected[i].getTarget()).toUri().toString(), actual.get(i).getTarget(),
-					" Target test '" + i + "' link");
+			assertEquals(Paths.get(expected[i].getTarget()).toUri().toString().replace("file:///", "file:/"),
+					actual.get(i).getTarget().replace("file:///", "file:/"), " Target test '" + i + "' link");
 		}
 	}
 

diff --git a/...emminx/src/test/java/org/eclipse/lemminx/extensions/contentmodel/DTDDocumentLinkTest.java b/...emminx/src/test/java/org/eclipse/lemminx/extensions/contentmodel/DTDDocumentLinkTest.java
@@ -45,7 +45,7 @@ public void docTypePUBLIC() throws BadLocationException {
 		XMLAssert.testDocumentLinkFor(xml, "src/test/resources/xml/base.xml",
 				dl(r(1, 38, 1, 62), "src/test/resources/dtd/entities/base.dtd"));
 	}
-	
+
 	@Test
 	public void noLinks() throws BadLocationException {
 		String xml = "<?xml version=\"1.0\" encoding=\"utf-8\" ?>\r\n" + //
@@ -55,4 +55,21 @@ public void noLinks() throws BadLocationException {
 				"<root-element />";
 		XMLAssert.testDocumentLinkFor(xml, "src/test/resources/xml/base.xml");
 	}
+
+	@Test
+	public void linkWithCatalogAndPublic() throws Exception {
+		// This test uses the local DTD with catalog-public.xml by using the PUBLIC ID
+		// -//Sun Microsystems, Inc.//DTD Web Application 2.3//EN
+		// <public publicId="-//Sun Microsystems, Inc.//DTD Web Application 2.3//EN"
+		// uri="../dtd/web-app_2_3.dtd" />
+		String xml = "<?xml version=\"1.0\" encoding=\"ISO-8859-1\"?> \r\n" + //
+				"<!DOCTYPE web-app\r\n" + //
+				"   PUBLIC \"-//Sun Microsystems, Inc.//DTD Web Application 2.3//EN\"\r\n" + //
+				"   \"ABCD.dtd\">\r\n" + //
+				"\r\n" + //
+				"<web-app></web-app>";
+		XMLAssert.testDocumentLinkFor(xml, "src/test/resources/xml/base.xml",
+				"src/test/resources/catalogs/catalog-public.xml",
+				dl(r(3,4,3,12), "src/test/resources/dtd/web-app_2_3.dtd"));
+	}
 }