Author: markt Date: Thu Feb 8 15:15:57 2018 New Revision: 1823565 URL: http://svn.apache.org/viewvc?rev=1823565&view=rev Log: Providing the error does not require the immediate closure of the connection, pass all errors (invalid requests etc.) that occur before the request processing pipeline is reached to the standard error handling mechanism so that the application error handling or the ErrorReportVlave can handle it.
Modified: tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java tomcat/trunk/java/org/apache/coyote/Response.java tomcat/trunk/java/org/apache/coyote/ajp/AjpProcessor.java tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java tomcat/trunk/test/org/apache/coyote/ajp/TestAbstractAjpProcessor.java tomcat/trunk/test/org/apache/tomcat/util/http/TestMimeHeadersIntegration.java tomcat/trunk/webapps/docs/changelog.xml Modified: tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java?rev=1823565&r1=1823564&r2=1823565&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java (original) +++ tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java Thu Feb 8 15:15:57 2018 @@ -393,14 +393,25 @@ public class CoyoteAdapter implements Ad // Log only if processing was invoked. // If postParseRequest() failed, it has already logged it. Context context = request.getContext(); + Host host = request.getHost(); // If the context is null, it is likely that the endpoint was // shutdown, this connection closed and the request recycled in // a different thread. That thread will have updated the access // log so it is OK not to update the access log here in that // case. + // The other possibility is that an error occurred early in + // processing and the request could not be mapped to a Context. + // Log via the host or engine in that case. + long time = System.currentTimeMillis() - req.getStartTime(); if (context != null) { - context.logAccess(request, response, - System.currentTimeMillis() - req.getStartTime(), false); + context.logAccess(request, response, time, false); + } else if (response.isError()) { + if (host != null) { + host.logAccess(request, response, time, false); + } else { + connector.getService().getContainer().logAccess( + request, response, time, false); + } } } Modified: tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java?rev=1823565&r1=1823564&r2=1823565&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java (original) +++ tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java Thu Feb 8 15:15:57 2018 @@ -84,6 +84,7 @@ public abstract class AbstractProcessor * @param t The error which occurred */ protected void setErrorState(ErrorState errorState, Throwable t) { + response.setError(); boolean blockIo = this.errorState.isIoAllowed() && !errorState.isIoAllowed(); this.errorState = this.errorState.getMostSevere(errorState); // Don't change the status code for IOException since that is almost Modified: tomcat/trunk/java/org/apache/coyote/Response.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/Response.java?rev=1823565&r1=1823564&r2=1823565&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/Response.java (original) +++ tomcat/trunk/java/org/apache/coyote/Response.java Thu Feb 8 15:15:57 2018 @@ -227,6 +227,10 @@ public final class Response { * @param status The status value to set */ public void setStatus(int status) { + if (this.status > 399) { + // Don't overwrite first recorded error status + return; + } this.status = status; } Modified: tomcat/trunk/java/org/apache/coyote/ajp/AjpProcessor.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/ajp/AjpProcessor.java?rev=1823565&r1=1823564&r2=1823565&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/ajp/AjpProcessor.java (original) +++ tomcat/trunk/java/org/apache/coyote/ajp/AjpProcessor.java Thu Feb 8 15:15:57 2018 @@ -364,10 +364,9 @@ public class AjpProcessor extends Abstra // 400 - Bad Request response.setStatus(400); setErrorState(ErrorState.CLOSE_CLEAN, t); - getAdapter().log(request, response, 0); } - if (!getErrorState().isError()) { + if (getErrorState().isIoAllowed()) { // Setting up filters, and parse some request headers rp.setStage(org.apache.coyote.Constants.STAGE_PREPARE); try { @@ -378,20 +377,18 @@ public class AjpProcessor extends Abstra // 500 - Internal Server Error response.setStatus(500); setErrorState(ErrorState.CLOSE_CLEAN, t); - getAdapter().log(request, response, 0); } } - if (!getErrorState().isError() && !cping && protocol.isPaused()) { + if (getErrorState().isIoAllowed() && !cping && protocol.isPaused()) { // 503 - Service unavailable response.setStatus(503); setErrorState(ErrorState.CLOSE_CLEAN, null); - getAdapter().log(request, response, 0); } cping = false; // Process the request in the adapter - if (!getErrorState().isError()) { + if (getErrorState().isIoAllowed()) { try { rp.setStage(org.apache.coyote.Constants.STAGE_SERVICE); getAdapter().service(request, response); @@ -852,7 +849,7 @@ public class AjpProcessor extends Abstra MessageBytes valueMB = request.getMimeHeaders().getValue("host"); parseHost(valueMB); - if (getErrorState().isError()) { + if (!getErrorState().isIoAllowed()) { getAdapter().log(request, response, 0); } } Modified: tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java?rev=1823565&r1=1823564&r2=1823565&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java (original) +++ tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java Thu Feb 8 15:15:57 2018 @@ -341,7 +341,6 @@ public class Http11Processor extends Abs // 400 - Bad Request response.setStatus(400); setErrorState(ErrorState.CLOSE_CLEAN, t); - getAdapter().log(request, response, 0); } // Has an upgrade been requested? @@ -377,7 +376,7 @@ public class Http11Processor extends Abs } } - if (!getErrorState().isError()) { + if (getErrorState().isIoAllowed()) { // Setting up filters, and parse some request headers rp.setStage(org.apache.coyote.Constants.STAGE_PREPARE); try { @@ -390,7 +389,6 @@ public class Http11Processor extends Abs // 500 - Internal Server Error response.setStatus(500); setErrorState(ErrorState.CLOSE_CLEAN, t); - getAdapter().log(request, response, 0); } } @@ -403,7 +401,7 @@ public class Http11Processor extends Abs } // Process the request in the adapter - if (!getErrorState().isError()) { + if (getErrorState().isIoAllowed()) { try { rp.setStage(org.apache.coyote.Constants.STAGE_SERVICE); getAdapter().service(request, response); @@ -529,7 +527,6 @@ public class Http11Processor extends Abs // Partially processed the request so need to respond response.setStatus(503); setErrorState(ErrorState.CLOSE_CLEAN, null); - getAdapter().log(request, response, 0); return false; } else { // Need to keep processor associated with socket @@ -771,7 +768,7 @@ public class Http11Processor extends Abs contentDelimitation = true; } - if (getErrorState().isError()) { + if (!getErrorState().isIoAllowed()) { getAdapter().log(request, response, 0); } } Modified: tomcat/trunk/test/org/apache/coyote/ajp/TestAbstractAjpProcessor.java URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/coyote/ajp/TestAbstractAjpProcessor.java?rev=1823565&r1=1823564&r2=1823565&view=diff ============================================================================== --- tomcat/trunk/test/org/apache/coyote/ajp/TestAbstractAjpProcessor.java (original) +++ tomcat/trunk/test/org/apache/coyote/ajp/TestAbstractAjpProcessor.java Thu Feb 8 15:15:57 2018 @@ -512,8 +512,8 @@ public class TestAbstractAjpProcessor ex TesterAjpMessage responseHeaders = ajpClient.sendMessage(forwardMessage); // Expect 3 packets: headers, body, end validateResponseHeaders(responseHeaders, 403, "403"); - //TesterAjpMessage responseBody = ajpClient.readMessage(); - //validateResponseBody(responseBody, HelloWorldServlet.RESPONSE_TEXT); + TesterAjpMessage responseBody = ajpClient.readMessage(); + validateResponseBody(responseBody, "<p><b>Type</b> Status Report</p>"); validateResponseEnd(ajpClient.readMessage(), false); ajpClient.connect(); @@ -526,8 +526,8 @@ public class TestAbstractAjpProcessor ex responseHeaders = ajpClient.sendMessage(forwardMessage); // Expect 3 packets: headers, body, end validateResponseHeaders(responseHeaders, 403, "403"); - //responseBody = ajpClient.readMessage(); - //validateResponseBody(responseBody, HelloWorldServlet.RESPONSE_TEXT); + responseBody = ajpClient.readMessage(); + validateResponseBody(responseBody, "<p><b>Type</b> Status Report</p>"); validateResponseEnd(ajpClient.readMessage(), false); ajpClient.connect(); @@ -540,7 +540,7 @@ public class TestAbstractAjpProcessor ex responseHeaders = ajpClient.sendMessage(forwardMessage); // Expect 3 packets: headers, body, end validateResponseHeaders(responseHeaders, 200, "200"); - TesterAjpMessage responseBody = ajpClient.readMessage(); + responseBody = ajpClient.readMessage(); validateResponseBody(responseBody, HelloWorldServlet.RESPONSE_TEXT); validateResponseEnd(ajpClient.readMessage(), true); @@ -641,7 +641,9 @@ public class TestAbstractAjpProcessor ex // Double check the connection is still open validateCpong(ajpClient.cping()); } else { - // Expect 2 messages: headers, end for an invalid request + // Expect 3 messages: headers, error report body, end for an invalid request + TesterAjpMessage responseBody = ajpClient.readMessage(); + validateResponseBody(responseBody, "<p><b>Type</b> Status Report</p>"); validateResponseEnd(ajpClient.readMessage(), false); } Modified: tomcat/trunk/test/org/apache/tomcat/util/http/TestMimeHeadersIntegration.java URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/tomcat/util/http/TestMimeHeadersIntegration.java?rev=1823565&r1=1823564&r2=1823565&view=diff ============================================================================== --- tomcat/trunk/test/org/apache/tomcat/util/http/TestMimeHeadersIntegration.java (original) +++ tomcat/trunk/test/org/apache/tomcat/util/http/TestMimeHeadersIntegration.java Thu Feb 8 15:15:57 2018 @@ -94,7 +94,7 @@ public class TestMimeHeadersIntegration client.getResponseLine() != null && client.isResponse200()); Assert.assertEquals("OK", client.getResponseBody()); } else { - alv.validateAccessLog(1, 400, 0, 0); + alv.validateAccessLog(1, 400, 0, 3000); // Connection aborted or response 400 Assert.assertTrue("Response line is: " + client.getResponseLine(), client.getResponseLine() == null || client.isResponse400()); Modified: tomcat/trunk/webapps/docs/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1823565&r1=1823564&r2=1823565&view=diff ============================================================================== --- tomcat/trunk/webapps/docs/changelog.xml (original) +++ tomcat/trunk/webapps/docs/changelog.xml Thu Feb 8 15:15:57 2018 @@ -50,6 +50,11 @@ <fix> Minor optimization when calling class transformers. (rjung) </fix> + <add> + Pass errors triggered by invalid requests or unavailable services to the + application provided error handling and/or the container provided error + handling (<code>ErrorReportValve</code>) as appropriate. (markt) + </add> </changelog> </subsection> <subsection name="Web applications"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org