Author: markt
Date: Mon Jan 4 18:12:07 2016
New Revision: 1722939
URL: http://svn.apache.org/viewvc?rev=1722939&view=rev
Log:
Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=58751
Correctly handle the case where an AsyncListener dispatches to a Servlet on an
asynchronous timeout and the Servlet uses <code>sendError()</code> to trigger
an error page.
Test case based on code provided by Andy Wilkinson.
Modified:
tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java
tomcat/trunk/test/org/apache/catalina/core/TestAsyncContextImpl.java
tomcat/trunk/webapps/docs/changelog.xml
Modified: tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java?rev=1722939&r1=1722938&r2=1722939&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java (original)
+++ tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java Mon Jan 4
18:12:07 2016
@@ -97,6 +97,29 @@ public class AsyncContextImpl implements
@Override
public void fireOnComplete() {
+ // Fire the listeners
+ doFireOnComplete();
+
+ // The application doesn't know it has to stop read and/or writing
until
+ // it receives the complete event so the request and response have to
be
+ // closed after firing the event.
+ try {
+ // First of all ensure that any data written to the response is
+ // written to the I/O layer.
+ request.getResponse().finishResponse();
+ // Close the request and the response.
+ request.getCoyoteRequest().action(ActionCode.END_REQUEST, null);
+ } catch (Throwable t) {
+ ExceptionUtils.handleThrowable(t);
+ // Catch this here and allow async context complete to continue
+ // normally so a dispatch takes place which ensures that the
+ // request and response objects are correctly recycled.
+ log.debug(sm.getString("asyncContextImpl.finishResponseError"), t);
+ }
+ }
+
+
+ private void doFireOnComplete() {
List<AsyncListenerWrapper> listenersCopy = new ArrayList<>();
listenersCopy.addAll(listeners);
@@ -115,25 +138,9 @@ public class AsyncContextImpl implements
clearServletRequestResponse();
context.unbind(Globals.IS_SECURITY_ENABLED, oldCL);
}
-
- // The application doesn't know it has to stop read and/or writing
until
- // it receives the complete event so the request and response have to
be
- // closed after firing the event.
- try {
- // First of all ensure that any data written to the response is
- // written to the I/O layer.
- request.getResponse().finishResponse();
- // Close the request and the response.
- request.getCoyoteRequest().action(ActionCode.END_REQUEST, null);
- } catch (Throwable t) {
- ExceptionUtils.handleThrowable(t);
- // Catch this here and allow async context complete to continue
- // normally so a dispatch takes place which ensures that the
- // request and response objects are correctly recycled.
- log.debug(sm.getString("asyncContextImpl.finishResponseError"), t);
- }
}
+
public boolean timeout() {
AtomicBoolean result = new AtomicBoolean();
request.getCoyoteRequest().action(ActionCode.ASYNC_TIMEOUT, result);
@@ -383,7 +390,9 @@ public class AsyncContextImpl implements
dispatch = null;
runnable.run();
if (!request.isAsync()) {
- fireOnComplete();
+ // Uses internal method since we don't want the
request/response
+ // to be closed. That will be handled in the adapter.
+ doFireOnComplete();
}
} catch (RuntimeException x) {
// doInternalComplete(true);
Modified: tomcat/trunk/test/org/apache/catalina/core/TestAsyncContextImpl.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/core/TestAsyncContextImpl.java?rev=1722939&r1=1722938&r2=1722939&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/core/TestAsyncContextImpl.java
(original)
+++ tomcat/trunk/test/org/apache/catalina/core/TestAsyncContextImpl.java Mon
Jan 4 18:12:07 2016
@@ -1347,7 +1347,7 @@ public class TestAsyncContextImpl extend
@Override
public void service(ServletRequest req, ServletResponse res)
throws ServletException, IOException {
- res.getWriter().println(ERROR_MESSAGE);
+ res.getWriter().print(ERROR_MESSAGE);
}
}
@@ -2288,4 +2288,74 @@ public class TestAsyncContextImpl extend
}
}
}
+
+
+ /*
+ * See https://bz.apache.org/bugzilla/show_bug.cgi?id=58751 comment 1
+ */
+ @Test
+ public void testTimeoutDispatchCustomErrorPage() throws Exception {
+ Tomcat tomcat = getTomcatInstance();
+ Context context = tomcat.addContext("", null);
+ tomcat.addServlet("", "timeout", Bug58751AsyncServlet.class.getName())
+ .setAsyncSupported(true);
+ CustomErrorServlet customErrorServlet = new CustomErrorServlet();
+ Tomcat.addServlet(context, "customErrorServlet", customErrorServlet);
+ context.addServletMapping("/timeout", "timeout");
+ context.addServletMapping("/error", "customErrorServlet");
+ ErrorPage errorPage = new ErrorPage();
+ errorPage.setLocation("/error");
+ context.addErrorPage(errorPage);
+ tomcat.start();
+
+ ByteChunk responseBody = new ByteChunk();
+ int rc = getUrl("http://localhost:"; + getPort() + "/timeout",
responseBody, null);
+
+ Assert.assertEquals(503, rc);
+ Assert.assertEquals(CustomErrorServlet.ERROR_MESSAGE,
responseBody.toString());
+ }
+
+
+ public static class Bug58751AsyncServlet extends HttpServlet {
+
+ private static final long serialVersionUID = 1L;
+
+ @Override
+ protected void doGet(HttpServletRequest req, HttpServletResponse resp)
+ throws ServletException, IOException {
+ if (req.getAttribute("timeout") != null) {
+ resp.sendError(503);
+ }
+ else {
+ final AsyncContext context = req.startAsync();
+ context.setTimeout(5000);
+ context.addListener(new AsyncListener() {
+
+ @Override
+ public void onTimeout(AsyncEvent event) throws IOException
{
+ HttpServletResponse response = (HttpServletResponse)
event
+ .getSuppliedResponse();
+ if (!response.isCommitted()) {
+ ((HttpServletRequest) event.getSuppliedRequest())
+ .setAttribute("timeout", Boolean.TRUE);
+ context.dispatch();
+ }
+ }
+
+ @Override
+ public void onStartAsync(AsyncEvent event) throws
IOException {
+ }
+
+ @Override
+ public void onError(AsyncEvent event) throws IOException {
+ }
+
+ @Override
+ public void onComplete(AsyncEvent event) throws
IOException {
+ }
+ });
+ }
+ }
+
+ }
}
Modified: tomcat/trunk/webapps/docs/changelog.xml
URL:
http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1722939&r1=1722938&r2=1722939&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Mon Jan 4 18:12:07 2016
@@ -164,6 +164,13 @@
<code>org.apache.catalina.core.DefaultInstanceManager</code> class.
(kkolinko)
</fix>
+ <fix>
+ <bug>58751</bug>: Correctly handle the case where an
+ <code>AsyncListener</code> dispatches to a Servlet on an asynchronous
+ timeout and the Servlet uses <code>sendError()</code> to trigger an
+ error page. Includes a test case based on code provided by Andy
+ Wilkinson.(markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Coyote">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
