[CXF-9105] add synchronized to the principal iteration (#2245)

apache · Feb 1, 2025 · 96aca2e · 96aca2e
1 parent 283c861
commit 96aca2e
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 13 deletions.
diff --git a/...eatures/logging/src/main/java/org/apache/cxf/ext/logging/event/DefaultLogEventMapper.java b/...eatures/logging/src/main/java/org/apache/cxf/ext/logging/event/DefaultLogEventMapper.java
@@ -19,6 +19,7 @@
 package org.apache.cxf.ext.logging.event;
 
 import java.security.AccessController;
+import java.security.Principal;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
@@ -109,7 +110,7 @@ public LogEvent map(final Message message, final Set<String> sensitiveProtocolHe
     }
 
     private String getPrincipal(Message message) {
-        String principal = getJAASPrincipal();
+        String principal = getConcatenatedJAASPrincipals();
         if (principal != null) {
             return principal;
         }
@@ -125,31 +126,41 @@ private String getPrincipal(Message message) {
         return null;
     }
 
-    private String getJAASPrincipal() {
-        StringBuilder principals = new StringBuilder();
-        Iterator<? extends Object> principalIt = getJAASPrincipals();
-        while (principalIt.hasNext()) {
-            principals.append(principalIt.next());
-            if (principalIt.hasNext()) {
-                principals.append(',');
+    private String getConcatenatedJAASPrincipals() {
+        StringBuilder principalsStringBuilder = new StringBuilder();
+        Set<Principal> principals = getJAASPrincipals();
+
+        if (principals.isEmpty()) {
+            return null;
+        }
+
+        synchronized (principals) {
+            Iterator<Principal> principalIt = principals.iterator();
+            while (principalIt.hasNext()) {
+                principalsStringBuilder.append(principalIt.next());
+                if (principalIt.hasNext()) {
+                    principalsStringBuilder.append(',');
+                }
             }
         }
-        if (principals.length() == 0) {
+
+        if (principalsStringBuilder.length() == 0) {
             return null;
         }
-        return principals.toString();
+
+        return principalsStringBuilder.toString();
     }
 
-    private Iterator<? extends Object> getJAASPrincipals() {
+    private Set<Principal> getJAASPrincipals() {
         try {
             Subject subject = Subject.getSubject(AccessController.getContext());
             return subject != null && subject.getPrincipals() != null
-                    ? subject.getPrincipals().iterator() : Collections.emptyIterator();
+                    ? subject.getPrincipals() : Collections.emptySet();
         } catch (UnsupportedOperationException e) {
             // JDK 23: The terminally deprecated method Subject.getSubject(AccessControlContext) has been re-specified
             // to throw UnsupportedOperationException if invoked when a Security Manager is not allowed.
             // see https://jdk.java.net/23/release-notes#JDK-8296244
-            return Collections.emptyIterator();
+            return Collections.emptySet();
         }
     }
 

diff --git a/rt/features/logging/src/test/java/org/apache/cxf/ext/logging/DefaultLogEventMapperTest.java b/rt/features/logging/src/test/java/org/apache/cxf/ext/logging/DefaultLogEventMapperTest.java
@@ -18,13 +18,20 @@
  */
 package org.apache.cxf.ext.logging;
 
+import java.security.Principal;
+import java.security.PrivilegedAction;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
 
+import javax.security.auth.Subject;
+
+import org.apache.cxf.common.security.SimplePrincipal;
 import org.apache.cxf.ext.logging.event.DefaultLogEventMapper;
 import org.apache.cxf.ext.logging.event.EventType;
 import org.apache.cxf.ext.logging.event.LogEvent;
@@ -147,4 +154,26 @@ public void testMap() {
         assertEquals("PUT[test]", event.getOperationName());
     }
 
+    @Test
+    public void testMultiplePrincipalsReturnedByAccessControllerContext() {
+        DefaultLogEventMapper mapper = new DefaultLogEventMapper();
+        Message message = new MessageImpl();
+        message.put(Message.HTTP_REQUEST_METHOD, "GET");
+        message.put(Message.REQUEST_URI, "test");
+        Exchange exchange = new ExchangeImpl();
+        message.setExchange(exchange);
+
+        Set<Principal> principals = IntStream.range(0, 3)
+                .mapToObj(i -> new SimplePrincipal("principal-" + i))
+                .collect(Collectors.toSet());
+
+        Subject subject = new Subject(false, principals, Set.of(), Set.of());
+
+        LogEvent event = Subject.doAs(subject, (PrivilegedAction<LogEvent>) () -> mapper.map(message));
+        String[] splitPrincipals = event.getPrincipal().split(",");
+        Set<String> expected = Set.of("principal-0", "principal-1", "principal-2");
+
+        assertEquals(expected, Arrays.stream(splitPrincipals).collect(Collectors.toSet()));
+    }
+
 }