Skip to content

Commit

Permalink
Prevent from NPE when there's a validation error
Browse files Browse the repository at this point in the history 
Fixes #927

Signed-off-by: azerr <azerr@redhat.com>
  • Loading branch information
@fbricon
azerr authored and fbricon committed Nov 10, 2020
1 parent 28685d8 commit b5c24a5
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 21 deletions.
2 changes: 1 addition & 1 deletion .../src/main/java/org/eclipse/lemminx/extensions/catalog/XMLCatalogURIResolverExtension.java
View file
Expand Up @@ -33,7 +33,7 @@ public class XMLCatalogURIResolverExtension implements URIResolverExtension {
*/
private static final String CATALOG_NAMESPACE_URI = "urn:oasis:names:tc:entity:xmlns:xml:catalog"; //$NON-NLS-1$

private static final String CATALOG_SYSTEM = "http://www.oasis-open.org/committees/entity/release/1.1/catalog.xsd";
private static final String CATALOG_SYSTEM = "https://www.oasis-open.org/committees/entity/release/1.1/catalog.xsd";

private final XMLExtensionsRegistry extensionsRegistry;

Expand Down
25 changes: 12 additions & 13 deletions ...ipse/lemminx/extensions/contentmodel/participants/diagnostics/LSPErrorReporterForXML.java
View file
Expand Up @@ -49,15 +49,18 @@ public class LSPErrorReporterForXML extends AbstractLSPErrorReporter {

private Set<ReferencedGrammarInfo> referencedGrammars;

private Map<String, ReferencedGrammarDiagnosticsInfo> referencedGrammarDiagnosticsInfoCache;
private final Map<String, ReferencedGrammarDiagnosticsInfo> referencedGrammarDiagnosticsInfoCache;

private final boolean hasRelatedInformation;

public LSPErrorReporterForXML(DOMDocument xmlDocument, List<Diagnostic> diagnostics,
ContentModelManager contentModelManager, boolean hasRelatedInformation) {
ContentModelManager contentModelManager, boolean hasRelatedInformation,
Map<String, ReferencedGrammarDiagnosticsInfo> referencedGrammarDiagnosticsInfoCache) {
super(XML_DIAGNOSTIC_SOURCE, xmlDocument, diagnostics);
this.contentModelManager = contentModelManager;
this.hasRelatedInformation = hasRelatedInformation;
this.referencedGrammarDiagnosticsInfoCache = referencedGrammarDiagnosticsInfoCache == null ? new HashMap<>()
: referencedGrammarDiagnosticsInfoCache;
}

/**
Expand All @@ -72,8 +75,8 @@ public LSPErrorReporterForXML(DOMDocument xmlDocument, List<Diagnostic> diagnost
@Override
protected Range toLSPRange(XMLLocator location, String key, Object[] arguments, String message,
DiagnosticSeverity diagnosticSeverity, boolean fatalError, DOMDocument document) {
String documentURI = location.getExpandedSystemId();
boolean errorForDocument = documentURI.endsWith(document.getDocumentURI());
String documentOrGrammarURI = location.getExpandedSystemId();
boolean errorForDocument = documentOrGrammarURI != null ? documentOrGrammarURI.endsWith(document.getDocumentURI()) : true;
// try adjust positions for XML syntax error
XMLSyntaxErrorCode syntaxCode = XMLSyntaxErrorCode.get(key);
if (syntaxCode != null) {
Expand All @@ -84,7 +87,7 @@ protected Range toLSPRange(XMLLocator location, String key, Object[] arguments,
}
} else {
fillReferencedGrammarDiagnostic(location, key, arguments, message, diagnosticSeverity, fatalError,
document.getResolverExtensionManager(), syntaxCode, null, null, documentURI);
document.getResolverExtensionManager(), syntaxCode, null, null, documentOrGrammarURI);
return NO_RANGE;
}
} else {
Expand All @@ -106,20 +109,19 @@ protected Range toLSPRange(XMLLocator location, String key, Object[] arguments,
}
} else {
fillReferencedGrammarDiagnostic(location, key, arguments, message, diagnosticSeverity,
fatalError, document.getResolverExtensionManager(), null, dtdCode, null, documentURI);
fatalError, document.getResolverExtensionManager(), null, dtdCode, null, documentOrGrammarURI);
return NO_RANGE;
}
} else {
XSDErrorCode xsdCode = XSDErrorCode.get(key);
if (xsdCode != null) {
if (xsdCode != null && !errorForDocument) {
// The error comes from the referenced XSD (with xsi:schemaLocation, xml-model,
// etc)

// Try to get the declared xsi:schemaLocation, xsi:noNamespaceLocation range
// which declares the XSD.
String grammarURI = location.getExpandedSystemId();
fillReferencedGrammarDiagnostic(location, key, arguments, message, diagnosticSeverity,
fatalError, document.getResolverExtensionManager(), null, null, xsdCode, grammarURI);
fatalError, document.getResolverExtensionManager(), null, null, xsdCode, documentOrGrammarURI);
return NO_RANGE;
}
}
Expand Down Expand Up @@ -197,9 +199,6 @@ private void fillReferencedGrammarDiagnostic(XMLLocator location, String key, Ob
*/
private ReferencedGrammarDiagnosticsInfo getReferencedGrammarDiagnosticsInfo(String grammarURI,
URIResolverExtensionManager resolverExtensionManager) {
if (referencedGrammarDiagnosticsInfoCache == null) {
referencedGrammarDiagnosticsInfoCache = new HashMap<>();
}
ReferencedGrammarDiagnosticsInfo info = referencedGrammarDiagnosticsInfoCache.get(grammarURI);
if (info == null) {
// Create diagnostic where DDT, XSD which have errors is declared
Expand Down Expand Up @@ -244,7 +243,7 @@ protected boolean isIgnoreFatalError(String key) {
}

public void endReport() {
if (referencedGrammarDiagnosticsInfoCache == null) {
if (referencedGrammarDiagnosticsInfoCache.isEmpty()) {
return;
}
// When a XML is validated by a DTD or XSD which have syntax error, the XSD, DTD
Expand Down
8 changes: 6 additions & 2 deletions ...va/org/eclipse/lemminx/extensions/contentmodel/participants/diagnostics/XMLValidator.java
View file
Expand Up @@ -14,6 +14,7 @@

import java.io.IOException;
import java.io.StringReader;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.CancellationException;
Expand Down Expand Up @@ -57,14 +58,17 @@ public static void doDiagnostics(DOMDocument document, XMLEntityResolver entityR
List<Diagnostic> diagnostics, XMLValidationSettings validationSettings,
ContentModelManager contentModelManager, CancelChecker monitor) {
XMLGrammarPool grammarPool = contentModelManager.getGrammarPool();
Map<String, ReferencedGrammarDiagnosticsInfo> referencedGrammarDiagnosticsInfoCache = new HashMap<>();
final LSPErrorReporterForXML reporterForXML = new LSPErrorReporterForXML(document, diagnostics,
contentModelManager, validationSettings != null ? validationSettings.isRelatedInformation() : false);
contentModelManager, validationSettings != null ? validationSettings.isRelatedInformation() : false,
referencedGrammarDiagnosticsInfoCache);
// When referenced grammar (XSD, DTD) have an error (ex : syntax error), the
// error must be reported.
// We create a reporter for grammar since Xerces reporter stores the XMLLocator
// for XML and Grammar.
final LSPErrorReporterForXML reporterForGrammar = new LSPErrorReporterForXML(document, diagnostics,
contentModelManager, validationSettings != null ? validationSettings.isRelatedInformation() : false);
contentModelManager, validationSettings != null ? validationSettings.isRelatedInformation() : false,
referencedGrammarDiagnosticsInfoCache);
try {
LSPXMLParserConfiguration configuration = new LSPXMLParserConfiguration(grammarPool,
isDisableOnlyDTDValidation(document), reporterForXML, reporterForGrammar, validationSettings);
Expand Down
3 changes: 2 additions & 1 deletion ...c/main/java/org/eclipse/lemminx/extensions/dtd/participants/diagnostics/DTDValidator.java
View file
Expand Up @@ -15,6 +15,7 @@
import java.io.ByteArrayInputStream;
import java.io.InputStream;
import java.nio.charset.StandardCharsets;
import java.util.HashMap;
import java.util.List;

import org.apache.xerces.impl.dtd.XMLDTDLoader;
Expand All @@ -37,7 +38,7 @@ public static void doDiagnostics(DOMDocument document, XMLEntityResolver entityR
try {
XMLDTDLoader loader = new XMLDTDLoader();
loader.setProperty("http://apache.org/xml/properties/internal/error-reporter",
new LSPErrorReporterForXML(document, diagnostics, contentModelManager, false));
new LSPErrorReporterForXML(document, diagnostics, contentModelManager, false, new HashMap<>()));

if (entityResolver != null) {
loader.setEntityResolver(entityResolver);
Expand Down
20 changes: 16 additions & 4 deletions ...x/src/test/java/org/eclipse/lemminx/extensions/contentmodel/XMLSchemaDiagnosticsTest.java
View file
Expand Up @@ -962,16 +962,20 @@ public void diagnosticRelatedInformationWithNoNamespaceSchemaLocationSyntaxProbl
String xsdFileURI = getGrammarFileURI("foo-invalid-syntax.xsd");
diagnostic.getRelatedInformation().add(new DiagnosticRelatedInformation(l(xsdFileURI, r(1, 1, 1, 54)), ""));

Diagnostic diagnosticBasedOnXSD = new Diagnostic(r(0, 1, 0, 4),
"cvc-elt.1.a: Cannot find the declaration of element 'foo'.", DiagnosticSeverity.Error, "xml",
XMLSchemaErrorCode.cvc_elt_1_a.getCode());

XMLLanguageService xmlLanguageService = new XMLLanguageService();
// First validation
XMLAssert.testDiagnosticsFor(xmlLanguageService, xml, null, null, "src/test/resources/test.xml", false,
settings, //
diagnostic);
diagnostic, diagnosticBasedOnXSD);
// Restart the validation to check the validation is working since Xerces cache
// the invalid XSD grammar
XMLAssert.testDiagnosticsFor(xmlLanguageService, xml, null, null, "src/test/resources/test.xml", false,
settings, //
diagnostic);
diagnostic, diagnosticBasedOnXSD);
}

@Test
Expand All @@ -994,16 +998,24 @@ public void diagnosticRelatedInformationWithSchemaLocationSyntaxProblem() throws
String xsdFileURI = getGrammarFileURI("foo-ns-invalid-syntax.xsd");
diagnostic.getRelatedInformation().add(new DiagnosticRelatedInformation(l(xsdFileURI, r(1, 1, 4, 29)), ""));

Diagnostic diagnosticBasedOnXSD1 = new Diagnostic(r(1, 8, 1, 20),
"TargetNamespace.1: Expecting namespace 'http://foo', but the target namespace of the schema document is 'xs:element name=\"foo\">'.",
DiagnosticSeverity.Error, "xml", XMLSchemaErrorCode.TargetNamespace_1.getCode());

Diagnostic diagnosticBasedOnXSD2 = new Diagnostic(r(0, 1, 0, 4),
"cvc-elt.1.a: Cannot find the declaration of element 'foo'.", DiagnosticSeverity.Error, "xml",
XMLSchemaErrorCode.cvc_elt_1_a.getCode());

XMLLanguageService xmlLanguageService = new XMLLanguageService();
// First validation
XMLAssert.testDiagnosticsFor(xmlLanguageService, xml, null, null, "src/test/resources/test.xml", false,
settings, //
diagnostic);
diagnostic, diagnosticBasedOnXSD1, diagnosticBasedOnXSD2);
// Restart the validation to check the validation is working since Xerces cache
// the invalid XSD grammar
XMLAssert.testDiagnosticsFor(xmlLanguageService, xml, null, null, "src/test/resources/test.xml", false,
settings, //
diagnostic);
diagnostic, diagnosticBasedOnXSD1, diagnosticBasedOnXSD2);
}

@Test
Expand Down

0 comments on commit b5c24a5

Please sign in to comment.