From 1c3f0aec14d00bdbca175713af70cb7c7b868e9f Mon Sep 17 00:00:00 2001 From: Adam Retter Date: Tue, 30 Oct 2018 15:09:32 +0800 Subject: [PATCH] [bugfix] Secure the XMLReader processing Closes https://github.com/eXist-db/exist/issues/2180 --- .../src/org/exist/debugger/Utils.java | 45 ++++++++--------- .../org/exist/debugger/dbgp/ResponseImpl.java | 3 +- .../spatial/AbstractGMLJDBCIndexWorker.java | 18 +++---- .../exist/indexing/spatial/GMLIndexTest.java | 3 +- .../xquery/modules/sql/ExecuteFunction.java | 27 +++++----- .../backup/AbstractBackupDescriptor.java | 26 +++++----- src/org/exist/backup/BackupDescriptor.java | 6 +-- src/org/exist/backup/Restore.java | 3 +- src/org/exist/backup/SystemExport.java | 2 +- src/org/exist/backup/SystemImport.java | 21 ++++---- .../exist/backup/restore/RestoreHandler.java | 3 +- .../backup/restore/SystemImportHandler.java | 23 +++++---- .../CollectionConfigurationManager.java | 25 ++++++---- src/org/exist/config/Configurator.java | 9 +++- src/org/exist/http/Descriptor.java | 10 +++- src/org/exist/http/RESTServer.java | 50 ++++++++----------- .../exist/http/urlrewrite/RewriteConfig.java | 27 +++++----- .../storage/serializers/XIncludeFilter.java | 19 ++++--- src/org/exist/util/Configuration.java | 11 +++- src/org/exist/util/ExistSAXParserFactory.java | 23 ++++----- src/org/exist/util/MimeTable.java | 9 +++- src/org/exist/validation/Validator.java | 7 ++- src/org/exist/xmldb/RemoteXMLResource.java | 6 ++- .../xquery/functions/fn/ParsingFunctions.java | 25 ++++------ src/org/exist/xquery/functions/util/Eval.java | 28 +++++------ .../exist/xquery/functions/util/Parse.java | 17 +++---- .../xquery/functions/validation/Jaxp.java | 7 ++- .../org/exist/config/ConfigurableTest.java | 3 +- test/src/org/exist/dom/memtree/DOMTest.java | 3 +- .../exist/dom/memtree/DocumentImplTest.java | 3 +- .../exist/dom/memtree/MemtreeBuilderTest.java | 3 +- test/src/org/exist/http/RESTServiceTest.java | 3 +- .../org/exist/test/runner/XMLTestRunner.java | 9 +++- test/src/org/exist/w3c/tests/TestCase.java | 5 +- test/src/org/exist/xmldb/ResourceTest.java | 3 +- 35 files changed, 266 insertions(+), 219 deletions(-) diff --git a/extensions/debuggee/src/org/exist/debugger/Utils.java b/extensions/debuggee/src/org/exist/debugger/Utils.java index 43adef7b9db..6a35a784e9f 100644 --- a/extensions/debuggee/src/org/exist/debugger/Utils.java +++ b/extensions/debuggee/src/org/exist/debugger/Utils.java @@ -24,9 +24,6 @@ import java.io.IOException; import java.io.StringReader; -import javax.xml.parsers.SAXParser; -import javax.xml.parsers.SAXParserFactory; - import org.exist.Namespaces; import org.exist.dom.memtree.NodeImpl; import org.exist.dom.memtree.SAXAdapter; @@ -44,28 +41,30 @@ public class Utils { public static NodeImpl nodeFromString(XQueryContext context, String source) throws IOException { SAXAdapter adapter = new SAXAdapter(context); - SAXParserFactory factory = SAXParserFactory.newInstance(); - factory.setNamespaceAware(true); - - XMLReader xr; - try { - SAXParser parser = factory.newSAXParser(); + final XMLReaderPool parserPool = context.getBroker().getBrokerPool().getParserPool(); + XMLReader xr = null; + try { + try { + xr = parserPool.borrowXMLReader(); + xr.setContentHandler(adapter); + xr.setProperty(Namespaces.SAX_LEXICAL_HANDLER, adapter); + + } catch (Exception e) { + throw new IOException(e); + } - xr = parser.getXMLReader(); - xr.setContentHandler(adapter); - xr.setProperty(Namespaces.SAX_LEXICAL_HANDLER, adapter); + try { + InputSource src = new InputSource(new StringReader(source)); + xr.parse(src); - } catch (Exception e) { - throw new IOException(e); - } - - try { - InputSource src = new InputSource(new StringReader(source)); - xr.parse(src); - - return (NodeImpl) adapter.getDocument(); - } catch (SAXException e) { - throw new IOException(e); + return (NodeImpl) adapter.getDocument(); + } catch (SAXException e) { + throw new IOException(e); + } + } finally { + if (xr != null) { + parserPool.returnXMLReader(xr); + } } } } diff --git a/extensions/debuggee/src/org/exist/debugger/dbgp/ResponseImpl.java b/extensions/debuggee/src/org/exist/debugger/dbgp/ResponseImpl.java index c6412bb44b2..168db79a3f3 100644 --- a/extensions/debuggee/src/org/exist/debugger/dbgp/ResponseImpl.java +++ b/extensions/debuggee/src/org/exist/debugger/dbgp/ResponseImpl.java @@ -32,6 +32,7 @@ import org.exist.debugger.DebuggerImpl; import org.exist.debugger.Response; import org.exist.dom.memtree.SAXAdapter; +import org.exist.util.ExistSAXParserFactory; import org.w3c.dom.Element; import org.w3c.dom.Node; import org.w3c.dom.NodeList; @@ -53,7 +54,7 @@ public class ResponseImpl implements Response { public ResponseImpl(IoSession session, InputStream inputStream) { this.session = session; - SAXParserFactory factory = SAXParserFactory.newInstance(); + SAXParserFactory factory = ExistSAXParserFactory.getSAXParserFactory(); factory.setNamespaceAware(true); InputSource src = new InputSource(inputStream); SAXParser parser; diff --git a/extensions/indexes/spatial/src/org/exist/indexing/spatial/AbstractGMLJDBCIndexWorker.java b/extensions/indexes/spatial/src/org/exist/indexing/spatial/AbstractGMLJDBCIndexWorker.java index 6247a663d78..e1ef6a5e66f 100644 --- a/extensions/indexes/spatial/src/org/exist/indexing/spatial/AbstractGMLJDBCIndexWorker.java +++ b/extensions/indexes/spatial/src/org/exist/indexing/spatial/AbstractGMLJDBCIndexWorker.java @@ -78,8 +78,6 @@ import org.xml.sax.helpers.XMLFilterImpl; import javax.xml.parsers.ParserConfigurationException; -import javax.xml.parsers.SAXParser; -import javax.xml.parsers.SAXParserFactory; import javax.xml.transform.TransformerException; import java.io.IOException; import java.io.StringReader; @@ -642,15 +640,13 @@ public Element streamGeometryToElement(Geometry geometry, String srsName, Receiv gmlString = gmlTransformer.transform(geometry); } catch (TransformerException e) { throw new SpatialIndexException(e); - } + } + final XMLReaderPool parserPool = pool.getParserPool(); + XMLReader reader = null; try { - //Copied from org.exist.xquery.functions.request.getData - SAXParserFactory factory = SAXParserFactory.newInstance(); - factory.setNamespaceAware(true); InputSource src = new InputSource(new StringReader(gmlString)); - SAXParser parser = factory.newSAXParser(); - XMLReader reader = parser.getXMLReader(); + reader = parserPool.borrowXMLReader(); reader.setContentHandler((ContentHandler)receiver); reader.parse(src); Document doc = receiver.getDocument(); @@ -660,7 +656,11 @@ public Element streamGeometryToElement(Geometry geometry, String srsName, Receiv } catch (SAXException e) { throw new SpatialIndexException(e); } catch (IOException e) { - throw new SpatialIndexException(e); + throw new SpatialIndexException(e); + } finally { + if (reader != null) { + parserPool.returnXMLReader(reader); + } } } diff --git a/extensions/indexes/spatial/test/src/org/exist/indexing/spatial/GMLIndexTest.java b/extensions/indexes/spatial/test/src/org/exist/indexing/spatial/GMLIndexTest.java index 00cd75e2412..b36e82cf979 100644 --- a/extensions/indexes/spatial/test/src/org/exist/indexing/spatial/GMLIndexTest.java +++ b/extensions/indexes/spatial/test/src/org/exist/indexing/spatial/GMLIndexTest.java @@ -46,6 +46,7 @@ import org.exist.security.PermissionDeniedException; import org.exist.storage.BrokerPool; import org.exist.storage.DBBroker; +import org.exist.util.ExistSAXParserFactory; import org.exist.xmldb.IndexQueryService; import org.exist.xmldb.XmldbURI; import org.exist.xquery.XPathException; @@ -203,7 +204,7 @@ public void testLowLevelSearch() throws EXistException, SAXException, ParserConf AbstractGMLJDBCIndexWorker indexWorker = (AbstractGMLJDBCIndexWorker) broker.getIndexController().getWorkerByIndexId(AbstractGMLJDBCIndex.ID); //Unplugged if (indexWorker != null) { - SAXParserFactory factory = SAXParserFactory.newInstance(); + SAXParserFactory factory = ExistSAXParserFactory.getSAXParserFactory(); factory.setNamespaceAware(true); InputSource src = new InputSource(new StringReader(IN_MEMORY_GML)); SAXParser parser = factory.newSAXParser(); diff --git a/extensions/modules/src/org/exist/xquery/modules/sql/ExecuteFunction.java b/extensions/modules/src/org/exist/xquery/modules/sql/ExecuteFunction.java index 99edb4293f9..1b5b6c83751 100644 --- a/extensions/modules/src/org/exist/xquery/modules/sql/ExecuteFunction.java +++ b/extensions/modules/src/org/exist/xquery/modules/sql/ExecuteFunction.java @@ -25,6 +25,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.exist.util.XMLReaderPool; import org.exist.util.io.FastByteArrayOutputStream; import org.w3c.dom.Element; import org.w3c.dom.Node; @@ -60,8 +61,6 @@ import java.sql.Statement; import java.sql.Timestamp; import java.sql.Types; -import javax.xml.parsers.SAXParser; -import javax.xml.parsers.SAXParserFactory; import org.exist.dom.memtree.AppendingSAXAdapter; import org.exist.dom.memtree.ReferenceNode; import org.exist.dom.memtree.SAXAdapter; @@ -263,17 +262,21 @@ public ExecuteFunction( XQueryContext context, FunctionSignature signature ) // Add a null indicator attribute if the value was SQL Null builder.addAttribute( new QName( "null", SQLModule.NAMESPACE_URI, SQLModule.PREFIX ), "true" ); } else { - - SAXParserFactory factory = SAXParserFactory.newInstance(); - factory.setNamespaceAware(true); InputSource src = new InputSource(sqlXml.getCharacterStream()); - SAXParser parser = factory.newSAXParser(); - XMLReader xr = parser.getXMLReader(); - - SAXAdapter adapter = new AppendingSAXAdapter(builder); - xr.setContentHandler(adapter); - xr.setProperty(Namespaces.SAX_LEXICAL_HANDLER, adapter); - xr.parse(src); + final XMLReaderPool parserPool = context.getBroker().getBrokerPool().getParserPool(); + XMLReader reader = null; + try { + reader = parserPool.borrowXMLReader(); + + SAXAdapter adapter = new AppendingSAXAdapter(builder); + reader.setContentHandler(adapter); + reader.setProperty(Namespaces.SAX_LEXICAL_HANDLER, adapter); + reader.parse(src); + } finally { + if (reader != null) { + parserPool.returnXMLReader(reader); + } + } } } catch(Exception e) { throw new XPathException("Could not parse column of type SQLXML: " + e.getMessage(), e); diff --git a/src/org/exist/backup/AbstractBackupDescriptor.java b/src/org/exist/backup/AbstractBackupDescriptor.java index 9ad2f129e71..311ed99a6dc 100644 --- a/src/org/exist/backup/AbstractBackupDescriptor.java +++ b/src/org/exist/backup/AbstractBackupDescriptor.java @@ -22,6 +22,7 @@ package org.exist.backup; +import org.exist.util.XMLReaderPool; import org.xml.sax.ContentHandler; import org.xml.sax.SAXException; import org.xml.sax.XMLReader; @@ -34,10 +35,6 @@ import java.util.Date; import java.util.Properties; -import javax.xml.parsers.ParserConfigurationException; -import javax.xml.parsers.SAXParser; -import javax.xml.parsers.SAXParserFactory; - public abstract class AbstractBackupDescriptor implements BackupDescriptor { @@ -74,15 +71,18 @@ public boolean before( long timestamp ) return( timestamp > getDate().getTime() ); } - - public void parse( ContentHandler handler ) throws IOException, SAXException, ParserConfigurationException + @Override + public void parse(final XMLReaderPool parserPool, final ContentHandler handler ) throws IOException, SAXException { - final SAXParserFactory saxFactory = SAXParserFactory.newInstance(); - saxFactory.setNamespaceAware( true ); - saxFactory.setValidating( false ); - final SAXParser sax = saxFactory.newSAXParser(); - final XMLReader reader = sax.getXMLReader(); - reader.setContentHandler( handler ); - reader.parse( getInputSource() ); + XMLReader reader = null; + try { + reader = parserPool.borrowXMLReader(); + reader.setContentHandler(handler); + reader.parse(getInputSource()); + } finally { + if (reader != null) { + parserPool.returnXMLReader(reader); + } + } } } diff --git a/src/org/exist/backup/BackupDescriptor.java b/src/org/exist/backup/BackupDescriptor.java index 116ac2b428b..30eebdb8edc 100644 --- a/src/org/exist/backup/BackupDescriptor.java +++ b/src/org/exist/backup/BackupDescriptor.java @@ -21,6 +21,7 @@ */ package org.exist.backup; +import org.exist.util.XMLReaderPool; import org.xml.sax.ContentHandler; import org.xml.sax.SAXException; @@ -32,8 +33,6 @@ import java.util.Date; import java.util.Properties; -import javax.xml.parsers.ParserConfigurationException; - public interface BackupDescriptor { @@ -86,7 +85,8 @@ public interface BackupDescriptor boolean before( long timestamp ); - void parse( ContentHandler handler ) throws IOException, SAXException, ParserConfigurationException; + void parse(XMLReaderPool parserPool, ContentHandler handler) + throws IOException, SAXException; Path getRepoBackup() throws IOException; } diff --git a/src/org/exist/backup/Restore.java b/src/org/exist/backup/Restore.java index 702ca5b9197..b70f64dab61 100644 --- a/src/org/exist/backup/Restore.java +++ b/src/org/exist/backup/Restore.java @@ -37,6 +37,7 @@ import org.exist.security.Account; import org.exist.security.SecurityManager; import org.exist.util.EXistInputSource; +import org.exist.util.ExistSAXParserFactory; import org.exist.util.FileUtils; import org.exist.xmldb.UserManagementService; import org.exist.xmldb.XmldbURI; @@ -67,7 +68,7 @@ public void restore(RestoreListener listener, String username, String password, //get the backup descriptors, can be more than one if it was an incremental backup final Deque descriptors = getBackupDescriptors(f); - final SAXParserFactory saxFactory = SAXParserFactory.newInstance(); + final SAXParserFactory saxFactory = ExistSAXParserFactory.getSAXParserFactory(); saxFactory.setNamespaceAware(true); saxFactory.setValidating(false); final SAXParser sax = saxFactory.newSAXParser(); diff --git a/src/org/exist/backup/SystemExport.java b/src/org/exist/backup/SystemExport.java index 25e7c4ffbca..d2f27ca89b8 100644 --- a/src/org/exist/backup/SystemExport.java +++ b/src/org/exist/backup/SystemExport.java @@ -462,7 +462,7 @@ private void export(BackupHandler bh, Collection current, BackupWriter output, D final CheckDeletedHandler check = new CheckDeletedHandler(current, serializer); try { - prevBackup.parse(check); + prevBackup.parse(broker.getBrokerPool().getParserPool(), check); } catch (final Exception e) { LOG.error("Caught exception while trying to parse previous backup descriptor: " + prevBackup.getSymbolicPath(), e); } diff --git a/src/org/exist/backup/SystemImport.java b/src/org/exist/backup/SystemImport.java index 1169c43dda5..c9af4dd02ee 100644 --- a/src/org/exist/backup/SystemImport.java +++ b/src/org/exist/backup/SystemImport.java @@ -29,8 +29,6 @@ import java.util.Deque; import java.util.Properties; import javax.xml.parsers.ParserConfigurationException; -import javax.xml.parsers.SAXParser; -import javax.xml.parsers.SAXParserFactory; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -44,6 +42,7 @@ import org.exist.storage.DBBroker; import org.exist.util.EXistInputSource; import org.exist.util.FileUtils; +import org.exist.util.XMLReaderPool; import org.exist.xmldb.XmldbURI; import org.xml.sax.SAXException; import org.xml.sax.XMLReader; @@ -74,15 +73,13 @@ public void restore(RestoreListener listener, String username, Object credential //get the backup descriptors, can be more than one if it was an incremental backup final Deque descriptors = getBackupDescriptors(f); - - final SAXParserFactory saxFactory = SAXParserFactory.newInstance(); - saxFactory.setNamespaceAware(true); - saxFactory.setValidating(false); - final SAXParser sax = saxFactory.newSAXParser(); - final XMLReader reader = sax.getXMLReader(); - + + final XMLReaderPool parserPool = broker.getBrokerPool().getParserPool(); + XMLReader reader = null; try { - listener.restoreStarting(); + reader = parserPool.borrowXMLReader(); + + listener.restoreStarting(); while(!descriptors.isEmpty()) { final BackupDescriptor descriptor = descriptors.pop(); @@ -96,6 +93,10 @@ public void restore(RestoreListener listener, String username, Object credential } } finally { listener.restoreFinished(); + + if (reader != null) { + parserPool.returnXMLReader(reader); + } } } } diff --git a/src/org/exist/backup/restore/RestoreHandler.java b/src/org/exist/backup/restore/RestoreHandler.java index 7401e98cf05..4f565133182 100644 --- a/src/org/exist/backup/restore/RestoreHandler.java +++ b/src/org/exist/backup/restore/RestoreHandler.java @@ -37,6 +37,7 @@ import org.exist.security.ACLPermission.ACE_TARGET; import org.exist.security.SecurityManager; import org.exist.util.EXistInputSource; +import org.exist.util.ExistSAXParserFactory; import org.exist.xmldb.EXistCollection; import org.exist.xmldb.EXistCollectionManagementService; import org.exist.xmldb.EXistResource; @@ -65,7 +66,7 @@ public class RestoreHandler extends DefaultHandler { private final static Logger LOG = LogManager.getLogger(RestoreHandler.class); - private final static SAXParserFactory saxFactory = SAXParserFactory.newInstance(); + private final static SAXParserFactory saxFactory = ExistSAXParserFactory.getSAXParserFactory(); static { saxFactory.setNamespaceAware(true); saxFactory.setValidating(false); diff --git a/src/org/exist/backup/restore/SystemImportHandler.java b/src/org/exist/backup/restore/SystemImportHandler.java index 689d9e88da0..1ecd009310d 100644 --- a/src/org/exist/backup/restore/SystemImportHandler.java +++ b/src/org/exist/backup/restore/SystemImportHandler.java @@ -23,6 +23,8 @@ import java.io.IOException; +import org.exist.util.ExistSAXParserFactory; +import org.exist.util.XMLReaderPool; import org.w3c.dom.DocumentType; import org.xml.sax.Attributes; import org.xml.sax.SAXException; @@ -47,8 +49,6 @@ import java.net.URISyntaxException; import java.util.*; -import javax.xml.parsers.ParserConfigurationException; -import javax.xml.parsers.SAXParser; import javax.xml.parsers.SAXParserFactory; import org.apache.logging.log4j.LogManager; @@ -74,7 +74,7 @@ public class SystemImportHandler extends DefaultHandler { private final static Logger LOG = LogManager.getLogger(SystemImportHandler.class); - private final static SAXParserFactory saxFactory = SAXParserFactory.newInstance(); + private final static SAXParserFactory saxFactory = ExistSAXParserFactory.getSAXParserFactory(); static { saxFactory.setNamespaceAware(true); saxFactory.setValidating(false); @@ -253,12 +253,11 @@ private void restoreSubCollectionEntry(Attributes atts) throws SAXException { //parse the sub-collection descriptor and restore final BackupDescriptor subDescriptor = descriptor.getChildBackupDescriptor(name); if(subDescriptor != null) { - - final SAXParser sax; - try { - sax = saxFactory.newSAXParser(); - - final XMLReader reader = sax.getXMLReader(); + + final XMLReaderPool parserPool = broker.getBrokerPool().getParserPool(); + XMLReader reader = null; + try { + reader = parserPool.borrowXMLReader(); final EXistInputSource is = subDescriptor.getInputSource(); is.setEncoding( "UTF-8" ); @@ -269,10 +268,12 @@ private void restoreSubCollectionEntry(Attributes atts) throws SAXException { reader.parse(is); } catch(final SAXParseException e) { throw new SAXException("Could not process collection: " + descriptor.getSymbolicPath(name, false), e); - } catch(final ParserConfigurationException pce) { - throw new SAXException("Could not initalise SAXParser for processing sub-collection: " + descriptor.getSymbolicPath(name, false), pce); } catch(final IOException ioe) { throw new SAXException("Could not read sub-collection for processing: " + ioe.getMessage(), ioe); + } finally { + if (reader != null) { + parserPool.returnXMLReader(reader); + } } } else { listener.error("Collection " + descriptor.getSymbolicPath(name, false) + " does not exist or is not readable."); diff --git a/src/org/exist/collections/CollectionConfigurationManager.java b/src/org/exist/collections/CollectionConfigurationManager.java index 180db27e6fe..305d5184d63 100644 --- a/src/org/exist/collections/CollectionConfigurationManager.java +++ b/src/org/exist/collections/CollectionConfigurationManager.java @@ -31,15 +31,14 @@ import org.exist.storage.txn.TransactionManager; import org.exist.storage.txn.Txn; import org.exist.util.LockException; +import org.exist.util.XMLReaderPool; import org.exist.util.sanity.SanityCheck; import org.exist.xmldb.XmldbURI; import org.w3c.dom.Document; import org.xml.sax.InputSource; import org.xml.sax.XMLReader; -import javax.xml.parsers.SAXParser; -import javax.xml.parsers.SAXParserFactory; - +import java.io.IOException; import java.io.StringReader; import java.util.*; import java.util.Map.Entry; @@ -155,15 +154,19 @@ public void addConfiguration(final Txn txn, final DBBroker broker, final Collect */ public void testConfiguration(DBBroker broker, String config) throws CollectionConfigurationException { try { - final SAXParserFactory factory = SAXParserFactory.newInstance(); - factory.setNamespaceAware(true); - final InputSource src = new InputSource(new StringReader(config)); - final SAXParser parser = factory.newSAXParser(); - final XMLReader reader = parser.getXMLReader(); final SAXAdapter adapter = new SAXAdapter(); - reader.setContentHandler(adapter); - reader.parse(src); - + final InputSource src = new InputSource(new StringReader(config)); + final XMLReaderPool parserPool = broker.getBrokerPool().getParserPool(); + XMLReader reader = null; + try { + reader = parserPool.borrowXMLReader(); + reader.setContentHandler(adapter); + reader.parse(src); + } finally { + if (reader != null) { + parserPool.returnXMLReader(reader); + } + } final Document doc = adapter.getDocument(); final CollectionConfiguration conf = new CollectionConfiguration(broker.getBrokerPool()); conf.read(broker, doc, true, null, null); diff --git a/src/org/exist/config/Configurator.java b/src/org/exist/config/Configurator.java index 29be30bf96e..7251697776b 100644 --- a/src/org/exist/config/Configurator.java +++ b/src/org/exist/config/Configurator.java @@ -65,6 +65,7 @@ import org.exist.storage.sync.Sync; import org.exist.storage.txn.TransactionManager; import org.exist.storage.txn.Txn; +import org.exist.util.ExistSAXParserFactory; import org.exist.util.MimeType; import com.evolvedbinary.j8fu.function.ConsumerE; import org.exist.util.io.FastByteArrayOutputStream; @@ -77,6 +78,7 @@ import org.xml.sax.XMLReader; import static java.lang.invoke.MethodType.methodType; +import static javax.xml.XMLConstants.FEATURE_SECURE_PROCESSING; /** * This class handle all configuration needs: extracting and saving, @@ -748,11 +750,16 @@ public static Configuration parse(final File file) throws ConfigurationException public static Configuration parse(final InputStream is) throws ConfigurationException { try { - final SAXParserFactory factory = SAXParserFactory.newInstance(); + final SAXParserFactory factory = ExistSAXParserFactory.getSAXParserFactory(); factory.setNamespaceAware(true); final InputSource src = new InputSource(is); final SAXParser parser = factory.newSAXParser(); final XMLReader reader = parser.getXMLReader(); + + reader.setFeature("http://xml.org/sax/features/external-general-entities", false); + reader.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + reader.setFeature(FEATURE_SECURE_PROCESSING, true); + final SAXAdapter adapter = new SAXAdapter(); reader.setContentHandler(adapter); reader.parse(src); diff --git a/src/org/exist/http/Descriptor.java b/src/org/exist/http/Descriptor.java index b2e11fef738..8ef48a5fd94 100644 --- a/src/org/exist/http/Descriptor.java +++ b/src/org/exist/http/Descriptor.java @@ -39,6 +39,7 @@ import org.apache.logging.log4j.Logger; import org.exist.dom.memtree.SAXAdapter; import org.exist.util.ConfigurationHelper; +import org.exist.util.ExistSAXParserFactory; import org.exist.util.SingleInstanceConfiguration; import org.w3c.dom.Document; import org.w3c.dom.Element; @@ -49,6 +50,8 @@ import org.xml.sax.SAXParseException; import org.xml.sax.XMLReader; +import static javax.xml.XMLConstants.FEATURE_SECURE_PROCESSING; + /** * Webapplication Descriptor *

@@ -114,12 +117,17 @@ private Descriptor() { // initialize xml parser // we use eXist's in-memory DOM implementation to work // around a bug in Xerces - final SAXParserFactory factory = SAXParserFactory.newInstance(); + final SAXParserFactory factory = ExistSAXParserFactory.getSAXParserFactory(); factory.setNamespaceAware(true); final InputSource src = new InputSource(is); final SAXParser parser = factory.newSAXParser(); final XMLReader reader = parser.getXMLReader(); + + reader.setFeature("http://xml.org/sax/features/external-general-entities", false); + reader.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + reader.setFeature(FEATURE_SECURE_PROCESSING, true); + final SAXAdapter adapter = new SAXAdapter(); reader.setContentHandler(adapter); reader.parse(src); diff --git a/src/org/exist/http/RESTServer.java b/src/org/exist/http/RESTServer.java index 8ead2dd652a..85f42c0cdf7 100644 --- a/src/org/exist/http/RESTServer.java +++ b/src/org/exist/http/RESTServer.java @@ -44,8 +44,6 @@ import javax.servlet.http.HttpServletResponse; import javax.xml.XMLConstants; import javax.xml.parsers.ParserConfigurationException; -import javax.xml.parsers.SAXParser; -import javax.xml.parsers.SAXParserFactory; import javax.xml.stream.XMLStreamException; import javax.xml.transform.OutputKeys; import javax.xml.transform.TransformerConfigurationException; @@ -93,10 +91,7 @@ import org.exist.storage.txn.TransactionException; import org.exist.storage.txn.TransactionManager; import org.exist.storage.txn.Txn; -import org.exist.util.Configuration; -import org.exist.util.LockException; -import org.exist.util.MimeTable; -import org.exist.util.MimeType; +import org.exist.util.*; import org.exist.util.io.CachingFilterInputStream; import org.exist.util.io.FilterInputStreamCache; import org.exist.util.io.FilterInputStreamCacheFactory; @@ -321,15 +316,12 @@ public void doGet(final DBBroker broker, final HttpServletRequest request, try { if (_var != null) { final NamespaceExtractor nsExtractor = new NamespaceExtractor(); - variables = parseXML(_var, nsExtractor); + variables = parseXML(broker.getBrokerPool(), _var, nsExtractor); namespaces = nsExtractor.getNamespaces(); } } catch (final SAXException e) { final XPathException x = new XPathException(e.toString()); writeXPathException(response, HttpServletResponse.SC_BAD_REQUEST, "UTF-8", query, path, x); - } catch (final ParserConfigurationException e) { - final XPathException x = new XPathException(e.toString()); - writeXPathException(response, HttpServletResponse.SC_BAD_REQUEST, "UTF-8", query, path, x); } if ((option = getParameter(request, HowMany)) != null) { @@ -743,7 +735,7 @@ public void doPost(final DBBroker broker, final HttpServletRequest request, try(final Txn transaction = transact.beginTransaction()) { final String content = getRequestContent(request); final NamespaceExtractor nsExtractor = new NamespaceExtractor(); - final ElementImpl root = parseXML(content, nsExtractor); + final ElementImpl root = parseXML(broker.getBrokerPool(), content, nsExtractor); final String rootNS = root.getNamespaceURI(); if (rootNS != null && rootNS.equals(Namespaces.EXIST_NS)) { @@ -956,25 +948,27 @@ public void doPost(final DBBroker broker, final HttpServletRequest request, } } - private ElementImpl parseXML(final String content, + private ElementImpl parseXML(final BrokerPool pool, final String content, final NamespaceExtractor nsExtractor) - throws ParserConfigurationException, SAXException, IOException { - - final SAXParserFactory factory = SAXParserFactory.newInstance(); - factory.setNamespaceAware(true); + throws SAXException, IOException { final InputSource src = new InputSource(new StringReader(content)); - final SAXParser parser = factory.newSAXParser(); - final XMLReader reader = parser.getXMLReader(); - final SAXAdapter adapter = new SAXAdapter(); - //reader.setContentHandler(adapter); - //reader.parse(src); - nsExtractor.setContentHandler(adapter); - nsExtractor.setParent(reader); - nsExtractor.parse(src); - - final Document doc = adapter.getDocument(); - - return (ElementImpl) doc.getDocumentElement(); + final XMLReaderPool parserPool = pool.getParserPool(); + XMLReader reader = null; + try { + reader = parserPool.borrowXMLReader(); + final SAXAdapter adapter = new SAXAdapter(); + nsExtractor.setContentHandler(adapter); + nsExtractor.setParent(reader); + nsExtractor.parse(src); + + final Document doc = adapter.getDocument(); + + return (ElementImpl) doc.getDocumentElement(); + } finally { + if (reader != null) { + parserPool.returnXMLReader(reader); + } + } } private class NamespaceExtractor extends XMLFilterImpl { diff --git a/src/org/exist/http/urlrewrite/RewriteConfig.java b/src/org/exist/http/urlrewrite/RewriteConfig.java index 9967f4c5bc0..034a17a916c 100644 --- a/src/org/exist/http/urlrewrite/RewriteConfig.java +++ b/src/org/exist/http/urlrewrite/RewriteConfig.java @@ -6,6 +6,7 @@ import org.exist.EXistException; import org.exist.security.PermissionDeniedException; import org.exist.dom.persistent.DocumentImpl; +import org.exist.util.XMLReaderPool; import org.exist.xmldb.XmldbURI; import org.exist.storage.DBBroker; import org.exist.storage.lock.Lock.LockMode; @@ -24,8 +25,6 @@ import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.xml.parsers.ParserConfigurationException; -import javax.xml.parsers.SAXParser; -import javax.xml.parsers.SAXParserFactory; import java.io.IOException; import java.io.InputStream; import java.nio.file.Files; @@ -223,18 +222,22 @@ private URLRewrite parseAction(final ServletConfig config, final String pattern, } private Document parseConfig(final Path file) throws ParserConfigurationException, SAXException, IOException { - final SAXParserFactory factory = SAXParserFactory.newInstance(); - factory.setNamespaceAware(true); - try (final InputStream is = Files.newInputStream(file)) { final InputSource src = new InputSource(is); - final SAXParser parser = factory.newSAXParser(); - final XMLReader xr = parser.getXMLReader(); - final SAXAdapter adapter = new SAXAdapter(); - xr.setContentHandler(adapter); - xr.setProperty(Namespaces.SAX_LEXICAL_HANDLER, adapter); - xr.parse(src); - return adapter.getDocument(); + final XMLReaderPool parserPool = urlRewrite.getBrokerPool().getParserPool(); + XMLReader xr = null; + try { + xr = parserPool.borrowXMLReader(); + final SAXAdapter adapter = new SAXAdapter(); + xr.setContentHandler(adapter); + xr.setProperty(Namespaces.SAX_LEXICAL_HANDLER, adapter); + xr.parse(src); + return adapter.getDocument(); + } finally { + if (xr != null) { + parserPool.returnXMLReader(xr); + } + } } } diff --git a/src/org/exist/storage/serializers/XIncludeFilter.java b/src/org/exist/storage/serializers/XIncludeFilter.java index 76b3816cb28..12fe3b453bf 100644 --- a/src/org/exist/storage/serializers/XIncludeFilter.java +++ b/src/org/exist/storage/serializers/XIncludeFilter.java @@ -36,6 +36,7 @@ import org.exist.source.StringSource; import org.exist.storage.XQueryPool; import com.evolvedbinary.j8fu.Either; +import org.exist.util.XMLReaderPool; import org.exist.util.serializer.AttrList; import org.exist.util.serializer.Receiver; import org.exist.xmldb.XmldbURI; @@ -45,9 +46,6 @@ import org.exist.xquery.XPathException; import org.exist.xquery.XQuery; import org.exist.xquery.XQueryContext; -import org.exist.xquery.functions.request.RequestModule; -import org.exist.xquery.functions.response.ResponseModule; -import org.exist.xquery.functions.session.SessionModule; import org.exist.xquery.util.ExpressionDumper; import org.exist.xquery.value.NodeValue; import org.exist.xquery.value.Sequence; @@ -60,8 +58,6 @@ import javax.xml.XMLConstants; import javax.xml.parsers.ParserConfigurationException; -import javax.xml.parsers.SAXParser; -import javax.xml.parsers.SAXParserFactory; import java.io.IOException; import java.io.InputStream; import java.io.UnsupportedEncodingException; @@ -492,19 +488,22 @@ private Either parseExternal( } // we use eXist's in-memory DOM implementation - final SAXParserFactory factory = SAXParserFactory.newInstance(); - factory.setNamespaceAware(true); - + final XMLReaderPool parserPool = serializer.broker.getBrokerPool().getParserPool(); + XMLReader reader = null; try (final InputStream is = con.getInputStream()) { final InputSource src = new InputSource(is); - final SAXParser parser = factory.newSAXParser(); - final XMLReader reader = parser.getXMLReader(); + + reader = parserPool.borrowXMLReader(); final SAXAdapter adapter = new SAXAdapter(); reader.setContentHandler(adapter); reader.parse(src); final org.exist.dom.memtree.DocumentImpl doc = adapter.getDocument(); doc.setDocumentURI(externalUri.toString()); return Either.Right(doc); + } finally { + if (reader != null) { + parserPool.returnXMLReader(reader); + } } } catch (final IOException e) { return Either.Left(new ResourceError("XInclude: unable to retrieve and parse document from URI: " + externalUri.toString(), e)); diff --git a/src/org/exist/util/Configuration.java b/src/org/exist/util/Configuration.java index a97fd374334..1bc6ab4aec9 100644 --- a/src/org/exist/util/Configuration.java +++ b/src/org/exist/util/Configuration.java @@ -83,6 +83,8 @@ import org.exist.scheduler.JobType; import org.exist.xquery.Module; +import static javax.xml.XMLConstants.FEATURE_SECURE_PROCESSING; + public class Configuration implements ErrorHandler { @@ -179,14 +181,19 @@ public Configuration(String configFilename, Optional existHomeDirname) thr // initialize xml parser // we use eXist's in-memory DOM implementation to work // around a bug in Xerces - final SAXParserFactory factory = SAXParserFactory.newInstance(); - factory.setNamespaceAware( true ); + final SAXParserFactory factory = ExistSAXParserFactory.getSAXParserFactory(); + factory.setNamespaceAware(true); // factory.setFeature("http://apache.org/xml/features/validation/schema", true); // factory.setFeature("http://apache.org/xml/features/validation/dynamic", true); final InputSource src = new InputSource(is); final SAXParser parser = factory.newSAXParser(); final XMLReader reader = parser.getXMLReader(); + + reader.setFeature("http://xml.org/sax/features/external-general-entities", false); + reader.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + reader.setFeature(FEATURE_SECURE_PROCESSING, true); + final SAXAdapter adapter = new SAXAdapter(); reader.setContentHandler(adapter); reader.parse(src); diff --git a/src/org/exist/util/ExistSAXParserFactory.java b/src/org/exist/util/ExistSAXParserFactory.java index 92de3db405b..d8d063dbc38 100644 --- a/src/org/exist/util/ExistSAXParserFactory.java +++ b/src/org/exist/util/ExistSAXParserFactory.java @@ -21,6 +21,7 @@ */ package org.exist.util; +import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import javax.xml.parsers.SAXParserFactory; @@ -28,30 +29,29 @@ import org.apache.logging.log4j.Logger; /** - * Helper class for creating an instance of javax.xml.parsers.SAXParserFactory - * + * Helper class for creating an instance of javax.xml.parsers.SAXParserFactory + * * @author dizzzz@exist-db.org */ public class ExistSAXParserFactory { private final static Logger LOG = LogManager.getLogger(ExistSAXParserFactory.class); - public final static String ORG_EXIST_SAXPARSERFACTORY="org.exist.SAXParserFactory"; + public final static String ORG_EXIST_SAXPARSERFACTORY = "org.exist.SAXParserFactory"; /** - * Get SAXParserFactory instance specified by factory class name. + * Get SAXParserFactory instance specified by factory class name. * * @param className Full class name of factory - * * @return A Sax parser factory or NULL when not available. */ - public static SAXParserFactory getSAXParserFactory(String className) { + public static SAXParserFactory getSAXParserFactory(final String className) { Class clazz = null; try { clazz = Class.forName(className); - } catch (final Exception ex) { // ClassNotFoundException + } catch (final ClassNotFoundException ex) { // quick escape LOG.debug(className + ": " + ex.getMessage(), ex); return null; @@ -61,7 +61,7 @@ public static SAXParserFactory getSAXParserFactory(String className) { Method method = null; try { method = clazz.getMethod("newInstance", (Class[]) null); - } catch (final Exception ex) { // SecurityException and NoSuchMethodException + } catch (final SecurityException | NoSuchMethodException ex) { // quick escape LOG.debug("Method " + className + ".newInstance not found.", ex); return null; @@ -72,7 +72,7 @@ public static SAXParserFactory getSAXParserFactory(String className) { try { result = method.invoke(null, (Object[]) null); - } catch (final Exception ex) { //IllegalAccessException and InvocationTargetException + } catch (final IllegalAccessException | InvocationTargetException ex) { // quick escape LOG.debug("Could not invoke method " + className + ".newInstance.", ex); return null; @@ -84,11 +84,10 @@ public static SAXParserFactory getSAXParserFactory(String className) { } return (SAXParserFactory) result; - } /** - * Get instance of a SAXParserFactory. Return factory specified by + * Get instance of a SAXParserFactory. Return factory specified by * system property org.exist.SAXParserFactory (if available) otherwise * return system default. * @@ -109,7 +108,7 @@ public static SAXParserFactory getSAXParserFactory() { // If no factory could be retrieved, create system default property. if (factory == null) { factory = SAXParserFactory.newInstance(); - LOG.debug(String.format("Using default SAXParserFactory '%s'", factory.getClass().getCanonicalName())); + LOG.info(String.format("Using default SAXParserFactory '%s'", factory.getClass().getCanonicalName())); } return factory; diff --git a/src/org/exist/util/MimeTable.java b/src/org/exist/util/MimeTable.java index 2be6f3097c0..a26677c8555 100644 --- a/src/org/exist/util/MimeTable.java +++ b/src/org/exist/util/MimeTable.java @@ -45,6 +45,8 @@ import org.xml.sax.XMLReader; import org.xml.sax.helpers.DefaultHandler; +import static javax.xml.XMLConstants.FEATURE_SECURE_PROCESSING; + /** * Global table of mime types. This singleton class maintains a list * of mime types known to the system. It is used to look up the @@ -273,12 +275,17 @@ private void load(final Path path) { * @throws IOException */ private void loadMimeTypes(InputStream stream) throws ParserConfigurationException, SAXException, IOException { - final SAXParserFactory factory = SAXParserFactory.newInstance(); + final SAXParserFactory factory = ExistSAXParserFactory.getSAXParserFactory(); factory.setNamespaceAware(true); factory.setValidating(false); final InputSource src = new InputSource(stream); final SAXParser parser = factory.newSAXParser(); final XMLReader reader = parser.getXMLReader(); + + reader.setFeature("http://xml.org/sax/features/external-general-entities", false); + reader.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + reader.setFeature(FEATURE_SECURE_PROCESSING, true); + reader.setContentHandler(new MimeTableHandler()); reader.parse(src); } diff --git a/src/org/exist/validation/Validator.java b/src/org/exist/validation/Validator.java index f56138c0281..ad3564f2395 100644 --- a/src/org/exist/validation/Validator.java +++ b/src/org/exist/validation/Validator.java @@ -36,6 +36,7 @@ import org.exist.Namespaces; import org.exist.storage.BrokerPool; import org.exist.util.Configuration; +import org.exist.util.ExistSAXParserFactory; import org.exist.util.XMLReaderObjectFactory; import org.exist.validation.resolver.AnyUriResolver; import org.exist.validation.resolver.SearchResourceResolver; @@ -46,6 +47,8 @@ import org.xml.sax.SAXException; import org.xml.sax.XMLReader; +import static javax.xml.XMLConstants.FEATURE_SECURE_PROCESSING; + /** * Validate XML documents with their grammars (DTD's and Schemas). * @@ -260,7 +263,7 @@ private XMLReader getXMLReader(ContentHandler contentHandler, ErrorHandler errorHandler) throws ParserConfigurationException, SAXException { // setup sax factory ; be sure just one instance! - final SAXParserFactory saxFactory = SAXParserFactory.newInstance(); + final SAXParserFactory saxFactory = ExistSAXParserFactory.getSAXParserFactory(); // Enable validation stuff saxFactory.setValidating(true); @@ -270,6 +273,8 @@ private XMLReader getXMLReader(ContentHandler contentHandler, final SAXParser saxParser = saxFactory.newSAXParser(); final XMLReader xmlReader = saxParser.getXMLReader(); + xmlReader.setFeature(FEATURE_SECURE_PROCESSING, true); + // Setup xmlreader xmlReader.setProperty(XMLReaderObjectFactory.APACHE_PROPERTIES_INTERNAL_GRAMMARPOOL, grammarPool); diff --git a/src/org/exist/xmldb/RemoteXMLResource.java b/src/org/exist/xmldb/RemoteXMLResource.java index e7ad90d9db8..1726400d4b2 100644 --- a/src/org/exist/xmldb/RemoteXMLResource.java +++ b/src/org/exist/xmldb/RemoteXMLResource.java @@ -26,6 +26,7 @@ import org.apache.xmlrpc.client.XmlRpcClient; import org.exist.Namespaces; import org.exist.dom.persistent.DocumentTypeImpl; +import org.exist.util.ExistSAXParserFactory; import org.exist.util.Leasable; import org.exist.util.MimeType; import org.exist.util.io.TemporaryFileManager; @@ -64,6 +65,8 @@ import java.util.Properties; import static java.nio.charset.StandardCharsets.UTF_8; +import static javax.xml.XMLConstants.FEATURE_SECURE_PROCESSING; + import java.util.Optional; public class RemoteXMLResource @@ -225,11 +228,12 @@ public void getContentAsSAX(final ContentHandler handler) throws XMLDBException XMLReader reader = xmlReader; if (reader == null) { - final SAXParserFactory saxFactory = SAXParserFactory.newInstance(); + final SAXParserFactory saxFactory = ExistSAXParserFactory.getSAXParserFactory(); saxFactory.setNamespaceAware(true); saxFactory.setValidating(false); final SAXParser sax = saxFactory.newSAXParser(); reader = sax.getXMLReader(); + reader.setFeature(FEATURE_SECURE_PROCESSING, true); } reader.setContentHandler(handler); diff --git a/src/org/exist/xquery/functions/fn/ParsingFunctions.java b/src/org/exist/xquery/functions/fn/ParsingFunctions.java index f39e604388f..aa979addb9f 100644 --- a/src/org/exist/xquery/functions/fn/ParsingFunctions.java +++ b/src/org/exist/xquery/functions/fn/ParsingFunctions.java @@ -29,6 +29,7 @@ import org.exist.dom.memtree.MemTreeBuilder; import org.exist.dom.memtree.NodeImpl; import org.exist.dom.memtree.SAXAdapter; +import org.exist.util.XMLReaderPool; import org.exist.validation.ValidationReport; import org.exist.xquery.*; import org.exist.xquery.functions.validation.Shared; @@ -45,9 +46,6 @@ import org.xml.sax.SAXException; import org.xml.sax.XMLReader; -import javax.xml.parsers.ParserConfigurationException; -import javax.xml.parsers.SAXParser; -import javax.xml.parsers.SAXParserFactory; import java.io.IOException; import java.io.StringReader; @@ -105,29 +103,24 @@ private Sequence parse(final InputSource src, XQueryContext theContext, Sequence Sequence resultSequence; final ValidationReport report = new ValidationReport(); final SAXAdapter adapter = new SAXAdapter(theContext); + + final XMLReaderPool parserPool = context.getBroker().getBrokerPool().getParserPool(); + XMLReader xr = null; try { - final SAXParserFactory factory = SAXParserFactory.newInstance(); - factory.setNamespaceAware(true); - - XMLReader xr = null; - - if (xr == null) { - final SAXParser parser = factory.newSAXParser(); - xr = parser.getXMLReader(); - } - + xr = parserPool.borrowXMLReader(); xr.setErrorHandler(report); xr.setContentHandler(adapter); xr.setProperty(Namespaces.SAX_LEXICAL_HANDLER, adapter); xr.parse(src); - - } catch (final ParserConfigurationException e) { - throw new XPathException(this, ErrorCodes.EXXQDY0002, "Error while constructing XML parser: " + e.getMessage(), args[0], e); } catch (final SAXException e) { logger.debug("Error while parsing XML: " + e.getMessage(), e); } catch (final IOException e) { throw new XPathException(this, ErrorCodes.FODC0006, ErrorCodes.FODC0006.getDescription() + ": " + e.getMessage(), args[0], e); + } finally { + if (xr != null) { + parserPool.returnXMLReader(xr); + } } if (report.isValid()) { diff --git a/src/org/exist/xquery/functions/util/Eval.java b/src/org/exist/xquery/functions/util/Eval.java index e8b1c907340..96f94e167c2 100644 --- a/src/org/exist/xquery/functions/util/Eval.java +++ b/src/org/exist/xquery/functions/util/Eval.java @@ -30,9 +30,6 @@ import java.util.SimpleTimeZone; import javax.xml.datatype.Duration; -import javax.xml.parsers.ParserConfigurationException; -import javax.xml.parsers.SAXParser; -import javax.xml.parsers.SAXParserFactory; import org.exist.Namespaces; import org.exist.dom.persistent.BinaryDocument; @@ -51,6 +48,7 @@ import org.exist.storage.DBBroker; import org.exist.storage.XQueryPool; import org.exist.storage.lock.Lock.LockMode; +import org.exist.util.XMLReaderPool; import org.exist.xmldb.XmldbURI; import org.exist.xquery.BasicFunction; import org.exist.xquery.Cardinality; @@ -552,28 +550,26 @@ private Sequence initContext(Node root, XQueryContext innerContext) throws XPath } private NodeImpl loadVarFromURI(String uri) throws XPathException { - try { + XMLReader xr = null; + final XMLReaderPool parserPool = context.getBroker().getBrokerPool().getParserPool(); + try { final URL url = new URL(uri); final InputStreamReader isr = new InputStreamReader(url.openStream(), "UTF-8"); - final SAXParserFactory factory = SAXParserFactory.newInstance(); - factory.setNamespaceAware(true); final InputSource src = new InputSource(isr); - final SAXParser parser = factory.newSAXParser(); - final XMLReader xr = parser.getXMLReader(); + + xr = parserPool.borrowXMLReader(); final SAXAdapter adapter = new SAXAdapter(context); xr.setContentHandler(adapter); xr.setProperty(Namespaces.SAX_LEXICAL_HANDLER, adapter); xr.parse(src); isr.close(); - return (NodeImpl) adapter.getDocument(); - } catch (final MalformedURLException e) { - throw new XPathException(this, e); - } catch (final IOException e) { + return adapter.getDocument(); + } catch (final SAXException | IOException e) { throw new XPathException(this, e); - } catch (final SAXException e) { - throw new XPathException(this, e); - } catch (final ParserConfigurationException e) { - throw new XPathException(this, e); + } finally { + if (xr != null) { + parserPool.returnXMLReader(xr); + } } } } diff --git a/src/org/exist/xquery/functions/util/Parse.java b/src/org/exist/xquery/functions/util/Parse.java index 7f9a0356a18..78f6156fc02 100644 --- a/src/org/exist/xquery/functions/util/Parse.java +++ b/src/org/exist/xquery/functions/util/Parse.java @@ -42,9 +42,6 @@ import org.xml.sax.SAXException; import org.xml.sax.XMLReader; -import javax.xml.parsers.ParserConfigurationException; -import javax.xml.parsers.SAXParser; -import javax.xml.parsers.SAXParserFactory; import java.io.IOException; import java.io.StringReader; import java.util.Optional; @@ -96,12 +93,9 @@ public Sequence eval(final Sequence[] args, final Sequence contextSequence) thro final StringReader reader = new StringReader(xmlContent); final ValidationReport report = new ValidationReport(); final SAXAdapter adapter = new SAXAdapter(context); + XMLReader xr = null; try { - final SAXParserFactory factory = SAXParserFactory.newInstance(); - factory.setNamespaceAware(true); final InputSource src = new InputSource(reader); - - final XMLReader xr; if (isCalledAs("parse-html")) { final Optional> maybeReaderInst = HtmlToXmlParser.getHtmlToXmlParser(context.getBroker().getConfiguration()); @@ -119,20 +113,21 @@ public Sequence eval(final Sequence[] args, final Sequence contextSequence) thro throw new XPathException(this, ErrorCodes.EXXQDY0002, "There is no HTML to XML parser configured in conf.xml"); } } else { - final SAXParser parser = factory.newSAXParser(); - xr = parser.getXMLReader(); + xr = context.getBroker().getBrokerPool().getParserPool().borrowXMLReader(); } xr.setErrorHandler(report); xr.setContentHandler(adapter); xr.setProperty(Namespaces.SAX_LEXICAL_HANDLER, adapter); xr.parse(src); - } catch (final ParserConfigurationException e) { - throw new XPathException(this, ErrorCodes.EXXQDY0002, "Error while constructing XML parser: " + e.getMessage(), args[0], e); } catch (final SAXException e) { logger.debug("Error while parsing XML: " + e.getMessage(), e); } catch (final IOException e) { throw new XPathException(this, ErrorCodes.EXXQDY0002, "Error while parsing XML: " + e.getMessage(), args[0], e); + } finally { + if (!isCalledAs("parse-html") && xr != null) { + context.getBroker().getBrokerPool().getParserPool().returnXMLReader(xr); + } } if (report.isValid()) { diff --git a/src/org/exist/xquery/functions/validation/Jaxp.java b/src/org/exist/xquery/functions/validation/Jaxp.java index 50860e8763c..53c33b7b2e5 100644 --- a/src/org/exist/xquery/functions/validation/Jaxp.java +++ b/src/org/exist/xquery/functions/validation/Jaxp.java @@ -45,6 +45,7 @@ import org.exist.dom.memtree.MemTreeBuilder; import org.exist.storage.BrokerPool; import org.exist.util.Configuration; +import org.exist.util.ExistSAXParserFactory; import org.exist.util.XMLReaderObjectFactory; import org.exist.util.io.TemporaryFileManager; import org.exist.validation.GrammarPool; @@ -72,6 +73,8 @@ import org.xml.sax.SAXNotSupportedException; import org.xml.sax.XMLReader; +import static javax.xml.XMLConstants.FEATURE_SECURE_PROCESSING; + /** * xQuery function for validation of XML instance documents * using grammars like XSDs and DTDs. @@ -310,7 +313,7 @@ public Sequence eval(Sequence[] args, Sequence contextSequence) private XMLReader getXMLReader() throws ParserConfigurationException, SAXException { // setup sax factory ; be sure just one instance! - final SAXParserFactory saxFactory = SAXParserFactory.newInstance(); + final SAXParserFactory saxFactory = ExistSAXParserFactory.getSAXParserFactory(); // Enable validation stuff saxFactory.setValidating(true); @@ -320,6 +323,8 @@ private XMLReader getXMLReader() throws ParserConfigurationException, SAXExcepti final SAXParser saxParser = saxFactory.newSAXParser(); final XMLReader xmlReader = saxParser.getXMLReader(); + xmlReader.setFeature(FEATURE_SECURE_PROCESSING, true); + setXmlReaderFeature(xmlReader, Namespaces.SAX_VALIDATION, true); setXmlReaderFeature(xmlReader, Namespaces.SAX_VALIDATION_DYNAMIC, false); setXmlReaderFeature(xmlReader, XMLReaderObjectFactory.APACHE_FEATURES_VALIDATION_SCHEMA, true); diff --git a/test/src/org/exist/config/ConfigurableTest.java b/test/src/org/exist/config/ConfigurableTest.java index e52d1e1bef7..39550c344a9 100644 --- a/test/src/org/exist/config/ConfigurableTest.java +++ b/test/src/org/exist/config/ConfigurableTest.java @@ -30,6 +30,7 @@ import com.googlecode.junittoolbox.ParallelRunner; import org.exist.dom.memtree.SAXAdapter; +import org.exist.util.ExistSAXParserFactory; import org.exist.util.io.FastByteArrayInputStream; import org.junit.Test; import org.junit.runner.RunWith; @@ -107,7 +108,7 @@ public void subelement() throws Exception { // initialize xml parser // we use eXist's in-memory DOM implementation to work // around a bug in Xerces - SAXParserFactory factory = SAXParserFactory.newInstance(); + SAXParserFactory factory = ExistSAXParserFactory.getSAXParserFactory(); factory.setNamespaceAware(true); InputSource src = new InputSource(is); SAXParser parser = factory.newSAXParser(); diff --git a/test/src/org/exist/dom/memtree/DOMTest.java b/test/src/org/exist/dom/memtree/DOMTest.java index b6f1d71923c..3c9831b61c5 100644 --- a/test/src/org/exist/dom/memtree/DOMTest.java +++ b/test/src/org/exist/dom/memtree/DOMTest.java @@ -16,6 +16,7 @@ import com.googlecode.junittoolbox.ParallelRunner; import org.exist.dom.QName; +import org.exist.util.ExistSAXParserFactory; import org.exist.util.serializer.DOMSerializer; import org.junit.Test; import org.junit.runner.RunWith; @@ -45,7 +46,7 @@ public class DOMTest { @Test public void documentBuilder() throws ParserConfigurationException, SAXException, IOException, TransformerException { DocumentBuilderReceiver receiver = new DocumentBuilderReceiver(); - SAXParserFactory factory = SAXParserFactory.newInstance(); + SAXParserFactory factory = ExistSAXParserFactory.getSAXParserFactory(); factory.setNamespaceAware(true); XMLReader reader = factory.newSAXParser().getXMLReader(); reader.setContentHandler(receiver); diff --git a/test/src/org/exist/dom/memtree/DocumentImplTest.java b/test/src/org/exist/dom/memtree/DocumentImplTest.java index 8753e6b0841..dd75d29251c 100644 --- a/test/src/org/exist/dom/memtree/DocumentImplTest.java +++ b/test/src/org/exist/dom/memtree/DocumentImplTest.java @@ -25,6 +25,7 @@ import net.sf.saxon.dom.DocumentBuilderImpl; import org.apache.xerces.dom.AttrNSImpl; import org.exist.Namespaces; +import org.exist.util.ExistSAXParserFactory; import org.exist.util.io.FastByteArrayInputStream; import org.junit.Test; import org.junit.runner.RunWith; @@ -168,7 +169,7 @@ private Document parseSaxon(final InputStream is) throws IOException, SAXExcepti } private DocumentImpl parseExist(final InputStream is) throws ParserConfigurationException, SAXException, IOException { - final SAXParserFactory saxParserFactory = SAXParserFactory.newInstance(); + final SAXParserFactory saxParserFactory = ExistSAXParserFactory.getSAXParserFactory(); final SAXParser saxParser = saxParserFactory.newSAXParser(); final XMLReader reader = saxParser.getXMLReader(); final InputSource src = new InputSource(is); diff --git a/test/src/org/exist/dom/memtree/MemtreeBuilderTest.java b/test/src/org/exist/dom/memtree/MemtreeBuilderTest.java index 3c7315316a8..3ebcdc8bff3 100644 --- a/test/src/org/exist/dom/memtree/MemtreeBuilderTest.java +++ b/test/src/org/exist/dom/memtree/MemtreeBuilderTest.java @@ -22,6 +22,7 @@ import com.googlecode.junittoolbox.ParallelParameterized; import org.exist.Namespaces; +import org.exist.util.ExistSAXParserFactory; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; @@ -80,7 +81,7 @@ public void parseSimple() throws IOException, SAXException, ParserConfigurationE } private DocumentImpl parse(final String xml) throws ParserConfigurationException, SAXException, IOException { - final SAXParserFactory saxParserFactory = SAXParserFactory.newInstance(); + final SAXParserFactory saxParserFactory = ExistSAXParserFactory.getSAXParserFactory(); saxParserFactory.setNamespaceAware(namespaceAware); final SAXAdapter saxAdapter = new SAXAdapter(); diff --git a/test/src/org/exist/http/RESTServiceTest.java b/test/src/org/exist/http/RESTServiceTest.java index ae5018d0b8d..5696d0ee192 100644 --- a/test/src/org/exist/http/RESTServiceTest.java +++ b/test/src/org/exist/http/RESTServiceTest.java @@ -37,6 +37,7 @@ import org.exist.dom.memtree.SAXAdapter; import org.exist.test.ExistWebServer; import org.exist.util.Base64Encoder; +import org.exist.util.ExistSAXParserFactory; import org.exist.xmldb.XmldbURI; import org.junit.runner.RunWith; import org.xml.sax.InputSource; @@ -866,7 +867,7 @@ private String readResponse(final InputStream is) throws IOException { } private int parseResponse(final String data) throws IOException, SAXException, ParserConfigurationException { - final SAXParserFactory factory = SAXParserFactory.newInstance(); + final SAXParserFactory factory = ExistSAXParserFactory.getSAXParserFactory(); factory.setNamespaceAware(true); final InputSource src = new InputSource(new StringReader(data)); final SAXParser parser = factory.newSAXParser(); diff --git a/test/src/org/exist/test/runner/XMLTestRunner.java b/test/src/org/exist/test/runner/XMLTestRunner.java index 1fc920f5094..0c6e811bd33 100644 --- a/test/src/org/exist/test/runner/XMLTestRunner.java +++ b/test/src/org/exist/test/runner/XMLTestRunner.java @@ -28,6 +28,7 @@ import org.exist.source.ClassLoaderSource; import org.exist.source.Source; import org.exist.util.DatabaseConfigurationException; +import org.exist.util.ExistSAXParserFactory; import org.exist.xquery.*; import org.exist.xquery.value.*; import org.junit.runner.Description; @@ -46,6 +47,8 @@ import java.nio.file.Path; import java.util.*; +import static javax.xml.XMLConstants.FEATURE_SECURE_PROCESSING; + /** * A JUnit test runner which can run the XML formatter XQuery tests * of eXist-db using $EXIST_HOME/src/org/exist/xquery/lib/test.xq. @@ -54,7 +57,7 @@ */ public class XMLTestRunner extends AbstractTestRunner { - private static final SAXParserFactory SAX_PARSER_FACTORY = SAXParserFactory.newInstance(); + private static final SAXParserFactory SAX_PARSER_FACTORY = ExistSAXParserFactory.getSAXParserFactory(); static { SAX_PARSER_FACTORY.setNamespaceAware(true); } @@ -167,6 +170,10 @@ private Document parse(final Path path) throws ParserConfigurationException, IOE final SAXParser parser = SAX_PARSER_FACTORY.newSAXParser(); final XMLReader xr = parser.getXMLReader(); + xr.setFeature("http://xml.org/sax/features/external-general-entities", false); + xr.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + xr.setFeature(FEATURE_SECURE_PROCESSING, true); + // we have to use eXist-db's SAXAdapter, otherwise un-referenced namespaces as used by xpath assertions may be stripped by Xerces. final SAXAdapter adapter = new SAXAdapter(); xr.setContentHandler(adapter); diff --git a/test/src/org/exist/w3c/tests/TestCase.java b/test/src/org/exist/w3c/tests/TestCase.java index 89bdd4b87e2..7ba2ec2c430 100644 --- a/test/src/org/exist/w3c/tests/TestCase.java +++ b/test/src/org/exist/w3c/tests/TestCase.java @@ -51,6 +51,7 @@ import org.exist.storage.DBBroker; import org.exist.storage.serializers.EXistOutputKeys; import org.exist.test.ExistEmbeddedServer; +import org.exist.util.ExistSAXParserFactory; import org.exist.util.FileUtils; import org.exist.xmldb.LocalCollection; import org.exist.xmldb.LocalXMLResource; @@ -331,7 +332,7 @@ public Resource getResource(Object r) throws XMLDBException { public NodeImpl loadVarFromURI(XQueryContext context, String uri) throws IOException { SAXAdapter adapter = new SAXAdapter(context); - SAXParserFactory factory = SAXParserFactory.newInstance(); + SAXParserFactory factory = ExistSAXParserFactory.getSAXParserFactory(); factory.setNamespaceAware(true); XMLReader xr; @@ -379,7 +380,7 @@ public NodeImpl loadVarFromURI(XQueryContext context, String uri) throws IOExcep public NodeImpl loadVarFromString(XQueryContext context, String source) throws IOException { SAXAdapter adapter = new SAXAdapter(context); - SAXParserFactory factory = SAXParserFactory.newInstance(); + SAXParserFactory factory = ExistSAXParserFactory.getSAXParserFactory(); factory.setNamespaceAware(true); XMLReader xr; diff --git a/test/src/org/exist/xmldb/ResourceTest.java b/test/src/org/exist/xmldb/ResourceTest.java index a2f5bbf4813..123be9b08e4 100644 --- a/test/src/org/exist/xmldb/ResourceTest.java +++ b/test/src/org/exist/xmldb/ResourceTest.java @@ -41,6 +41,7 @@ import org.exist.dom.QName; import org.exist.security.Account; import org.exist.test.ExistXmldbEmbeddedServer; +import org.exist.util.ExistSAXParserFactory; import org.exist.util.serializer.AttrList; import org.exist.util.serializer.SAXSerializer; import org.exist.util.XMLFilenameFilter; @@ -183,7 +184,7 @@ public void setContentAsSAX() throws SAXException, ParserConfigurationException, + "Paragraph2" + ""; ContentHandler handler = doc.setContentAsSAX(); - SAXParserFactory saxFactory = SAXParserFactory.newInstance(); + SAXParserFactory saxFactory = ExistSAXParserFactory.getSAXParserFactory(); saxFactory.setNamespaceAware(true); saxFactory.setValidating(false); SAXParser sax = saxFactory.newSAXParser();