This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/main by this push:
     new fa0f256  Improve custom error page handling for committed responses
fa0f256 is described below

commit fa0f2567aae91a13137094ebcdb74d72b8222f3b
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Tue Aug 17 11:32:27 2021 +0100

    Improve custom error page handling for committed responses
---
 .../apache/catalina/core/StandardHostValve.java    | 13 +++++
 .../catalina/core/TestStandardHostValve.java       | 58 ++++++++++++++++++++++
 webapps/docs/changelog.xml                         |  8 +++
 3 files changed, 79 insertions(+)

diff --git a/java/org/apache/catalina/core/StandardHostValve.java 
b/java/org/apache/catalina/core/StandardHostValve.java
index 81cbff4..2c7004e 100644
--- a/java/org/apache/catalina/core/StandardHostValve.java
+++ b/java/org/apache/catalina/core/StandardHostValve.java
@@ -366,6 +366,19 @@ final class StandardHostValve extends ValveBase {
                 // Response is committed - including the error page is the
                 // best we can do
                 rd.include(request.getRequest(), response.getResponse());
+
+                // Ensure the combined incomplete response and error page is
+                // written to the client
+                try {
+                    response.flushBuffer();
+                } catch (Throwable t) {
+                    ExceptionUtils.handleThrowable(t);
+                }
+
+                // Now close immediately as an additional signal to the client
+                // that something went wrong
+                response.getCoyoteResponse().action(ActionCode.CLOSE_NOW,
+                        
request.getAttribute(RequestDispatcher.ERROR_EXCEPTION));
             } else {
                 // Reset the response (keeping the real error code and message)
                 response.resetBuffer(true);
diff --git a/test/org/apache/catalina/core/TestStandardHostValve.java 
b/test/org/apache/catalina/core/TestStandardHostValve.java
index 130173a..9e6927c 100644
--- a/test/org/apache/catalina/core/TestStandardHostValve.java
+++ b/test/org/apache/catalina/core/TestStandardHostValve.java
@@ -130,6 +130,7 @@ public class TestStandardHostValve extends TomcatBaseTest {
         Assert.assertTrue(result.contains("Visit requestDestroyed"));
     }
 
+
     private void doTestErrorPageHandling(int error, String report)
             throws Exception {
 
@@ -142,6 +143,49 @@ public class TestStandardHostValve extends TomcatBaseTest {
         Assert.assertEquals(report, bc.toString());
     }
 
+
+    @Test
+    public void testIncompleteResponse() throws Exception {
+        // Set up a container
+        Tomcat tomcat = getTomcatInstance();
+
+        // No file system docBase required
+        Context ctx = tomcat.addContext("", null);
+
+        // Add the error page
+        Tomcat.addServlet(ctx, "error", new ExceptionServlet());
+        ctx.addServletMappingDecoded("/error", "error");
+
+        // Add the error handling page
+        Tomcat.addServlet(ctx, "report", new ReportServlet());
+        ctx.addServletMappingDecoded("/report/*", "report");
+
+        // And the handling for 500 responses
+        ErrorPage errorPage500 = new ErrorPage();
+        errorPage500.setErrorCode(Response.SC_INTERNAL_SERVER_ERROR);
+        errorPage500.setLocation("/report/500");
+        ctx.addErrorPage(errorPage500);
+
+        // And the default error handling
+        ErrorPage errorPageDefault = new ErrorPage();
+        errorPageDefault.setLocation("/report/default");
+        ctx.addErrorPage(errorPageDefault);
+
+        tomcat.start();
+
+        // Request a page that triggers an error
+        ByteChunk bc = new ByteChunk();
+        Throwable t = null;
+        try {
+            getUrl("http://localhost:"; + getPort() + "/error", bc, null);
+            System.out.println(bc.toString());
+        } catch (IOException ioe) {
+            t = ioe;
+        }
+        Assert.assertNotNull(t);
+    }
+
+
     private static class ErrorServlet extends HttpServlet {
 
         private static final long serialVersionUID = 1L;
@@ -154,6 +198,20 @@ public class TestStandardHostValve extends TomcatBaseTest {
         }
     }
 
+
+    private static class ExceptionServlet extends HttpServlet {
+
+        private static final long serialVersionUID = 1L;
+
+        @Override
+        protected void doGet(HttpServletRequest req, HttpServletResponse resp)
+                throws ServletException, IOException {
+            resp.flushBuffer();
+            throw new IOException();
+        }
+    }
+
+
     private static class ReportServlet extends HttpServlet {
 
         private static final long serialVersionUID = 1L;
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index b29ecfd..5330154 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -112,6 +112,14 @@
         not support <code>allow-java-encodings</code>. A warning will be logged
         if such an XML parser is detected. (markt)
       </fix>
+      <fix>
+        Change the behaviour of custom error pages. If an error occurs after 
the
+        response is committed, once the custom error page content has been 
added
+        to the response the connection is now closed immediately rather than
+        closed cleanly. i.e. the last chunk that marks the end of the response
+        body is no longer sent. This acts as an additional signal to the client
+        that the request experienced an error. (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

Reply via email to