This is an automated email from the ASF dual-hosted git repository. lukaszlenart pushed a commit to branch feature/WW-3576-session-map-staleness in repository https://gitbox.apache.org/repos/asf/struts.git
commit 32212876dd54ccecc9702ba6201f64b0a26dba0f Author: Lukasz Lenart <[email protected]> AuthorDate: Sun Oct 18 10:30:42 2026 +0200 WW-3576 fix(core): detach the request SessionMap at request end A detached map never calls its request again: with a live pinned session it keeps working, and a put with no live pinned session throws IllegalStateException instead of resolving a session through a request that has already ended. Co-Authored-By: Claude Opus 5.5 <[email protected]> --- .../struts2/dispatcher/PrepareOperations.java | 8 ++++ .../org/apache/struts2/dispatcher/SessionMap.java | 10 ++++- .../struts2/dispatcher/PrepareOperationsTest.java | 27 ++++++++++++ .../apache/struts2/dispatcher/SessionMapTest.java | 49 ++++++++++++++++++++++ 4 files changed, 93 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/apache/struts2/dispatcher/PrepareOperations.java b/core/src/main/java/org/apache/struts2/dispatcher/PrepareOperations.java index eb37b4812..95fa4bf92 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/PrepareOperations.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/PrepareOperations.java @@ -75,6 +75,7 @@ public class PrepareOperations { try { dispatcher.cleanUpRequest(request); } finally { + detachSessionMap(); ActionContext.clear(); Dispatcher.clearInstance(); ContainerHolder.clear(); @@ -83,6 +84,13 @@ public class PrepareOperations { }); } + private static void detachSessionMap() { + ActionContext context = ActionContext.getContext(); + if (context != null && context.getSession() instanceof SessionMap sessionMap) { + sessionMap.detach(); + } + } + /** * Creates the action context and initializes the thread local * diff --git a/core/src/main/java/org/apache/struts2/dispatcher/SessionMap.java b/core/src/main/java/org/apache/struts2/dispatcher/SessionMap.java index 9f587b723..3448ffffa 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/SessionMap.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/SessionMap.java @@ -47,7 +47,7 @@ public class SessionMap extends AbstractMap<String, Object> implements Serializa @Deprecated(since = "7.5.0", forRemoval = true) protected Set<Entry<String, Object>> entries; protected HttpServletRequest request; - + private volatile boolean detached; protected SessionMap() { } @@ -66,6 +66,10 @@ public class SessionMap extends AbstractMap<String, Object> implements Serializa this.session = request.getSession(false); } + void detach() { + detached = true; + } + /** * Invalidate the http session. */ @@ -202,6 +206,10 @@ public class SessionMap extends AbstractMap<String, Object> implements Serializa synchronized (this) { HttpSession current = session; if (current == null) { + if (detached) { + throw new IllegalStateException("The request of this session map has ended and its session is gone; " + + "it cannot resolve a new session"); + } current = request.getSession(true); session = current; } diff --git a/core/src/test/java/org/apache/struts2/dispatcher/PrepareOperationsTest.java b/core/src/test/java/org/apache/struts2/dispatcher/PrepareOperationsTest.java index abc060c4c..902e7ee8c 100644 --- a/core/src/test/java/org/apache/struts2/dispatcher/PrepareOperationsTest.java +++ b/core/src/test/java/org/apache/struts2/dispatcher/PrepareOperationsTest.java @@ -28,6 +28,8 @@ import jakarta.servlet.ServletException; import jakarta.servlet.http.HttpServletRequest; import java.util.stream.IntStream; +import static org.junit.Assert.assertThrows; + public class PrepareOperationsTest extends StrutsInternalTestCase { public void testCreateActionContextWhenRequestHasOne() { HttpServletRequest req = new MockHttpServletRequest(); @@ -60,6 +62,31 @@ public class PrepareOperationsTest extends StrutsInternalTestCase { assertNull(ContainerHolder.get()); } + public void testRequestCleanupDetachesTheContextSessionMap() { + MockHttpServletRequest req = new MockHttpServletRequest(); + SessionMap sessionMap = new SessionMap(req); + ActionContext.getContext().withSession(sessionMap); + PrepareOperations prepare = new PrepareOperations(dispatcher); + prepare.trackRecursion(req); + + prepare.cleanupRequest(req); + + assertThrows(IllegalStateException.class, () -> sessionMap.put("key", "value")); + assertNull(req.getSession(false)); + } + + public void testRequestCleanupWithoutActionContext() { + HttpServletRequest req = new MockHttpServletRequest(); + PrepareOperations prepare = new PrepareOperations(dispatcher); + prepare.trackRecursion(req); + ActionContext.clear(); + + prepare.cleanupRequest(req); + + assertNull(Dispatcher.getInstance()); + assertNull(ContainerHolder.get()); + } + public void testWrappedRequestCleanup() { HttpServletRequest req = new MockHttpServletRequest(); PrepareOperations prepare = new PrepareOperations(dispatcher); diff --git a/core/src/test/java/org/apache/struts2/dispatcher/SessionMapTest.java b/core/src/test/java/org/apache/struts2/dispatcher/SessionMapTest.java index 135ca1954..3f7c3a8fc 100644 --- a/core/src/test/java/org/apache/struts2/dispatcher/SessionMapTest.java +++ b/core/src/test/java/org/apache/struts2/dispatcher/SessionMapTest.java @@ -416,6 +416,55 @@ public class SessionMapTest extends TestCase { assertEquals("value", sessionMap.remove("key")); } + public void testDetachedPutAfterInvalidationNeverReachesTheRecycledRequest() { + MockHttpServletRequest request = new MockHttpServletRequest(); + MockHttpSession pinned = new MockHttpSession(); + request.setSession(pinned); + SessionMap sessionMap = new SessionMap(request); + pinned.invalidate(); + MockHttpSession otherUsers = new MockHttpSession(); + request.setSession(otherUsers); + sessionMap.detach(); + + try { + sessionMap.put("key", "value"); + fail("a detached map with no live session must not resolve a session through its request"); + } catch (IllegalStateException expected) { + assertFalse(expected.getMessage().contains("value")); + } + assertFalse(otherUsers.getAttributeNames().hasMoreElements()); + assertSame(otherUsers, request.getSession(false)); + } + + public void testDetachedNeverPinnedPutThrows() { + MockHttpServletRequest request = new MockHttpServletRequest(); + SessionMap sessionMap = new SessionMap(request); + sessionMap.detach(); + + try { + sessionMap.put("key", "value"); + fail("a detached map with no pinned session must not create one through its request"); + } catch (IllegalStateException expected) { + assertNull(request.getSession(false)); + } + } + + public void testDetachedMapKeepsWorkingOnItsLivePinnedSession() { + MockHttpServletRequest request = new MockHttpServletRequest(); + MockHttpSession pinned = new MockHttpSession(); + request.setSession(pinned); + SessionMap sessionMap = new SessionMap(request); + sessionMap.detach(); + + sessionMap.put("key", "value"); + + assertEquals("value", pinned.getAttribute("key")); + assertEquals("value", sessionMap.get("key")); + assertTrue(sessionMap.containsKey("key")); + assertEquals(1, sessionMap.entrySet().size()); + assertEquals("value", sessionMap.remove("key")); + } + @Override protected void setUp() throws Exception { sessionMock = new Mock(HttpSession.class);
