This is an automated email from the ASF dual-hosted git repository. robertlazarski pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/axis-axis2-java-rampart.git
commit 29d775470afd8ab17c3331d7364c91af70fb9ab5 Author: Robert Lazarski <[email protected]> AuthorDate: Fri Oct 16 03:12:44 2026 -1000 RAMPART-459: record a failed OpenSAML init instead of retrying per message When InitializationService.initialize() failed (e.g. a standalone Xerces rejecting jdk.xml.maxElementDepth), every message retried it under the lock, reintroducing the contention. The failure is now logged once and rethrown without retrying; a fixed environment needs a restart for SAML to work. Co-Authored-By: Claude Opus 5.5 <[email protected]> --- .../org/apache/rampart/RampartMessageData.java | 6 ++- .../rahas/impl/util/OpenSAMLInitializer.java | 61 +++++++++++++++++++--- .../rahas/impl/util/OpenSAMLInitializerTest.java | 49 +++++++++++++++++ 3 files changed, 108 insertions(+), 8 deletions(-) diff --git a/modules/rampart-core/src/main/java/org/apache/rampart/RampartMessageData.java b/modules/rampart-core/src/main/java/org/apache/rampart/RampartMessageData.java index a649d0d9..c4ffe877 100644 --- a/modules/rampart-core/src/main/java/org/apache/rampart/RampartMessageData.java +++ b/modules/rampart-core/src/main/java/org/apache/rampart/RampartMessageData.java @@ -278,7 +278,11 @@ public class RampartMessageData { try { OpenSAMLInitializer.ensureInitialized(); } catch (Exception e) { - log.warn("WSS4J OpenSAML initialization failed: " + e.getMessage(), e); + // OpenSAMLInitializer logs the failure once; repeating it per message + // would flood the log of a deployment that does not use SAML. + if (log.isDebugEnabled()) { + log.debug("OpenSAML is not initialized: " + e.getMessage()); + } } // Set the WSSConfig diff --git a/modules/rampart-trust/src/main/java/org/apache/rahas/impl/util/OpenSAMLInitializer.java b/modules/rampart-trust/src/main/java/org/apache/rahas/impl/util/OpenSAMLInitializer.java index 50a504d9..eb9fcf17 100644 --- a/modules/rampart-trust/src/main/java/org/apache/rahas/impl/util/OpenSAMLInitializer.java +++ b/modules/rampart-trust/src/main/java/org/apache/rahas/impl/util/OpenSAMLInitializer.java @@ -46,9 +46,31 @@ public final class OpenSAMLInitializer { private static final Log log = LogFactory.getLog(OpenSAMLInitializer.class); - private static volatile boolean initialized; + /** The OpenSAML initialisation steps, separable so tests can substitute a failing one. */ + interface Steps { + void run() throws InitializationException; + } + + private static final OpenSAMLInitializer INSTANCE = new OpenSAMLInitializer(() -> { + OpenSAMLUtil.initSamlEngine(); + InitializationService.initialize(); + }); + + private final Steps steps; + + private volatile boolean initialized; + + /** + * The failure of the one initialisation attempt, if it failed. Failures here are + * environmental and repeat identically (for example a standalone Xerces on the + * classpath rejecting {@code jdk.xml.maxElementDepth} in OpenSAML's parser pool), + * so retrying per message would put every thread back on the lock for the same + * result. A fix to the environment needs a restart. + */ + private volatile InitializationException failure; - private OpenSAMLInitializer() { + OpenSAMLInitializer(Steps steps) { + this.steps = steps; } /** @@ -78,23 +100,48 @@ public final class OpenSAMLInitializer { } /** - * Initialises OpenSAML once. After the first successful call this is a single - * volatile read. A failed attempt is not recorded, so the next call retries. + * Initialises OpenSAML once. After the first attempt, successful or not, this never + * takes the lock: a success is a volatile read, and a failure is logged once and + * then rethrown unchanged on every later call without retrying. */ public static void ensureInitialized() throws InitializationException { + INSTANCE.ensure(); + } + + void ensure() throws InitializationException { if (initialized) { return; } - synchronized (OpenSAMLInitializer.class) { + InitializationException previous = failure; + if (previous != null) { + throw previous; + } + synchronized (this) { if (initialized) { return; } - OpenSAMLUtil.initSamlEngine(); - InitializationService.initialize(); + if (failure != null) { + throw failure; + } + try { + steps.run(); + } catch (InitializationException e) { + throw recordFailure(e); + } catch (RuntimeException e) { + // Module initialisers can fail unchecked; InitializationService does not wrap. + throw recordFailure(new InitializationException("OpenSAML initialization failed: " + e, e)); + } initialized = true; if (log.isDebugEnabled()) { log.debug("OpenSAML initialization complete"); } } } + + private InitializationException recordFailure(InitializationException e) { + failure = e; + log.error("OpenSAML initialization failed; SAML processing is unavailable until" + + " restart. Non-SAML messages are unaffected. Cause: " + e.getMessage(), e); + return e; + } } diff --git a/modules/rampart-trust/src/test/java/org/apache/rahas/impl/util/OpenSAMLInitializerTest.java b/modules/rampart-trust/src/test/java/org/apache/rahas/impl/util/OpenSAMLInitializerTest.java index d47adcc1..3073e573 100644 --- a/modules/rampart-trust/src/test/java/org/apache/rahas/impl/util/OpenSAMLInitializerTest.java +++ b/modules/rampart-trust/src/test/java/org/apache/rahas/impl/util/OpenSAMLInitializerTest.java @@ -23,11 +23,13 @@ import java.net.URLClassLoader; import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.atomic.AtomicInteger; import junit.framework.TestCase; import org.apache.axis2.AxisFault; +import org.opensaml.core.config.InitializationException; import org.opensaml.core.xml.config.XMLObjectProviderRegistrySupport; import org.opensaml.xmlsec.SecurityConfigurationSupport; import org.opensaml.xmlsec.SignatureSigningConfiguration; @@ -80,6 +82,53 @@ public class OpenSAMLInitializerTest extends TestCase { } } + /** + * A failed attempt must not be retried per message: that put every thread back on + * the lock to fail the same way, for deployments that never use SAML. + */ + public void testFailureIsRecordedAndNotRetried() throws Exception { + final AtomicInteger attempts = new AtomicInteger(); + final InitializationException cause = new InitializationException("parser pool"); + OpenSAMLInitializer initializer = new OpenSAMLInitializer(() -> { + attempts.incrementAndGet(); + throw cause; + }); + + for (int i = 0; i < 3; i++) { + try { + initializer.ensure(); + fail("a failed initialisation must keep failing"); + } catch (InitializationException expected) { + assertSame(cause, expected); + } + } + assertEquals("initialisation must be attempted once", 1, attempts.get()); + } + + public void testUncheckedFailureIsWrappedAndNotRetried() throws Exception { + final AtomicInteger attempts = new AtomicInteger(); + final IllegalArgumentException cause = new IllegalArgumentException("maxElementDepth"); + OpenSAMLInitializer initializer = new OpenSAMLInitializer(() -> { + attempts.incrementAndGet(); + throw cause; + }); + + InitializationException first = null; + for (int i = 0; i < 3; i++) { + try { + initializer.ensure(); + fail("a failed initialisation must keep failing"); + } catch (InitializationException expected) { + assertSame(cause, expected.getCause()); + if (first == null) { + first = expected; + } + assertSame(first, expected); + } + } + assertEquals("initialisation must be attempted once", 1, attempts.get()); + } + public void testConcurrentCallsSucceed() throws Exception { final int threadCount = 16; final CountDownLatch startGate = new CountDownLatch(1);
