From a4c2f291d986ccf988f37af5466265a7f534c16a Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Tue, 21 May 2024 11:16:19 +0200 Subject: [PATCH] Avoid creation of SAXParserFactory for every read operation Includes JAXBContext locking revision (avoiding synchronization) and consistent treatment of DocumentBuilderFactory (in terms of caching as well as locking). Closes gh-32851 --- .../oxm/jaxb/Jaxb2Marshaller.java | 47 +++++++++--- .../oxm/support/AbstractMarshaller.java | 46 +++++++----- .../Jaxb2RootElementHttpMessageConverter.java | 23 ++++-- .../xml/SourceHttpMessageConverter.java | 74 +++++++++++++------ 4 files changed, 132 insertions(+), 58 deletions(-) diff --git a/spring-oxm/src/main/java/org/springframework/oxm/jaxb/Jaxb2Marshaller.java b/spring-oxm/src/main/java/org/springframework/oxm/jaxb/Jaxb2Marshaller.java index 9ed4a30a7c00..2d903c092cd8 100644 --- a/spring-oxm/src/main/java/org/springframework/oxm/jaxb/Jaxb2Marshaller.java +++ b/spring-oxm/src/main/java/org/springframework/oxm/jaxb/Jaxb2Marshaller.java @@ -37,6 +37,8 @@ import java.util.Date; import java.util.Map; import java.util.UUID; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import javax.xml.XMLConstants; import javax.xml.datatype.Duration; @@ -192,7 +194,7 @@ public class Jaxb2Marshaller implements MimeMarshaller, MimeUnmarshaller, Generi @Nullable private ClassLoader beanClassLoader; - private final Object jaxbContextMonitor = new Object(); + private final Lock jaxbContextLock = new ReentrantLock(); @Nullable private volatile JAXBContext jaxbContext; @@ -204,6 +206,12 @@ public class Jaxb2Marshaller implements MimeMarshaller, MimeUnmarshaller, Generi private boolean processExternalEntities = false; + @Nullable + private volatile SAXParserFactory schemaParserFactory; + + @Nullable + private volatile SAXParserFactory sourceParserFactory; + /** * Set multiple JAXB context paths. The given array of context paths gets @@ -426,6 +434,7 @@ public void setMappedClass(Class mappedClass) { */ public void setSupportDtd(boolean supportDtd) { this.supportDtd = supportDtd; + this.sourceParserFactory = null; } /** @@ -450,6 +459,7 @@ public void setProcessExternalEntities(boolean processExternalEntities) { if (processExternalEntities) { this.supportDtd = true; } + this.sourceParserFactory = null; } /** @@ -497,7 +507,9 @@ public JAXBContext getJaxbContext() { if (context != null) { return context; } - synchronized (this.jaxbContextMonitor) { + + this.jaxbContextLock.lock(); + try { context = this.jaxbContext; if (context == null) { try { @@ -521,6 +533,9 @@ else if (!ObjectUtils.isEmpty(this.packagesToScan)) { } return context; } + finally { + this.jaxbContextLock.unlock(); + } } private JAXBContext createJaxbContextFromContextPath(String contextPath) throws JAXBException { @@ -587,17 +602,24 @@ private Schema loadSchema(Resource[] resources, String schemaLanguage) throws IO Assert.notEmpty(resources, "No resources given"); Assert.hasLength(schemaLanguage, "No schema language provided"); Source[] schemaSources = new Source[resources.length]; - SAXParserFactory saxParserFactory = SAXParserFactory.newInstance(); - saxParserFactory.setNamespaceAware(true); - saxParserFactory.setFeature("http://xml.org/sax/features/namespace-prefixes", true); + + SAXParserFactory saxParserFactory = this.schemaParserFactory; + if (saxParserFactory == null) { + saxParserFactory = SAXParserFactory.newInstance(); + saxParserFactory.setNamespaceAware(true); + saxParserFactory.setFeature("http://xml.org/sax/features/namespace-prefixes", true); + this.schemaParserFactory = saxParserFactory; + } SAXParser saxParser = saxParserFactory.newSAXParser(); XMLReader xmlReader = saxParser.getXMLReader(); + for (int i = 0; i < resources.length; i++) { Resource resource = resources[i]; Assert.isTrue(resource != null && resource.exists(), () -> "Resource does not exist: " + resource); InputSource inputSource = SaxResourceUtils.createInputSource(resource); schemaSources[i] = new SAXSource(xmlReader, inputSource); } + SchemaFactory schemaFactory = SchemaFactory.newInstance(schemaLanguage); if (this.schemaResourceResolver != null) { schemaFactory.setResourceResolver(this.schemaResourceResolver); @@ -886,11 +908,16 @@ else if (streamSource.getReader() != null) { try { if (xmlReader == null) { - SAXParserFactory saxParserFactory = SAXParserFactory.newInstance(); - saxParserFactory.setNamespaceAware(true); - saxParserFactory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", !isSupportDtd()); - String name = "http://xml.org/sax/features/external-general-entities"; - saxParserFactory.setFeature(name, isProcessExternalEntities()); + SAXParserFactory saxParserFactory = this.sourceParserFactory; + if (saxParserFactory == null) { + saxParserFactory = SAXParserFactory.newInstance(); + saxParserFactory.setNamespaceAware(true); + saxParserFactory.setFeature( + "http://apache.org/xml/features/disallow-doctype-decl", !isSupportDtd()); + saxParserFactory.setFeature( + "http://xml.org/sax/features/external-general-entities", isProcessExternalEntities()); + this.sourceParserFactory = saxParserFactory; + } SAXParser saxParser = saxParserFactory.newSAXParser(); xmlReader = saxParser.getXMLReader(); } diff --git a/spring-oxm/src/main/java/org/springframework/oxm/support/AbstractMarshaller.java b/spring-oxm/src/main/java/org/springframework/oxm/support/AbstractMarshaller.java index 7c6182c4a77d..a3925acb5039 100644 --- a/spring-oxm/src/main/java/org/springframework/oxm/support/AbstractMarshaller.java +++ b/spring-oxm/src/main/java/org/springframework/oxm/support/AbstractMarshaller.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2023 the original author or authors. + * Copyright 2002-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -83,9 +83,10 @@ public abstract class AbstractMarshaller implements Marshaller, Unmarshaller { private boolean processExternalEntities = false; @Nullable - private DocumentBuilderFactory documentBuilderFactory; + private volatile DocumentBuilderFactory documentBuilderFactory; - private final Object documentBuilderFactoryMonitor = new Object(); + @Nullable + private volatile SAXParserFactory saxParserFactory; /** @@ -94,6 +95,8 @@ public abstract class AbstractMarshaller implements Marshaller, Unmarshaller { */ public void setSupportDtd(boolean supportDtd) { this.supportDtd = supportDtd; + this.documentBuilderFactory = null; + this.saxParserFactory = null; } /** @@ -118,6 +121,8 @@ public void setProcessExternalEntities(boolean processExternalEntities) { if (processExternalEntities) { this.supportDtd = true; } + this.documentBuilderFactory = null; + this.saxParserFactory = null; } /** @@ -137,14 +142,13 @@ public boolean isProcessExternalEntities() { */ protected Document buildDocument() { try { - DocumentBuilder documentBuilder; - synchronized (this.documentBuilderFactoryMonitor) { - if (this.documentBuilderFactory == null) { - this.documentBuilderFactory = createDocumentBuilderFactory(); - } - documentBuilder = createDocumentBuilder(this.documentBuilderFactory); + DocumentBuilderFactory builderFactory = this.documentBuilderFactory; + if (builderFactory == null) { + builderFactory = createDocumentBuilderFactory(); + this.documentBuilderFactory = builderFactory; } - return documentBuilder.newDocument(); + DocumentBuilder builder = createDocumentBuilder(builderFactory); + return builder.newDocument(); } catch (ParserConfigurationException ex) { throw new UnmarshallingFailureException("Could not create document placeholder: " + ex.getMessage(), ex); @@ -179,11 +183,11 @@ protected DocumentBuilderFactory createDocumentBuilderFactory() throws ParserCon protected DocumentBuilder createDocumentBuilder(DocumentBuilderFactory factory) throws ParserConfigurationException { - DocumentBuilder documentBuilder = factory.newDocumentBuilder(); + DocumentBuilder builder = factory.newDocumentBuilder(); if (!isProcessExternalEntities()) { - documentBuilder.setEntityResolver(NO_OP_ENTITY_RESOLVER); + builder.setEntityResolver(NO_OP_ENTITY_RESOLVER); } - return documentBuilder; + return builder; } /** @@ -193,11 +197,17 @@ protected DocumentBuilder createDocumentBuilder(DocumentBuilderFactory factory) * @throws ParserConfigurationException if thrown by JAXP methods */ protected XMLReader createXmlReader() throws SAXException, ParserConfigurationException { - SAXParserFactory saxParserFactory = SAXParserFactory.newInstance(); - saxParserFactory.setNamespaceAware(true); - saxParserFactory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", !isSupportDtd()); - saxParserFactory.setFeature("http://xml.org/sax/features/external-general-entities", isProcessExternalEntities()); - SAXParser saxParser = saxParserFactory.newSAXParser(); + SAXParserFactory parserFactory = this.saxParserFactory; + if (parserFactory == null) { + parserFactory = SAXParserFactory.newInstance(); + parserFactory.setNamespaceAware(true); + parserFactory.setFeature( + "http://apache.org/xml/features/disallow-doctype-decl", !isSupportDtd()); + parserFactory.setFeature( + "http://xml.org/sax/features/external-general-entities", isProcessExternalEntities()); + this.saxParserFactory = parserFactory; + } + SAXParser saxParser = parserFactory.newSAXParser(); XMLReader xmlReader = saxParser.getXMLReader(); if (!isProcessExternalEntities()) { xmlReader.setEntityResolver(NO_OP_ENTITY_RESOLVER); diff --git a/spring-web/src/main/java/org/springframework/http/converter/xml/Jaxb2RootElementHttpMessageConverter.java b/spring-web/src/main/java/org/springframework/http/converter/xml/Jaxb2RootElementHttpMessageConverter.java index a08e1d2172ee..dda603995a16 100644 --- a/spring-web/src/main/java/org/springframework/http/converter/xml/Jaxb2RootElementHttpMessageConverter.java +++ b/spring-web/src/main/java/org/springframework/http/converter/xml/Jaxb2RootElementHttpMessageConverter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2023 the original author or authors. + * Copyright 2002-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -61,6 +61,7 @@ * @author Arjen Poutsma * @author Sebastien Deleuze * @author Rossen Stoyanchev + * @author Juergen Hoeller * @since 3.0 * @see MarshallingHttpMessageConverter */ @@ -70,6 +71,9 @@ public class Jaxb2RootElementHttpMessageConverter extends AbstractJaxb2HttpMessa private boolean processExternalEntities = false; + @Nullable + private volatile SAXParserFactory sourceParserFactory; + /** * Indicate whether DTD parsing should be supported. @@ -77,6 +81,7 @@ public class Jaxb2RootElementHttpMessageConverter extends AbstractJaxb2HttpMessa */ public void setSupportDtd(boolean supportDtd) { this.supportDtd = supportDtd; + this.sourceParserFactory = null; } /** @@ -97,6 +102,7 @@ public void setProcessExternalEntities(boolean processExternalEntities) { if (processExternalEntities) { this.supportDtd = true; } + this.sourceParserFactory = null; } /** @@ -156,11 +162,16 @@ protected Source processSource(Source source) { if (source instanceof StreamSource streamSource) { InputSource inputSource = new InputSource(streamSource.getInputStream()); try { - SAXParserFactory saxParserFactory = SAXParserFactory.newInstance(); - saxParserFactory.setNamespaceAware(true); - saxParserFactory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", !isSupportDtd()); - String featureName = "http://xml.org/sax/features/external-general-entities"; - saxParserFactory.setFeature(featureName, isProcessExternalEntities()); + SAXParserFactory saxParserFactory = this.sourceParserFactory; + if (saxParserFactory == null) { + saxParserFactory = SAXParserFactory.newInstance(); + saxParserFactory.setNamespaceAware(true); + saxParserFactory.setFeature( + "http://apache.org/xml/features/disallow-doctype-decl", !isSupportDtd()); + saxParserFactory.setFeature( + "http://xml.org/sax/features/external-general-entities", isProcessExternalEntities()); + this.sourceParserFactory = saxParserFactory; + } SAXParser saxParser = saxParserFactory.newSAXParser(); XMLReader xmlReader = saxParser.getXMLReader(); if (!isProcessExternalEntities()) { diff --git a/spring-web/src/main/java/org/springframework/http/converter/xml/SourceHttpMessageConverter.java b/spring-web/src/main/java/org/springframework/http/converter/xml/SourceHttpMessageConverter.java index d8355227767f..c99d0bb033b7 100644 --- a/spring-web/src/main/java/org/springframework/http/converter/xml/SourceHttpMessageConverter.java +++ b/spring-web/src/main/java/org/springframework/http/converter/xml/SourceHttpMessageConverter.java @@ -63,6 +63,7 @@ * * @author Arjen Poutsma * @author Rossen Stoyanchev + * @author Juergen Hoeller * @since 3.0 * @param the converted object type */ @@ -75,11 +76,7 @@ public class SourceHttpMessageConverter extends AbstractHttpMe (publicID, systemID, base, ns) -> InputStream.nullInputStream(); private static final Set> SUPPORTED_CLASSES = Set.of( - DOMSource.class, - SAXSource.class, - StAXSource.class, - StreamSource.class, - Source.class); + DOMSource.class, SAXSource.class, StAXSource.class, StreamSource.class, Source.class); private final TransformerFactory transformerFactory = TransformerFactory.newInstance(); @@ -88,6 +85,15 @@ public class SourceHttpMessageConverter extends AbstractHttpMe private boolean processExternalEntities = false; + @Nullable + private volatile DocumentBuilderFactory documentBuilderFactory; + + @Nullable + private volatile SAXParserFactory saxParserFactory; + + @Nullable + private volatile XMLInputFactory xmlInputFactory; + /** * Sets the {@link #setSupportedMediaTypes(java.util.List) supportedMediaTypes} @@ -104,6 +110,9 @@ public SourceHttpMessageConverter() { */ public void setSupportDtd(boolean supportDtd) { this.supportDtd = supportDtd; + this.documentBuilderFactory = null; + this.saxParserFactory = null; + this.xmlInputFactory = null; } /** @@ -124,6 +133,9 @@ public void setProcessExternalEntities(boolean processExternalEntities) { if (processExternalEntities) { this.supportDtd = true; } + this.documentBuilderFactory = null; + this.saxParserFactory = null; + this.xmlInputFactory = null; } /** @@ -165,17 +177,21 @@ else if (StreamSource.class == clazz || Source.class == clazz) { private DOMSource readDOMSource(InputStream body, HttpInputMessage inputMessage) throws IOException { try { - DocumentBuilderFactory documentBuilderFactory = DocumentBuilderFactory.newInstance(); - documentBuilderFactory.setNamespaceAware(true); - documentBuilderFactory.setFeature( - "http://apache.org/xml/features/disallow-doctype-decl", !isSupportDtd()); - documentBuilderFactory.setFeature( - "http://xml.org/sax/features/external-general-entities", isProcessExternalEntities()); - DocumentBuilder documentBuilder = documentBuilderFactory.newDocumentBuilder(); + DocumentBuilderFactory builderFactory = this.documentBuilderFactory; + if (builderFactory == null) { + builderFactory = DocumentBuilderFactory.newInstance(); + builderFactory.setNamespaceAware(true); + builderFactory.setFeature( + "http://apache.org/xml/features/disallow-doctype-decl", !isSupportDtd()); + builderFactory.setFeature( + "http://xml.org/sax/features/external-general-entities", isProcessExternalEntities()); + this.documentBuilderFactory = builderFactory; + } + DocumentBuilder builder = builderFactory.newDocumentBuilder(); if (!isProcessExternalEntities()) { - documentBuilder.setEntityResolver(NO_OP_ENTITY_RESOLVER); + builder.setEntityResolver(NO_OP_ENTITY_RESOLVER); } - Document document = documentBuilder.parse(body); + Document document = builder.parse(body); return new DOMSource(document); } catch (NullPointerException ex) { @@ -197,11 +213,17 @@ private DOMSource readDOMSource(InputStream body, HttpInputMessage inputMessage) private SAXSource readSAXSource(InputStream body, HttpInputMessage inputMessage) throws IOException { try { - SAXParserFactory saxParserFactory = SAXParserFactory.newInstance(); - saxParserFactory.setNamespaceAware(true); - saxParserFactory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", !isSupportDtd()); - saxParserFactory.setFeature("http://xml.org/sax/features/external-general-entities", isProcessExternalEntities()); - SAXParser saxParser = saxParserFactory.newSAXParser(); + SAXParserFactory parserFactory = this.saxParserFactory; + if (parserFactory == null) { + parserFactory = SAXParserFactory.newInstance(); + parserFactory.setNamespaceAware(true); + parserFactory.setFeature( + "http://apache.org/xml/features/disallow-doctype-decl", !isSupportDtd()); + parserFactory.setFeature( + "http://xml.org/sax/features/external-general-entities", isProcessExternalEntities()); + this.saxParserFactory = parserFactory; + } + SAXParser saxParser = parserFactory.newSAXParser(); XMLReader xmlReader = saxParser.getXMLReader(); if (!isProcessExternalEntities()) { xmlReader.setEntityResolver(NO_OP_ENTITY_RESOLVER); @@ -217,11 +239,15 @@ private SAXSource readSAXSource(InputStream body, HttpInputMessage inputMessage) private Source readStAXSource(InputStream body, HttpInputMessage inputMessage) { try { - XMLInputFactory inputFactory = XMLInputFactory.newInstance(); - inputFactory.setProperty(XMLInputFactory.SUPPORT_DTD, isSupportDtd()); - inputFactory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, isProcessExternalEntities()); - if (!isProcessExternalEntities()) { - inputFactory.setXMLResolver(NO_OP_XML_RESOLVER); + XMLInputFactory inputFactory = this.xmlInputFactory; + if (inputFactory == null) { + inputFactory = XMLInputFactory.newInstance(); + inputFactory.setProperty(XMLInputFactory.SUPPORT_DTD, isSupportDtd()); + inputFactory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, isProcessExternalEntities()); + if (!isProcessExternalEntities()) { + inputFactory.setXMLResolver(NO_OP_XML_RESOLVER); + } + this.xmlInputFactory = inputFactory; } XMLStreamReader streamReader = inputFactory.createXMLStreamReader(body); return new StAXSource(streamReader);