tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ma...@apache.org
Subject svn commit: r1722939 - in /tomcat/trunk: java/org/apache/catalina/core/AsyncContextImpl.java test/org/apache/catalina/core/TestAsyncContextImpl.java webapps/docs/changelog.xml
Date Mon, 04 Jan 2016 18:12:07 GMT
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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Mime
View raw message