Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Separate instructions for testing well formed XML files and validation against DTD #1602

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -338,8 +338,9 @@ public Collection<DOMDocument> getAllDocuments() {
}

@Override
public void validate(DOMDocument document, Map<String, Object> validationArgs) {
xmlTextDocumentService.validate(document, validationArgs);
public void validate(DOMDocument document, Map<String, Object> validationArgs,
XMLValidationRootSettings validationSettings) {
xmlTextDocumentService.validate(document, validationArgs, validationSettings);
}

public XMLCapabilityManager getCapabilityManager() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ public XMLTextDocumentService(XMLLanguageServer xmlLanguageServer) {
this.limitExceededWarner = null;
this.xmlValidatorDelayer = new ModelValidatorDelayer<DOMDocument>((document) -> {
DOMDocument xmlDocument = document.getModel();
validate(xmlDocument, Collections.emptyMap());
validate(xmlDocument, Collections.emptyMap(), null);

getXMLLanguageService().getDocumentLifecycleParticipants().forEach(participant -> {
try {
Expand Down Expand Up @@ -653,7 +653,7 @@ private void triggerValidationFor(Collection<ModelTextDocument<DOMDocument>> doc
xmlLanguageServer.schedule(() -> {
documents.forEach(document -> {
try {
validate(document.getModel(), Collections.emptyMap());
validate(document.getModel(), Collections.emptyMap(), null);
} catch (CancellationException e) {
// Ignore the error and continue to validate other documents
}
Expand Down Expand Up @@ -692,7 +692,7 @@ void validate(TextDocument document, boolean withDelay) throws CancellationExcep
} else {
CompletableFuture.runAsync(() -> {
DOMDocument xmlDocument = ((ModelTextDocument<DOMDocument>) document).getModel();
validate(xmlDocument, Collections.emptyMap());
validate(xmlDocument, Collections.emptyMap(), null);
getXMLLanguageService().getDocumentLifecycleParticipants().forEach(participant -> {
try {
participant.didOpen(xmlDocument);
Expand All @@ -708,19 +708,22 @@ void validate(TextDocument document, boolean withDelay) throws CancellationExcep
/**
* Validate and publish diagnostics for the given DOM document.
*
* @param xmlDocument the DOM document.
* @param validationArgs the validation arguments.
* @param xmlDocument the DOM document.
* @param validationArgs the validation arguments.
* @param validationSettings
*
* @throws CancellationException when the DOM document content changed and
* diagnostics must be stopped.
*/
void validate(DOMDocument xmlDocument, Map<String, Object> validationArgs) throws CancellationException {
void validate(DOMDocument xmlDocument, Map<String, Object> validationArgs,
XMLValidationRootSettings validationSettings)
throws CancellationException {
CancelChecker cancelChecker = xmlDocument.getCancelChecker();
cancelChecker.checkCanceled();
getXMLLanguageService().publishDiagnostics(xmlDocument,
params -> xmlLanguageServer.getLanguageClient().publishDiagnostics(params),
(doc) -> triggerValidationFor(doc, TriggeredBy.Other),
sharedSettings.getValidationSettings(),
validationSettings != null ? validationSettings : sharedSettings.getValidationSettings(),
validationArgs, cancelChecker);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

import org.eclipse.lemminx.dom.DOMDocument;
import org.eclipse.lemminx.extensions.contentmodel.model.ContentModelManager;
import org.eclipse.lemminx.extensions.contentmodel.settings.XMLValidationRootSettings;
import org.eclipse.lemminx.services.IXMLDocumentProvider;
import org.eclipse.lemminx.services.IXMLValidationService;
import org.eclipse.lemminx.services.extensions.commands.AbstractDOMDocumentCommandHandler;
Expand Down Expand Up @@ -59,15 +60,25 @@ protected Object executeCommand(DOMDocument document, ExecuteCommandParams param
CancelChecker cancelChecker) throws Exception {
// Validation args can contains the external resource ('url' as key map) to
// force do download.
JsonObject validationArgs = params.getArguments().size() > 1 ? (JsonObject) params.getArguments().get(1) : null;
JsonObject validationArgs = getJsonObject(params, 1);
JsonObject validationSettings = getJsonObject(params, 2);
// 1. remove the referenced grammar in the XML file from the Xerces grammar pool
// (used by the Xerces validation) and the content model documents cache (used
// by the XML completion/hover based on the grammar)
contentModelManager.evictCacheFor(document);
// 2. trigger the validation for the given XML file
Map map = JSONUtility.toModel(validationArgs, Map.class);
validationService.validate(document, map);
XMLValidationRootSettings settings = JSONUtility.toModel(validationSettings, XMLValidationRootSettings.class);
validationService.validate(document, map, settings);
return null;
}

private static JsonObject getJsonObject(ExecuteCommandParams params, int index) {
if (params.getArguments().size() <= index) {
return null;
}
Object result = params.getArguments().get(index);
return result instanceof JsonObject ? (JsonObject) result : null;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,10 @@
import org.eclipse.lemminx.dom.XMLModel;
import org.eclipse.lemminx.extensions.contentmodel.model.ContentModelManager;
import org.eclipse.lemminx.extensions.contentmodel.participants.XMLSyntaxErrorCode;
import org.eclipse.lemminx.extensions.contentmodel.settings.DTDEnabled;
import org.eclipse.lemminx.extensions.contentmodel.settings.NamespacesEnabled;
import org.eclipse.lemminx.extensions.contentmodel.settings.SchemaEnabled;
import org.eclipse.lemminx.extensions.contentmodel.settings.XMLDTDSettings;
import org.eclipse.lemminx.extensions.contentmodel.settings.XMLNamespacesSettings;
import org.eclipse.lemminx.extensions.contentmodel.settings.XMLSchemaSettings;
import org.eclipse.lemminx.extensions.contentmodel.settings.XMLValidationSettings;
Expand Down Expand Up @@ -86,8 +88,10 @@ public static void doDiagnostics(DOMDocument document, XMLEntityResolver entityR
LSPXMLEntityManager entityManager = new LSPXMLEntityManager(reporterForXML, grammarPool);
try {

boolean dtdValidationEnabled = !isDisableOnlyDTDValidation(document, validationSettings);
LSPXMLParserConfiguration configuration = new LSPXMLParserConfiguration(grammarPool,
isDisableOnlyDTDValidation(document), reporterForXML, reporterForGrammar, entityManager,
!dtdValidationEnabled, reporterForXML, reporterForGrammar,
entityManager,
validationSettings);

if (entityResolver != null) {
Expand Down Expand Up @@ -119,6 +123,8 @@ && isSchemaValidationEnabled(document, validationSettings)
|| (!hasRelaxNG && document.hasExternalGrammar());
if (hasSchemaGrammar && !schemaValidationEnabled) {
hasGrammar = false;
} else if (document.hasDTD() && !dtdValidationEnabled) {
hasGrammar = false;
}
parser.setFeature("http://xml.org/sax/features/validation", hasGrammar); //$NON-NLS-1$

Expand Down Expand Up @@ -324,7 +330,7 @@ private static boolean hasExternalSchemaGrammar(DOMDocument document) {
* @param document the DOM document
* @return true is DTD validation must be disabled and false otherwise.
*/
private static boolean isDisableOnlyDTDValidation(DOMDocument document) {
private static boolean isDisableOnlyDTDValidation(DOMDocument document, XMLValidationSettings validationSettings) {
Map<String, String> externalGrammarLocation = document.getExternalGrammarLocation();
if (externalGrammarLocation != null
&& externalGrammarLocation.containsKey(IExternalGrammarLocationProvider.DOCTYPE)) {
Expand All @@ -343,12 +349,41 @@ private static boolean isDisableOnlyDTDValidation(DOMDocument document) {
}
DOMDocumentType docType = document.getDoctype();
if (docType.getKindNode() != null) {
return false;
return !isDTDValidationEnabled(document, validationSettings);
}
// Disable the DTD validation only if there are not <!ELEMENT or an <!ATTRLIST
return !docType.getChildren().stream().anyMatch(node -> node.isDTDElementDecl() || node.isDTDAttListDecl());
}

private static boolean isValidDTDLocation(DOMDocument document) {
DOMDocumentType docType = document.getDoctype();
if (docType == null) {
return false;
}
return isValidLocation(document.getDocumentURI(), docType.getPublicId());
}

private static boolean isDTDValidationEnabled(DOMDocument document, XMLValidationSettings validationSettings) {
if (validationSettings == null) {
return true;
}
DTDEnabled enabled = DTDEnabled.always;
XMLDTDSettings dtdSettings = validationSettings.getDtd();
if (dtdSettings != null && dtdSettings.getEnabled() != null) {
enabled = dtdSettings.getEnabled();
}
switch (enabled) {
case always:
return true;
case never:
return false;
case onValidDTD:
return isValidDTDLocation(document);
default:
return true;
}
}

/**
* Warn if XML document is not bound to a grammar according the settings
*
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/**
* Copyright (c) 2023 Red Hat Inc. and others.
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v2.0
* which accompanies this distribution, and is available at
* http://www.eclipse.org/legal/epl-v20.html
*
* SPDX-License-Identifier: EPL-2.0
*
* Contributors:
* Red Hat Inc. - initial API and implementation
*/
package org.eclipse.lemminx.extensions.contentmodel.settings;

public enum DTDEnabled {
always, never, onValidDTD;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/**
* Copyright (c) 2023 Red Hat Inc. and others.
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v2.0
* which accompanies this distribution, and is available at
* http://www.eclipse.org/legal/epl-v20.html
*
* SPDX-License-Identifier: EPL-2.0
*
* Contributors:
* Red Hat Inc. - initial API and implementation
*/
package org.eclipse.lemminx.extensions.contentmodel.settings;

/**
* XML DTD settings.
*
*/
public class XMLDTDSettings {

public XMLDTDSettings() {
setEnabled(DTDEnabled.always);
}

private DTDEnabled enabled;

public void setEnabled(DTDEnabled enabled) {
this.enabled = enabled;
}

@Override
public int hashCode() {
final int prime = 31;
int result = 1;
result = prime * result + ((enabled == null) ? 0 : enabled.hashCode());
return result;
}

@Override
public boolean equals(Object obj) {
if (this == obj) {
return true;
}
if (obj == null) {
return false;
}
if (getClass() != obj.getClass()) {
return false;
}
XMLDTDSettings other = (XMLDTDSettings) obj;
if (enabled != other.enabled) {
return false;
}
return true;
}

public DTDEnabled getEnabled() {
return enabled;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ public class XMLValidationSettings {
private XMLNamespacesSettings namespaces;

private XMLSchemaSettings schema;

private XMLDTDSettings dtd;

private boolean disallowDocTypeDecl;

Expand All @@ -50,6 +52,7 @@ public XMLValidationSettings() {
setResolveExternalEntities(false);
setNamespaces(new XMLNamespacesSettings());
setSchema(new XMLSchemaSettings());
setDtd(new XMLDTDSettings());
setXInclude(new XMLXIncludeSettings());
}

Expand Down Expand Up @@ -107,6 +110,24 @@ public void setSchema(XMLSchemaSettings schema) {
this.schema = schema;
}

/**
* Returns the XML DTD validation settings.
*
* @return the XML DTD validation settings.
*/
public XMLDTDSettings getDtd() {
return dtd;
}

/**
* Set the XML DTD validation settings.
*
* @param schema the XML DTD validation settings.
*/
public void setDtd(XMLDTDSettings dtd) {
this.dtd = dtd;
}

public void setNoGrammar(String noGrammar) {
this.noGrammar = noGrammar;
}
Expand Down Expand Up @@ -203,6 +224,7 @@ public XMLValidationSettings merge(XMLValidationSettings settings) {
if (settings != null) {
this.namespaces = settings.namespaces;
this.schema = settings.schema;
this.dtd = settings.dtd;
this.enabled = settings.enabled;
this.noGrammar = settings.noGrammar;
this.disallowDocTypeDecl = settings.disallowDocTypeDecl;
Expand Down Expand Up @@ -231,6 +253,7 @@ public int hashCode() {
result = prime * result + ((noGrammar == null) ? 0 : noGrammar.hashCode());
result = prime * result + (resolveExternalEntities ? 1231 : 1237);
result = prime * result + ((schema == null) ? 0 : schema.hashCode());
result = prime * result + ((dtd == null) ? 0 : dtd.hashCode());
result = prime * result + ((xInclude == null) ? 0 : xInclude.hashCode());
return result;
}
Expand Down Expand Up @@ -288,6 +311,13 @@ public boolean equals(Object obj) {
} else if (!schema.equals(other.schema)) {
return false;
}
if (dtd == null) {
if (other.dtd != null) {
return false;
}
} else if (!dtd.equals(other.dtd)) {
return false;
}
return true;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import java.util.Map;

import org.eclipse.lemminx.dom.DOMDocument;
import org.eclipse.lemminx.extensions.contentmodel.settings.XMLValidationRootSettings;

/**
* XML Document validation service available to XML LS extensions
Expand All @@ -31,15 +32,16 @@ public interface IXMLValidationService {
* @param document the XML document
* @param validationArgs validation arguments.
*/
void validate(DOMDocument document, Map<String, Object> validationArgs);
void validate(DOMDocument document, Map<String, Object> validationArgs,
XMLValidationRootSettings validationSettings);

/**
* Performs XML document validation
*
* @param document the XML document
*/
default void validate(DOMDocument document) {
validate(document, Collections.emptyMap());
validate(document, Collections.emptyMap(), null);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,18 @@
"name": "org.eclipse.lemminx.extensions.contentmodel.settings.SchemaEnabled",
"allDeclaredFields": true
},
{
"name": "org.eclipse.lemminx.extensions.contentmodel.settings.XMLDTDSettings",
"allDeclaredFields": true,
"methods": [{
"name": "<init>",
"parameterTypes": []
}]
},
{
"name": "org.eclipse.lemminx.extensions.contentmodel.settings.DTDEnabled",
"allDeclaredFields": true
},
{
"name": "org.eclipse.lemminx.settings.XMLSymbolExpressionFilter",
"allDeclaredFields": true,
Expand Down
Loading