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 c09388b5161004121dd299e4dd87245924476ae6 Author: Lukasz Lenart <[email protected]> AuthorDate: Sun Oct 18 09:19:33 2026 +0200 WW-3576 feat(core): warn when a background process writes around its session sandbox Co-Authored-By: Claude Opus 5.5 <[email protected]> --- .../org/apache/struts2/dispatcher/SessionMap.java | 12 ++++++++++ .../interceptor/ExecuteAndWaitInterceptor.java | 10 ++++++++ .../interceptor/exec/BackgroundProcess.java | 10 +++++++- .../interceptor/exec/SandboxedSessionMap.java | 27 ++++++++++++++++++++++ .../interceptor/exec/StrutsBackgroundProcess.java | 21 ++++++++++++++++- .../interceptor/exec/SandboxedSessionMapTest.java | 27 ++++++++++++++++++++++ 6 files changed, 105 insertions(+), 2 deletions(-) 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 d59ea0a79..9f587b723 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/SessionMap.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/SessionMap.java @@ -20,6 +20,7 @@ package org.apache.struts2.dispatcher; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpSession; +import org.apache.struts2.interceptor.exec.SandboxedSessionMap; import java.io.Serial; import java.io.Serializable; @@ -69,6 +70,7 @@ public class SessionMap extends AbstractMap<String, Object> implements Serializa * Invalidate the http session. */ public void invalidate() { + warnIfSandboxBypassed(); HttpSession current = session; if (current == null) { return; @@ -87,6 +89,7 @@ public class SessionMap extends AbstractMap<String, Object> implements Serializa */ @Override public void clear() { + warnIfSandboxBypassed(); HttpSession current = session; if (current == null) { return; @@ -171,6 +174,7 @@ public class SessionMap extends AbstractMap<String, Object> implements Serializa */ @Override public Object put(final String key, final Object value) { + warnIfSandboxBypassed(); HttpSession current = session; if (current == null) { current = pin(); @@ -187,6 +191,13 @@ public class SessionMap extends AbstractMap<String, Object> implements Serializa return oldValue; } + private void warnIfSandboxBypassed() { + SandboxedSessionMap sandbox = SandboxedSessionMap.current(); + if (sandbox != null && !(this instanceof SandboxedSessionMap)) { + sandbox.warnEscapeOnce(); + } + } + private HttpSession pin() { synchronized (this) { HttpSession current = session; @@ -217,6 +228,7 @@ public class SessionMap extends AbstractMap<String, Object> implements Serializa */ @Override public Object remove(final Object key) { + warnIfSandboxBypassed(); HttpSession current = session; if (current == null) { return null; diff --git a/core/src/main/java/org/apache/struts2/interceptor/ExecuteAndWaitInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/ExecuteAndWaitInterceptor.java index 7572a6d4e..3885d52fa 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/ExecuteAndWaitInterceptor.java +++ b/core/src/main/java/org/apache/struts2/interceptor/ExecuteAndWaitInterceptor.java @@ -86,6 +86,16 @@ import java.util.Map; * rather than calling ActionContext.getSession(). * </p> * + * <p>The background process works on a session sandbox, a snapshot of the session taken when the process starts. Use + * <code>ActionContext.getContext().getSession()</code> or {@link SessionAware} to reach it. Its writes are applied to + * the real session by the polling request that sees the process finish, only if the session id is unchanged. + * Conflicts keep the real session's value by default and can be resolved per action with + * {@link org.apache.struts2.action.BackgroundSessionConflictAware}. OGNL <code>#session</code> and + * <code>invocation.getInvocationContext().getSession()</code> bypass the sandbox, write to the live session and log a + * warning. A custom {@link BackgroundProcess} that does not extend {@link StrutsBackgroundProcess}, or a subclass + * overriding <code>prepare()</code>, does not get a sandbox. + * </p> + * * <p>The thread kicked off by this interceptor will be named in the form <b><u>actionName</u>BackgroundProcess</b>. * For example, the <i>search</i> action would run as a thread named <i>searchBackgroundProcess</i>. * </p> diff --git a/core/src/main/java/org/apache/struts2/interceptor/exec/BackgroundProcess.java b/core/src/main/java/org/apache/struts2/interceptor/exec/BackgroundProcess.java index 7b1392ac8..b17100b09 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/exec/BackgroundProcess.java +++ b/core/src/main/java/org/apache/struts2/interceptor/exec/BackgroundProcess.java @@ -23,7 +23,15 @@ import org.apache.struts2.interceptor.ExecuteAndWaitInterceptor; /** * Interface used to create a background process which will be executed by - * {@link ExecuteAndWaitInterceptor} + * {@link ExecuteAndWaitInterceptor}. + * + * <p>A process built on {@link StrutsBackgroundProcess} works on a session sandbox, a snapshot of the session taken + * when the process starts, and receives it through {@link #withSessionSandbox(SandboxedSessionMap)}. Its writes are + * applied to the real session by the polling request that sees the process finish, and only if the session id is + * unchanged; conflicts keep the real session's value unless the action implements + * {@link org.apache.struts2.action.BackgroundSessionConflictAware}. A custom implementation that does not extend + * {@link StrutsBackgroundProcess}, or a subclass overriding {@link StrutsBackgroundProcess#prepare()}, gets no + * sandbox.</p> */ public interface BackgroundProcess extends Runnable { diff --git a/core/src/main/java/org/apache/struts2/interceptor/exec/SandboxedSessionMap.java b/core/src/main/java/org/apache/struts2/interceptor/exec/SandboxedSessionMap.java index 32991ea55..e9e8b2619 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/exec/SandboxedSessionMap.java +++ b/core/src/main/java/org/apache/struts2/interceptor/exec/SandboxedSessionMap.java @@ -19,6 +19,8 @@ package org.apache.struts2.interceptor.exec; import jakarta.servlet.http.HttpSession; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.apache.struts2.dispatcher.SessionMap; import java.io.Serial; @@ -31,6 +33,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.atomic.AtomicBoolean; /** * The session view an execAndWait background process works on: a shallow snapshot of the session taken at @@ -41,11 +44,15 @@ public class SandboxedSessionMap extends SessionMap { @Serial private static final long serialVersionUID = 2907451127739228064L; + private static final Logger LOG = LogManager.getLogger(SandboxedSessionMap.class); + private static final ThreadLocal<SandboxedSessionMap> CURRENT = new ThreadLocal<>(); + private final transient Map<String, Object> snapshot = new LinkedHashMap<>(); private final transient Map<String, Object> view = new LinkedHashMap<>(); private final transient List<SessionOperation> operations = new ArrayList<>(); private final transient String startSessionId; private final transient String processName; + private final transient AtomicBoolean escapeWarned = new AtomicBoolean(); public SandboxedSessionMap(HttpSession session, String processName) { super(); @@ -59,6 +66,26 @@ public class SandboxedSessionMap extends SessionMap { view.putAll(snapshot); } + public static SandboxedSessionMap current() { + return CURRENT.get(); + } + + public static void runInSandbox(SandboxedSessionMap sandbox, Runnable work) { + CURRENT.set(sandbox); + try { + work.run(); + } finally { + CURRENT.remove(); + } + } + + public void warnEscapeOnce() { + if (escapeWarned.compareAndSet(false, true)) { + LOG.warn("Background process [{}] wrote to the request session, bypassing its session sandbox; " + + "use ActionContext.getContext().getSession() or SessionAware", processName); + } + } + public String getStartSessionId() { return startSessionId; } diff --git a/core/src/main/java/org/apache/struts2/interceptor/exec/StrutsBackgroundProcess.java b/core/src/main/java/org/apache/struts2/interceptor/exec/StrutsBackgroundProcess.java index 8c41c3a8a..d05220a74 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/exec/StrutsBackgroundProcess.java +++ b/core/src/main/java/org/apache/struts2/interceptor/exec/StrutsBackgroundProcess.java @@ -70,7 +70,7 @@ public class StrutsBackgroundProcess implements BackgroundProcess, Serializable try { beforeInvocation(); installSessionSandbox(); - result = invocation.invokeActionOnly(); + result = invokeAction(); } catch (Exception e) { LOG.warn("Exception during invokeActionOnly() execution", e); exception = e; @@ -106,6 +106,25 @@ public class StrutsBackgroundProcess implements BackgroundProcess, Serializable return sessionSandbox; } + private String invokeAction() throws Exception { + if (sessionSandbox == null) { + return invocation.invokeActionOnly(); + } + String[] outcome = new String[1]; + Exception[] failure = new Exception[1]; + SandboxedSessionMap.runInSandbox(sessionSandbox, () -> { + try { + outcome[0] = invocation.invokeActionOnly(); + } catch (Exception e) { + failure[0] = e; + } + }); + if (failure[0] != null) { + throw failure[0]; + } + return outcome[0]; + } + private void installSessionSandbox() { if (sessionSandbox == null) { return; diff --git a/core/src/test/java/org/apache/struts2/interceptor/exec/SandboxedSessionMapTest.java b/core/src/test/java/org/apache/struts2/interceptor/exec/SandboxedSessionMapTest.java index e8269a378..115ce1da2 100644 --- a/core/src/test/java/org/apache/struts2/interceptor/exec/SandboxedSessionMapTest.java +++ b/core/src/test/java/org/apache/struts2/interceptor/exec/SandboxedSessionMapTest.java @@ -19,6 +19,9 @@ package org.apache.struts2.interceptor.exec; import junit.framework.TestCase; +import org.apache.struts2.dispatcher.SessionMap; +import org.apache.struts2.test.LogCapture; +import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpSession; import java.util.HashMap; @@ -89,4 +92,28 @@ public class SandboxedSessionMapTest extends TestCase { assertTrue(sandbox.getOperations().contains(new SessionOperation.Remove("b"))); assertEquals("1", live.getAttribute("a")); } + + public void testWriteThroughRequestMapFromBackgroundThreadWarnsOnce() throws Exception { + MockHttpServletRequest request = new MockHttpServletRequest(); + MockHttpSession live = new MockHttpSession(); + request.setSession(live); + SessionMap requestMap = new SessionMap(request); + SandboxedSessionMap sandbox = new SandboxedSessionMap(live, "longJob"); + + try (LogCapture logs = new LogCapture(SandboxedSessionMap.class)) { + Thread background = new Thread(() -> SandboxedSessionMap.runInSandbox(sandbox, () -> { + requestMap.put("a", "1"); + requestMap.put("b", "2"); + sandbox.put("c", "3"); + })); + background.start(); + background.join(5000); + + assertEquals("1", live.getAttribute("a")); + assertEquals(1, logs.messages().size()); + assertTrue(logs.messages().get(0).contains("longJob")); + assertFalse(logs.messages().get(0).contains("1")); + } + assertNull(SandboxedSessionMap.current()); + } }
