This is an automated email from the ASF dual-hosted git repository.

rmaucher pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit 514dccd3aa2b5636000388bad16a03331f27d731
Author: opencode <[email protected]>
AuthorDate: Wed Oct 7 18:05:37 2026 +0200

    Validate the principal object graph before persisting it in StandardSession
    
    With persistAuthentication enabled, only the top-level Principal was
    checked for Serializable. A principal whose object graph contained a
    non-serializable element threw NotSerializableException mid-write from
    the swallowing try/catch around stream.writeObject(principal), leaving
    partially written object bytes in the stream. The later load then
    failed with a WriteAbortedException or ClassCastException, corrupting
    the whole persisted session record while the write side reported
    success.
    
    Serialize the graph into a throwaway buffer first, dropping the
    principal and warning when the graph is not serializable, matching the
    behaviour of the existing top-level check.
---
 .../apache/catalina/session/StandardSession.java   | 22 +++++--
 .../catalina/session/TestStandardManager.java      | 67 ++++++++++++++++++++++
 2 files changed, 85 insertions(+), 4 deletions(-)

diff --git a/java/org/apache/catalina/session/StandardSession.java 
b/java/org/apache/catalina/session/StandardSession.java
index ef53c342a3..794ef9a89d 100644
--- a/java/org/apache/catalina/session/StandardSession.java
+++ b/java/org/apache/catalina/session/StandardSession.java
@@ -17,6 +17,7 @@
 package org.apache.catalina.session;
 
 import java.beans.PropertyChangeSupport;
+import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.NotSerializableException;
 import java.io.ObjectInputStream;
@@ -1301,11 +1302,24 @@ public class StandardSession implements HttpSession, 
Session, Serializable {
 
         // Write authentication information (may be null values)
         stream.writeObject(sessionAuthType);
-        try {
-            stream.writeObject(sessionPrincipal);
-        } catch (NotSerializableException e) {
-            
manager.getContext().getLogger().warn(sm.getString("standardSession.principalNotSerializable",
 id), e);
+        if (sessionPrincipal != null) {
+            /*
+             * The instanceof check above only covers the principal itself.
+             * Verify that the complete object graph is serializable before
+             * writing to the session stream, since a NotSerializableException
+             * thrown for a nested element mid-write would leave the session
+             * record corrupted and unreadable. Discard the principal in the
+             * same way as above if the graph is not serializable.
+             */
+            try (ByteArrayOutputStream baos = new ByteArrayOutputStream();
+                    ObjectOutputStream principalStream = new 
ObjectOutputStream(baos)) {
+                principalStream.writeObject(sessionPrincipal);
+            } catch (NotSerializableException e) {
+                sessionPrincipal = null;
+                
manager.getContext().getLogger().warn(sm.getString("standardSession.principalNotSerializable",
 id), e);
+            }
         }
+        stream.writeObject(sessionPrincipal);
         stream.writeObject(expectedSessionId);
         stream.writeObject(savedRequest);
 
diff --git a/test/org/apache/catalina/session/TestStandardManager.java 
b/test/org/apache/catalina/session/TestStandardManager.java
index 2a9edeba26..4f42f526aa 100644
--- a/test/org/apache/catalina/session/TestStandardManager.java
+++ b/test/org/apache/catalina/session/TestStandardManager.java
@@ -20,6 +20,7 @@ import java.io.File;
 import java.io.FileOutputStream;
 import java.io.ObjectOutputStream;
 import java.io.Serializable;
+import java.security.Principal;
 import java.util.concurrent.atomic.AtomicInteger;
 
 import jakarta.servlet.http.HttpSessionActivationListener;
@@ -29,6 +30,7 @@ import jakarta.servlet.http.HttpSessionListener;
 import org.junit.Assert;
 import org.junit.Test;
 
+import org.apache.catalina.Session;
 import org.apache.catalina.startup.ExpandWar;
 import org.apache.tomcat.unittest.TesterContext;
 import org.apache.tomcat.unittest.TesterHost;
@@ -60,6 +62,26 @@ public class TestStandardManager {
         }
     }
 
+    public static class TesterNonSerializableGraphPrincipal implements 
Principal, Serializable {
+
+        private static final long serialVersionUID = 1L;
+
+        private final String name;
+
+        @SuppressWarnings("unused")
+        private final Object notSerializable = new Object();
+
+        public TesterNonSerializableGraphPrincipal(String name) {
+            this.name = name;
+        }
+
+        @Override
+        public String getName() {
+            return name;
+        }
+    }
+
+
     /*
      * A session that was persisted as invalid (which can happen when a
      * session expires concurrently with unload()) must be discarded on load
@@ -141,4 +163,49 @@ public class TestStandardManager {
             ExpandWar.delete(dir);
         }
     }
+
+    /*
+     * A principal whose own class is Serializable but whose object graph
+     * contains a non-serializable element must not corrupt the persisted
+     * session record. The principal is dropped instead, keeping the rest of
+     * the session loadable.
+     */
+    @Test
+    public void testUnloadDropsPrincipalWithNonSerializableGraph() throws 
Exception {
+        File dir = new File("SESS_TEMP_STANDARD_PRINCIPAL");
+        ExpandWar.delete(dir);
+        Assert.assertTrue(dir.mkdirs());
+        File file = new File(dir, "TEST_SESSIONS.ser");
+        try {
+            StandardManager manager = new StandardManager();
+
+            TesterContext context = new TesterContext();
+            context.setServletContext(new TesterServletContext());
+            context.setParent(new TesterHost());
+            manager.setContext(context);
+
+            manager.setPathname(file.getAbsolutePath());
+            manager.setPersistAuthentication(true);
+            manager.start();
+
+            Session session = manager.createSession(VALID_ID);
+            session.setPrincipal(new 
TesterNonSerializableGraphPrincipal("testUser"));
+
+            manager.unload();
+
+            try {
+                manager.load();
+            } catch (Throwable t) {
+                // Before the fix, the NotSerializableException was swallowed
+                // mid-write, corrupting the stream, so loading the file failed
+                Assert.fail("Loading the persisted session failed: " + t);
+            }
+
+            Session loaded = manager.findSession(VALID_ID);
+            Assert.assertNotNull(loaded);
+            Assert.assertNull(loaded.getPrincipal());
+        } finally {
+            ExpandWar.delete(dir);
+        }
+    }
 }


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to