diff --git a/apitools/org.eclipse.pde.api.tools/src/org/eclipse/pde/api/tools/internal/APIFileGenerator.java b/apitools/org.eclipse.pde.api.tools/src/org/eclipse/pde/api/tools/internal/APIFileGenerator.java index 47b15e6e18..f215781646 100644 --- a/apitools/org.eclipse.pde.api.tools/src/org/eclipse/pde/api/tools/internal/APIFileGenerator.java +++ b/apitools/org.eclipse.pde.api.tools/src/org/eclipse/pde/api/tools/internal/APIFileGenerator.java @@ -32,10 +32,8 @@ import java.util.zip.ZipEntry; import java.util.zip.ZipFile; -import javax.xml.parsers.FactoryConfigurationError; import javax.xml.parsers.ParserConfigurationException; import javax.xml.parsers.SAXParser; -import javax.xml.parsers.SAXParserFactory; import org.eclipse.core.runtime.CoreException; import org.eclipse.jdt.core.JavaCore; @@ -48,6 +46,7 @@ import org.eclipse.pde.api.tools.internal.provisional.model.IApiTypeContainer; import org.eclipse.pde.api.tools.internal.provisional.scanner.TagScanner; import org.eclipse.pde.api.tools.internal.util.Util; +import org.eclipse.pde.internal.core.util.XmlParserFactory; import org.osgi.framework.BundleException; import org.osgi.framework.Constants; import org.xml.sax.Attributes; @@ -459,15 +458,9 @@ private boolean isAPIToolsNature(File dotProjectFile) { * @return true if it contains a source extension point, false otherwise */ private boolean containsAPIToolsNature(String pluginXMLContents) { - SAXParserFactory factory = null; - try { - factory = SAXParserFactory.newInstance(); - } catch (FactoryConfigurationError e) { - return false; - } SAXParser saxParser = null; try { - saxParser = factory.newSAXParser(); + saxParser = XmlParserFactory.createSAXParserIgnoringDOCTYPE(); } catch (ParserConfigurationException | SAXException e) { // ignore } diff --git a/apitools/org.eclipse.pde.api.tools/src/org/eclipse/pde/api/tools/internal/model/BundleComponent.java b/apitools/org.eclipse.pde.api.tools/src/org/eclipse/pde/api/tools/internal/model/BundleComponent.java index b33b3ad998..feca9f1f64 100644 --- a/apitools/org.eclipse.pde.api.tools/src/org/eclipse/pde/api/tools/internal/model/BundleComponent.java +++ b/apitools/org.eclipse.pde.api.tools/src/org/eclipse/pde/api/tools/internal/model/BundleComponent.java @@ -36,10 +36,8 @@ import java.util.zip.ZipEntry; import java.util.zip.ZipFile; -import javax.xml.parsers.FactoryConfigurationError; import javax.xml.parsers.ParserConfigurationException; import javax.xml.parsers.SAXParser; -import javax.xml.parsers.SAXParserFactory; import org.eclipse.core.runtime.CoreException; import org.eclipse.core.runtime.IPath; @@ -79,6 +77,7 @@ import org.eclipse.pde.api.tools.internal.util.Util; import org.eclipse.pde.internal.core.TargetWeaver; import org.eclipse.pde.internal.core.util.ManifestUtils; +import org.eclipse.pde.internal.core.util.XmlParserFactory; import org.osgi.framework.BundleException; import org.osgi.framework.Constants; import org.osgi.framework.Version; @@ -1068,15 +1067,9 @@ private static boolean isSourceComponent(Map manifest, File loca * @return true if it contains a source extension point, false otherwise */ private static boolean containsSourceExtensionPoint(String pluginXMLContents) { - SAXParserFactory factory = null; - try { - factory = SAXParserFactory.newInstance(); - } catch (FactoryConfigurationError e) { - return false; - } SAXParser saxParser = null; try { - saxParser = factory.newSAXParser(); + saxParser = XmlParserFactory.createSAXParserWithErrorOnDOCTYPE(); } catch (ParserConfigurationException | SAXException e) { // ignore } diff --git a/apitools/org.eclipse.pde.api.tools/src/org/eclipse/pde/api/tools/internal/search/UseReportConverter.java b/apitools/org.eclipse.pde.api.tools/src/org/eclipse/pde/api/tools/internal/search/UseReportConverter.java index 0cae1b47c0..29eab3af08 100644 --- a/apitools/org.eclipse.pde.api.tools/src/org/eclipse/pde/api/tools/internal/search/UseReportConverter.java +++ b/apitools/org.eclipse.pde.api.tools/src/org/eclipse/pde/api/tools/internal/search/UseReportConverter.java @@ -38,7 +38,6 @@ import javax.xml.parsers.ParserConfigurationException; import javax.xml.parsers.SAXParser; -import javax.xml.parsers.SAXParserFactory; import javax.xml.transform.Result; import javax.xml.transform.Source; import javax.xml.transform.Transformer; @@ -66,6 +65,7 @@ import org.eclipse.pde.api.tools.internal.provisional.search.IMetadata; import org.eclipse.pde.api.tools.internal.util.Signatures; import org.eclipse.pde.api.tools.internal.util.Util; +import org.eclipse.pde.internal.core.util.XmlParserFactory; import org.eclipse.pde.internal.core.util.XmlTransformerFactory; import org.osgi.framework.Version; import org.w3c.dom.Element; @@ -741,9 +741,8 @@ protected List parse(IProgressMonitor monitor) throws Exception { */ SAXParser getParser() throws Exception { if (this.parser == null) { - SAXParserFactory factory = SAXParserFactory.newInstance(); try { - this.parser = factory.newSAXParser(); + this.parser = XmlParserFactory.createSAXParserWithErrorOnDOCTYPE(); } catch (ParserConfigurationException pce) { throw new Exception(SearchMessages.UseReportConverter_pce_error_getting_parser, pce); } catch (SAXException se) { diff --git a/apitools/org.eclipse.pde.api.tools/src/org/eclipse/pde/api/tools/internal/search/UseScanParser.java b/apitools/org.eclipse.pde.api.tools/src/org/eclipse/pde/api/tools/internal/search/UseScanParser.java index 0fc1ac9316..ec8e79e8e3 100644 --- a/apitools/org.eclipse.pde.api.tools/src/org/eclipse/pde/api/tools/internal/search/UseScanParser.java +++ b/apitools/org.eclipse.pde.api.tools/src/org/eclipse/pde/api/tools/internal/search/UseScanParser.java @@ -24,7 +24,6 @@ import javax.xml.parsers.ParserConfigurationException; import javax.xml.parsers.SAXParser; -import javax.xml.parsers.SAXParserFactory; import org.eclipse.core.runtime.IProgressMonitor; import org.eclipse.core.runtime.SubMonitor; @@ -36,6 +35,7 @@ import org.eclipse.pde.api.tools.internal.provisional.descriptors.IComponentDescriptor; import org.eclipse.pde.api.tools.internal.provisional.descriptors.IMemberDescriptor; import org.eclipse.pde.api.tools.internal.util.Util; +import org.eclipse.pde.internal.core.util.XmlParserFactory; import org.xml.sax.Attributes; import org.xml.sax.SAXException; import org.xml.sax.helpers.DefaultHandler; @@ -307,9 +307,9 @@ public void parse(String xmlLocation, IProgressMonitor monitor, UseScanVisitor u * builds */ SAXParser getParser() throws Exception { - SAXParserFactory factory = SAXParserFactory.newInstance(); try { - return factory.newSAXParser(); + return XmlParserFactory.createSAXParserWithErrorOnDOCTYPE(); + } catch (ParserConfigurationException pce) { throw new Exception(SearchMessages.UseReportConverter_pce_error_getting_parser, pce); } catch (SAXException se) { diff --git a/apitools/org.eclipse.pde.api.tools/src_ant/org/eclipse/pde/api/tools/internal/tasks/APIDeprecationReportConversionTask.java b/apitools/org.eclipse.pde.api.tools/src_ant/org/eclipse/pde/api/tools/internal/tasks/APIDeprecationReportConversionTask.java index d5ae67f2b0..b222016dda 100644 --- a/apitools/org.eclipse.pde.api.tools/src_ant/org/eclipse/pde/api/tools/internal/tasks/APIDeprecationReportConversionTask.java +++ b/apitools/org.eclipse.pde.api.tools/src_ant/org/eclipse/pde/api/tools/internal/tasks/APIDeprecationReportConversionTask.java @@ -25,7 +25,6 @@ import javax.xml.parsers.ParserConfigurationException; import javax.xml.parsers.SAXParser; -import javax.xml.parsers.SAXParserFactory; import org.apache.tools.ant.BuildException; import org.apache.tools.ant.Task; @@ -34,6 +33,7 @@ import org.eclipse.pde.api.tools.internal.IApiXmlConstants; import org.eclipse.pde.api.tools.internal.provisional.comparator.IDelta; import org.eclipse.pde.api.tools.internal.util.Util; +import org.eclipse.pde.internal.core.util.XmlParserFactory; import org.xml.sax.Attributes; import org.xml.sax.SAXException; import org.xml.sax.helpers.DefaultHandler; @@ -311,10 +311,9 @@ public void execute() throws BuildException { System.out.println("xmlFileLocation : " + this.xmlFileLocation); //$NON-NLS-1$ System.out.println("htmlFileLocation : " + this.htmlFileLocation); //$NON-NLS-1$ } - SAXParserFactory factory = SAXParserFactory.newInstance(); SAXParser parser = null; try { - parser = factory.newSAXParser(); + parser = XmlParserFactory.createSAXParserWithErrorOnDOCTYPE(); } catch (ParserConfigurationException | SAXException e) { e.printStackTrace(); } diff --git a/apitools/org.eclipse.pde.api.tools/src_ant/org/eclipse/pde/api/tools/internal/tasks/APIFreezeReportConversionTask.java b/apitools/org.eclipse.pde.api.tools/src_ant/org/eclipse/pde/api/tools/internal/tasks/APIFreezeReportConversionTask.java index ecd076ee70..6e0411113e 100644 --- a/apitools/org.eclipse.pde.api.tools/src_ant/org/eclipse/pde/api/tools/internal/tasks/APIFreezeReportConversionTask.java +++ b/apitools/org.eclipse.pde.api.tools/src_ant/org/eclipse/pde/api/tools/internal/tasks/APIFreezeReportConversionTask.java @@ -25,7 +25,6 @@ import javax.xml.parsers.ParserConfigurationException; import javax.xml.parsers.SAXParser; -import javax.xml.parsers.SAXParserFactory; import org.apache.tools.ant.BuildException; import org.apache.tools.ant.Task; @@ -34,6 +33,7 @@ import org.eclipse.pde.api.tools.internal.IApiXmlConstants; import org.eclipse.pde.api.tools.internal.provisional.comparator.IDelta; import org.eclipse.pde.api.tools.internal.util.Util; +import org.eclipse.pde.internal.core.util.XmlParserFactory; import org.xml.sax.Attributes; import org.xml.sax.SAXException; import org.xml.sax.helpers.DefaultHandler; @@ -491,10 +491,9 @@ public void execute() throws BuildException { } } } - SAXParserFactory factory = SAXParserFactory.newInstance(); SAXParser parser = null; try { - parser = factory.newSAXParser(); + parser = XmlParserFactory.createSAXParserWithErrorOnDOCTYPE(); } catch (ParserConfigurationException | SAXException e) { e.printStackTrace(); } diff --git a/apitools/org.eclipse.pde.api.tools/src_ant/org/eclipse/pde/api/tools/internal/tasks/AnalysisReportConversionTask.java b/apitools/org.eclipse.pde.api.tools/src_ant/org/eclipse/pde/api/tools/internal/tasks/AnalysisReportConversionTask.java index 2a4b416503..985e7865e0 100644 --- a/apitools/org.eclipse.pde.api.tools/src_ant/org/eclipse/pde/api/tools/internal/tasks/AnalysisReportConversionTask.java +++ b/apitools/org.eclipse.pde.api.tools/src_ant/org/eclipse/pde/api/tools/internal/tasks/AnalysisReportConversionTask.java @@ -27,7 +27,6 @@ import javax.xml.parsers.ParserConfigurationException; import javax.xml.parsers.SAXParser; -import javax.xml.parsers.SAXParserFactory; import org.apache.tools.ant.BuildException; import org.apache.tools.ant.Task; @@ -35,6 +34,7 @@ import org.eclipse.pde.api.tools.internal.IApiXmlConstants; import org.eclipse.pde.api.tools.internal.provisional.ApiPlugin; import org.eclipse.pde.api.tools.internal.util.Util; +import org.eclipse.pde.internal.core.util.XmlParserFactory; import org.xml.sax.Attributes; import org.xml.sax.SAXException; import org.xml.sax.helpers.DefaultHandler; @@ -429,10 +429,9 @@ public void execute() throws BuildException { if (!this.reportsRoot.exists() || !this.reportsRoot.isDirectory()) { throw new BuildException(NLS.bind(Messages.invalid_directory_name, this.xmlReportsLocation)); } - SAXParserFactory factory = SAXParserFactory.newInstance(); SAXParser parser = null; try { - parser = factory.newSAXParser(); + parser = XmlParserFactory.createSAXParserWithErrorOnDOCTYPE(); } catch (ParserConfigurationException | SAXException e) { e.printStackTrace(); } diff --git a/build/org.eclipse.pde.build/src_ant/org/eclipse/pde/internal/build/tasks/JNLPGenerator.java b/build/org.eclipse.pde.build/src_ant/org/eclipse/pde/internal/build/tasks/JNLPGenerator.java index af3abf2909..3fe1fb2a8b 100644 --- a/build/org.eclipse.pde.build/src_ant/org/eclipse/pde/internal/build/tasks/JNLPGenerator.java +++ b/build/org.eclipse.pde.build/src_ant/org/eclipse/pde/internal/build/tasks/JNLPGenerator.java @@ -37,7 +37,6 @@ import javax.xml.parsers.ParserConfigurationException; import javax.xml.parsers.SAXParser; -import javax.xml.parsers.SAXParserFactory; import org.xml.sax.Attributes; import org.xml.sax.InputSource; @@ -66,7 +65,6 @@ public class JNLPGenerator extends DefaultHandler { * feature.includes = extension * feature.plugin = jar */ - private final static SAXParserFactory parserFactory = SAXParserFactory.newInstance(); private PrintWriter out; private String destination; private String provider; @@ -109,8 +107,7 @@ public JNLPGenerator(String feature, String destination, String codebase, String this.locale = locale; this.generateOfflineAllowed = generateOfflineAllowed; try { - parserFactory.setNamespaceAware(true); - parser = parserFactory.newSAXParser(); + parser = XmlParserFactory.createNsAwareSAXParserWithErrorOnDOCTYPE(); } catch (ParserConfigurationException | SAXException e) { System.out.println(e); } diff --git a/build/org.eclipse.pde.build/src_ant/org/eclipse/pde/internal/build/tasks/XmlParserFactory.java b/build/org.eclipse.pde.build/src_ant/org/eclipse/pde/internal/build/tasks/XmlParserFactory.java new file mode 100644 index 0000000000..db56c5607e --- /dev/null +++ b/build/org.eclipse.pde.build/src_ant/org/eclipse/pde/internal/build/tasks/XmlParserFactory.java @@ -0,0 +1,42 @@ +/******************************************************************************* + * Copyright (c) 2023 Joerg Kubitz and others. + * + * This program and the accompanying materials + * are made available under the terms of the Eclipse Public License 2.0 + * which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + *******************************************************************************/ +package org.eclipse.pde.internal.build.tasks; + +import javax.xml.parsers.ParserConfigurationException; +import javax.xml.parsers.SAXParser; +import javax.xml.parsers.SAXParserFactory; + +import org.xml.sax.SAXException; + +public class XmlParserFactory { + private XmlParserFactory() { + // static Utility only + } + + private static final SAXParserFactory SAX_FACTORY_ERROR_ON_DOCTYPE_NS = createSAXFactoryWithErrorOnDOCTYPE(); + + private static synchronized SAXParserFactory createSAXFactoryWithErrorOnDOCTYPE() { + SAXParserFactory f = SAXParserFactory.newInstance(); + f.setNamespaceAware(true); + try { + // force org.xml.sax.SAXParseException for any DOCTYPE: + f.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); //$NON-NLS-1$ + } catch (Exception e) { + throw new RuntimeException(e); + } + return f; + } + + public static SAXParser createNsAwareSAXParserWithErrorOnDOCTYPE() throws ParserConfigurationException, SAXException { + return SAX_FACTORY_ERROR_ON_DOCTYPE_NS.newSAXParser(); + } + +} diff --git a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/AbstractModel.java b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/AbstractModel.java index 7e8f58690a..817b5ce10b 100644 --- a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/AbstractModel.java +++ b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/AbstractModel.java @@ -24,7 +24,6 @@ import javax.xml.parsers.FactoryConfigurationError; import javax.xml.parsers.ParserConfigurationException; import javax.xml.parsers.SAXParser; -import javax.xml.parsers.SAXParserFactory; import org.eclipse.core.filebuffers.FileBuffers; import org.eclipse.core.filebuffers.ITextFileBuffer; @@ -41,6 +40,7 @@ import org.eclipse.pde.core.IModelChangedEvent; import org.eclipse.pde.core.IModelChangedListener; import org.eclipse.pde.core.ModelChangedEvent; +import org.eclipse.pde.internal.core.util.XmlParserFactory; import org.xml.sax.SAXException; public abstract class AbstractModel extends PlatformObject implements IModel, IModelChangeProviderExtension, Serializable { @@ -237,7 +237,7 @@ public void throwParseErrorsException(Throwable e) throws CoreException { } protected SAXParser getSaxParser() throws ParserConfigurationException, SAXException, FactoryConfigurationError { - return SAXParserFactory.newInstance().newSAXParser(); + return XmlParserFactory.createSAXParserWithErrorOnDOCTYPE(); } @Override diff --git a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/plugin/AbstractExtensions.java b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/plugin/AbstractExtensions.java index 6f08358a6b..e044ae2b1b 100644 --- a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/plugin/AbstractExtensions.java +++ b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/plugin/AbstractExtensions.java @@ -23,7 +23,6 @@ import java.util.List; import javax.xml.parsers.ParserConfigurationException; -import javax.xml.parsers.SAXParserFactory; import org.eclipse.core.resources.IFile; import org.eclipse.core.runtime.CoreException; @@ -36,6 +35,7 @@ import org.eclipse.pde.core.plugin.ISharedPluginModel; import org.eclipse.pde.internal.core.PDECore; import org.eclipse.pde.internal.core.PDECoreMessages; +import org.eclipse.pde.internal.core.util.XmlParserFactory; import org.w3c.dom.Node; import org.xml.sax.SAXException; @@ -199,7 +199,7 @@ public String getSchemaVersion() { try { InputStream stream = new BufferedInputStream(((IFile) res).getContents(true)); PluginHandler handler = new PluginHandler(true); - SAXParserFactory.newInstance().newSAXParser().parse(stream, handler); + XmlParserFactory.createSAXParserWithErrorOnDOCTYPE().parse(stream, handler); return handler.getSchemaVersion(); } catch (CoreException | SAXException | IOException | ParserConfigurationException e) { } diff --git a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/plugin/PluginBase.java b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/plugin/PluginBase.java index de70b7354c..055099ebfd 100644 --- a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/plugin/PluginBase.java +++ b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/plugin/PluginBase.java @@ -20,7 +20,6 @@ import javax.xml.parsers.FactoryConfigurationError; import javax.xml.parsers.ParserConfigurationException; import javax.xml.parsers.SAXParser; -import javax.xml.parsers.SAXParserFactory; import org.eclipse.core.runtime.CoreException; import org.eclipse.osgi.service.resolver.BundleDescription; @@ -36,6 +35,7 @@ import org.eclipse.pde.internal.core.PDECoreMessages; import org.eclipse.pde.internal.core.PDEState; import org.eclipse.pde.internal.core.bundle.BundlePluginBase; +import org.eclipse.pde.internal.core.util.XmlParserFactory; import org.osgi.framework.Version; import org.w3c.dom.Node; import org.w3c.dom.NodeList; @@ -404,7 +404,8 @@ protected boolean hasRequiredAttributes() { } protected SAXParser getSaxParser() throws ParserConfigurationException, SAXException, FactoryConfigurationError { - return SAXParserFactory.newInstance().newSAXParser(); + return XmlParserFactory.createSAXParserWithErrorOnDOCTYPE(); + } public static int getMatchRule(VersionRange versionRange) { @@ -447,6 +448,7 @@ public String getBundleSourceEntry() { return fBundleSourceEntry; } + @Override public boolean exportsExternalAnnotations() { return fExportsExternalAnnotations; } diff --git a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/update/configurator/ConfigurationParser.java b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/update/configurator/ConfigurationParser.java index ede90d4784..986aa6cca9 100644 --- a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/update/configurator/ConfigurationParser.java +++ b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/update/configurator/ConfigurationParser.java @@ -26,10 +26,10 @@ import javax.xml.parsers.ParserConfigurationException; import javax.xml.parsers.SAXParser; -import javax.xml.parsers.SAXParserFactory; import org.eclipse.core.runtime.URIUtil; import org.eclipse.osgi.util.NLS; +import org.eclipse.pde.internal.core.util.XmlParserFactory; import org.xml.sax.Attributes; import org.xml.sax.InputSource; import org.xml.sax.SAXException; @@ -43,7 +43,6 @@ class ConfigurationParser extends DefaultHandler implements IConfigurationConsta private static final String URL_PROPERTY = "org.eclipse.update.resolution_url"; //$NON-NLS-1$ private static final String EMPTY_STRING = ""; //$NON-NLS-1$ - private final static SAXParserFactory parserFactory = SAXParserFactory.newInstance(); private SAXParser parser; private URL currentSiteURL; @@ -58,8 +57,7 @@ class ConfigurationParser extends DefaultHandler implements IConfigurationConsta public ConfigurationParser() throws InvocationTargetException { try { - parserFactory.setNamespaceAware(true); - this.parser = parserFactory.newSAXParser(); + this.parser = XmlParserFactory.createSAXParserWithErrorOnDOCTYPE(true); } catch (ParserConfigurationException | SAXException e) { Utils.log(Utils.newStatus("ConfigurationParser", e)); //$NON-NLS-1$ throw new InvocationTargetException(e); diff --git a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/update/configurator/FeatureParser.java b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/update/configurator/FeatureParser.java index b373d0b1cb..5f82d1d100 100644 --- a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/update/configurator/FeatureParser.java +++ b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/update/configurator/FeatureParser.java @@ -20,9 +20,9 @@ import javax.xml.parsers.ParserConfigurationException; import javax.xml.parsers.SAXParser; -import javax.xml.parsers.SAXParserFactory; import org.eclipse.osgi.util.NLS; +import org.eclipse.pde.internal.core.util.XmlParserFactory; import org.xml.sax.Attributes; import org.xml.sax.InputSource; import org.xml.sax.SAXException; @@ -40,16 +40,13 @@ class FeatureParser extends DefaultHandler { private FeatureEntry feature; private URL url; - private final static SAXParserFactory parserFactory = SAXParserFactory.newInstance(); - /** * Constructs a feature parser. */ public FeatureParser() { super(); try { - parserFactory.setNamespaceAware(true); - this.parser = parserFactory.newSAXParser(); + this.parser = XmlParserFactory.createSAXParserWithErrorOnDOCTYPE(true); } catch (ParserConfigurationException | SAXException e) { System.out.println(e); } diff --git a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/util/PDEXMLHelper.java b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/util/PDEXMLHelper.java index a54b8045e4..ea50bc7c83 100644 --- a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/util/PDEXMLHelper.java +++ b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/util/PDEXMLHelper.java @@ -22,10 +22,6 @@ import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.FactoryConfigurationError; import javax.xml.parsers.ParserConfigurationException; -import javax.xml.parsers.SAXParser; -import javax.xml.parsers.SAXParserFactory; - -import org.xml.sax.SAXException; /** * PDEXMLHelper @@ -33,40 +29,20 @@ */ public class PDEXMLHelper { - protected static SAXParserFactory fSAXFactory; protected static PDEXMLHelper fPinstance; protected static DocumentBuilderFactory fDOMFactory; - protected static List> fSAXParserQueue; protected static List> fDOMParserQueue; protected static int fSAXPoolLimit; protected static int fDOMPoolLimit; protected static final int FMAXPOOLLIMIT = 1; protected PDEXMLHelper() throws FactoryConfigurationError { - fSAXFactory = SAXParserFactory.newInstance(); fDOMFactory = XmlDocumentBuilderFactory.createDocumentBuilderFactoryWithErrorOnDOCTYPE(); - fSAXParserQueue = Collections.synchronizedList(new LinkedList<>()); fDOMParserQueue = Collections.synchronizedList(new LinkedList<>()); fSAXPoolLimit = FMAXPOOLLIMIT; fDOMPoolLimit = FMAXPOOLLIMIT; } - public synchronized SAXParser getDefaultSAXParser() throws ParserConfigurationException, SAXException { - - SAXParser parser = null; - if (fSAXParserQueue.isEmpty()) { - parser = fSAXFactory.newSAXParser(); - } else { - SoftReference reference = fSAXParserQueue.remove(0); - if (reference.get() != null) { - parser = reference.get(); - } else { - parser = fSAXFactory.newSAXParser(); - } - } - return parser; - } - public synchronized DocumentBuilder getDefaultDOMParser() throws ParserConfigurationException { DocumentBuilder parser = null; @@ -90,13 +66,6 @@ public static PDEXMLHelper Instance() throws FactoryConfigurationError { return fPinstance; } - public synchronized void recycleSAXParser(SAXParser parser) { - if (fSAXParserQueue.size() < fSAXPoolLimit) { - SoftReference reference = new SoftReference<>(parser); - fSAXParserQueue.add(reference); - } - } - public synchronized void recycleDOMParser(DocumentBuilder parser) { if (fDOMParserQueue.size() < fDOMPoolLimit) { SoftReference reference = new SoftReference<>(parser); diff --git a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/util/SAXParserWrapper.java b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/util/SAXParserWrapper.java index 1ab43db0a7..352c981829 100644 --- a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/util/SAXParserWrapper.java +++ b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/util/SAXParserWrapper.java @@ -36,28 +36,14 @@ private SAXParserWrapper() { // static use only public static void parse(File f, DefaultHandler dh) throws SAXException, IOException, ParserConfigurationException, FactoryConfigurationError { - parse(p -> p.parse(f, dh)); + SAXParser fParser = XmlParserFactory.createSAXParserWithErrorOnDOCTYPE(); + fParser.parse(f, dh); } public static void parse(InputStream is, DefaultHandler dh) throws SAXException, IOException, ParserConfigurationException, FactoryConfigurationError { - parse(p -> p.parse(is, dh)); + SAXParser fParser = XmlParserFactory.createSAXParserWithErrorOnDOCTYPE(); + fParser.parse(is, dh); } - // NOTE: If other parser method calls are required, the corresponding - // wrapper method needs to be added here - - private interface ParseProcess { - void parseWith(SAXParser parser) throws SAXException, IOException; - } - - private static void parse(ParseProcess pp) - throws ParserConfigurationException, SAXException, FactoryConfigurationError, IOException { - SAXParser fParser = PDEXMLHelper.Instance().getDefaultSAXParser(); - try { - pp.parseWith(fParser); - } finally { - PDEXMLHelper.Instance().recycleSAXParser(fParser); - } - } } diff --git a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/util/XmlParserFactory.java b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/util/XmlParserFactory.java new file mode 100644 index 0000000000..979865fa6c --- /dev/null +++ b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/util/XmlParserFactory.java @@ -0,0 +1,76 @@ +/******************************************************************************* + * Copyright (c) 2023 Joerg Kubitz and others. + * + * This program and the accompanying materials + * are made available under the terms of the Eclipse Public License 2.0 + * which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + *******************************************************************************/ +package org.eclipse.pde.internal.core.util; + +import javax.xml.XMLConstants; +import javax.xml.parsers.ParserConfigurationException; +import javax.xml.parsers.SAXParser; +import javax.xml.parsers.SAXParserFactory; + +import org.xml.sax.SAXException; +import org.xml.sax.SAXNotRecognizedException; +import org.xml.sax.SAXNotSupportedException; + +public class XmlParserFactory { + private XmlParserFactory() { + // static Utility only + } + + private static final SAXParserFactory SAX_FACTORY_ERROR_ON_DOCTYPE = createSAXFactoryWithErrorOnDOCTYPE(false); + private static final SAXParserFactory SAX_FACTORY_ERROR_ON_DOCTYPE_NS = createSAXFactoryWithErrorOnDOCTYPE(true); + private static final SAXParserFactory SAX_FACTORY_IGNORING_DOCTYPE = createSAXFactoryIgnoringDOCTYPE(); + + private static synchronized SAXParserFactory createSAXFactoryWithErrorOnDOCTYPE(boolean awareness) { + SAXParserFactory f = SAXParserFactory.newInstance(); + f.setNamespaceAware(awareness); + try { + // force org.xml.sax.SAXParseException for any DOCTYPE: + f.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); //$NON-NLS-1$ + } catch (Exception e) { + throw new RuntimeException(e); + } + return f; + } + + private static synchronized SAXParserFactory createSAXFactoryIgnoringDOCTYPE() { + SAXParserFactory f = SAXParserFactory.newInstance(); + try { + // ignore DOCTYPE: + f.setFeature("http://xml.org/sax/features/external-general-entities", false); //$NON-NLS-1$ + f.setFeature("http://xml.org/sax/features/external-parameter-entities", false); //$NON-NLS-1$ + f.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); //$NON-NLS-1$ + } catch (Exception e) { + throw new RuntimeException(e); + } + return f; + } + + public static SAXParser createSAXParserWithErrorOnDOCTYPE() throws ParserConfigurationException, SAXException { + return createSAXParserWithErrorOnDOCTYPE(false); + } + + public static SAXParser createSAXParserWithErrorOnDOCTYPE(boolean awareness) + throws ParserConfigurationException, SAXException { + if (awareness) { + return SAX_FACTORY_ERROR_ON_DOCTYPE_NS.newSAXParser(); + } + return SAX_FACTORY_ERROR_ON_DOCTYPE.newSAXParser(); + } + + public static SAXParser createSAXParserIgnoringDOCTYPE() + throws ParserConfigurationException, SAXNotRecognizedException, SAXNotSupportedException, SAXException { + SAXParser parser = SAX_FACTORY_IGNORING_DOCTYPE.newSAXParser(); + parser.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, ""); //$NON-NLS-1$ + parser.setProperty(XMLConstants.ACCESS_EXTERNAL_SCHEMA, ""); //$NON-NLS-1$ + return parser; + } + +} diff --git a/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/core/tests/internal/AllPDECoreTests.java b/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/core/tests/internal/AllPDECoreTests.java index 1c8deea918..67aaa7cc66 100644 --- a/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/core/tests/internal/AllPDECoreTests.java +++ b/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/core/tests/internal/AllPDECoreTests.java @@ -11,6 +11,7 @@ XmlTransformerTest.class, // WorkspaceModelManagerTest.class, // WorkspaceProductModelManagerTest.class, // + XmlParserFactoryTest.class // }) public class AllPDECoreTests { } diff --git a/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/core/tests/internal/XmlParserFactoryTest.java b/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/core/tests/internal/XmlParserFactoryTest.java new file mode 100644 index 0000000000..5257f808ca --- /dev/null +++ b/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/core/tests/internal/XmlParserFactoryTest.java @@ -0,0 +1,203 @@ +/******************************************************************************* + * Copyright (c) 2023 Joerg Kubitz and others. + * + * This program and the accompanying materials + * are made available under the terms of the Eclipse Public License 2.0 + * which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + *******************************************************************************/ +package org.eclipse.pde.core.tests.internal; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import java.io.BufferedReader; +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.InputStreamReader; +import java.io.OutputStream; +import java.lang.reflect.InvocationTargetException; +import java.net.ServerSocket; +import java.net.Socket; +import java.net.SocketException; +import java.net.URI; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.concurrent.ConcurrentLinkedQueue; +import java.util.function.IntFunction; + +import javax.xml.parsers.SAXParser; + +import org.eclipse.pde.internal.core.util.XmlParserFactory; +import org.junit.Test; +import org.xml.sax.InputSource; +import org.xml.sax.SAXException; +import org.xml.sax.SAXParseException; +import org.xml.sax.helpers.DefaultHandler; + +public class XmlParserFactoryTest { + + List tmpFiles = new ArrayList<>(); + + @Test + public void testParseXmlWithExternalEntity() throws Exception { + SAXParser parser =XmlParserFactory.createSAXParserWithErrorOnDOCTYPE(); + try { + testParseXmlWithExternalEntity(parser, this::createMalciousXml); + assertTrue("SAXParseException expected", false); + } catch (SAXParseException e) { + String message = e.getMessage(); + assertTrue(message, message.contains("DOCTYPE")); + assertTrue(message, message.contains("http://apache.org/xml/features/disallow-doctype-decl")); + } + } + + @Test + public void testParseXmlWithoutExternalEntity() throws Exception { + SAXParser parser = XmlParserFactory.createSAXParserWithErrorOnDOCTYPE(); + testParseXmlWithExternalEntity(parser, this::createNormalXml); + } + + @Test + public void testParseXmlWithIgnoredExternalEntity() throws Exception { + SAXParser parser = XmlParserFactory.createSAXParserIgnoringDOCTYPE(); + testParseXmlWithExternalEntity(parser, this::createMalciousXml); + } + + @Test + public void testParseXmlWithoutIgnoredExternalEntity() throws Exception { + SAXParser parser = XmlParserFactory.createSAXParserIgnoringDOCTYPE(); + testParseXmlWithExternalEntity(parser, this::createNormalXml); + } + + InputStream createMalciousXml(int localPort) { + try { + Path tempSecret = Files.createTempFile("test", ".txt"); + tmpFiles.add(tempSecret); + Files.writeString(tempSecret, "secret"); + Path tempDtd = Files.createTempFile("test", ".dtd"); + tmpFiles.add(tempDtd); + String dtdContent = "\n" // + + "\n" // + + "\">\n" // + + "%var2;\n"; + Files.writeString(tempDtd, dtdContent); + StringBuilder sb = new StringBuilder(); + sb.append("\n"); + URI dtdURi = tempDtd.toUri(); + String dtdUrl = dtdURi.toURL().toString(); + sb.append("\n"); + sb.append("&var3;&var4;"); + String xml = sb.toString(); + return new ByteArrayInputStream(xml.getBytes(StandardCharsets.UTF_8)); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + InputStream createNormalXml(int localPort) { + StringBuilder sb = new StringBuilder(); + sb.append("\n"); + sb.append("hello"); + String xml = sb.toString(); + return new ByteArrayInputStream(xml.getBytes(StandardCharsets.UTF_8)); + } + + private void cleanup() throws IOException { + for (Path path : tmpFiles) { + Files.delete(path); + } + } + + public void testParseXmlWithExternalEntity(SAXParser parser, IntFunction xmlSupplier) + throws Exception { + Collection exceptionsInOtherThreads = new ConcurrentLinkedQueue<>(); + Thread httpServerThread = null; + try (ServerSocket serverSocket = new ServerSocket(0)) { + int localPort = serverSocket.getLocalPort(); + httpServerThread = new Thread("httpServerThread") { + @Override + public void run() { + try (Socket socket = serverSocket.accept()) { + try (BufferedReader in = new BufferedReader(new InputStreamReader(socket.getInputStream()))) { + String firstLine = in.readLine(); + try (OutputStream outputStream = socket.getOutputStream()) { + outputStream.write("HTTP/1.1 200 OK\r\n".getBytes(StandardCharsets.UTF_8)); + } + assertTrue(firstLine, firstLine.startsWith("GET")); + assertFalse("Server received secret: " + firstLine, firstLine.contains("secret")); // var3 + assertFalse("Server was contacted", true); + } + } catch (SocketException closed) { + // expected + } catch (Throwable e) { + exceptionsInOtherThreads.add(e); + } + } + }; + httpServerThread.start(); + List elements = new ArrayList<>(); + DefaultHandler handler = new DefaultHandler() { + @Override + public void startElement(String uri, String localName, String qName, org.xml.sax.Attributes attributes) + throws org.xml.sax.SAXException { + elements.add(qName); + } + + @Override + public void characters(char ch[], int start, int length) throws SAXException { + String content = new String(ch, start, length); + assertFalse("Secret was injected into xml: " + content, content.contains("secret")); // var4 + } + + @Override + public InputSource resolveEntity(String publicId, String systemId) throws IOException, SAXException { + // implementation that would do any remote call: + try { + return new InputSource(URI.create(systemId).toURL().openStream()); + } catch (IOException exception) { + throw new SAXException(exception); + } + // Also the default impl injects files: + // return null; + + // Does also prevent access to external files: + // return new InputSource(new StringReader("")); + } + + }; + try (InputStream xmlStream = xmlSupplier.apply(localPort)) { + parser.parse(xmlStream, handler); + } + assertEquals(List.of("Body"), elements); + } finally { + cleanup(); + } + httpServerThread.join(5000); + assertFalse(httpServerThread.isAlive()); + for (Throwable e : exceptionsInOtherThreads) { + throw new InvocationTargetException(e, e.getMessage()); + } + } + + static volatile Object sink; + + public static void main(String[] args) throws Exception { + // testParserCreatePerformance + for (int i = 1; i < 1000; i++) { + long n0 = System.nanoTime(); + sink = XmlParserFactory.createSAXParserIgnoringDOCTYPE(); + long n1 = System.nanoTime(); + System.out.println("run " + i + ": " + (n1 - n0) + "ns"); + // ~ run 999: 60000ns + } + } +}