From: Robert Varga Date: Fri, 20 Jan 2017 10:51:18 +0000 (+0100) Subject: BUG-7506: use common DocumentBuilderFactory X-Git-Tag: release/carbon~317 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=1071bff328641dadd4e44d8c0571a069f8747da0 BUG-7506: use common DocumentBuilderFactory Yangtools exports UntrustedXML utility class, which contains pre-configured document builder factories for dealing with XMLs which are not completely trusted. Reuse that instead of rolling our own, especially in the XML parse path. Change-Id: I83d0ea60104f2266669493548e2d40b5ab6e4772 Signed-off-by: Robert Varga --- diff --git a/opendaylight/blueprint/src/main/java/org/opendaylight/controller/blueprint/ext/DataStoreAppConfigMetadata.java b/opendaylight/blueprint/src/main/java/org/opendaylight/controller/blueprint/ext/DataStoreAppConfigMetadata.java index b479d39ffe..fcbe1dea67 100644 --- a/opendaylight/blueprint/src/main/java/org/opendaylight/controller/blueprint/ext/DataStoreAppConfigMetadata.java +++ b/opendaylight/blueprint/src/main/java/org/opendaylight/controller/blueprint/ext/DataStoreAppConfigMetadata.java @@ -24,8 +24,6 @@ import java.util.Objects; import java.util.concurrent.atomic.AtomicBoolean; import javax.annotation.Nonnull; import javax.annotation.Nullable; -import javax.xml.parsers.DocumentBuilderFactory; -import javax.xml.parsers.ParserConfigurationException; import org.apache.aries.blueprint.services.ExtendedBlueprintContainer; import org.opendaylight.controller.md.sal.binding.api.ClusteredDataTreeChangeListener; import org.opendaylight.controller.md.sal.binding.api.DataBroker; @@ -39,6 +37,7 @@ import org.opendaylight.controller.md.sal.common.api.data.ReadFailedException; import org.opendaylight.controller.sal.core.api.model.SchemaService; import org.opendaylight.mdsal.binding.dom.codec.api.BindingNormalizedNodeSerializer; import org.opendaylight.yangtools.concepts.ListenerRegistration; +import org.opendaylight.yangtools.util.xml.UntrustedXML; import org.opendaylight.yangtools.yang.binding.DataObject; import org.opendaylight.yangtools.yang.binding.Identifiable; import org.opendaylight.yangtools.yang.binding.Identifier; @@ -81,17 +80,6 @@ public class DataStoreAppConfigMetadata extends AbstractDependentComponentFactor private static final String DEFAULT_APP_CONFIG_FILE_PATH = "etc" + File.separator + "opendaylight" + File.separator + "datastore" + File.separator + "initial" + File.separator + "config"; - private static final DocumentBuilderFactory DOC_BUILDER_FACTORY; - - static { - DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); - factory.setNamespaceAware(true); - factory.setCoalescing(true); - factory.setIgnoringElementContentWhitespace(true); - factory.setIgnoringComments(true); - DOC_BUILDER_FACTORY = factory; - } - private final Element defaultAppConfigElement; private final String defaultAppConfigFileName; private final String appConfigBindingClassName; @@ -108,9 +96,9 @@ public class DataStoreAppConfigMetadata extends AbstractDependentComponentFactor // project are still used - conversion to the mdsal binding classes hasn't occurred yet. private volatile BindingNormalizedNodeSerializer bindingSerializer; - public DataStoreAppConfigMetadata(@Nonnull String id, @Nonnull String appConfigBindingClassName, - @Nullable String appConfigListKeyValue, @Nullable String defaultAppConfigFileName, - @Nonnull UpdateStrategy updateStrategyValue, @Nullable Element defaultAppConfigElement) { + public DataStoreAppConfigMetadata(@Nonnull final String id, @Nonnull final String appConfigBindingClassName, + @Nullable final String appConfigListKeyValue, @Nullable final String defaultAppConfigFileName, + @Nonnull final UpdateStrategy updateStrategyValue, @Nullable final Element defaultAppConfigElement) { super(id); this.defaultAppConfigElement = defaultAppConfigElement; this.defaultAppConfigFileName = defaultAppConfigFileName; @@ -121,7 +109,7 @@ public class DataStoreAppConfigMetadata extends AbstractDependentComponentFactor @Override @SuppressWarnings("unchecked") - public void init(ExtendedBlueprintContainer container) { + public void init(final ExtendedBlueprintContainer container) { super.init(container); Class appConfigBindingClass; @@ -189,7 +177,7 @@ public class DataStoreAppConfigMetadata extends AbstractDependentComponentFactor retrieveService("data-broker", DataBroker.class, service -> retrieveInitialAppConfig((DataBroker)service)); } - private void retrieveInitialAppConfig(DataBroker dataBroker) { + private void retrieveInitialAppConfig(final DataBroker dataBroker) { LOG.debug("{}: Got DataBroker instance - reading app config {}", logName(), bindingContext.appConfigPath); setDependendencyDesc("Initial app config " + bindingContext.appConfigBindingClass.getSimpleName()); @@ -214,7 +202,7 @@ public class DataStoreAppConfigMetadata extends AbstractDependentComponentFactor LogicalDatastoreType.CONFIGURATION, bindingContext.appConfigPath); Futures.addCallback(future, new FutureCallback>() { @Override - public void onSuccess(Optional possibleAppConfig) { + public void onSuccess(final Optional possibleAppConfig) { LOG.debug("{}: Read of app config {} succeeded: {}", logName(), bindingContext .appConfigBindingClass.getName(), possibleAppConfig); @@ -223,7 +211,7 @@ public class DataStoreAppConfigMetadata extends AbstractDependentComponentFactor } @Override - public void onFailure(Throwable failure) { + public void onFailure(final Throwable failure) { readOnlyTx.close(); // We may have gotten the app config via the data tree change listener so only retry if not. @@ -237,7 +225,7 @@ public class DataStoreAppConfigMetadata extends AbstractDependentComponentFactor }); } - private void onAppConfigChanged(Collection> changes) { + private void onAppConfigChanged(final Collection> changes) { for (DataTreeModification change: changes) { DataObjectModification changeRoot = change.getRootNode(); ModificationType type = changeRoot.getModificationType(); @@ -267,7 +255,7 @@ public class DataStoreAppConfigMetadata extends AbstractDependentComponentFactor } } - private boolean setInitialAppConfig(Optional possibleAppConfig) { + private boolean setInitialAppConfig(final Optional possibleAppConfig) { boolean result = readingInitialAppConfig.compareAndSet(true, false); if (result) { DataObject localAppConfig; @@ -345,8 +333,8 @@ public class DataStoreAppConfigMetadata extends AbstractDependentComponentFactor return appConfig; } - private NormalizedNode parsePossibleDefaultAppConfigXMLFile(SchemaContext schemaContext, - DataSchemaNode dataSchema) { + private NormalizedNode parsePossibleDefaultAppConfigXMLFile(final SchemaContext schemaContext, + final DataSchemaNode dataSchema) { String appConfigFileName = defaultAppConfigFileName; if (Strings.isNullOrEmpty(appConfigFileName)) { @@ -373,14 +361,14 @@ public class DataStoreAppConfigMetadata extends AbstractDependentComponentFactor XmlUtils.DEFAULT_XML_CODEC_PROVIDER, schemaContext); try (FileInputStream fis = new FileInputStream(appConfigFile)) { - Document root = DOC_BUILDER_FACTORY.newDocumentBuilder().parse(fis); + Document root = UntrustedXML.newDocumentBuilder().parse(fis); NormalizedNode dataNode = bindingContext.parseDataElement(root.getDocumentElement(), dataSchema, parserFactory); LOG.debug("{}: Parsed data node: {}", logName(), dataNode); return dataNode; - } catch (SAXException | IOException | ParserConfigurationException e) { + } catch (SAXException | IOException e) { String msg = String.format("%s: Could not read/parse app config file %s", logName(), appConfigFile); LOG.error(msg, e); setFailureMessage(msg); @@ -389,7 +377,7 @@ public class DataStoreAppConfigMetadata extends AbstractDependentComponentFactor return null; } - private String findYangModuleName(QName qname, SchemaContext schemaContext) { + private String findYangModuleName(final QName qname, final SchemaContext schemaContext) { for (Module m : schemaContext.getModules()) { if (qname.getModule().equals(m.getQNameModule())) { return m.getName(); @@ -401,8 +389,8 @@ public class DataStoreAppConfigMetadata extends AbstractDependentComponentFactor } @Nullable - private NormalizedNode parsePossibleDefaultAppConfigElement(SchemaContext schemaContext, - DataSchemaNode dataSchema) { + private NormalizedNode parsePossibleDefaultAppConfigElement(final SchemaContext schemaContext, + final DataSchemaNode dataSchema) { if (defaultAppConfigElement == null) { return null; } @@ -425,7 +413,7 @@ public class DataStoreAppConfigMetadata extends AbstractDependentComponentFactor @Override - public void destroy(Object instance) { + public void destroy(final Object instance) { super.destroy(instance); if (appConfigChangeListenerReg != null) { @@ -443,8 +431,8 @@ public class DataStoreAppConfigMetadata extends AbstractDependentComponentFactor final Class schemaType; final QName bindingQName; - protected BindingContext(Class appConfigBindingClass, InstanceIdentifier appConfigPath, - Class schemaType) { + protected BindingContext(final Class appConfigBindingClass, + final InstanceIdentifier appConfigPath, final Class schemaType) { this.appConfigBindingClass = appConfigBindingClass; this.appConfigPath = appConfigPath; this.schemaType = schemaType; @@ -462,18 +450,18 @@ public class DataStoreAppConfigMetadata extends AbstractDependentComponentFactor * BindingContext implementation for a container binding. */ private static class ContainerBindingContext extends BindingContext { - ContainerBindingContext(Class appConfigBindingClass) { + ContainerBindingContext(final Class appConfigBindingClass) { super(appConfigBindingClass, InstanceIdentifier.create(appConfigBindingClass), ContainerSchemaNode.class); } @Override - NormalizedNode newDefaultNode(DataSchemaNode dataSchema) { + NormalizedNode newDefaultNode(final DataSchemaNode dataSchema) { return ImmutableNodes.containerNode(bindingQName); } @Override - NormalizedNode parseDataElement(Element element, DataSchemaNode dataSchema, - DomToNormalizedNodeParserFactory parserFactory) { + NormalizedNode parseDataElement(final Element element, final DataSchemaNode dataSchema, + final DomToNormalizedNodeParserFactory parserFactory) { return parserFactory.getContainerNodeParser().parse(Collections.singletonList(element), (ContainerSchemaNode)dataSchema); } @@ -485,14 +473,14 @@ public class DataStoreAppConfigMetadata extends AbstractDependentComponentFactor private static class ListBindingContext extends BindingContext { final String appConfigListKeyValue; - ListBindingContext(Class appConfigBindingClass, InstanceIdentifier appConfigPath, - String appConfigListKeyValue) { + ListBindingContext(final Class appConfigBindingClass, + final InstanceIdentifier appConfigPath, final String appConfigListKeyValue) { super(appConfigBindingClass, appConfigPath, ListSchemaNode.class); this.appConfigListKeyValue = appConfigListKeyValue; } @SuppressWarnings({ "rawtypes", "unchecked" }) - private static ListBindingContext newInstance(Class bindingClass, String listKeyValue) + private static ListBindingContext newInstance(final Class bindingClass, final String listKeyValue) throws InstantiationException, IllegalAccessException, IllegalArgumentException, InvocationTargetException, NoSuchMethodException, SecurityException { // We assume the yang list key type is string. @@ -503,7 +491,7 @@ public class DataStoreAppConfigMetadata extends AbstractDependentComponentFactor } @Override - NormalizedNode newDefaultNode(DataSchemaNode dataSchema) { + NormalizedNode newDefaultNode(final DataSchemaNode dataSchema) { // We assume there's only one key for the list. List keys = ((ListSchemaNode)dataSchema).getKeyDefinition(); Preconditions.checkArgument(keys.size() == 1, "Expected only 1 key for list %s", appConfigBindingClass); @@ -512,8 +500,8 @@ public class DataStoreAppConfigMetadata extends AbstractDependentComponentFactor } @Override - NormalizedNode parseDataElement(Element element, DataSchemaNode dataSchema, - DomToNormalizedNodeParserFactory parserFactory) { + NormalizedNode parseDataElement(final Element element, final DataSchemaNode dataSchema, + final DomToNormalizedNodeParserFactory parserFactory) { return parserFactory.getMapEntryNodeParser().parse(Collections.singletonList(element), (ListSchemaNode)dataSchema); } diff --git a/opendaylight/blueprint/src/main/java/org/opendaylight/controller/blueprint/ext/OpendaylightNamespaceHandler.java b/opendaylight/blueprint/src/main/java/org/opendaylight/controller/blueprint/ext/OpendaylightNamespaceHandler.java index 05359b9e39..45f1e50017 100644 --- a/opendaylight/blueprint/src/main/java/org/opendaylight/controller/blueprint/ext/OpendaylightNamespaceHandler.java +++ b/opendaylight/blueprint/src/main/java/org/opendaylight/controller/blueprint/ext/OpendaylightNamespaceHandler.java @@ -13,8 +13,6 @@ import java.io.StringReader; import java.net.URL; import java.util.Collections; import java.util.Set; -import javax.xml.parsers.DocumentBuilderFactory; -import javax.xml.parsers.ParserConfigurationException; import org.apache.aries.blueprint.ComponentDefinitionRegistry; import org.apache.aries.blueprint.NamespaceHandler; import org.apache.aries.blueprint.ParserContext; @@ -28,6 +26,7 @@ import org.apache.aries.blueprint.mutable.MutableValueMetadata; import org.opendaylight.controller.blueprint.BlueprintContainerRestartService; import org.opendaylight.controller.md.sal.binding.api.NotificationService; import org.opendaylight.controller.sal.binding.api.RpcProviderRegistry; +import org.opendaylight.yangtools.util.xml.UntrustedXML; import org.osgi.service.blueprint.container.ComponentDefinitionException; import org.osgi.service.blueprint.reflect.BeanMetadata; import org.osgi.service.blueprint.reflect.ComponentMetadata; @@ -392,16 +391,9 @@ public final class OpendaylightNamespaceHandler implements NamespaceHandler { } private static Element parseXML(final String name, final String xml) { - DocumentBuilderFactory builderFactory = DocumentBuilderFactory.newInstance(); - builderFactory.setNamespaceAware(true); - builderFactory.setCoalescing(true); - builderFactory.setIgnoringElementContentWhitespace(true); - builderFactory.setIgnoringComments(true); - try { - return builderFactory.newDocumentBuilder().parse(new InputSource( - new StringReader(xml))).getDocumentElement(); - } catch (SAXException | IOException | ParserConfigurationException e) { + return UntrustedXML.newDocumentBuilder().parse(new InputSource(new StringReader(xml))).getDocumentElement(); + } catch (SAXException | IOException e) { throw new ComponentDefinitionException(String.format("Error %s parsing XML: %s", name, xml), e); } }