Author: violetagg
Date: Wed Sep 24 19:48:54 2014
New Revision: 1627401
URL: http://svn.apache.org/r1627401
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=56739
Merged revision 1616441 from tomcat/trunk:
If an application handles an error on an application thread during asynchronous
processing by calling HttpServletResponse.sendError(), then ensure that the
application is given an opportunity to report that error via an appropriate
application defined error page if one is configured.
Modified:
tomcat/tc7.0.x/trunk/ (props changed)
tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java
tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/Response.java
tomcat/tc7.0.x/trunk/java/org/apache/catalina/core/LocalStrings.properties
tomcat/tc7.0.x/trunk/java/org/apache/catalina/core/StandardHostValve.java
tomcat/tc7.0.x/trunk/java/org/apache/catalina/valves/ErrorReportValve.java
tomcat/tc7.0.x/trunk/test/org/apache/catalina/core/TestAsyncContextImpl.java
tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml
Propchange: tomcat/tc7.0.x/trunk/
------------------------------------------------------------------------------
Merged /tomcat/trunk:r1616441
Modified:
tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java
URL:
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java?rev=1627401&r1=1627400&r2=1627401&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java
(original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java
Wed Sep 24 19:48:54 2014
@@ -294,6 +294,14 @@ public class CoyoteAdapter implements Ad
asyncConImpl.setErrorState(null, false);
}
}
+ // Has an error occurred during async processing that needs to be
+ // processed by the application's error page mechanism (or Tomcat's
+ // if the application doesn't define one)?
+ if (!request.isAsyncDispatching() && request.isAsync() &&
+ response.isErrorReportRequired()) {
+
connector.getService().getContainer().getPipeline().getFirst().invoke(request,
response);
+ }
+
if (request.isAsyncDispatching()) {
success = true;
connector.getService().getContainer().getPipeline().getFirst().invoke(request,
response);
Modified: tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/Response.java
URL:
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/Response.java?rev=1627401&r1=1627400&r2=1627401&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/Response.java
(original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/Response.java Wed
Sep 24 19:48:54 2014
@@ -34,6 +34,7 @@ import java.util.List;
import java.util.Locale;
import java.util.TimeZone;
import java.util.Vector;
+import java.util.concurrent.atomic.AtomicInteger;
import javax.servlet.ServletOutputStream;
import javax.servlet.SessionTrackingMode;
@@ -235,10 +236,35 @@ public class Response
private boolean isCharacterEncodingSet = false;
/**
- * The error flag.
+ * With the introduction of async processing and the possibility of
+ * non-container threads calling sendError() tracking the current error
+ * state and ensuring that the correct error page is called becomes more
+ * complicated. This state attribute helps by tracking the current error
+ * state and informing callers that attempt to change state if the change
+ * was successful or if another thread got there first.
+ *
+ * <pre>
+ * The state machine is very simple:
+ *
+ * 0 - NONE
+ * 1 - NOT_REPORTED
+ * 2 - REPORTED
+ *
+ *
+ * -->---->-- >NONE
+ * | | |
+ * | | | setError()
+ * ^ ^ |
+ * | | \|/
+ * | |-<-NOT_REPORTED
+ * | |
+ * ^ | report()
+ * | |
+ * | \|/
+ * |----<----REPORTED
+ * </pre>
*/
- protected boolean error = false;
- private boolean errorAfterCommit = false;
+ private AtomicInteger errorState = new AtomicInteger(0);
/**
@@ -279,8 +305,7 @@ public class Response
usingWriter = false;
appCommitted = false;
included = false;
- error = false;
- errorAfterCommit = false;
+ errorState.set(0);
isCharacterEncodingSet = false;
if (Globals.IS_SECURITY_ENABLED || Connector.RECYCLE_FACADES) {
@@ -470,15 +495,15 @@ public class Response
/**
* Set the error flag.
*/
- public void setError() {
- if (!error) {
- error = true;
- errorAfterCommit = coyoteResponse.isCommitted();
+ public boolean setError() {
+ boolean result = errorState.compareAndSet(0, 1);
+ if (result) {
Wrapper wrapper = getRequest().getWrapper();
if (wrapper != null) {
wrapper.incrementErrorCount();
}
}
+ return result;
}
@@ -486,12 +511,17 @@ public class Response
* Error flag accessor.
*/
public boolean isError() {
- return error;
+ return errorState.get() > 0;
+ }
+
+
+ public boolean isErrorReportRequired() {
+ return errorState.get() == 1;
}
- public boolean isErrorAfterCommit() {
- return errorAfterCommit;
+ public boolean setErrorReported() {
+ return errorState.compareAndSet(1, 2);
}
Modified:
tomcat/tc7.0.x/trunk/java/org/apache/catalina/core/LocalStrings.properties
URL:
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/core/LocalStrings.properties?rev=1627401&r1=1627400&r2=1627401&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/catalina/core/LocalStrings.properties
(original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/catalina/core/LocalStrings.properties
Wed Sep 24 19:48:54 2014
@@ -200,6 +200,7 @@ standardEngine.unregister.mbeans.failed=
standardHost.accessBase=Cannot access document base directory {0}
standardHost.alreadyStarted=Host has already been started
standardHost.appBase=Application base directory {0} does not exist
+standardHost.asyncStateError=An asynchronous request was received for
processing that was neither an async dispatch nor an error to process
standardHost.clientAbort=Remote Client Aborted Request, IOException: {0}
standardHost.configRequired=URL to configuration file is required
standardHost.configNotAllowed=Use of configuration file is not allowed
Modified:
tomcat/tc7.0.x/trunk/java/org/apache/catalina/core/StandardHostValve.java
URL:
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/core/StandardHostValve.java?rev=1627401&r1=1627400&r2=1627401&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/catalina/core/StandardHostValve.java
(original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/catalina/core/StandardHostValve.java
Wed Sep 24 19:48:54 2014
@@ -157,10 +157,8 @@ final class StandardHostValve extends Va
request.setAsyncSupported(context.getPipeline().isAsyncSupported());
}
- // Don't fire listeners during async processing
- // If a request init listener throws an exception, the request is
- // aborted
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();
@@ -168,7 +166,13 @@ final class StandardHostValve extends Va
// Ask this Context to process this request
try {
- context.getPipeline().getFirst().invoke(request, response);
+ if (!asyncAtStart || asyncDispatching) {
+ context.getPipeline().getFirst().invoke(request, response);
+ } else {
+ if (!errorAtStart) {
+ throw new
IllegalStateException(sm.getString("standardHost.asyncStateError"));
+ }
+ }
} catch (Throwable t) {
ExceptionUtils.handleThrowable(t);
if (errorAtStart) {
@@ -314,7 +318,7 @@ final class StandardHostValve extends Va
// Look for a default error page
errorPage = context.findErrorPage(0);
}
- if (errorPage != null) {
+ if (errorPage != null && response.setErrorReported()) {
response.setAppCommitted(false);
request.setAttribute(RequestDispatcher.ERROR_STATUS_CODE,
Integer.valueOf(statusCode));
@@ -390,30 +394,32 @@ final class StandardHostValve extends Va
}
if (errorPage != null) {
- response.setAppCommitted(false);
- request.setAttribute(Globals.DISPATCHER_REQUEST_PATH_ATTR,
- errorPage.getLocation());
- request.setAttribute(Globals.DISPATCHER_TYPE_ATTR,
- DispatcherType.ERROR);
- request.setAttribute(RequestDispatcher.ERROR_STATUS_CODE,
- new Integer(HttpServletResponse.SC_INTERNAL_SERVER_ERROR));
- request.setAttribute(RequestDispatcher.ERROR_MESSAGE,
- throwable.getMessage());
- request.setAttribute(RequestDispatcher.ERROR_EXCEPTION,
- realError);
- Wrapper wrapper = request.getWrapper();
- if (wrapper != null)
- request.setAttribute(RequestDispatcher.ERROR_SERVLET_NAME,
- wrapper.getName());
- request.setAttribute(RequestDispatcher.ERROR_REQUEST_URI,
- request.getRequestURI());
- request.setAttribute(RequestDispatcher.ERROR_EXCEPTION_TYPE,
- realError.getClass());
- if (custom(request, response, errorPage)) {
- try {
- response.finishResponse();
- } catch (IOException e) {
- container.getLogger().warn("Exception Processing " +
errorPage, e);
+ if (response.setErrorReported()) {
+ response.setAppCommitted(false);
+ request.setAttribute(Globals.DISPATCHER_REQUEST_PATH_ATTR,
+ errorPage.getLocation());
+ request.setAttribute(Globals.DISPATCHER_TYPE_ATTR,
+ DispatcherType.ERROR);
+ request.setAttribute(RequestDispatcher.ERROR_STATUS_CODE,
+ new
Integer(HttpServletResponse.SC_INTERNAL_SERVER_ERROR));
+ request.setAttribute(RequestDispatcher.ERROR_MESSAGE,
+ throwable.getMessage());
+ request.setAttribute(RequestDispatcher.ERROR_EXCEPTION,
+ realError);
+ Wrapper wrapper = request.getWrapper();
+ if (wrapper != null)
+ request.setAttribute(RequestDispatcher.ERROR_SERVLET_NAME,
+ wrapper.getName());
+ request.setAttribute(RequestDispatcher.ERROR_REQUEST_URI,
+ request.getRequestURI());
+ request.setAttribute(RequestDispatcher.ERROR_EXCEPTION_TYPE,
+ realError.getClass());
+ if (custom(request, response, errorPage)) {
+ try {
+ response.finishResponse();
+ } catch (IOException e) {
+ container.getLogger().warn("Exception Processing " +
errorPage, e);
+ }
}
}
} else {
Modified:
tomcat/tc7.0.x/trunk/java/org/apache/catalina/valves/ErrorReportValve.java
URL:
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/valves/ErrorReportValve.java?rev=1627401&r1=1627400&r2=1627401&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/catalina/valves/ErrorReportValve.java
(original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/catalina/valves/ErrorReportValve.java
Wed Sep 24 19:48:54 2014
@@ -103,9 +103,10 @@ public class ErrorReportValve extends Va
getNext().invoke(request, response);
if (response.isCommitted()) {
- if (response.isErrorAfterCommit()) {
- // Attempt to flush any data that is still to be written to the
- // client
+ if (response.setErrorReported()) {
+ // Error wasn't previously reported but we can't write an error
+ // page because the response has already been committed.
Attempt
+ // to flush any data that is still to be written to the client.
try {
response.flushBuffer();
} catch (Throwable t) {
@@ -170,7 +171,8 @@ public class ErrorReportValve extends Va
// Do nothing on a 1xx, 2xx and 3xx status
// Do nothing if anything has been written already
// Do nothing if the response hasn't been explicitly marked as in error
- if (statusCode < 400 || response.getContentWritten() > 0 ||
!response.isError()) {
+ // and that error has not been reported.
+ if (statusCode < 400 || response.getContentWritten() > 0 ||
!response.setErrorReported()) {
return;
}
String message = RequestUtil.filter(response.getMessage());
Modified:
tomcat/tc7.0.x/trunk/test/org/apache/catalina/core/TestAsyncContextImpl.java
URL:
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/test/org/apache/catalina/core/TestAsyncContextImpl.java?rev=1627401&r1=1627400&r2=1627401&view=diff
==============================================================================
---
tomcat/tc7.0.x/trunk/test/org/apache/catalina/core/TestAsyncContextImpl.java
(original)
+++
tomcat/tc7.0.x/trunk/test/org/apache/catalina/core/TestAsyncContextImpl.java
Wed Sep 24 19:48:54 2014
@@ -1303,14 +1303,11 @@ public class TestAsyncContextImpl extend
assertEquals(HttpServletResponse.SC_BAD_REQUEST, rc);
- // SRV 10.9.2 - Writing the response is entirely the application's
+ // SRV 10.9.2 - Handling an error is entirely the application's
// responsibility when an error occurs on an application thread.
- // The test servlet writes no content in this case.
- if (threaded) {
- assertEquals(0, res.getLength());
- } else {
- assertTrue(res.getLength() > 0);
- }
+ // 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);
// Without this test may complete before access log has a chance to log
// the request
Modified: tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml
URL:
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml?rev=1627401&r1=1627400&r2=1627401&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Wed Sep 24 19:48:54 2014
@@ -77,6 +77,13 @@
starts processing the dispatch. (markt)
</fix>
<fix>
+ <bug>56739</bug>: If an application handles an error on an application
+ thread during asynchronous processing by calling
+ <code>HttpServletResponse.sendError()</code>, then ensure that the
+ application is given an opportunity to report that error via an
+ appropriate application defined error page if one is configured.
(markt)
+ </fix>
+ <fix>
<bug>56771</bug>: When lookup for a resource in all the alternate or
backup <code>javax.naming.directory.DirContext</code>,
<code>javax.naming.NameNotFoundException</code> will be thrown at the
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
