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

Fixes #847 Signed-off-by: azerr <[email protected]>
eclipse-lemminx · Aug 20, 2020 · ebb62a7 · ebb62a7
1 parent e6fa099
commit ebb62a7
Show file tree

Hide file tree

Showing 3 changed files with 115 additions and 48 deletions.
diff --git a/...va/org/eclipse/lemminx/extensions/contentmodel/participants/diagnostics/LSPSAXParser.java b/...va/org/eclipse/lemminx/extensions/contentmodel/participants/diagnostics/LSPSAXParser.java
@@ -29,11 +29,13 @@
 import org.apache.xerces.xni.XMLLocator;
 import org.apache.xerces.xni.XNIException;
 import org.apache.xerces.xni.grammars.XMLGrammarPool;
+import org.apache.xerces.xni.parser.XMLInputSource;
 import org.apache.xerces.xni.parser.XMLParserConfiguration;
 import org.eclipse.lemminx.commons.BadLocationException;
 import org.eclipse.lemminx.dom.DOMDocument;
 import org.eclipse.lemminx.dom.DOMDocumentType;
 import org.eclipse.lemminx.extensions.contentmodel.participants.DTDErrorCode;
+import org.eclipse.lemminx.uriresolver.URIResolverExtensionManager;
 import org.eclipse.lsp4j.DiagnosticSeverity;
 import org.eclipse.lsp4j.Range;
 import org.xml.sax.SAXNotRecognizedException;
@@ -105,21 +107,22 @@ public void doctypeDecl(String rootElement, String publicId, String systemId, Au
 		if (systemId != null) {
 			// There a declared DTD in the DOCTYPE
 			// <!DOCTYPE root-element SYSTEM "./extended.dtd" []>
-			String eid = null;
-			try {
-				eid = XMLEntityManager.expandSystemId(systemId, locator.getExpandedSystemId(), false);
-			} catch (java.io.IOException e) {
-			}
-			if (!isDTDExists(eid)) {
+
+			XMLEntityManager entityManager = (XMLEntityManager) fConfiguration.getProperty(ENTITY_MANAGER);
+			XMLDTDDescription grammarDesc = createGrammarDescription(rootElement, publicId, systemId);
+
+			// Expand the system ID of the DTD and resolve it.
+			String expandedSystemId = getExpandedSystemId(grammarDesc, entityManager);
+			if (!isDTDExists(expandedSystemId)) {
 				// The declared DTD doesn't exist
 				// <!DOCTYPE root-element SYSTEM "./dtd-doesnt-exist.dtd" []>
 				try {
 					// Report the error
 					DOMDocumentType docType = document.getDoctype();
 					Range range = new Range(document.positionAt(docType.getSystemIdNode().getStart()),
 							document.positionAt(docType.getSystemIdNode().getEnd()));
-					reporter.addDiagnostic(range, MessageFormat.format(DTD_NOT_FOUND, eid), DiagnosticSeverity.Error,
-							DTDErrorCode.dtd_not_found.getCode());
+					reporter.addDiagnostic(range, MessageFormat.format(DTD_NOT_FOUND, expandedSystemId),
+							DiagnosticSeverity.Error, DTDErrorCode.dtd_not_found.getCode());
 				} catch (BadLocationException e) {
 					// Do nothing
 				}
@@ -140,9 +143,7 @@ public void doctypeDecl(String rootElement, String publicId, String systemId, Au
 				if (grammarPool != null) {
 					// FIX [BUG 2]
 					// DTD exists, get the DTD grammar from the cache
-					XMLEntityManager entityManager = (XMLEntityManager) fConfiguration.getProperty(ENTITY_MANAGER);
-					XMLDTDDescription grammarDesc = new XMLDTDDescription(publicId, systemId,
-							locator.getExpandedSystemId(), eid, rootElement);
+
 					DTDGrammar grammar = (DTDGrammar) grammarPool.retrieveGrammar(grammarDesc);
 					if (grammar != null) {
 						// The DTD grammar is in cache, we need to fill XML entity manager with the
@@ -155,6 +156,44 @@ public void doctypeDecl(String rootElement, String publicId, String systemId, Au
 		super.doctypeDecl(rootElement, publicId, systemId, augs);
 	}
 
+	/**
+	 * Create DTD grammar description by expanding the system id.
+	 * 
+	 * @param rootElement the root element
+	 * @param publicId    the public ID.
+	 * @param systemId    the system ID.
+	 * @return the DTD grammar description by expanding the system id.
+	 */
+	private XMLDTDDescription createGrammarDescription(String rootElement, String publicId, String systemId) {
+		String eid = null;
+		try {
+			eid = XMLEntityManager.expandSystemId(systemId, locator.getExpandedSystemId(), false);
+		} catch (java.io.IOException e) {
+		}
+
+		return new XMLDTDDescription(publicId, systemId, locator.getExpandedSystemId(), eid, rootElement);
+	}
+
+	/**
+	 * Resolve the expanded system ID by resolving the system ID of the given
+	 * grammar description with uri resolver (XML catalog, cache, etc with
+	 * {@link URIResolverExtensionManager}).
+	 * 
+	 * @param grammarDesc   the DTD grammar description.
+	 * @param entityManager the entity manager.
+	 * @return the expanded system ID by resolving the system ID of the given
+	 *         grammar description with uri resolver (XML catalog, cache, etc with
+	 *         {@link URIResolverExtensionManager}).
+	 */
+	private static String getExpandedSystemId(XMLDTDDescription grammarDesc, XMLEntityManager entityManager) {
+		try {
+			XMLInputSource input = entityManager.resolveEntity(grammarDesc);
+			return input.getSystemId();
+		} catch (Exception e) {
+		}
+		return grammarDesc.getExpandedSystemId();
+	}
+
 	private static boolean isDTDExists(String expandedSystemId) {
 		if (expandedSystemId == null || expandedSystemId.isEmpty()) {
 			return true;

diff --git a/...lemminx/src/test/java/org/eclipse/lemminx/extensions/contentmodel/DTDDiagnosticsTest.java b/...lemminx/src/test/java/org/eclipse/lemminx/extensions/contentmodel/DTDDiagnosticsTest.java
@@ -12,8 +12,8 @@
  */
 package org.eclipse.lemminx.extensions.contentmodel;
 
-import static org.eclipse.lemminx.XMLAssert.d;
 import static org.eclipse.lemminx.XMLAssert.ca;
+import static org.eclipse.lemminx.XMLAssert.d;
 import static org.eclipse.lemminx.XMLAssert.te;
 import static org.eclipse.lemminx.XMLAssert.testCodeActionsFor;
 
@@ -46,6 +46,24 @@ public void MSG_ELEMENT_NOT_DECLARED() throws Exception {
 				d(5, 1, 8, DTDErrorCode.MSG_CONTENT_INVALID));
 	}
 
+	@Test
+	public void MSG_ELEMENT_NOT_DECLARED_Public() 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>\r\n" + //
+				"	<XXX></XXX>\r\n" + //
+				"</web-app>";
+		testPublicDiagnosticsFor(xml, d(6, 2, 5, DTDErrorCode.MSG_ELEMENT_NOT_DECLARED),
+				d(5, 1, 8, DTDErrorCode.MSG_CONTENT_INVALID));
+	}
+
 	@Test
 	public void MSG_CONTENT_INVALID() throws Exception {
 		String xml = "<?xml version=\"1.0\"?>\r\n" + //
@@ -296,7 +314,8 @@ public void EntityNotDeclaredAddToSubset() throws Exception {
 				"	&nbsp;\r\n" + //
 				"</article>";
 
-		Diagnostic d = d(5, 1, 5, 7, DTDErrorCode.EntityNotDeclared, "The entity \"nbsp\" was referenced, but not declared.");
+		Diagnostic d = d(5, 1, 5, 7, DTDErrorCode.EntityNotDeclared,
+				"The entity \"nbsp\" was referenced, but not declared.");
 		XMLAssert.testDiagnosticsFor(xml, d);
 		testCodeActionsFor(xml, d, ca(d, te(2, 29, 2, 29, "\r\n\t<!ENTITY nbsp \"entity-value\">")));
 	}
@@ -311,7 +330,8 @@ public void EntityNotDeclaredAddToSubsetOneChar() throws Exception {
 				"	&a;\r\n" + //
 				"</article>";
 
-		Diagnostic d = d(5, 1, 5, 4, DTDErrorCode.EntityNotDeclared, "The entity \"a\" was referenced, but not declared.");
+		Diagnostic d = d(5, 1, 5, 4, DTDErrorCode.EntityNotDeclared,
+				"The entity \"a\" was referenced, but not declared.");
 		XMLAssert.testDiagnosticsFor(xml, d);
 		testCodeActionsFor(xml, d, ca(d, te(2, 29, 2, 29, "\r\n\t<!ENTITY a \"entity-value\">")));
 	}
@@ -322,12 +342,12 @@ public void EntityNotDeclaredNoPrologNoDoctype() throws Exception {
 				"	&nbsp;\r\n" + //
 				"</article>";
 
-		Diagnostic d = d(1, 1, 1, 7, DTDErrorCode.EntityNotDeclared, "The entity \"nbsp\" was referenced, but not declared.");
+		Diagnostic d = d(1, 1, 1, 7, DTDErrorCode.EntityNotDeclared,
+				"The entity \"nbsp\" was referenced, but not declared.");
 		XMLAssert.testDiagnosticsFor(xml, d);
 
-		testCodeActionsFor(xml, d, ca(d, te(0, 0, 0, 0, "<!DOCTYPE article [\r\n" +
-				"\t<!ENTITY nbsp \"entity-value\">\r\n" +
-				"]>\r\n")));
+		testCodeActionsFor(xml, d,
+				ca(d, te(0, 0, 0, 0, "<!DOCTYPE article [\r\n" + "\t<!ENTITY nbsp \"entity-value\">\r\n" + "]>\r\n")));
 	}
 
 	@Test
@@ -337,28 +357,25 @@ public void EntityNotDeclaredWithPrologNoDoctype() throws Exception {
 				"	&nbsp;\r\n" + //
 				"</article>";
 
-		Diagnostic d = d(2, 1, 2, 7, DTDErrorCode.EntityNotDeclared, "The entity \"nbsp\" was referenced, but not declared.");
+		Diagnostic d = d(2, 1, 2, 7, DTDErrorCode.EntityNotDeclared,
+				"The entity \"nbsp\" was referenced, but not declared.");
 		XMLAssert.testDiagnosticsFor(xml, d);
 
-		testCodeActionsFor(xml, d, ca(d, te(0, 38, 0, 38, "\r\n<!DOCTYPE article [\r\n" +
-				"\t<!ENTITY nbsp \"entity-value\">\r\n" +
-				"]>")));
+		testCodeActionsFor(xml, d, ca(d,
+				te(0, 38, 0, 38, "\r\n<!DOCTYPE article [\r\n" + "\t<!ENTITY nbsp \"entity-value\">\r\n" + "]>")));
 	}
 
 	@Test
 	public void EntityNotDeclaredWithPrologWithRootSameLine() throws Exception {
-		String xml = "<?xml version=\"1.0\" encoding=\"UTF-8\"?><text1>\n" +
-				"<text2>\n" +
-				"\t&c;\n" +
-				"</text2>\n" +
-				"</text1>";
+		String xml = "<?xml version=\"1.0\" encoding=\"UTF-8\"?><text1>\n" + "<text2>\n" + "\t&c;\n" + "</text2>\n"
+				+ "</text1>";
 
-		Diagnostic d = d(2, 1, 2, 4, DTDErrorCode.EntityNotDeclared, "The entity \"c\" was referenced, but not declared.");
+		Diagnostic d = d(2, 1, 2, 4, DTDErrorCode.EntityNotDeclared,
+				"The entity \"c\" was referenced, but not declared.");
 		XMLAssert.testDiagnosticsFor(xml, d);
 
-		testCodeActionsFor(xml, d, ca(d, te(0, 38, 0, 38, "\n<!DOCTYPE text1 [\n" +
-				"\t<!ENTITY c \"entity-value\">\n" +
-				"]>\n")));
+		testCodeActionsFor(xml, d,
+				ca(d, te(0, 38, 0, 38, "\n<!DOCTYPE text1 [\n" + "\t<!ENTITY c \"entity-value\">\n" + "]>\n")));
 	}
 
 	@Test
@@ -369,11 +386,11 @@ public void EntityNotDeclaredDoctypeNoSubset() throws Exception {
 				"	&nbsp;\n" + //
 				"</article>";
 
-		Diagnostic d = d(3, 1, 3, 7, DTDErrorCode.EntityNotDeclared, "The entity \"nbsp\" was referenced, but not declared.");
+		Diagnostic d = d(3, 1, 3, 7, DTDErrorCode.EntityNotDeclared,
+				"The entity \"nbsp\" was referenced, but not declared.");
 		XMLAssert.testDiagnosticsFor(xml, d);
 
-		testCodeActionsFor(xml, d,
-				ca(d, te(1, 18, 1, 18, "[\n\t<!ENTITY nbsp \"entity-value\">\n]")));
+		testCodeActionsFor(xml, d, ca(d, te(1, 18, 1, 18, "[\n\t<!ENTITY nbsp \"entity-value\">\n]")));
 	}
 
 	@Test
@@ -384,11 +401,11 @@ public void EntityNotDeclaredDoctypeNoSubsetNoSpace() throws Exception {
 				"	&nbsp;\n" + //
 				"</article>";
 
-		Diagnostic d = d(3, 1, 3, 7, DTDErrorCode.EntityNotDeclared, "The entity \"nbsp\" was referenced, but not declared.");
+		Diagnostic d = d(3, 1, 3, 7, DTDErrorCode.EntityNotDeclared,
+				"The entity \"nbsp\" was referenced, but not declared.");
 		XMLAssert.testDiagnosticsFor(xml, d);
 
-		testCodeActionsFor(xml, d,
-				ca(d, te(1, 17, 1, 17, " [\n\t<!ENTITY nbsp \"entity-value\">\n]")));
+		testCodeActionsFor(xml, d, ca(d, te(1, 17, 1, 17, " [\n\t<!ENTITY nbsp \"entity-value\">\n]")));
 	}
 
 	@Test
@@ -400,11 +417,11 @@ public void EntityNotDeclaredDoctypeNoSubsetEndBracketNewLine() throws Exception
 				"	&nbsp;\n" + //
 				"</article>";
 
-		Diagnostic d = d(4, 1, 4, 7, DTDErrorCode.EntityNotDeclared, "The entity \"nbsp\" was referenced, but not declared.");
+		Diagnostic d = d(4, 1, 4, 7, DTDErrorCode.EntityNotDeclared,
+				"The entity \"nbsp\" was referenced, but not declared.");
 		XMLAssert.testDiagnosticsFor(xml, d);
 
-		testCodeActionsFor(xml, d,
-				ca(d, te(2, 0, 2, 0, "[\n\t<!ENTITY nbsp \"entity-value\">\n]")));
+		testCodeActionsFor(xml, d, ca(d, te(2, 0, 2, 0, "[\n\t<!ENTITY nbsp \"entity-value\">\n]")));
 	}
 
 	@Test
@@ -415,11 +432,11 @@ public void EntityNotDeclaredDoctypeEmptySubset() throws Exception {
 				"	&nbsp;\n" + //
 				"</article>";
 
-		Diagnostic d = d(3, 1, 3, 7, DTDErrorCode.EntityNotDeclared, "The entity \"nbsp\" was referenced, but not declared.");
+		Diagnostic d = d(3, 1, 3, 7, DTDErrorCode.EntityNotDeclared,
+				"The entity \"nbsp\" was referenced, but not declared.");
 		XMLAssert.testDiagnosticsFor(xml, d);
 
-		testCodeActionsFor(xml, d,
-				ca(d, te(1, 19, 1, 19, "\n\t<!ENTITY nbsp \"entity-value\">\n")));
+		testCodeActionsFor(xml, d, ca(d, te(1, 19, 1, 19, "\n\t<!ENTITY nbsp \"entity-value\">\n")));
 	}
 
 	@Test
@@ -431,11 +448,11 @@ public void EntityNotDeclaredDoctypeEmptySubsetWithNewline() throws Exception {
 				"	&nbsp;\n" + //
 				"</article>";
 
-		Diagnostic d = d(4, 1, 4, 7, DTDErrorCode.EntityNotDeclared, "The entity \"nbsp\" was referenced, but not declared.");
+		Diagnostic d = d(4, 1, 4, 7, DTDErrorCode.EntityNotDeclared,
+				"The entity \"nbsp\" was referenced, but not declared.");
 		XMLAssert.testDiagnosticsFor(xml, d);
 
-		testCodeActionsFor(xml, d,
-				ca(d, te(2, 0, 2, 0, "\t<!ENTITY nbsp \"entity-value\">\n")));
+		testCodeActionsFor(xml, d, ca(d, te(2, 0, 2, 0, "\t<!ENTITY nbsp \"entity-value\">\n")));
 	}
 
 	@Test
@@ -451,7 +468,8 @@ public void EntityNotDeclaredSingleQuotes() throws Exception {
 		settings.getPreferences().setQuoteStyle(QuoteStyle.singleQuotes);
 		settings.getFormattingSettings().setEnforceQuoteStyle(EnforceQuoteStyle.preferred);
 		settings.getFormattingSettings().setInsertSpaces(false);
-		Diagnostic d = d(5, 1, 5, 7, DTDErrorCode.EntityNotDeclared, "The entity \"nbsp\" was referenced, but not declared.");
+		Diagnostic d = d(5, 1, 5, 7, DTDErrorCode.EntityNotDeclared,
+				"The entity \"nbsp\" was referenced, but not declared.");
 		XMLAssert.testDiagnosticsFor(xml, d);
 		testCodeActionsFor(xml, d, settings, ca(d, te(2, 29, 2, 29, "\r\n\t<!ENTITY nbsp \'entity-value\'>")));
 	}
@@ -534,7 +552,7 @@ public void testDTDNotFoundWithSYSTEM() throws Exception {
 				"   <Term>36</Term>\r\n" + //
 				"   \r\n" + //
 				"</inEQUAL_PMT>";
-		XMLAssert.testDiagnosticsFor(xml, d(1, 29, 1, 46, DTDErrorCode.dtd_not_found),// [1]
+		XMLAssert.testDiagnosticsFor(xml, d(1, 29, 1, 46, DTDErrorCode.dtd_not_found), // [1]
 				d(2, 1, 12, DTDErrorCode.MSG_ELEMENT_NOT_DECLARED), // [2]
 				d(5, 4, 12, DTDErrorCode.MSG_ELEMENT_NOT_DECLARED), // [3]
 				d(5, 23, 5, 30, XMLSyntaxErrorCode.ETagRequired)); // [4]
@@ -561,4 +579,7 @@ private static void testDiagnosticsFor(String xml, Diagnostic... expected) {
 		XMLAssert.testDiagnosticsFor(xml, "src/test/resources/catalogs/catalog.xml", expected);
 	}
 
+	private static void testPublicDiagnosticsFor(String xml, Diagnostic... expected) {
+		XMLAssert.testDiagnosticsFor(xml, "src/test/resources/catalogs/catalog-public.xml", expected);
+	}
 }
diff --git a/org.eclipse.lemminx/src/test/resources/catalogs/catalog-public.xml b/org.eclipse.lemminx/src/test/resources/catalogs/catalog-public.xml
@@ -0,0 +1,7 @@
+<?xml version="1.0"?>
+<catalog xmlns="urn:oasis:names:tc:entity:xmlns:xml:catalog">
+
+	<public publicId="-//Sun Microsystems, Inc.//DTD Web Application 2.3//EN"
+		uri="../dtd/web-app_2_3.dtd" />
+
+</catalog>