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 8ce0aeb037fd5a45e93dd815e5e13263e77e5be7 Author: Robert Lazarski <[email protected]> AuthorDate: Tue Jun 9 16:38:22 2026 -1000 RAMPART-427: add the CLIENT_SIDE marker parameter race-free RampartMessageData's constructor set the per-service CLIENT_SIDE marker parameter with a non-atomic check-then-add: concurrent first requests to a service all saw the parameter as absent, then each tried to add it with setLocked(true). The first won; the rest failed with "The CLIENT_SIDE parameter is already locked and the value cannot be overridden", failing those requests intermittently under load. The add is now done by addClientSideParameterIfAbsent(AxisService), which synchronizes on the service and re-checks before adding, so the parameter is added at most once. The lock is per-service (not a global static), and the common path - parameter already present - never enters the synchronized method, so steady-state message processing stays lock-free. Adds RampartMessageDataTest covering idempotent and concurrent adds. (The original report's integration test relied on commons-httpclient 3.x, which is no longer a dependency, and was an inherently timing-dependent test; this deterministic unit test replaces it.) Verified with a full clean -Papache-release verify across all modules including the integration scenarios and the nine policy samples on JDK 25. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> --- .../org/apache/rampart/RampartMessageData.java | 26 +++++-- .../org/apache/rampart/RampartMessageDataTest.java | 83 ++++++++++++++++++++++ 2 files changed, 105 insertions(+), 4 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 e2a122f9..8ba45a90 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 @@ -209,6 +209,27 @@ public class RampartMessageData { private SOAPConstants soapConstants; + /** + * Adds the locked CLIENT_SIDE marker parameter to the service, once, if it is + * not already present. The check-and-add is synchronized on the service so that + * concurrent first requests do not race to add it. Without this, the first + * thread added and locked the parameter and the remaining threads failed with + * "The CLIENT_SIDE parameter is already locked and the value cannot be + * overridden" (RAMPART-427). The lock is per-service rather than global, and the + * common case (parameter already present) never reaches this method, so there is + * no contention once a service has handled its first client request. + */ + static void addClientSideParameterIfAbsent(AxisService axisService) throws AxisFault { + synchronized (axisService) { + if (axisService.getParameter(PARAM_CLIENT_SIDE) == null) { + Parameter clientSideParam = new Parameter(); + clientSideParam.setName(PARAM_CLIENT_SIDE); + clientSideParam.setLocked(true); + axisService.addParameter(clientSideParam); + } + } + } + public RampartMessageData(MessageContext msgCtx, boolean sender) throws RampartException { this.msgContext = msgCtx; @@ -267,10 +288,7 @@ public class RampartMessageData { this.isInitiator = !msgCtx.isServerSide(); //TODO if Axis Service is null at this point, do we have to create a dummy one ?? if(this.isInitiator && axisService != null ) { - Parameter clientSideParam = new Parameter(); - clientSideParam.setName(PARAM_CLIENT_SIDE); - clientSideParam.setLocked(true); - msgCtx.getAxisService().addParameter(clientSideParam); + addClientSideParameterIfAbsent(axisService); } } diff --git a/modules/rampart-core/src/test/java/org/apache/rampart/RampartMessageDataTest.java b/modules/rampart-core/src/test/java/org/apache/rampart/RampartMessageDataTest.java new file mode 100644 index 00000000..fcb493c4 --- /dev/null +++ b/modules/rampart-core/src/test/java/org/apache/rampart/RampartMessageDataTest.java @@ -0,0 +1,83 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.rampart; + +import java.util.List; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.CountDownLatch; + +import junit.framework.TestCase; + +import org.apache.axis2.description.AxisService; + +/** + * RAMPART-427: adding the locked CLIENT_SIDE marker parameter to a service must be + * idempotent and safe under concurrent first requests; otherwise all but the first + * thread fails with "The CLIENT_SIDE parameter is already locked and the value + * cannot be overridden". + */ +public class RampartMessageDataTest extends TestCase { + + /** + * Deterministic guard: a second add must not throw because the parameter is + * already present and locked. + */ + public void testAddClientSideParameterIsIdempotent() throws Exception { + AxisService service = new AxisService("TestService"); + RampartMessageData.addClientSideParameterIfAbsent(service); + RampartMessageData.addClientSideParameterIfAbsent(service); + assertNotNull("CLIENT_SIDE parameter should be present", + service.getParameter(RampartMessageData.PARAM_CLIENT_SIDE)); + } + + /** + * Many threads racing to mark the same service client-side must all succeed, + * with the parameter added exactly once. + */ + public void testAddClientSideParameterConcurrent() throws Exception { + final AxisService service = new AxisService("TestService"); + final int threadCount = 16; + final CountDownLatch startGate = new CountDownLatch(1); + final CountDownLatch doneGate = new CountDownLatch(threadCount); + final List<Throwable> failures = new CopyOnWriteArrayList<Throwable>(); + + for (int i = 0; i < threadCount; i++) { + new Thread(new Runnable() { + public void run() { + try { + startGate.await(); + RampartMessageData.addClientSideParameterIfAbsent(service); + } catch (Throwable t) { + failures.add(t); + } finally { + doneGate.countDown(); + } + } + }).start(); + } + + startGate.countDown(); // release all threads at once to maximise contention + doneGate.await(); + + assertTrue("concurrent CLIENT_SIDE add must not fail (RAMPART-427): " + failures, + failures.isEmpty()); + assertNotNull("CLIENT_SIDE parameter should be present", + service.getParameter(RampartMessageData.PARAM_CLIENT_SIDE)); + } +}
