This is an automated email from the ASF dual-hosted git repository. markt-asf pushed a commit to branch 9.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 28201d056be23b8c126d7d92797cdc54b1f9ed2c Author: Mark Thomas <[email protected]> AuthorDate: Wed Jun 3 17:55:00 2026 +0100 Fix BZ 70048. Correctly handle asynchronous requests. --- .../apache/catalina/valves/LocalStrings.properties | 1 + .../apache/catalina/valves/PersistentValve.java | 121 ++++++++++++++++----- webapps/docs/changelog.xml | 4 + 3 files changed, 97 insertions(+), 29 deletions(-) diff --git a/java/org/apache/catalina/valves/LocalStrings.properties b/java/org/apache/catalina/valves/LocalStrings.properties index 908cb6f3ab..771bcc6f63 100644 --- a/java/org/apache/catalina/valves/LocalStrings.properties +++ b/java/org/apache/catalina/valves/LocalStrings.properties @@ -150,6 +150,7 @@ persistentValve.filter.failure=Unable to compile filter=[{0}] persistentValve.requestIgnore=The request for [{0}] was ignored by this Valve as it matches the configured filter persistentValve.requestProcess=The request for [{0}] will be processed by this Valve as it does not match the configured filter persistentValve.sessionLoadFail=Loading session [{0}] from the store failed +persistentValve.sessionSaveFail=Saving session [{0}] to the store failed proxyErrorReportValve.error=Proxy error to [{0}] diff --git a/java/org/apache/catalina/valves/PersistentValve.java b/java/org/apache/catalina/valves/PersistentValve.java index f3301367d2..d6474e6875 100644 --- a/java/org/apache/catalina/valves/PersistentValve.java +++ b/java/org/apache/catalina/valves/PersistentValve.java @@ -24,6 +24,9 @@ import java.util.concurrent.atomic.AtomicLong; import java.util.regex.Pattern; import java.util.regex.PatternSyntaxException; +import javax.servlet.AsyncContext; +import javax.servlet.AsyncEvent; +import javax.servlet.AsyncListener; import javax.servlet.ServletException; import javax.servlet.http.HttpServletResponse; @@ -202,31 +205,48 @@ public class PersistentValve extends ValveBase { // Ask the next valve to process the request. getNext().invoke(request, response); + } finally { + if (request.isAsync()) { + /* + * Need to continue to hold the semaphore until asynchronous processing is complete. Register a listener + * that will release the Semaphore once asynchronous processing is complete. Also need to delay session + * persistence until completion of the asynchronous processing. + */ + AsyncContext asyncContext = request.getAsyncContext(); + asyncContext.addListener( + new StoreSessionAsyncListener(request, context, sessionId, semaphore, mustReleaseSemaphore)); + } else { + storeSession(request, context, sessionId, semaphore, mustReleaseSemaphore); + } + } + } - // If still processing async, don't try to store the session - if (!request.isAsync()) { - // Read the sessionid after the response. - // HttpSession hsess = hreq.getSession(false); - Session hsess; - try { - hsess = request.getSessionInternal(false); - } catch (Exception e) { - hsess = null; - } - String newsessionId = null; - if (hsess != null) { - newsessionId = hsess.getIdInternal(); - } - if (containerLog.isTraceEnabled()) { - containerLog.trace("newsessionId: " + newsessionId); - } - if (newsessionId != null) { - try { - bind(context); + private void storeSession(Request request, Context context, String originalSessionId, + UsageCountingSemaphore semaphore, boolean mustReleaseSemaphore) { + try { + Manager manager = context.getManager(); + Session hsess; + try { + hsess = request.getSessionInternal(false); + } catch (Exception e) { + hsess = null; + } + String newsessionId = null; + if (hsess != null) { + newsessionId = hsess.getIdInternal(); + } + + if (containerLog.isTraceEnabled()) { + containerLog.trace("newsessionId: " + newsessionId); + } + if (newsessionId != null) { + try { + bind(context); - /* store the session and remove it from the manager */ - if (manager instanceof StoreManager) { + /* store the session and remove it from the manager */ + if (manager instanceof StoreManager) { + try { Session session = manager.findSession(newsessionId); Store store = ((StoreManager) manager).getStore(); boolean stored = false; @@ -247,14 +267,16 @@ public class PersistentValve extends ValveBase { " stale: " + isSessionStale(session, System.currentTimeMillis())); } } - } else { - if (containerLog.isTraceEnabled()) { - containerLog.trace("newsessionId Manager: " + manager); - } + } catch (IOException ioe) { + containerLog.warn(sm.getString("persistentValve.sessionSaveFail", newsessionId)); + } + } else { + if (containerLog.isTraceEnabled()) { + containerLog.trace("newsessionId Manager: " + manager); } - } finally { - unbind(context); } + } finally { + unbind(context); } } } finally { @@ -262,7 +284,7 @@ public class PersistentValve extends ValveBase { if (mustReleaseSemaphore) { semaphore.release(); } - sessionToSemaphoreMap.computeIfPresent(sessionId, + sessionToSemaphoreMap.computeIfPresent(originalSessionId, (k, v) -> v.decrementAndGetUsageCount() == 0 ? null : v); } } @@ -322,6 +344,7 @@ public class PersistentValve extends ValveBase { * Determines whether the given URI should bypass session persistence based on the configured filter. * * @param uri the request URI to check + * * @return {@code true} if the request should bypass session persistence, otherwise {@code false} */ protected boolean isRequestWithoutSession(String uri) { @@ -466,4 +489,44 @@ public class PersistentValve extends ValveBase { semaphore.release(); } } + + + private class StoreSessionAsyncListener implements AsyncListener { + + private final Request request; + private final Context context; + private final String originalSessionId; + private final UsageCountingSemaphore semaphore; + private final boolean mustReleaseSemaphore; + + StoreSessionAsyncListener(Request request, Context context, String originalSessionId, + UsageCountingSemaphore semaphore, boolean mustReleaseSemaphore) { + this.request = request; + this.context = context; + this.originalSessionId = originalSessionId; + this.semaphore = semaphore; + this.mustReleaseSemaphore = mustReleaseSemaphore; + } + + + @Override + public void onComplete(AsyncEvent event) throws IOException { + storeSession(request, context, originalSessionId, semaphore, mustReleaseSemaphore); + } + + @Override + public void onTimeout(AsyncEvent event) throws IOException { + // NO-OP. + } + + @Override + public void onError(AsyncEvent event) throws IOException { + // NO-OP. + } + + @Override + public void onStartAsync(AsyncEvent event) throws IOException { + event.getAsyncContext().addListener(this); + } + } } diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index fa94536bfe..858a74b1d0 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -227,6 +227,10 @@ Fix <code>CombinedRealm</code> <code>isAvailable</code>, it allows authentication if at least one sub realm is available. (remm) </fix> + <fix> + <bug>70048</bug>: Correctly handle asynchronous requests in + <code>PersistentValve</code>. (markt) + </fix> </changelog> </subsection> <subsection name="Coyote"> --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
