warn on use of deprecated xml features (#23583)

* warn on use of deprecated xml features

* ignore zip extraction issues for tests

4 files changed, 105 insertions, 9 deletions

diff --git a/config-application-package/src/main/java/com/yahoo/config/application/ValidationProcessor.java b/config-application-package/src/main/java/com/yahoo/config/application/ValidationProcessor.java
new file mode 100644
index 00000000000..5f430f70584
--- /dev/null
+++ b/config-application-package/src/main/java/com/yahoo/config/application/ValidationProcessor.java
@@ -0,0 +1,18 @@
+package com.yahoo.config.application;
+
+import org.w3c.dom.Document;
+import org.w3c.dom.NodeList;
+
+import javax.xml.transform.TransformerException;
+import java.io.IOException;
+
+public class ValidationProcessor implements PreProcessor {
+
+    @Override
+    public Document process(Document input) throws IOException, TransformerException {
+        NodeList includeitems = input.getElementsByTagNameNS("http://www.w3.org/2001/XInclude", "*");
+        if (includeitems.getLength() > 0)
+            throw new UnsupportedOperationException("XInclude not supported, use preprocess:include instead");
+        return input;
+    }
+}
\ No newline at end of file
diff --git a/config-application-package/src/main/java/com/yahoo/config/application/XmlPreProcessor.java b/config-application-package/src/main/java/com/yahoo/config/application/XmlPreProcessor.java
index 640cc7bcfa7..ba68894c9f9 100644
--- a/config-application-package/src/main/java/com/yahoo/config/application/XmlPreProcessor.java
+++ b/config-application-package/src/main/java/com/yahoo/config/application/XmlPreProcessor.java
@@ -5,6 +5,7 @@ import com.yahoo.config.application.FileSystemWrapper.FileWrapper;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.InstanceName;
 import com.yahoo.config.provision.RegionName;
+import com.yahoo.text.XML;
 import org.w3c.dom.Document;
 import org.xml.sax.InputSource;
 import org.xml.sax.SAXException;
@@ -57,7 +58,7 @@ public class XmlPreProcessor {
     }
 
     public Document run() throws ParserConfigurationException, IOException, SAXException, TransformerException {
-        DocumentBuilder docBuilder = Xml.getPreprocessDocumentBuilder();
+        DocumentBuilder docBuilder = XML.getDocumentBuilder();
         Document document = docBuilder.parse(new InputSource(xmlInput));
         return execute(document);
     }
@@ -74,6 +75,7 @@ public class XmlPreProcessor {
         chain.add(new IncludeProcessor(applicationDir));
         chain.add(new OverrideProcessor(instance, environment, region));
         chain.add(new PropertiesProcessor());
+        chain.add(new ValidationProcessor()); // must be last in chain
         return chain;
     }
 
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/AbstractBundleValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/AbstractBundleValidator.java
index 1642a8cb3cc..b732c7a7e19 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/AbstractBundleValidator.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/AbstractBundleValidator.java
@@ -9,6 +9,7 @@ import com.yahoo.config.application.api.ComponentInfo;
 import com.yahoo.config.application.api.DeployLogger;
 import com.yahoo.config.model.deploy.DeployState;
 import com.yahoo.path.Path;
+import com.yahoo.text.XML;
 import com.yahoo.vespa.model.VespaModel;
 import org.w3c.dom.Document;
 import org.w3c.dom.Element;
@@ -16,8 +17,6 @@ import org.w3c.dom.NodeList;
 import org.xml.sax.InputSource;
 import org.xml.sax.SAXException;
 
-import javax.xml.parsers.DocumentBuilderFactory;
-import javax.xml.parsers.ParserConfigurationException;
 import javax.xml.xpath.XPathConstants;
 import javax.xml.xpath.XPathExpressionException;
 import javax.xml.xpath.XPathFactory;
@@ -96,20 +95,19 @@ public abstract class AbstractBundleValidator extends Validator {
     }
 
     private static final Pattern POM_FILE_LOCATION = Pattern.compile("META-INF/maven/.+?/.+?/pom.xml");
-    private Optional<Document> getPomXmlContent(DeployLogger deployLogger, JarFile jar) {
+    public Optional<Document> getPomXmlContent(DeployLogger deployLogger, JarFile jar) {
         return jar.stream()
                 .filter(f -> POM_FILE_LOCATION.matcher(f.getName()).matches())
                 .findFirst()
                 .map(f -> {
                     try {
                         String text = new String(jar.getInputStream(f).readAllBytes());
-                        return DocumentBuilderFactory.newDefaultInstance().newDocumentBuilder()
+                        return XML.getDocumentBuilder()
                                 .parse(new InputSource(new StringReader(text)));
-                    } catch (ParserConfigurationException e) {
-                        throw new RuntimeException(e);
                     } catch (SAXException e) {
-                        deployLogger.log(Level.INFO, String.format("Unable to parse pom.xml from %s", filename(jar)));
-                        return null;
+                        String message = String.format("Unable to parse pom.xml from %s", filename(jar));
+                        deployLogger.log(Level.SEVERE, message);
+                        throw new RuntimeException(message, e);
                     } catch (IOException e) {
                         deployLogger.log(Level.INFO,
                                 String.format("Unable to read '%s' from '%s'", f.getName(), jar.getName()));
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionPreparer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionPreparer.java
index 1e073ac3458..a90c3b958d7 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionPreparer.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionPreparer.java
@@ -6,6 +6,9 @@ import com.yahoo.component.Version;
 import com.yahoo.component.Vtag;
 import com.yahoo.concurrent.UncheckedTimeoutException;
 import com.yahoo.config.FileReference;
+import com.yahoo.config.application.ValidationProcessor;
+import com.yahoo.config.application.XmlPreProcessor;
+import com.yahoo.config.application.api.ApplicationMetaData;
 import com.yahoo.config.application.api.ApplicationPackage;
 import com.yahoo.config.application.api.DeployLogger;
 import com.yahoo.config.application.api.FileRegistry;
@@ -26,6 +29,7 @@ import com.yahoo.config.provision.Zone;
 import com.yahoo.container.jdisc.secretstore.SecretStore;
 import com.yahoo.net.HostName;
 import com.yahoo.path.Path;
+import com.yahoo.text.XML;
 import com.yahoo.vespa.config.server.ConfigServerSpec;
 import com.yahoo.vespa.config.server.TimeoutBudget;
 import com.yahoo.vespa.config.server.application.ApplicationSet;
@@ -47,9 +51,14 @@ import com.yahoo.vespa.config.server.tenant.EndpointCertificateRetriever;
 import com.yahoo.vespa.config.server.tenant.TenantRepository;
 import com.yahoo.vespa.curator.Curator;
 import com.yahoo.vespa.flags.FlagSource;
+import com.yahoo.vespa.model.application.validation.BundleValidator;
+import org.xml.sax.SAXException;
 
+import javax.xml.parsers.ParserConfigurationException;
+import javax.xml.transform.TransformerException;
 import java.io.File;
 import java.io.IOException;
+import java.nio.file.Files;
 import java.security.cert.X509Certificate;
 import java.time.Instant;
 import java.util.Collection;
@@ -58,9 +67,11 @@ import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.ExecutorService;
+import java.util.jar.JarFile;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
+import java.util.zip.ZipException;
 
 import static com.yahoo.vespa.config.server.ConfigServerSpec.fromConfig;
 
@@ -238,6 +249,7 @@ public class SessionPreparer {
 
         void preprocess() {
             try {
+                validateXmlFeatures(applicationPackage, logger);
                 this.preprocessedApplicationPackage = applicationPackage.preprocess(zone, logger);
             } catch (IOException | RuntimeException e) {
                 throw new IllegalArgumentException("Error preprocessing application package for " + applicationId +
@@ -246,6 +258,72 @@ public class SessionPreparer {
             checkTimeout("preprocess");
         }
 
+        /**
+         * Warn on use of deprecated XML features
+         */
+        private void validateXmlFeatures(ApplicationPackage applicationPackage, DeployLogger logger) {
+            // TODO: Validate no use of XInclude, datatype definitions or external entities
+            //       in any xml file we parse, such as services.xml, deployment.xml, hosts.xml,
+            //       validation-overrides.xml and any pom.xml files in OSGi bundles
+            //       services.xml and hosts.xml will need to be preprocessed by our own processors first
+
+            File applicationPackageDir = applicationPackage.getFileReference(Path.fromString("."));
+            File servicesXml = applicationPackage.getFileReference(Path.fromString("services.xml"));
+            File hostsXml = applicationPackage.getFileReference(Path.fromString("hosts.xml"));
+
+            // Validate after doing our own preprocessing on these two files
+            if(servicesXml.exists()) {
+                vespaPreprocess(applicationPackageDir.getAbsoluteFile(), servicesXml, applicationPackage.getMetaData());
+            }
+            if(hostsXml.exists()) {
+                vespaPreprocess(applicationPackageDir.getAbsoluteFile(), hostsXml, applicationPackage.getMetaData());
+            }
+
+            // Validate all other XML files
+            try (var paths = Files.find(applicationPackageDir.getAbsoluteFile().toPath(), Integer.MAX_VALUE,
+                    (path, attr) -> attr.isRegularFile() && path.getFileName().toString().matches(".*\\.[Xx][Mm][Ll]"))) {
+                paths.filter(p -> !(p.equals(servicesXml.getAbsoluteFile().toPath()) || p.equals(hostsXml.getAbsoluteFile().toPath())))
+                        .forEach(xmlPath -> {
+                            try {
+                                new ValidationProcessor().process(XML.getDocument(xmlPath.toFile()));
+                            } catch (IOException | TransformerException e) {
+                                throw new RuntimeException(e);
+                            }
+                        });
+            } catch (IOException e) {
+                throw new RuntimeException(e);
+            }
+
+            // Validate pom.xml files in OSGi bundles
+            try (var paths = Files.find(applicationPackageDir.getAbsoluteFile().toPath(), Integer.MAX_VALUE,
+                    (path, attr) -> attr.isRegularFile() && path.getFileName().toString().matches(".*\\.[Jj][Aa][Rr]"))) {
+                        paths.forEach(jarPath -> {
+                            try {
+                                new BundleValidator().getPomXmlContent(logger, new JarFile(jarPath.toFile()));
+                            } catch (ZipException e) {
+                                // ignore for tests
+                            } catch (IOException e) {
+                                throw new RuntimeException(e);
+                            }
+                        });
+            } catch (IOException e) {
+                throw new RuntimeException(e);
+            }
+        }
+
+        void vespaPreprocess(File appDir, File inputXml, ApplicationMetaData metaData) {
+            try {
+                new XmlPreProcessor(appDir,
+                        inputXml,
+                        metaData.getApplicationId().instance(),
+                        zone.environment(),
+                        zone.region())
+                        .run();
+            } catch (ParserConfigurationException | IOException | SAXException | TransformerException e) {
+                throw new RuntimeException(e);
+            }
+        }
+
         AllocatedHosts buildModels(Instant now) {
             var allocatedHosts = new AllocatedHostsFromAllModels();
             this.modelResultList = preparedModelsBuilder.buildModels(applicationId, dockerImageRepository, vespaVersion,