From af7a5716e800db463f0e6913eb729f1bae050eb9 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Tue, 21 Apr 2026 20:34:29 +0200 Subject: [PATCH 1/2] Consistently ignore exceptions for Xerces-specific properties Closes gh-36682 --- .../oxm/jaxb/Jaxb2Marshaller.java | 36 ++++++---- .../oxm/support/AbstractMarshaller.java | 54 ++++++++++---- .../Jaxb2RootElementHttpMessageConverter.java | 44 +++++++----- .../xml/SourceHttpMessageConverter.java | 70 +++++++++++++------ 4 files changed, 140 insertions(+), 64 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 e7d5ae287d3..32a95381414 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 @@ -892,17 +892,29 @@ public class Jaxb2Marshaller implements MimeMarshaller, MimeUnmarshaller, Generi // By default, Spring will prevent the processing of external entities. // This is a mitigation against XXE attacks. if (xmlReader == null) { - 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; + SAXParserFactory factory = this.sourceParserFactory; + if (factory == null) { + factory = SAXParserFactory.newInstance(); + factory.setNamespaceAware(true); + try { + factory.setFeature( + "http://apache.org/xml/features/disallow-doctype-decl", !isSupportDtd()); + } + catch (Exception ex) { + // Xerces properties not recognized/supported - ignore + } + try { + factory.setFeature( + "http://xml.org/sax/features/external-general-entities", isProcessExternalEntities()); + factory.setFeature( + "http://xml.org/sax/features/external-parameter-entities", isProcessExternalEntities()); + } + catch (Exception ex) { + // SAX properties not recognized/supported - ignore + } + this.sourceParserFactory = factory; } - SAXParser saxParser = saxParserFactory.newSAXParser(); + SAXParser saxParser = factory.newSAXParser(); xmlReader = saxParser.getXMLReader(); } if (!isProcessExternalEntities()) { @@ -910,8 +922,8 @@ public class Jaxb2Marshaller implements MimeMarshaller, MimeUnmarshaller, Generi } return new SAXSource(xmlReader, inputSource); } - catch (SAXException | ParserConfigurationException ex) { - logger.info("Processing of external entities could not be disabled", ex); + catch (Exception ex) { + logger.warn("Processing of external entities could not be disabled", ex); return source; } } 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 1f142540952..1cdfc56d3c3 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 @@ -75,6 +75,7 @@ public abstract class AbstractMarshaller implements Marshaller, Unmarshaller { private static final EntityResolver NO_OP_ENTITY_RESOLVER = (publicId, systemId) -> new InputSource(new StringReader("")); + /** Logger available to subclasses. */ protected final Log logger = LogFactory.getLog(getClass()); @@ -165,8 +166,22 @@ public abstract class AbstractMarshaller implements Marshaller, Unmarshaller { DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); factory.setValidating(false); factory.setNamespaceAware(true); - factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", !isSupportDtd()); - factory.setFeature("http://xml.org/sax/features/external-general-entities", isProcessExternalEntities()); + try { + factory.setFeature( + "http://apache.org/xml/features/disallow-doctype-decl", !isSupportDtd()); + } + catch (Exception ex) { + // Xerces properties not recognized/supported - ignore + } + try { + factory.setFeature( + "http://xml.org/sax/features/external-general-entities", isProcessExternalEntities()); + factory.setFeature( + "http://xml.org/sax/features/external-parameter-entities", isProcessExternalEntities()); + } + catch (Exception ex) { + // SAX properties not recognized/supported - ignore + } return factory; } @@ -195,17 +210,29 @@ public abstract class AbstractMarshaller implements Marshaller, Unmarshaller { * @throws ParserConfigurationException if thrown by JAXP methods */ protected XMLReader createXmlReader() throws SAXException, ParserConfigurationException { - 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; + SAXParserFactory factory = this.saxParserFactory; + if (factory == null) { + factory = SAXParserFactory.newInstance(); + factory.setNamespaceAware(true); + try { + factory.setFeature( + "http://apache.org/xml/features/disallow-doctype-decl", !isSupportDtd()); + } + catch (Exception ex) { + // Xerces properties not recognized/supported - ignore + } + try { + factory.setFeature( + "http://xml.org/sax/features/external-general-entities", isProcessExternalEntities()); + factory.setFeature( + "http://xml.org/sax/features/external-parameter-entities", isProcessExternalEntities()); + } + catch (Exception ex) { + // SAX properties not recognized/supported - ignore + } + this.saxParserFactory = factory; } - SAXParser saxParser = parserFactory.newSAXParser(); + SAXParser saxParser = factory.newSAXParser(); XMLReader xmlReader = saxParser.getXMLReader(); if (!isProcessExternalEntities()) { xmlReader.setEntityResolver(NO_OP_ENTITY_RESOLVER); @@ -456,8 +483,7 @@ public abstract class AbstractMarshaller implements Marshaller, Unmarshaller { catch (NullPointerException ex) { if (!isSupportDtd()) { throw new UnmarshallingFailureException("NPE while unmarshalling. " + - "This can happen on JDK 1.6 due to the presence of DTD " + - "declarations, which are disabled."); + "This can happen due to the presence of DTD declarations, which are disabled."); } throw ex; } 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 fee6d426eea..fc286d3d049 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 @@ -19,7 +19,6 @@ package org.springframework.http.converter.xml; import java.io.StringReader; import java.nio.charset.Charset; -import javax.xml.parsers.ParserConfigurationException; import javax.xml.parsers.SAXParser; import javax.xml.parsers.SAXParserFactory; import javax.xml.transform.Result; @@ -39,7 +38,6 @@ import jakarta.xml.bind.annotation.XmlType; import org.jspecify.annotations.Nullable; import org.xml.sax.EntityResolver; import org.xml.sax.InputSource; -import org.xml.sax.SAXException; import org.xml.sax.XMLReader; import org.springframework.core.annotation.AnnotationUtils; @@ -68,6 +66,10 @@ import org.springframework.util.ClassUtils; */ public class Jaxb2RootElementHttpMessageConverter extends AbstractJaxb2HttpMessageConverter { + private static final EntityResolver NO_OP_ENTITY_RESOLVER = + (publicId, systemId) -> new InputSource(new StringReader("")); + + private boolean supportDtd = false; private boolean processExternalEntities = false; @@ -176,24 +178,36 @@ public class Jaxb2RootElementHttpMessageConverter extends AbstractJaxb2HttpMessa try { // By default, Spring will prevent the processing of external entities. // This is a mitigation against XXE attacks. - 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; + SAXParserFactory factory = this.sourceParserFactory; + if (factory == null) { + factory = SAXParserFactory.newInstance(); + factory.setNamespaceAware(true); + try { + factory.setFeature( + "http://apache.org/xml/features/disallow-doctype-decl", !isSupportDtd()); + } + catch (Exception ex) { + // Xerces properties not recognized/supported - ignore + } + try { + factory.setFeature( + "http://xml.org/sax/features/external-general-entities", isProcessExternalEntities()); + factory.setFeature( + "http://xml.org/sax/features/external-parameter-entities", isProcessExternalEntities()); + } + catch (Exception ex) { + // SAX properties not recognized/supported - ignore + } + this.sourceParserFactory = factory; } - SAXParser saxParser = saxParserFactory.newSAXParser(); + SAXParser saxParser = factory.newSAXParser(); XMLReader xmlReader = saxParser.getXMLReader(); if (!isProcessExternalEntities()) { xmlReader.setEntityResolver(NO_OP_ENTITY_RESOLVER); } return new SAXSource(xmlReader, inputSource); } - catch (SAXException | ParserConfigurationException ex) { + catch (Exception ex) { logger.warn("Processing of external entities could not be disabled", ex); return source; } @@ -239,8 +253,4 @@ public class Jaxb2RootElementHttpMessageConverter extends AbstractJaxb2HttpMessa return true; } - - private static final EntityResolver NO_OP_ENTITY_RESOLVER = - (publicId, systemId) -> new InputSource(new StringReader("")); - } 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 3cc4cb533e4..dc5d04ae824 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 @@ -176,17 +176,29 @@ public class SourceHttpMessageConverter extends AbstractHttpMe try { // By default, Spring will prevent the processing of external entities. // This is a mitigation against XXE attacks. - 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; + DocumentBuilderFactory factory = this.documentBuilderFactory; + if (factory == null) { + factory = DocumentBuilderFactory.newInstance(); + factory.setNamespaceAware(true); + try { + factory.setFeature( + "http://apache.org/xml/features/disallow-doctype-decl", !isSupportDtd()); + } + catch (Exception ex) { + // Xerces properties not recognized/supported - ignore + } + try { + factory.setFeature( + "http://xml.org/sax/features/external-general-entities", isProcessExternalEntities()); + factory.setFeature( + "http://xml.org/sax/features/external-parameter-entities", isProcessExternalEntities()); + } + catch (Exception ex) { + // SAX properties not recognized/supported - ignore + } + this.documentBuilderFactory = factory; } - DocumentBuilder builder = builderFactory.newDocumentBuilder(); + DocumentBuilder builder = factory.newDocumentBuilder(); if (!isProcessExternalEntities()) { builder.setEntityResolver(NO_OP_ENTITY_RESOLVER); } @@ -212,17 +224,29 @@ public class SourceHttpMessageConverter extends AbstractHttpMe private SAXSource readSAXSource(InputStream body, HttpInputMessage inputMessage) throws IOException { try { - 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; + SAXParserFactory factory = this.saxParserFactory; + if (factory == null) { + factory = SAXParserFactory.newInstance(); + factory.setNamespaceAware(true); + try { + factory.setFeature( + "http://apache.org/xml/features/disallow-doctype-decl", !isSupportDtd()); + } + catch (Exception ex) { + // Xerces properties not recognized/supported - ignore + } + try { + factory.setFeature( + "http://xml.org/sax/features/external-general-entities", isProcessExternalEntities()); + factory.setFeature( + "http://xml.org/sax/features/external-parameter-entities", isProcessExternalEntities()); + } + catch (Exception ex) { + // SAX properties not recognized/supported - ignore + } + this.saxParserFactory = factory; } - SAXParser saxParser = parserFactory.newSAXParser(); + SAXParser saxParser = factory.newSAXParser(); XMLReader xmlReader = saxParser.getXMLReader(); if (!isProcessExternalEntities()) { xmlReader.setEntityResolver(NO_OP_ENTITY_RESOLVER); @@ -230,7 +254,11 @@ public class SourceHttpMessageConverter extends AbstractHttpMe byte[] bytes = StreamUtils.copyToByteArray(body); return new SAXSource(xmlReader, new InputSource(new ByteArrayInputStream(bytes))); } - catch (SAXException | ParserConfigurationException ex) { + catch (ParserConfigurationException ex) { + throw new HttpMessageNotReadableException( + "Could not set feature: " + ex.getMessage(), ex, inputMessage); + } + catch (SAXException ex) { throw new HttpMessageNotReadableException( "Could not parse document: " + ex.getMessage(), ex, inputMessage); } From 8965d9bccf408f7be20108cd497af17fb48becf6 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Tue, 21 Apr 2026 20:35:25 +0200 Subject: [PATCH 2/2] Polishing --- .../util/ConcurrentLruCache.java | 75 +++++++++---------- 1 file changed, 35 insertions(+), 40 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/util/ConcurrentLruCache.java b/spring-core/src/main/java/org/springframework/util/ConcurrentLruCache.java index ee11b0ca8f7..279e75b0ebf 100644 --- a/spring-core/src/main/java/org/springframework/util/ConcurrentLruCache.java +++ b/spring-core/src/main/java/org/springframework/util/ConcurrentLruCache.java @@ -102,7 +102,7 @@ public final class ConcurrentLruCache { if (this.capacity == 0) { return this.generator.apply(key); } - final Node node = this.cache.get(key); + Node node = this.cache.get(key); if (node == null) { V value = this.generator.apply(key); put(key, value); @@ -115,9 +115,9 @@ public final class ConcurrentLruCache { private void put(K key, V value) { Assert.notNull(key, "key must not be null"); Assert.notNull(value, "value must not be null"); - final CacheEntry cacheEntry = new CacheEntry<>(value, CacheEntryState.ACTIVE); - final Node node = new Node<>(key, cacheEntry); - final Node prior = this.cache.putIfAbsent(node.key, node); + CacheEntry cacheEntry = new CacheEntry<>(value, CacheEntryState.ACTIVE); + Node node = new Node<>(key, cacheEntry); + Node prior = this.cache.putIfAbsent(node.key, node); if (prior == null) { processWrite(new AddTask(node)); } @@ -128,7 +128,7 @@ public final class ConcurrentLruCache { private void processRead(Node node) { boolean drainRequested = this.readOperations.recordRead(node); - final DrainStatus status = this.drainStatus.get(); + DrainStatus status = this.drainStatus.get(); if (status.shouldDrainBuffers(drainRequested)) { drainOperations(); } @@ -228,7 +228,7 @@ public final class ConcurrentLruCache { * {@code false} if there was no matching key */ public boolean remove(K key) { - final Node node = this.cache.remove(key); + Node node = this.cache.remove(key); if (node == null) { return false; } @@ -237,27 +237,29 @@ public final class ConcurrentLruCache { return true; } - /* + /** * Transition the node from the {@code active} state to the {@code pending removal} state, * if the transition is valid. */ private void markForRemoval(Node node) { - for (; ; ) { - final CacheEntry current = node.get(); + while (true) { + CacheEntry current = node.get(); if (!current.isActive()) { return; } - final CacheEntry pendingRemoval = new CacheEntry<>(current.value, CacheEntryState.PENDING_REMOVAL); + CacheEntry pendingRemoval = new CacheEntry<>(current.value, CacheEntryState.PENDING_REMOVAL); if (node.compareAndSet(current, pendingRemoval)) { return; } } } + /** * Write operation recorded when a new entry is added to the cache. */ private final class AddTask implements Runnable { + final Node node; AddTask(Node node) { @@ -275,7 +277,7 @@ public final class ConcurrentLruCache { private void evictEntries() { while (currentSize.get() > capacity) { - final Node node = evictionQueue.poll(); + Node node = evictionQueue.poll(); if (node == null) { return; } @@ -283,7 +285,6 @@ public final class ConcurrentLruCache { markAsRemoved(node); } } - } @@ -291,6 +292,7 @@ public final class ConcurrentLruCache { * Write operation recorded when an entry is removed to the cache. */ private final class RemovalTask implements Runnable { + final Node node; RemovalTask(Node node) { @@ -310,7 +312,7 @@ public final class ConcurrentLruCache { */ private enum DrainStatus { - /* + /** * No drain operation currently running. */ IDLE { @@ -320,7 +322,7 @@ public final class ConcurrentLruCache { } }, - /* + /** * A drain operation is required due to a pending write modification. */ REQUIRED { @@ -330,7 +332,7 @@ public final class ConcurrentLruCache { } }, - /* + /** * A drain operation is in progress. */ PROCESSING { @@ -367,12 +369,6 @@ public final class ConcurrentLruCache { private static final int BUFFER_COUNT = detectNumberOfBuffers(); - private static int detectNumberOfBuffers() { - int availableProcessors = Runtime.getRuntime().availableProcessors(); - int nextPowerOfTwo = 1 << (Integer.SIZE - Integer.numberOfLeadingZeros(availableProcessors - 1)); - return Math.min(4, nextPowerOfTwo); - } - private static final int BUFFERS_MASK = BUFFER_COUNT - 1; private static final int MAX_PENDING_OPERATIONS = 32; @@ -383,19 +379,13 @@ public final class ConcurrentLruCache { private static final int BUFFER_INDEX_MASK = BUFFER_SIZE - 1; - /* - * Number of operations recorded, for each buffer - */ + // Number of operations recorded, for each buffer private final AtomicLongArray recordedCount = new AtomicLongArray(BUFFER_COUNT); - /* - * Number of operations read, for each buffer - */ + // Number of operations read, for each buffer private final long[] readCount = new long[BUFFER_COUNT]; - /* - * Number of operations processed, for each buffer - */ + // Number of operations processed, for each buffer private final AtomicLongArray processedCount = new AtomicLongArray(BUFFER_COUNT); @SuppressWarnings("rawtypes") @@ -403,10 +393,11 @@ public final class ConcurrentLruCache { private final EvictionQueue evictionQueue; + @SuppressWarnings("rawtypes") ReadOperations(EvictionQueue evictionQueue) { this.evictionQueue = evictionQueue; for (int i = 0; i < BUFFER_COUNT; i++) { - this.buffers[i] = new AtomicReferenceArray<>(BUFFER_SIZE); + this.buffers[i] = new AtomicReferenceArray(BUFFER_SIZE); } } @@ -458,6 +449,12 @@ public final class ConcurrentLruCache { } this.processedCount.lazySet(bufferIndex, writeCount); } + + private static int detectNumberOfBuffers() { + int availableProcessors = Runtime.getRuntime().availableProcessors(); + int nextPowerOfTwo = 1 << (Integer.SIZE - Integer.numberOfLeadingZeros(availableProcessors - 1)); + return Math.min(4, nextPowerOfTwo); + } } @@ -536,10 +533,9 @@ public final class ConcurrentLruCache { if (this.first == null) { return null; } - final Node f = this.first; - final Node next = f.getNext(); + Node f = this.first; + Node next = f.getNext(); f.setNext(null); - this.first = next; if (next == null) { this.last = null; @@ -558,13 +554,12 @@ public final class ConcurrentLruCache { } private boolean contains(Node e) { - return (e.getPrevious() != null) || (e.getNext() != null) || (e == this.first); + return (e.getPrevious() != null || e.getNext() != null || e == this.first); } - private void linkLast(final Node e) { - final Node l = this.last; + private void linkLast(Node e) { + Node l = this.last; this.last = e; - if (l == null) { this.first = e; } @@ -575,8 +570,8 @@ public final class ConcurrentLruCache { } private void unlink(Node e) { - final Node prev = e.getPrevious(); - final Node next = e.getNext(); + Node prev = e.getPrevious(); + Node next = e.getNext(); if (prev == null) { this.first = next; }