diff options
author | Harald Musum <musum@yahooinc.com> | 2022-04-20 08:23:39 +0200 |
---|---|---|
committer | Harald Musum <musum@yahooinc.com> | 2022-04-20 08:23:39 +0200 |
commit | 3889010d96f7b844b182e7fccffdf7cbb07a85b2 (patch) | |
tree | 6049a31e5e4bdddb6bf2f7ecfb24c37a6478efe9 /config-model/src/main/java | |
parent | 2dcb262db7e0087cb7f2ea455c732fd1ea84dd6b (diff) |
Add error handler for XML parsing
Will output name of source if there is a warning or error
Diffstat (limited to 'config-model/src/main/java')
6 files changed, 74 insertions, 49 deletions
diff --git a/config-model/src/main/java/com/yahoo/config/model/ConfigModelRepo.java b/config-model/src/main/java/com/yahoo/config/model/ConfigModelRepo.java index 0e0f992952a..8985dac91aa 100644 --- a/config-model/src/main/java/com/yahoo/config/model/ConfigModelRepo.java +++ b/config-model/src/main/java/com/yahoo/config/model/ConfigModelRepo.java @@ -4,27 +4,16 @@ package com.yahoo.config.model; import com.yahoo.config.application.api.ApplicationFile; import com.yahoo.config.application.api.ApplicationPackage; import com.yahoo.config.model.ConfigModelContext.ApplicationType; -import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.model.builder.xml.ConfigModelBuilder; import com.yahoo.config.model.builder.xml.ConfigModelId; import com.yahoo.config.model.builder.xml.XmlHelper; +import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.model.graph.ModelGraphBuilder; import com.yahoo.config.model.graph.ModelNode; +import com.yahoo.config.model.producer.AbstractConfigProducer; import com.yahoo.config.model.provision.HostsXmlProvisioner; - -import java.util.ArrayList; -import java.util.Collection; -import java.util.Collections; -import java.util.Iterator; -import java.util.LinkedHashMap; -import java.util.List; -import java.util.Map; -import java.util.Optional; -import java.util.TreeMap; -import java.util.logging.Level; import com.yahoo.path.Path; import com.yahoo.text.XML; -import com.yahoo.config.model.producer.AbstractConfigProducer; import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.builder.VespaModelBuilder; import com.yahoo.vespa.model.clients.Clients; @@ -32,13 +21,22 @@ import com.yahoo.vespa.model.content.Content; import com.yahoo.vespa.model.routing.Routing; import org.w3c.dom.Document; import org.w3c.dom.Element; -import org.xml.sax.InputSource; import org.xml.sax.SAXException; import java.io.IOException; import java.io.Reader; import java.io.Serializable; import java.io.StringReader; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.Iterator; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.TreeMap; +import java.util.logging.Level; import java.util.logging.Logger; /** @@ -91,7 +89,7 @@ public class ConfigModelRepo implements ConfigModelRepoAdder, Serializable, Iter builder.postProc(deployState.getDeployLogger(), root, this); } - private Element getServicesFromApp(ApplicationPackage applicationPackage) throws IOException, SAXException { + private Element getServicesFromApp(ApplicationPackage applicationPackage) throws IOException { try (Reader servicesFile = applicationPackage.getServices()) { return getServicesFromReader(servicesFile); } @@ -188,8 +186,8 @@ public class ConfigModelRepo implements ConfigModelRepoAdder, Serializable, Iter return permanentServices; } - private Element getServicesFromReader(Reader reader) throws IOException, SAXException { - Document doc = XmlHelper.getDocumentBuilder().parse(new InputSource(reader)); + private Element getServicesFromReader(Reader reader) { + Document doc = XmlHelper.getDocument(reader); return doc.getDocumentElement(); } @@ -274,10 +272,10 @@ public class ConfigModelRepo implements ConfigModelRepoAdder, Serializable, Iter } // TODO: Doctoring on the XML is the wrong level for this. We should be able to mark a model as default instead -Jon - private static Element getImplicitAdmin(DeployState deployState) throws IOException, SAXException { + private static Element getImplicitAdmin(DeployState deployState) { String defaultAdminElement = deployState.isHosted() ? getImplicitAdminV4() : getImplicitAdminV2(); log.log(Level.FINE, () -> "No <admin> defined, using " + defaultAdminElement); - return XmlHelper.getDocumentBuilder().parse(new InputSource(new StringReader(defaultAdminElement))).getDocumentElement(); + return XmlHelper.getDocument(new StringReader(defaultAdminElement)).getDocumentElement(); } private static String getImplicitAdminV2() { diff --git a/config-model/src/main/java/com/yahoo/config/model/builder/xml/XmlHelper.java b/config-model/src/main/java/com/yahoo/config/model/builder/xml/XmlHelper.java index 5763be301a6..220d6bbb768 100644 --- a/config-model/src/main/java/com/yahoo/config/model/builder/xml/XmlHelper.java +++ b/config-model/src/main/java/com/yahoo/config/model/builder/xml/XmlHelper.java @@ -3,12 +3,14 @@ package com.yahoo.config.model.builder.xml; import com.yahoo.component.ComponentId; import com.yahoo.component.ComponentSpecification; -import java.util.logging.Level; import com.yahoo.text.XML; +import com.yahoo.yolean.Exceptions; import org.w3c.dom.Document; import org.w3c.dom.Element; +import org.xml.sax.ErrorHandler; import org.xml.sax.InputSource; import org.xml.sax.SAXException; +import org.xml.sax.SAXParseException; import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; @@ -19,6 +21,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.Optional; +import java.util.logging.Level; import java.util.logging.Logger; @@ -74,9 +77,15 @@ public final class XmlHelper { } public static Document getDocument(Reader reader) { + return getDocument(reader, "unknown source"); + } + + public static Document getDocument(Reader reader, String source) { Document doc; try { - doc = getDocumentBuilder().parse(new InputSource(reader)); + InputSource inputSource = new InputSource(reader); + inputSource.setPublicId(source); + doc = getDocumentBuilder().parse(inputSource); } catch (SAXException | IOException e) { throw new IllegalArgumentException(e); } @@ -121,6 +130,7 @@ public final class XmlHelper { public static synchronized DocumentBuilder getDocumentBuilder() { try { DocumentBuilder docBuilder = factory.newDocumentBuilder(); + docBuilder.setErrorHandler(new CustomErrorHandler(log)); log.log(Level.FINE, "XML parser now operational!"); return docBuilder; } catch (ParserConfigurationException e) { @@ -144,4 +154,34 @@ public final class XmlHelper { if (child.getFirstChild() == null) return Optional.empty(); return Optional.ofNullable(child.getFirstChild().getNodeValue()); } + + /** Error handler which will output name of source for warnings and errors */ + private static class CustomErrorHandler implements ErrorHandler { + + private final Logger logger; + + CustomErrorHandler(Logger logger) { + super(); + this.logger = logger; + } + + public void warning(SAXParseException e) { + logger.log(Level.WARNING, message(e)); + } + + public void error(SAXParseException e) { + throw new IllegalArgumentException(message(e)); + } + + public void fatalError(SAXParseException e) { throw new IllegalArgumentException(message(e)); } + + private String message(SAXParseException e) { + String sourceId = e.getPublicId() == null ? "" : e.getPublicId(); + return "Invalid XML" + (sourceId.isEmpty() ? " (unknown source)" : " in " + sourceId) + + ": " + Exceptions.toMessageString(e) + + " [" + e.getLineNumber() + ":" + e.getColumnNumber() + "]"; + } + + } + } diff --git a/config-model/src/main/java/com/yahoo/config/model/provision/Hosts.java b/config-model/src/main/java/com/yahoo/config/model/provision/Hosts.java index c9a03dad65e..2ef8cb4a0bf 100644 --- a/config-model/src/main/java/com/yahoo/config/model/provision/Hosts.java +++ b/config-model/src/main/java/com/yahoo/config/model/provision/Hosts.java @@ -8,12 +8,13 @@ import com.yahoo.text.XML; import com.yahoo.vespa.model.builder.xml.dom.VespaDomBuilder; import org.w3c.dom.Document; import org.w3c.dom.Element; -import org.xml.sax.InputSource; -import org.xml.sax.SAXException; -import java.io.IOException; import java.io.Reader; -import java.util.*; +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashSet; +import java.util.List; +import java.util.Set; import java.util.logging.Logger; /** @@ -63,12 +64,7 @@ public class Hosts { */ public static Hosts readFrom(Reader hostsFile) { List<Host> hosts = new ArrayList<>(); - Document doc; - try { - doc = XmlHelper.getDocumentBuilder().parse(new InputSource(hostsFile)); - } catch (SAXException | IOException e) { - throw new IllegalArgumentException(e); - } + Document doc = XmlHelper.getDocument(hostsFile); for (Element hostE : XML.getChildren(doc.getDocumentElement(), "host")) { String name = hostE.getAttribute("name"); if (name.equals("")) { diff --git a/config-model/src/main/java/com/yahoo/config/model/test/MockRoot.java b/config-model/src/main/java/com/yahoo/config/model/test/MockRoot.java index b29ff1ee58b..7e2953e6606 100644 --- a/config-model/src/main/java/com/yahoo/config/model/test/MockRoot.java +++ b/config-model/src/main/java/com/yahoo/config/model/test/MockRoot.java @@ -19,10 +19,7 @@ import com.yahoo.vespa.model.builder.xml.dom.DomAdminV2Builder; import com.yahoo.vespa.model.filedistribution.FileDistributionConfigProducer; import com.yahoo.vespa.model.filedistribution.FileReferencesRepository; import org.w3c.dom.Document; -import org.xml.sax.InputSource; -import org.xml.sax.SAXException; -import java.io.IOException; import java.io.StringReader; import java.util.ArrayList; import java.util.Collections; @@ -143,13 +140,9 @@ public class MockRoot extends AbstractConfigProducerRoot { "<?xml version='1.0' encoding='utf-8' ?>" + "<services>" + xml + "</services>"; - try { - Document doc = XmlHelper.getDocumentBuilder().parse(new InputSource(new StringReader(servicesXml))); - setAdmin(new DomAdminV2Builder(ConfigModelContext.ApplicationType.DEFAULT, false, new ArrayList<>()). - build(deployState, this, XML.getChildren(doc.getDocumentElement(), "admin").get(0))); - } catch (SAXException | IOException e) { - throw new RuntimeException(e); - } + Document doc = XmlHelper.getDocument(new StringReader(servicesXml)); + setAdmin(new DomAdminV2Builder(ConfigModelContext.ApplicationType.DEFAULT, false, new ArrayList<>()) + .build(deployState, this, XML.getChildren(doc.getDocumentElement(), "admin").get(0))); } public final void setAdmin(Admin admin) { diff --git a/config-model/src/main/java/com/yahoo/config/model/test/TestUtil.java b/config-model/src/main/java/com/yahoo/config/model/test/TestUtil.java index df916907472..c05d7bf4942 100644 --- a/config-model/src/main/java/com/yahoo/config/model/test/TestUtil.java +++ b/config-model/src/main/java/com/yahoo/config/model/test/TestUtil.java @@ -25,9 +25,7 @@ public class TestUtil { lines.addAll(Arrays.asList(xmlLines)); try { - return XmlHelper.getDocumentBuilder().parse( - inputSource((CollectionUtil.mkString(lines, "\n").replace("'", "\"")))) - .getDocumentElement(); + return XmlHelper.getDocument(new StringReader(CollectionUtil.mkString(lines, "\n").replace("'", "\""))).getDocumentElement(); } catch (Exception e) { throw new RuntimeException(e); } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/VespaDomBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/VespaDomBuilder.java index df77d83da83..b619210155f 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/VespaDomBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/VespaDomBuilder.java @@ -4,11 +4,10 @@ package com.yahoo.vespa.model.builder.xml.dom; import com.yahoo.config.application.api.DeployLogger; import com.yahoo.config.model.ApplicationConfigProducerRoot; import com.yahoo.config.model.ConfigModelRepo; -import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.model.builder.xml.XmlHelper; +import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.model.producer.AbstractConfigProducer; import com.yahoo.config.model.producer.UserConfigRepo; -import java.util.logging.Level; import com.yahoo.text.XML; import com.yahoo.vespa.model.AbstractService; import com.yahoo.vespa.model.Affinity; @@ -24,7 +23,6 @@ import com.yahoo.vespa.model.content.Content; import com.yahoo.vespa.model.generic.builder.DomServiceClusterBuilder; import com.yahoo.vespa.model.generic.service.ServiceCluster; import com.yahoo.vespa.model.search.SearchCluster; - import org.w3c.dom.Document; import org.w3c.dom.Element; import org.w3c.dom.Node; @@ -33,6 +31,7 @@ import org.w3c.dom.NodeList; import java.util.ArrayList; import java.util.LinkedList; import java.util.List; +import java.util.logging.Level; import java.util.logging.Logger; /** @@ -94,7 +93,8 @@ public class VespaDomBuilder extends VespaModelBuilder { public ApplicationConfigProducerRoot getRoot(String name, DeployState deployState, AbstractConfigProducer parent) { try { return new DomRootBuilder(name). - build(deployState, parent, XmlHelper.getDocument(deployState.getApplicationPackage().getServices()).getDocumentElement()); + build(deployState, parent, XmlHelper.getDocument(deployState.getApplicationPackage().getServices(), "services.xml") + .getDocumentElement()); } catch (Exception e) { throw new IllegalArgumentException(e); } @@ -303,7 +303,7 @@ public class VespaDomBuilder extends VespaModelBuilder { @Override public List<ServiceCluster> getClusters(DeployState deployState, AbstractConfigProducer parent) { List<ServiceCluster> clusters = new ArrayList<>(); - Document services = XmlHelper.getDocument(deployState.getApplicationPackage().getServices()); + Document services = XmlHelper.getDocument(deployState.getApplicationPackage().getServices(), "services.xml"); for (Element clusterSpec : XML.getChildren(services.getDocumentElement(), "cluster")) { DomServiceClusterBuilder clusterBuilder = new DomServiceClusterBuilder(clusterSpec.getAttribute("name")); clusters.add(clusterBuilder.build(deployState, parent.getRoot(), clusterSpec)); |