Author: markt
Date: Fri Dec 5 10:54:21 2014
New Revision: 1643232
URL: http://svn.apache.org/viewvc?rev=1643232&view=rev
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=57252
Provide application configured error pages with a chance to handle an
async error before the built-in error reporting.
Modified:
tomcat/tc8.0.x/trunk/ (props changed)
tomcat/tc8.0.x/trunk/java/org/apache/catalina/core/StandardHostValve.java
tomcat/tc8.0.x/trunk/java/org/apache/catalina/valves/ErrorReportValve.java
tomcat/tc8.0.x/trunk/test/org/apache/catalina/core/TestAsyncContextImpl.java
tomcat/tc8.0.x/trunk/webapps/docs/changelog.xml
Propchange: tomcat/tc8.0.x/trunk/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Fri Dec 5 10:54:21 2014
@@ -1 +1 @@
-/tomcat/trunk:1636524,1637156,1637176,1637188,1637331,1637684,1637695,1638720-1638725,1639653,1640010,1640084,1640088,1640275,1640322,1640347,1640361,1640365,1640403,1640410,1640652,1640655-1640658,1640688,1640700-1640883,1640903,1640976,1640978,1641000,1641026,1641038-1641039,1641051-1641052,1641058,1641064,1641300,1641369,1641374,1641380,1641486,1641634,1641656-1641692,1641704,1641707-1641718,1641720-1641722,1641735,1641981,1642668,1642679,1642697,1642699,1643002,1643066,1643121,1643206,1643216
+/tomcat/trunk:1636524,1637156,1637176,1637188,1637331,1637684,1637695,1638720-1638725,1639653,1640010,1640084,1640088,1640275,1640322,1640347,1640361,1640365,1640403,1640410,1640652,1640655-1640658,1640688,1640700-1640883,1640903,1640976,1640978,1641000,1641026,1641038-1641039,1641051-1641052,1641058,1641064,1641300,1641369,1641374,1641380,1641486,1641634,1641656-1641692,1641704,1641707-1641718,1641720-1641722,1641735,1641981,1642668,1642679,1642697,1642699,1643002,1643066,1643121,1643206,1643209-1643210,1643216
Modified:
tomcat/tc8.0.x/trunk/java/org/apache/catalina/core/StandardHostValve.java
URL:
http://svn.apache.org/viewvc/tomcat/tc8.0.x/trunk/java/org/apache/catalina/core/StandardHostValve.java?rev=1643232&r1=1643231&r2=1643232&view=diff
==============================================================================
--- tomcat/tc8.0.x/trunk/java/org/apache/catalina/core/StandardHostValve.java
(original)
+++ tomcat/tc8.0.x/trunk/java/org/apache/catalina/core/StandardHostValve.java
Fri Dec 5 10:54:21 2014
@@ -121,9 +121,6 @@ final class StandardHostValve extends Va
boolean asyncAtStart = request.isAsync();
boolean asyncDispatching = request.isAsyncDispatching();
- // An async error page may dispatch to another resource. This flag
helps
- // ensure an infinite error handling loop is not entered
- boolean errorAtStart = response.isError();
try {
context.bind(Globals.IS_SECURITY_ENABLED, MY_CLASSLOADER);
@@ -136,18 +133,26 @@ final class StandardHostValve extends Va
return;
}
- // Ask this Context to process this request
+ // Ask this Context to process this request. Requests that are in
+ // async mode and are not being dispatched to this resource must be
+ // in error and have been routed here to check for application
+ // defined error pages.
try {
if (!asyncAtStart || asyncDispatching) {
context.getPipeline().getFirst().invoke(request, response);
} else {
- if (!errorAtStart) {
+ // Make sure this request/response is here because an error
+ // report is required.
+ if (!response.isErrorReportRequired()) {
throw new
IllegalStateException(sm.getString("standardHost.asyncStateError"));
}
}
} catch (Throwable t) {
ExceptionUtils.handleThrowable(t);
- if (errorAtStart) {
+ // If a new error occurred while trying to report a previous
+ // error simply log the new error and allow the original error
+ // to be reported.
+ if (response.isErrorReportRequired()) {
container.getLogger().error("Exception Processing " +
request.getRequestURI(), t);
} else {
@@ -158,28 +163,27 @@ final class StandardHostValve extends Va
Throwable t = (Throwable)
request.getAttribute(RequestDispatcher.ERROR_EXCEPTION);
- // If the request was async at the start and an error occurred
- // then the async error handling will kick-in and that will fire
- // the request destroyed event *after* the error handling has
- // taken place.
- if (!(request.isAsync() || (asyncAtStart && t != null))) {
- // Protect against NPEs if context was destroyed during a
- // long running request.
- if (context.getState().isAvailable()) {
- if (!errorAtStart) {
- // Error page processing
- response.setSuspended(false);
-
- if (t != null) {
- throwable(request, response, t);
- } else {
- status(request, response);
- }
- }
+ // Protect against NPEs if the context was destroyed during a
+ // long running request.
+ if (!context.getState().isAvailable()) {
+ return;
+ }
+
+ // Look for (and render if found) an application level error page
+ if (response.isErrorReportRequired()) {
+ // Error page processing
+ response.setSuspended(false);
- context.fireRequestDestroyEvent(request);
+ if (t != null) {
+ throwable(request, response, t);
+ } else {
+ status(request, response);
}
}
+
+ if (!request.isAsync() && !response.isErrorReportRequired()) {
+ context.fireRequestDestroyEvent(request);
+ }
} finally {
// Access a session (if present) to update last accessed time,
based
// on a strict interpretation of the specification
Modified:
tomcat/tc8.0.x/trunk/java/org/apache/catalina/valves/ErrorReportValve.java
URL:
http://svn.apache.org/viewvc/tomcat/tc8.0.x/trunk/java/org/apache/catalina/valves/ErrorReportValve.java?rev=1643232&r1=1643231&r2=1643232&view=diff
==============================================================================
--- tomcat/tc8.0.x/trunk/java/org/apache/catalina/valves/ErrorReportValve.java
(original)
+++ tomcat/tc8.0.x/trunk/java/org/apache/catalina/valves/ErrorReportValve.java
Fri Dec 5 10:54:21 2014
@@ -97,8 +97,10 @@ public class ErrorReportValve extends Va
Throwable throwable = (Throwable)
request.getAttribute(RequestDispatcher.ERROR_EXCEPTION);
- if (request.isAsyncStarted() && ((response.getStatus() < 400 &&
- throwable == null) || request.isAsyncDispatching())) {
+ // If an async request is in progress and is not going to end once this
+ // container thread finishes, do not trigger error page handling - it
+ // will be triggered later if required.
+ if (request.isAsync() && !request.isAsyncCompleting()) {
return;
}
@@ -122,11 +124,6 @@ public class ErrorReportValve extends Va
} catch (Throwable tt) {
ExceptionUtils.handleThrowable(tt);
}
-
- if (request.isAsyncStarted() && !request.isAsyncCompleting() &&
- !request.isAsyncDispatching()) {
- request.getAsyncContext().complete();
- }
}
Modified:
tomcat/tc8.0.x/trunk/test/org/apache/catalina/core/TestAsyncContextImpl.java
URL:
http://svn.apache.org/viewvc/tomcat/tc8.0.x/trunk/test/org/apache/catalina/core/TestAsyncContextImpl.java?rev=1643232&r1=1643231&r2=1643232&view=diff
==============================================================================
---
tomcat/tc8.0.x/trunk/test/org/apache/catalina/core/TestAsyncContextImpl.java
(original)
+++
tomcat/tc8.0.x/trunk/test/org/apache/catalina/core/TestAsyncContextImpl.java
Fri Dec 5 10:54:21 2014
@@ -1243,15 +1243,25 @@ public class TestAsyncContextImpl extend
@Test
public void testBug51197a() throws Exception {
- doTestBug51197(false);
+ doTestBug51197(false, false);
}
@Test
public void testBug51197b() throws Exception {
- doTestBug51197(true);
+ doTestBug51197(true, false);
}
- private void doTestBug51197(boolean threaded) throws Exception {
+ @Test
+ public void testBug51197c() throws Exception {
+ doTestBug51197(false, true);
+ }
+
+ @Test
+ public void testBug51197d() throws Exception {
+ doTestBug51197(true, true);
+ }
+
+ private void doTestBug51197(boolean threaded, boolean customError) throws
Exception {
// Setup Tomcat instance
Tomcat tomcat = getTomcatInstance();
@@ -1265,6 +1275,17 @@ public class TestAsyncContextImpl extend
wrapper.setAsyncSupported(true);
ctx.addServletMapping("/asyncErrorServlet", "asyncErrorServlet");
+ if (customError) {
+ CustomErrorServlet customErrorServlet = new CustomErrorServlet();
+ Tomcat.addServlet(ctx, "customErrorServlet", customErrorServlet);
+ ctx.addServletMapping("/customErrorServlet", "customErrorServlet");
+
+ ErrorPage ep = new ErrorPage();
+ ep.setLocation("/customErrorServlet");
+
+ ctx.addErrorPage(ep);
+ }
+
TesterAccessLogValve alv = new TesterAccessLogValve();
ctx.getPipeline().addValve(alv);
@@ -1284,7 +1305,14 @@ public class TestAsyncContextImpl extend
// responsibility when an error occurs on an application thread.
// Calling sendError() followed by complete() and expecting the
standard
// error page mechanism to kick in could be viewed as handling the
error
- assertTrue(res.getLength() > 0);
+ String responseBody = res.toString();
+ Assert.assertNotNull(responseBody);
+ assertTrue(responseBody.length() > 0);
+ if (customError) {
+ assertTrue(responseBody,
responseBody.contains(CustomErrorServlet.ERROR_MESSAGE));
+ } else {
+ assertTrue(responseBody,
responseBody.contains(AsyncErrorServlet.ERROR_MESSAGE));
+ }
// Without this test may complete before access log has a chance to log
// the request
@@ -1295,6 +1323,21 @@ public class TestAsyncContextImpl extend
REQUEST_TIME);
}
+ private static class CustomErrorServlet extends GenericServlet {
+
+ private static final long serialVersionUID = 1L;
+
+ public static final String ERROR_MESSAGE = "Custom error page";
+
+ @Override
+ public void service(ServletRequest req, ServletResponse res)
+ throws ServletException, IOException {
+ res.getWriter().println(ERROR_MESSAGE);
+ }
+
+ }
+
+
private static class AsyncErrorServlet extends HttpServlet {
private static final long serialVersionUID = 1L;
@@ -1310,7 +1353,7 @@ public class TestAsyncContextImpl extend
}
@Override
- protected void doGet(HttpServletRequest req, HttpServletResponse resp)
+ protected void doGet(HttpServletRequest req, final HttpServletResponse
resp)
throws ServletException, IOException {
final AsyncContext actxt = req.startAsync();
@@ -1320,11 +1363,7 @@ public class TestAsyncContextImpl extend
@Override
public void run() {
try {
- HttpServletResponse resp =
- (HttpServletResponse) actxt.getResponse();
resp.sendError(status, ERROR_MESSAGE);
- // Complete so there is no delay waiting for the
- // timeout
actxt.complete();
} catch (IOException e) {
// Ignore
@@ -1332,7 +1371,8 @@ public class TestAsyncContextImpl extend
}
});
} else {
- resp.sendError(status);
+ resp.sendError(status, ERROR_MESSAGE);
+ actxt.complete();
}
}
}
Modified: tomcat/tc8.0.x/trunk/webapps/docs/changelog.xml
URL:
http://svn.apache.org/viewvc/tomcat/tc8.0.x/trunk/webapps/docs/changelog.xml?rev=1643232&r1=1643231&r2=1643232&view=diff
==============================================================================
--- tomcat/tc8.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc8.0.x/trunk/webapps/docs/changelog.xml Fri Dec 5 10:54:21 2014
@@ -98,6 +98,11 @@
thread can not change the async state until the container thread has
completed. (markt)
</fix>
+ <fix>
+ <bug>57252</bug>: Provide application configured error pages with a
+ chance to handle an async error before the built-in error reporting.
+ (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Coyote">
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
