Factory for SAXParser parsing XML without DOCTYPE

External entity resolution is not supported by PDE (see
PDECoreMessages.XMLErrorReporter_ExternalEntityResolution) but still the
SAXParser did follow external links where DefaultHandler.resolveEntity
was not overwritten.
At many places PDE already overwrote DefaultHandler.resolveEntity to
prevent external resolution. With the new configuration that method is
not even called anymore.

This change offers and uses a configuration that
* reports an Exception if .xml contains DOCTYPE or
* does just ignore external references (as a fall back if the exception
would show up to cause trouble).

Also the caching of used parsers in possibly other threads is removed
because the SAXParser is not guaranteed to be thread-safe. Only the
factory is reused, because that is effectively final after creation.
Reusing SAXParser is not a big help nowadays - see
XmlParserFactoryTest.main(String[]) for performance test.
In my measurement successive parser creations takes only ~ 0.06 ms.

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
 		}

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<String, String> 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
 		}

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) {

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) {

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();
 		}

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();
 		}

apitools/org.eclipse.pde.api.tools/src_ant/org/eclipse/pde/api/tools/internal/tasks/AnalysisReportConversionTask.java

@@ -27,14 +27,14 @@
 
 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;
 import org.eclipse.osgi.util.NLS;
 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();
 		}

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);
 		}

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();
+	}
+
+}

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

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) {
 					}

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;
 	}

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);