This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch 9.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit ccb5a83b0b71929d6020a33b3b55d4703755f014 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 f5bee5740b..85259f7eec 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; @@ -1373,11 +1374,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); if (manager instanceof ManagerBase && ((ManagerBase) manager).getPersistAuthenticationNotes()) { // Write the notes associated with authentication. Without these, // authentication can fail if there is a persist/restore during 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]
