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 e79f945762b4daab2c43937cfcef326f808b1aaf Author: Lukasz Lenart <[email protected]> AuthorDate: Sun Oct 18 12:27:09 2026 +0200 WW-3576 fix(core): detach every request SessionMap at request end Co-Authored-By: Claude Opus 5.5 <[email protected]> --- .../struts2/dispatcher/PrepareOperations.java | 1 + .../org/apache/struts2/dispatcher/SessionMap.java | 30 +++++++++++++++++++++ .../struts2/dispatcher/PrepareOperationsTest.java | 31 ++++++++++++++++++++++ .../apache/struts2/dispatcher/SessionMapTest.java | 2 ++ .../interceptor/CreateSessionInterceptorTest.java | 3 +++ .../debugging/DebuggingInterceptorTest.java | 12 +++++++-- 6 files changed, 77 insertions(+), 2 deletions(-) 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 95fa4bf92..d9398e301 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 { + SessionMap.detachAll(request); detachSessionMap(); ActionContext.clear(); Dispatcher.clearInstance(); 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 801f9c3e3..d7346a22c 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/SessionMap.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/SessionMap.java @@ -29,6 +29,8 @@ import java.util.Enumeration; import java.util.HashSet; import java.util.Set; import java.util.concurrent.Callable; +import java.util.concurrent.ConcurrentLinkedQueue; +import java.util.Queue; /** * A simple implementation of the {@link java.util.Map} interface to handle a collection of HTTP session @@ -40,6 +42,8 @@ public class SessionMap extends AbstractMap<String, Object> implements Serializa @Serial private static final long serialVersionUID = 4678843241638046854L; + private static final String REQUEST_MAPS = SessionMap.class.getName() + ".maps"; + private static final ThreadLocal<Runnable> LIVE_WRITE_LISTENER = new ThreadLocal<>(); protected volatile HttpSession session; @@ -66,12 +70,38 @@ public class SessionMap extends AbstractMap<String, Object> implements Serializa // "execAndWait" interceptor this.request = request; this.session = request.getSession(false); + register(request); } void detach() { detached = true; } + private void register(HttpServletRequest request) { + RequestMaps registered = request.getAttribute(REQUEST_MAPS) instanceof RequestMaps existing ? existing : null; + if (registered == null) { + registered = new RequestMaps(); + request.setAttribute(REQUEST_MAPS, registered); + } + registered.maps.add(this); + } + + static void detachAll(HttpServletRequest request) { + if (request.getAttribute(REQUEST_MAPS) instanceof RequestMaps registered) { + registered.maps.forEach(SessionMap::detach); + request.removeAttribute(REQUEST_MAPS); + } + } + + private static final class RequestMaps { + private final Queue<SessionMap> maps = new ConcurrentLinkedQueue<>(); + + @Override + public String toString() { + return "RequestMaps"; + } + } + /** * Invalidate the http session. */ 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 902e7ee8c..b01b87cad 100644 --- a/core/src/test/java/org/apache/struts2/dispatcher/PrepareOperationsTest.java +++ b/core/src/test/java/org/apache/struts2/dispatcher/PrepareOperationsTest.java @@ -75,6 +75,37 @@ public class PrepareOperationsTest extends StrutsInternalTestCase { assertNull(req.getSession(false)); } + public void testRequestCleanupDetachesTheHelperSessionMapAfterCreateSession() { + MockHttpServletRequest req = new MockHttpServletRequest(); + SessionMap original = new SessionMap(req); + ActionContext context = ActionContext.getContext() + .withSession(original) + .with(DispatcherConstants.SESSION, original) + .withSession(new SessionMap(req)); + PrepareOperations prepare = new PrepareOperations(dispatcher); + prepare.trackRecursion(req); + + prepare.cleanupRequest(req); + + SessionMap helper = (SessionMap) context.get(DispatcherConstants.SESSION); + assertThrows(IllegalStateException.class, () -> helper.put("key", "value")); + assertNull(req.getSession(false)); + } + + public void testRequestCleanupDetachesEveryRequestSessionMap() { + MockHttpServletRequest req = new MockHttpServletRequest(); + SessionMap first = new SessionMap(req); + SessionMap second = new SessionMap(req); + PrepareOperations prepare = new PrepareOperations(dispatcher); + prepare.trackRecursion(req); + + prepare.cleanupRequest(req); + + assertThrows(IllegalStateException.class, () -> first.put("key", "value")); + assertThrows(IllegalStateException.class, () -> second.put("key", "value")); + assertNull(req.getSession(false)); + } + public void testRequestCleanupWithoutActionContext() { 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 3f7c3a8fc..f1bb72edc 100644 --- a/core/src/test/java/org/apache/struts2/dispatcher/SessionMapTest.java +++ b/core/src/test/java/org/apache/struts2/dispatcher/SessionMapTest.java @@ -471,6 +471,8 @@ public class SessionMapTest extends TestCase { sessionMock.matchAndReturn("getId", "1"); requestMock = new Mock(HttpServletRequest.class); requestMock.matchAndReturn("getSession", new Constraint[]{new IsEqual(Boolean.FALSE)}, sessionMock.proxy()); + requestMock.matchAndReturn("getAttribute", new Constraint[]{new IsAnything()}, null); + requestMock.matchAndReturn("setAttribute", new Constraint[]{new IsAnything(), new IsAnything()}, null); } diff --git a/core/src/test/java/org/apache/struts2/interceptor/CreateSessionInterceptorTest.java b/core/src/test/java/org/apache/struts2/interceptor/CreateSessionInterceptorTest.java index 43a098a0d..4e1deae85 100644 --- a/core/src/test/java/org/apache/struts2/interceptor/CreateSessionInterceptorTest.java +++ b/core/src/test/java/org/apache/struts2/interceptor/CreateSessionInterceptorTest.java @@ -25,6 +25,7 @@ import org.apache.struts2.StrutsInternalTestCase; import org.jmock.Mock; import org.jmock.core.constraint.IsEqual; import org.jmock.core.matcher.InvokeOnceMatcher; +import org.jmock.core.stub.ReturnStub; import jakarta.servlet.http.HttpServletRequest; @@ -48,6 +49,8 @@ public class CreateSessionInterceptorTest extends StrutsInternalTestCase { httpServletRequestMock.expects(new InvokeOnceMatcher()).method("getSession").with(new IsEqual(Boolean.FALSE)); httpServletRequestMock.expects(new InvokeOnceMatcher()).method("getSession").with(new IsEqual(Boolean.TRUE)); httpServletRequestMock.expects(new InvokeOnceMatcher()).method("getSession").with(new IsEqual(Boolean.FALSE)); + httpServletRequestMock.stubs().method("getAttribute").will(new ReturnStub(null)); + httpServletRequestMock.stubs().method("setAttribute"); HttpServletRequest request = (HttpServletRequest) httpServletRequestMock.proxy(); ServletActionContext.setRequest(request); diff --git a/core/src/test/java/org/apache/struts2/interceptor/debugging/DebuggingInterceptorTest.java b/core/src/test/java/org/apache/struts2/interceptor/debugging/DebuggingInterceptorTest.java index 01977f1ce..b9caaa6a5 100644 --- a/core/src/test/java/org/apache/struts2/interceptor/debugging/DebuggingInterceptorTest.java +++ b/core/src/test/java/org/apache/struts2/interceptor/debugging/DebuggingInterceptorTest.java @@ -68,7 +68,11 @@ public class DebuggingInterceptorTest extends StrutsJUnit4InternalTestCase { <debug> <parameters/> <context/> - <request/> + <request> + <org.apache.struts2.dispatcher.SessionMap.maps> + <class>class org.apache.struts2.dispatcher.SessionMap$RequestMaps</class> + </org.apache.struts2.dispatcher.SessionMap.maps> + </request> <session/> <valueStack> <value> @@ -171,7 +175,11 @@ public class DebuggingInterceptorTest extends StrutsJUnit4InternalTestCase { &lt;debug&gt; &lt;parameters/&gt; &lt;context/&gt; - &lt;request/&gt; + &lt;request&gt; + &lt;org.apache.struts2.dispatcher.SessionMap.maps&gt; + &lt;class&gt;class org.apache.struts2.dispatcher.SessionMap$RequestMaps&lt;/class&gt; + &lt;/org.apache.struts2.dispatcher.SessionMap.maps&gt; + &lt;/request&gt; &lt;session/&gt; &lt;valueStack&gt; &lt;value&gt;
