Author: markt Date: Fri Jun 13 13:57:27 2014 New Revision: 1602429 URL: http://svn.apache.org/r1602429 Log: Refactoring. Switch from a boolean to an Enum for error state so we can differentiate between an error that requires the connection is closed after the current response is completed and an error that requires that the connection is closed immediately. This commit should be a NO-OP. While the different error states are set, the only the presence of an error (or not) is tested - i.e. no change from the implementation prior to this commit. Try to be consistent when an error occurs. Set the status code first (if required), then set the error state and finally log (if required).
Added: tomcat/tc7.0.x/trunk/java/org/apache/coyote/ErrorState.java - copied unchanged from r1600109, tomcat/trunk/java/org/apache/coyote/ErrorState.java Modified: tomcat/tc7.0.x/trunk/ (props changed) tomcat/tc7.0.x/trunk/java/org/apache/coyote/AbstractProcessor.java tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AjpAprProcessor.java tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AjpNioProcessor.java tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AjpProcessor.java tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java Propchange: tomcat/tc7.0.x/trunk/ ------------------------------------------------------------------------------ Merged /tomcat/trunk:r1600109 Modified: tomcat/tc7.0.x/trunk/java/org/apache/coyote/AbstractProcessor.java URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/coyote/AbstractProcessor.java?rev=1602429&r1=1602428&r2=1602429&view=diff ============================================================================== --- tomcat/tc7.0.x/trunk/java/org/apache/coyote/AbstractProcessor.java (original) +++ tomcat/tc7.0.x/trunk/java/org/apache/coyote/AbstractProcessor.java Fri Jun 13 13:57:27 2014 @@ -38,9 +38,9 @@ public abstract class AbstractProcessor< protected SocketWrapper<S> socketWrapper = null; /** - * Error flag. + * Error state for the request/response currently being processed. */ - protected boolean error; + private ErrorState errorState; /** @@ -64,6 +64,24 @@ public abstract class AbstractProcessor< /** + * Update the current error state to the new error state if the new error + * state is more severe than the current error state. + */ + protected void setErrorState(ErrorState errorState) { + this.errorState = this.errorState.getMostSevere(errorState); + } + + + protected void resetErrorState() { + errorState = ErrorState.NONE; + } + + + protected ErrorState getErrorState() { + return errorState; + } + + /** * The endpoint receiving connections that are handled by this processor. */ protected AbstractEndpoint getEndpoint() { Modified: tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java?rev=1602429&r1=1602428&r2=1602429&view=diff ============================================================================== --- tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java (original) +++ tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java Fri Jun 13 13:57:27 2014 @@ -30,6 +30,7 @@ import javax.servlet.http.HttpServletRes import org.apache.coyote.AbstractProcessor; import org.apache.coyote.ActionCode; import org.apache.coyote.AsyncContextCallback; +import org.apache.coyote.ErrorState; import org.apache.coyote.InputBuffer; import org.apache.coyote.OutputBuffer; import org.apache.coyote.Request; @@ -322,15 +323,13 @@ public abstract class AbstractAjpProcess try { prepareResponse(); } catch (IOException e) { - // Set error flag - error = true; + setErrorState(ErrorState.CLOSE_NOW); } try { flush(false); } catch (IOException e) { - // Set error flag - error = true; + setErrorState(ErrorState.CLOSE_NOW); } break; } @@ -340,8 +339,7 @@ public abstract class AbstractAjpProcess try { prepareResponse(); } catch (IOException e) { - // Set error flag - error = true; + setErrorState(ErrorState.CLOSE_NOW); return; } } @@ -349,19 +347,18 @@ public abstract class AbstractAjpProcess try { flush(true); } catch (IOException e) { - // Set error flag - error = true; + setErrorState(ErrorState.CLOSE_NOW); } break; } case IS_ERROR: { - ((AtomicBoolean) param).set(error); + ((AtomicBoolean) param).set(getErrorState().isError()); break; } case DISABLE_SWALLOW_INPUT: { // TODO: Do not swallow request input but // make sure we are closing the connection - error = true; + setErrorState(ErrorState.CLOSE_CLEAN); break; } case CLOSE: { @@ -372,8 +369,7 @@ public abstract class AbstractAjpProcess try { finish(); } catch (IOException e) { - // Set error flag - error = true; + setErrorState(ErrorState.CLOSE_NOW); } break; } @@ -509,16 +505,18 @@ public abstract class AbstractAjpProcess RequestInfo rp = request.getRequestProcessor(); try { rp.setStage(org.apache.coyote.Constants.STAGE_SERVICE); - error = !adapter.asyncDispatch(request, response, status); + if(!getAdapter().asyncDispatch(request, response, status)) { + setErrorState(ErrorState.CLOSE_NOW); + } resetTimeouts(); } catch (InterruptedIOException e) { - error = true; + setErrorState(ErrorState.CLOSE_NOW); } catch (Throwable t) { ExceptionUtils.handleThrowable(t); + setErrorState(ErrorState.CLOSE_NOW); getLog().error(sm.getString("http11processor.request.process"), t); - error = true; } finally { - if (error) { + if (getErrorState().isError()) { // 500 - Internal Server Error response.setStatus(500); adapter.log(request, response, 0); @@ -528,7 +526,7 @@ public abstract class AbstractAjpProcess rp.setStage(org.apache.coyote.Constants.STAGE_ENDED); if (isAsync()) { - if (error) { + if (getErrorState().isError()) { request.updateCounters(); return SocketState.CLOSED; } else { @@ -536,7 +534,7 @@ public abstract class AbstractAjpProcess } } else { request.updateCounters(); - if (error) { + if (getErrorState().isError()) { return SocketState.CLOSED; } else { return SocketState.OPEN; @@ -760,7 +758,7 @@ public abstract class AbstractAjpProcess long cl = vMB.getLong(); if (contentLengthSet) { response.setStatus(HttpServletResponse.SC_BAD_REQUEST); - error = true; + setErrorState(ErrorState.CLOSE_CLEAN); } else { contentLengthSet = true; // Set the content-length header for the request @@ -878,7 +876,7 @@ public abstract class AbstractAjpProcess secret = true; if (!tmpMB.equals(requiredSecret)) { response.setStatus(403); - error = true; + setErrorState(ErrorState.CLOSE_CLEAN); } } break; @@ -894,7 +892,7 @@ public abstract class AbstractAjpProcess // Check if secret was submitted if required if ((requiredSecret != null) && !secret) { response.setStatus(403); - error = true; + setErrorState(ErrorState.CLOSE_CLEAN); } // Check for a full URI (including protocol://host:port/) @@ -927,7 +925,7 @@ public abstract class AbstractAjpProcess MessageBytes valueMB = request.getMimeHeaders().getValue("host"); parseHost(valueMB); - if (error) { + if (getErrorState().isError()) { adapter.log(request, response, 0); } } @@ -945,7 +943,7 @@ public abstract class AbstractAjpProcess request.serverName().duplicate(request.localName()); } catch (IOException e) { response.setStatus(400); - error = true; + setErrorState(ErrorState.CLOSE_CLEAN); } return; } @@ -993,9 +991,9 @@ public abstract class AbstractAjpProcess int charValue = HexUtils.getDec(valueB[i + valueS]); if (charValue == -1) { // Invalid character - error = true; // 400 - Bad request response.setStatus(400); + setErrorState(ErrorState.CLOSE_CLEAN); break; } port = port + (charValue * mult); @@ -1111,8 +1109,7 @@ public abstract class AbstractAjpProcess try { prepareResponse(); } catch (IOException e) { - // Set error flag - error = true; + setErrorState(ErrorState.CLOSE_NOW); } } @@ -1127,7 +1124,7 @@ public abstract class AbstractAjpProcess } // Add the end message - if (error) { + if (getErrorState().isError()) { output(endAndCloseMessageArray, 0, endAndCloseMessageArray.length); } else { output(endMessageArray, 0, endMessageArray.length); @@ -1195,8 +1192,7 @@ public abstract class AbstractAjpProcess try { prepareResponse(); } catch (IOException e) { - // Set error flag - error = true; + setErrorState(ErrorState.CLOSE_NOW); } } Modified: tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AjpAprProcessor.java URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AjpAprProcessor.java?rev=1602429&r1=1602428&r2=1602429&view=diff ============================================================================== --- tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AjpAprProcessor.java (original) +++ tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AjpAprProcessor.java Fri Jun 13 13:57:27 2014 @@ -21,6 +21,7 @@ import java.io.InterruptedIOException; import java.nio.ByteBuffer; import org.apache.coyote.ActionCode; +import org.apache.coyote.ErrorState; import org.apache.coyote.RequestInfo; import org.apache.juli.logging.Log; import org.apache.juli.logging.LogFactory; @@ -108,12 +109,11 @@ public class AjpAprProcessor extends Abs Socket.setsbb(socketRef, outputBuffer); boolean cping = false; - // Error flag - error = false; + resetErrorState(); boolean keptAlive = false; - while (!error && !endpoint.isPaused()) { + while (!getErrorState().isError() && !endpoint.isPaused()) { // Parsing the request header try { // Get first message of the request @@ -134,7 +134,7 @@ public class AjpAprProcessor extends Abs cping = true; if (Socket.send(socketRef, pongMessageArray, 0, pongMessageArray.length) < 0) { - error = true; + setErrorState(ErrorState.CLOSE_NOW); } continue; } else if(type != Constants.JK_AJP13_FORWARD_REQUEST) { @@ -143,24 +143,24 @@ public class AjpAprProcessor extends Abs if (log.isDebugEnabled()) { log.debug("Unexpected message: " + type); } - error = true; + setErrorState(ErrorState.CLOSE_NOW); break; } keptAlive = true; request.setStartTime(System.currentTimeMillis()); } catch (IOException e) { - error = true; + setErrorState(ErrorState.CLOSE_NOW); break; } catch (Throwable t) { ExceptionUtils.handleThrowable(t); log.debug(sm.getString("ajpprocessor.header.error"), t); // 400 - Bad Request response.setStatus(400); - adapter.log(request, response, 0); - error = true; + setErrorState(ErrorState.CLOSE_CLEAN); + getAdapter().log(request, response, 0); } - if (!error) { + if (!getErrorState().isError()) { // Setting up filters, and parse some request headers rp.setStage(org.apache.coyote.Constants.STAGE_PREPARE); try { @@ -170,37 +170,37 @@ public class AjpAprProcessor extends Abs log.debug(sm.getString("ajpprocessor.request.prepare"), t); // 500 - Internal Server Error response.setStatus(500); - adapter.log(request, response, 0); - error = true; + setErrorState(ErrorState.CLOSE_CLEAN); + getAdapter().log(request, response, 0); } } - if (!error && !cping && endpoint.isPaused()) { + if (!getErrorState().isError() && !cping && endpoint.isPaused()) { // 503 - Service unavailable response.setStatus(503); - adapter.log(request, response, 0); - error = true; + setErrorState(ErrorState.CLOSE_CLEAN); + getAdapter().log(request, response, 0); } cping = false; // Process the request in the adapter - if (!error) { + if (!getErrorState().isError()) { try { rp.setStage(org.apache.coyote.Constants.STAGE_SERVICE); adapter.service(request, response); } catch (InterruptedIOException e) { - error = true; + setErrorState(ErrorState.CLOSE_NOW); } catch (Throwable t) { ExceptionUtils.handleThrowable(t); log.error(sm.getString("ajpprocessor.request.process"), t); // 500 - Internal Server Error response.setStatus(500); - adapter.log(request, response, 0); - error = true; + setErrorState(ErrorState.CLOSE_CLEAN); + getAdapter().log(request, response, 0); } } - if (isAsync() && !error) { + if (isAsync() && !getErrorState().isError()) { break; } @@ -210,13 +210,13 @@ public class AjpAprProcessor extends Abs finish(); } catch (Throwable t) { ExceptionUtils.handleThrowable(t); - error = true; + setErrorState(ErrorState.CLOSE_NOW); } } // If there was an error, make sure the request is counted as // and error, and update the statistics counter - if (error) { + if (getErrorState().isError()) { response.setStatus(500); } request.updateCounters(); @@ -227,7 +227,7 @@ public class AjpAprProcessor extends Abs rp.setStage(org.apache.coyote.Constants.STAGE_ENDED); - if (!error && !endpoint.isPaused()) { + if (!getErrorState().isError() && !endpoint.isPaused()) { if (isAsync()) { return SocketState.LONG; } else { Modified: tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AjpNioProcessor.java URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AjpNioProcessor.java?rev=1602429&r1=1602428&r2=1602429&view=diff ============================================================================== --- tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AjpNioProcessor.java (original) +++ tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AjpNioProcessor.java Fri Jun 13 13:57:27 2014 @@ -23,6 +23,7 @@ import java.nio.ByteBuffer; import java.nio.channels.Selector; import org.apache.coyote.ActionCode; +import org.apache.coyote.ErrorState; import org.apache.coyote.RequestInfo; import org.apache.juli.logging.Log; import org.apache.juli.logging.LogFactory; @@ -93,10 +94,9 @@ public class AjpNioProcessor extends Abs long soTimeout = endpoint.getSoTimeout(); boolean cping = false; - // Error flag - error = false; + resetErrorState(); - while (!error && !endpoint.isPaused()) { + while (!getErrorState().isError() && !endpoint.isPaused()) { // Parsing the request header try { // Get first message of the request @@ -120,7 +120,7 @@ public class AjpNioProcessor extends Abs try { output(pongMessageArray, 0, pongMessageArray.length); } catch (IOException e) { - error = true; + setErrorState(ErrorState.CLOSE_NOW); } recycle(false); continue; @@ -130,24 +130,24 @@ public class AjpNioProcessor extends Abs if (log.isDebugEnabled()) { log.debug("Unexpected message: " + type); } - error = true; + setErrorState(ErrorState.CLOSE_NOW); recycle(true); break; } request.setStartTime(System.currentTimeMillis()); } catch (IOException e) { - error = true; + setErrorState(ErrorState.CLOSE_NOW); break; } catch (Throwable t) { ExceptionUtils.handleThrowable(t); log.debug(sm.getString("ajpprocessor.header.error"), t); // 400 - Bad Request response.setStatus(400); - adapter.log(request, response, 0); - error = true; + setErrorState(ErrorState.CLOSE_CLEAN); + getAdapter().log(request, response, 0); } - if (!error) { + if (!getErrorState().isError()) { // Setting up filters, and parse some request headers rp.setStage(org.apache.coyote.Constants.STAGE_PREPARE); try { @@ -157,37 +157,37 @@ public class AjpNioProcessor extends Abs log.debug(sm.getString("ajpprocessor.request.prepare"), t); // 500 - Internal Server Error response.setStatus(500); - adapter.log(request, response, 0); - error = true; + setErrorState(ErrorState.CLOSE_CLEAN); + getAdapter().log(request, response, 0); } } - if (!error && !cping && endpoint.isPaused()) { + if (!getErrorState().isError() && !cping && endpoint.isPaused()) { // 503 - Service unavailable response.setStatus(503); - adapter.log(request, response, 0); - error = true; + setErrorState(ErrorState.CLOSE_CLEAN); + getAdapter().log(request, response, 0); } cping = false; // Process the request in the adapter - if (!error) { + if (!getErrorState().isError()) { try { rp.setStage(org.apache.coyote.Constants.STAGE_SERVICE); adapter.service(request, response); } catch (InterruptedIOException e) { - error = true; + setErrorState(ErrorState.CLOSE_NOW); } catch (Throwable t) { ExceptionUtils.handleThrowable(t); log.error(sm.getString("ajpprocessor.request.process"), t); // 500 - Internal Server Error response.setStatus(500); - adapter.log(request, response, 0); - error = true; + setErrorState(ErrorState.CLOSE_CLEAN); + getAdapter().log(request, response, 0); } } - if (isAsync() && !error) { + if (isAsync() && !getErrorState().isError()) { break; } @@ -197,13 +197,13 @@ public class AjpNioProcessor extends Abs finish(); } catch (Throwable t) { ExceptionUtils.handleThrowable(t); - error = true; + setErrorState(ErrorState.CLOSE_NOW); } } // If there was an error, make sure the request is counted as // and error, and update the statistics counter - if (error) { + if (getErrorState().isError()) { response.setStatus(500); } request.updateCounters(); @@ -219,7 +219,7 @@ public class AjpNioProcessor extends Abs rp.setStage(org.apache.coyote.Constants.STAGE_ENDED); - if (!error && !endpoint.isPaused()) { + if (!getErrorState().isError() && !endpoint.isPaused()) { if (isAsync()) { return SocketState.LONG; } else { @@ -278,7 +278,7 @@ public class AjpNioProcessor extends Abs // finished. final KeyAttachment attach = (KeyAttachment)socketWrapper.getSocket().getAttachment(false); - if (!error && attach != null && + if (!getErrorState().isError() && attach != null && asyncStateMachine.isAsyncDispatching()) { long soTimeout = endpoint.getSoTimeout(); Modified: tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AjpProcessor.java URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AjpProcessor.java?rev=1602429&r1=1602428&r2=1602429&view=diff ============================================================================== --- tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AjpProcessor.java (original) +++ tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AjpProcessor.java Fri Jun 13 13:57:27 2014 @@ -23,6 +23,7 @@ import java.io.OutputStream; import java.net.Socket; import org.apache.coyote.ActionCode; +import org.apache.coyote.ErrorState; import org.apache.coyote.RequestInfo; import org.apache.juli.logging.Log; import org.apache.juli.logging.LogFactory; @@ -106,11 +107,9 @@ public class AjpProcessor extends Abstra } boolean cping = false; - // Error flag - error = false; - - while (!error && !endpoint.isPaused()) { - + resetErrorState(); + + while (!getErrorState().isError() && !endpoint.isPaused()) { // Parsing the request header try { // Set keep alive timeout if enabled @@ -138,7 +137,7 @@ public class AjpProcessor extends Abstra try { output.write(pongMessageArray); } catch (IOException e) { - error = true; + setErrorState(ErrorState.CLOSE_NOW); } continue; } else if(type != Constants.JK_AJP13_FORWARD_REQUEST) { @@ -147,23 +146,23 @@ public class AjpProcessor extends Abstra if (log.isDebugEnabled()) { log.debug("Unexpected message: " + type); } - error = true; + setErrorState(ErrorState.CLOSE_NOW); break; } request.setStartTime(System.currentTimeMillis()); } catch (IOException e) { - error = true; + setErrorState(ErrorState.CLOSE_NOW); break; } catch (Throwable t) { ExceptionUtils.handleThrowable(t); log.debug(sm.getString("ajpprocessor.header.error"), t); // 400 - Bad Request response.setStatus(400); - adapter.log(request, response, 0); - error = true; + setErrorState(ErrorState.CLOSE_CLEAN); + getAdapter().log(request, response, 0); } - if (!error) { + if (!getErrorState().isError()) { // Setting up filters, and parse some request headers rp.setStage(org.apache.coyote.Constants.STAGE_PREPARE); try { @@ -173,37 +172,37 @@ public class AjpProcessor extends Abstra log.debug(sm.getString("ajpprocessor.request.prepare"), t); // 500 - Internal Server Error response.setStatus(500); - adapter.log(request, response, 0); - error = true; + setErrorState(ErrorState.CLOSE_CLEAN); + getAdapter().log(request, response, 0); } } - if (!error && !cping && endpoint.isPaused()) { + if (!getErrorState().isError() && !cping && endpoint.isPaused()) { // 503 - Service unavailable response.setStatus(503); - adapter.log(request, response, 0); - error = true; + setErrorState(ErrorState.CLOSE_CLEAN); + getAdapter().log(request, response, 0); } cping = false; // Process the request in the adapter - if (!error) { + if (!getErrorState().isError()) { try { rp.setStage(org.apache.coyote.Constants.STAGE_SERVICE); adapter.service(request, response); } catch (InterruptedIOException e) { - error = true; + setErrorState(ErrorState.CLOSE_NOW); } catch (Throwable t) { ExceptionUtils.handleThrowable(t); log.error(sm.getString("ajpprocessor.request.process"), t); // 500 - Internal Server Error response.setStatus(500); - adapter.log(request, response, 0); - error = true; + setErrorState(ErrorState.CLOSE_CLEAN); + getAdapter().log(request, response, 0); } } - if (isAsync() && !error) { + if (isAsync() && !getErrorState().isError()) { break; } @@ -213,13 +212,13 @@ public class AjpProcessor extends Abstra finish(); } catch (Throwable t) { ExceptionUtils.handleThrowable(t); - error = true; + setErrorState(ErrorState.CLOSE_NOW); } } // If there was an error, make sure the request is counted as // and error, and update the statistics counter - if (error) { + if (getErrorState().isError()) { response.setStatus(500); } request.updateCounters(); @@ -230,7 +229,7 @@ public class AjpProcessor extends Abstra rp.setStage(org.apache.coyote.Constants.STAGE_ENDED); - if (isAsync() && !error && !endpoint.isPaused()) { + if (isAsync() && !getErrorState().isError() && !endpoint.isPaused()) { return SocketState.LONG; } else { input = null; Modified: tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java?rev=1602429&r1=1602428&r2=1602429&view=diff ============================================================================== --- tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java (original) +++ tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java Fri Jun 13 13:57:27 2014 @@ -28,6 +28,7 @@ import javax.servlet.http.HttpServletRes import org.apache.coyote.AbstractProcessor; import org.apache.coyote.ActionCode; import org.apache.coyote.AsyncContextCallback; +import org.apache.coyote.ErrorState; import org.apache.coyote.RequestInfo; import org.apache.coyote.http11.filters.BufferedInputFilter; import org.apache.coyote.http11.filters.ChunkedInputFilter; @@ -735,7 +736,7 @@ public abstract class AbstractHttp11Proc // Unsupported transfer encoding // 501 - Unimplemented response.setStatus(501); - error = true; + setErrorState(ErrorState.CLOSE_CLEAN); if (getLog().isDebugEnabled()) { getLog().debug(sm.getString("http11processor.request.prepare") + " Unsupported transfer encoding [" + encodingName + "]"); @@ -760,8 +761,7 @@ public abstract class AbstractHttp11Proc try { getOutputBuffer().endRequest(); } catch (IOException e) { - // Set error flag - error = true; + setErrorState(ErrorState.CLOSE_NOW); } break; } @@ -776,8 +776,7 @@ public abstract class AbstractHttp11Proc prepareResponse(); getOutputBuffer().commit(); } catch (IOException e) { - // Set error flag - error = true; + setErrorState(ErrorState.CLOSE_NOW); } break; } @@ -793,8 +792,7 @@ public abstract class AbstractHttp11Proc try { getOutputBuffer().sendAck(); } catch (IOException e) { - // Set error flag - error = true; + setErrorState(ErrorState.CLOSE_NOW); } break; } @@ -802,20 +800,19 @@ public abstract class AbstractHttp11Proc try { getOutputBuffer().flush(); } catch (IOException e) { - // Set error flag - error = true; + setErrorState(ErrorState.CLOSE_NOW); response.setErrorException(e); } break; } case IS_ERROR: { - ((AtomicBoolean) param).set(error); + ((AtomicBoolean) param).set(getErrorState().isError()); break; } case DISABLE_SWALLOW_INPUT: { // Do not swallow request input and make sure we are closing the // connection - error = true; + setErrorState(ErrorState.CLOSE_CLEAN); getInputBuffer().setSwallowInput(false); break; } @@ -956,7 +953,6 @@ public abstract class AbstractHttp11Proc getOutputBuffer().init(socketWrapper, endpoint); // Flags - error = false; keepAlive = true; comet = false; openSocket = false; @@ -967,12 +963,13 @@ public abstract class AbstractHttp11Proc } else { keptAlive = socketWrapper.isKeptAlive(); } + resetErrorState(); if (disableKeepAlive()) { socketWrapper.setKeepAliveLeft(0); } - while (!error && keepAlive && !comet && !isAsync() && + while (!getErrorState().isError() && keepAlive && !comet && !isAsync() && upgradeInbound == null && httpUpgradeHandler == null && !endpoint.isPaused()) { @@ -989,7 +986,7 @@ public abstract class AbstractHttp11Proc if (endpoint.isPaused()) { // 503 - Service unavailable response.setStatus(503); - error = true; + setErrorState(ErrorState.CLOSE_CLEAN); } else { // Make sure that connectors that are non-blocking during // header processing (NIO) only set the start time the first @@ -1017,7 +1014,7 @@ public abstract class AbstractHttp11Proc getLog().debug( sm.getString("http11processor.header.parse"), e); } - error = true; + setErrorState(ErrorState.CLOSE_NOW); break; } catch (Throwable t) { ExceptionUtils.handleThrowable(t); @@ -1039,11 +1036,11 @@ public abstract class AbstractHttp11Proc } // 400 - Bad Request response.setStatus(400); - adapter.log(request, response, 0); - error = true; + setErrorState(ErrorState.CLOSE_CLEAN); + getAdapter().log(request, response, 0); } - if (!error) { + if (!getErrorState().isError()) { // Setting up filters, and parse some request headers rp.setStage(org.apache.coyote.Constants.STAGE_PREPARE); try { @@ -1056,8 +1053,8 @@ public abstract class AbstractHttp11Proc } // 500 - Internal Server Error response.setStatus(500); - adapter.log(request, response, 0); - error = true; + setErrorState(ErrorState.CLOSE_CLEAN); + getAdapter().log(request, response, 0); } } @@ -1069,7 +1066,7 @@ public abstract class AbstractHttp11Proc } // Process the request in the adapter - if (!error) { + if (!getErrorState().isError()) { try { rp.setStage(org.apache.coyote.Constants.STAGE_SERVICE); adapter.service(request, response); @@ -1078,22 +1075,25 @@ public abstract class AbstractHttp11Proc // set the status to 500 and set the errorException. // If we fail here, then the response is likely already // committed, so we can't try and set headers. - if(keepAlive && !error) { // Avoid checking twice. - error = response.getErrorException() != null || - (!isAsync() && - statusDropsConnection(response.getStatus())); + if(keepAlive && !getErrorState().isError() && ( + response.getErrorException() != null || + (!isAsync() && + statusDropsConnection(response.getStatus())))) { + setErrorState(ErrorState.CLOSE_CLEAN); } setCometTimeouts(socketWrapper); } catch (InterruptedIOException e) { - error = true; + setErrorState(ErrorState.CLOSE_NOW); } catch (HeadersTooLargeException e) { - error = true; // The response should not have been committed but check it // anyway to be safe - if (!response.isCommitted()) { + if (response.isCommitted()) { + setErrorState(ErrorState.CLOSE_NOW); + } else { response.reset(); response.setStatus(500); - response.setHeader("Connection", "close"); + setErrorState(ErrorState.CLOSE_CLEAN); + response.setHeader("Connection", "close"); // TODO: Remove } } catch (Throwable t) { ExceptionUtils.handleThrowable(t); @@ -1101,8 +1101,8 @@ public abstract class AbstractHttp11Proc "http11processor.request.process"), t); // 500 - Internal Server Error response.setStatus(500); - adapter.log(request, response, 0); - error = true; + setErrorState(ErrorState.CLOSE_CLEAN); + getAdapter().log(request, response, 0); } } @@ -1110,7 +1110,7 @@ public abstract class AbstractHttp11Proc rp.setStage(org.apache.coyote.Constants.STAGE_ENDINPUT); if (!isAsync() && !comet) { - if (error) { + if (getErrorState().isError()) { // If we know we are closing the connection, don't drain // input. This way uploading a 100GB file doesn't tie up the // thread if the servlet has rejected it. @@ -1133,12 +1133,12 @@ public abstract class AbstractHttp11Proc // If there was an error, make sure the request is counted as // and error, and update the statistics counter - if (error) { + if (getErrorState().isError()) { response.setStatus(500); } request.updateCounters(); - if (!isAsync() && !comet || error) { + if (!isAsync() && !comet || getErrorState().isError()) { getInputBuffer().nextRequest(); getOutputBuffer().nextRequest(); } @@ -1160,7 +1160,7 @@ public abstract class AbstractHttp11Proc rp.setStage(org.apache.coyote.Constants.STAGE_ENDED); - if (error || endpoint.isPaused()) { + if (getErrorState().isError() || endpoint.isPaused()) { return SocketState.CLOSED; } else if (isAsync() || comet) { return SocketState.LONG; @@ -1217,13 +1217,13 @@ public abstract class AbstractHttp11Proc } else { // Unsupported protocol http11 = false; - error = true; // Send 505; Unsupported HTTP version + response.setStatus(505); + setErrorState(ErrorState.CLOSE_CLEAN); if (getLog().isDebugEnabled()) { getLog().debug(sm.getString("http11processor.request.prepare")+ " Unsupported HTTP version \""+protocolMB+"\""); } - response.setStatus(505); } MessageBytes methodMB = request.method(); @@ -1256,8 +1256,8 @@ public abstract class AbstractHttp11Proc getInputBuffer().setSwallowInput(false); expectation = true; } else { - error = true; response.setStatus(HttpServletResponse.SC_EXPECTATION_FAILED); + setErrorState(ErrorState.CLOSE_CLEAN); } } @@ -1349,13 +1349,13 @@ public abstract class AbstractHttp11Proc // Check host header if (http11 && (valueMB == null)) { - error = true; // 400 - Bad request + response.setStatus(400); + setErrorState(ErrorState.CLOSE_CLEAN); if (getLog().isDebugEnabled()) { getLog().debug(sm.getString("http11processor.request.prepare")+ " host header missing"); } - response.setStatus(400); } parseHost(valueMB); @@ -1388,8 +1388,7 @@ public abstract class AbstractHttp11Proc org.apache.coyote.Constants.COMET_TIMEOUT_SUPPORTED_ATTR, Boolean.TRUE); } - - if (error) { + if (getErrorState().isError()) { adapter.log(request, response, 0); } } @@ -1540,7 +1539,7 @@ public abstract class AbstractHttp11Proc headers.addValue(Constants.CONNECTION).setString( Constants.CLOSE); } - } else if (!http11 && !error) { + } else if (!http11 && !getErrorState().isError()) { headers.addValue(Constants.CONNECTION).setString(Constants.KEEPALIVE); } @@ -1630,9 +1629,9 @@ public abstract class AbstractHttp11Proc int charValue = HexUtils.getDec(valueB[i + valueS]); if (charValue == -1 || charValue > 9) { // Invalid character - error = true; // 400 - Bad request response.setStatus(400); + setErrorState(ErrorState.CLOSE_CLEAN); break; } port = port + (charValue * mult); @@ -1650,16 +1649,18 @@ public abstract class AbstractHttp11Proc RequestInfo rp = request.getRequestProcessor(); try { rp.setStage(org.apache.coyote.Constants.STAGE_SERVICE); - error = !adapter.asyncDispatch(request, response, status); + if(!getAdapter().asyncDispatch(request, response, status)) { + setErrorState(ErrorState.CLOSE_NOW); + } resetTimeouts(); } catch (InterruptedIOException e) { - error = true; + setErrorState(ErrorState.CLOSE_NOW); } catch (Throwable t) { ExceptionUtils.handleThrowable(t); + setErrorState(ErrorState.CLOSE_NOW); getLog().error(sm.getString("http11processor.request.process"), t); - error = true; } finally { - if (error) { + if (getErrorState().isError()) { // 500 - Internal Server Error response.setStatus(500); adapter.log(request, response, 0); @@ -1668,7 +1669,7 @@ public abstract class AbstractHttp11Proc rp.setStage(org.apache.coyote.Constants.STAGE_ENDED); - if (error) { + if (getErrorState().isError()) { return SocketState.CLOSED; } else if (isAsync()) { return SocketState.LONG; @@ -1749,26 +1750,25 @@ public abstract class AbstractHttp11Proc try { getInputBuffer().endRequest(); } catch (IOException e) { - error = true; + setErrorState(ErrorState.CLOSE_NOW); } catch (Throwable t) { ExceptionUtils.handleThrowable(t); - getLog().error(sm.getString("http11processor.request.finish"), t); // 500 - Internal Server Error // Can't add a 500 to the access log since that has already been // written in the Adapter.service method. response.setStatus(500); - error = true; + setErrorState(ErrorState.CLOSE_NOW); + getLog().error(sm.getString("http11processor.request.finish"), t); } try { getOutputBuffer().endRequest(); } catch (IOException e) { - error = true; + setErrorState(ErrorState.CLOSE_NOW); } catch (Throwable t) { ExceptionUtils.handleThrowable(t); + setErrorState(ErrorState.CLOSE_NOW); getLog().error(sm.getString("http11processor.response.finish"), t); - error = true; } - } @@ -1805,6 +1805,7 @@ public abstract class AbstractHttp11Proc remotePort = -1; localPort = -1; comet = false; + resetErrorState(); recycleInternal(); } Modified: tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java?rev=1602429&r1=1602428&r2=1602429&view=diff ============================================================================== --- tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java (original) +++ tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java Fri Jun 13 13:57:27 2014 @@ -23,6 +23,7 @@ import java.security.cert.CertificateFac import java.security.cert.X509Certificate; import org.apache.coyote.ActionCode; +import org.apache.coyote.ErrorState; import org.apache.coyote.RequestInfo; import org.apache.coyote.http11.filters.BufferedInputFilter; import org.apache.juli.logging.Log; @@ -124,21 +125,23 @@ public class Http11AprProcessor extends try { rp.setStage(org.apache.coyote.Constants.STAGE_SERVICE); - error = !adapter.event(request, response, status); + if (!getAdapter().event(request, response, status)) { + setErrorState(ErrorState.CLOSE_NOW); + } } catch (InterruptedIOException e) { - error = true; + setErrorState(ErrorState.CLOSE_NOW); } catch (Throwable t) { ExceptionUtils.handleThrowable(t); - log.error(sm.getString("http11processor.request.process"), t); // 500 - Internal Server Error response.setStatus(500); - adapter.log(request, response, 0); - error = true; + setErrorState(ErrorState.CLOSE_NOW); + getAdapter().log(request, response, 0); + log.error(sm.getString("http11processor.request.process"), t); } rp.setStage(org.apache.coyote.Constants.STAGE_ENDED); - if (error || status==SocketStatus.STOP) { + if (getErrorState().isError() || status==SocketStatus.STOP) { return SocketState.CLOSED; } else if (!comet) { inputBuffer.nextRequest(); @@ -188,8 +191,8 @@ public class Http11AprProcessor extends if (endpoint.isPaused()) { // 503 - Service unavailable response.setStatus(503); - adapter.log(request, response, 0); - error = true; + setErrorState(ErrorState.CLOSE_CLEAN); + getAdapter().log(request, response, 0); } else { return true; } @@ -213,7 +216,7 @@ public class Http11AprProcessor extends protected boolean breakKeepAliveLoop(SocketWrapper<Long> socketWrapper) { openSocket = keepAlive; // Do sendfile as needed: add socket to sendfile and end - if (sendfileData != null && !error) { + if (sendfileData != null && !getErrorState().isError()) { sendfileData.socket = socketWrapper.getSocket().longValue(); sendfileData.keepAlive = keepAlive; if (!((AprEndpoint)endpoint).getSendfile().add(sendfileData)) { @@ -225,7 +228,7 @@ public class Http11AprProcessor extends log.debug(sm.getString( "http11processor.sendfile.error")); } - error = true; + setErrorState(ErrorState.CLOSE_NOW); } else { // The sendfile Poller will add the socket to the main // Poller once sendfile processing is complete Modified: tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java?rev=1602429&r1=1602428&r2=1602429&view=diff ============================================================================== --- tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java (original) +++ tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java Fri Jun 13 13:57:27 2014 @@ -24,6 +24,7 @@ import java.nio.channels.SelectionKey; import javax.net.ssl.SSLEngine; import org.apache.coyote.ActionCode; +import org.apache.coyote.ErrorState; import org.apache.coyote.RequestInfo; import org.apache.coyote.http11.filters.BufferedInputFilter; import org.apache.juli.logging.Log; @@ -106,8 +107,7 @@ public class Http11NioProcessor extends * @throws IOException error during an I/O operation */ @Override - public SocketState event(SocketStatus status) - throws IOException { + public SocketState event(SocketStatus status) throws IOException { long soTimeout = endpoint.getSoTimeout(); @@ -115,8 +115,10 @@ public class Http11NioProcessor extends final NioEndpoint.KeyAttachment attach = (NioEndpoint.KeyAttachment)socketWrapper.getSocket().getAttachment(false); try { rp.setStage(org.apache.coyote.Constants.STAGE_SERVICE); - error = !adapter.event(request, response, status); - if ( !error ) { + if (!getAdapter().event(request, response, status)) { + setErrorState(ErrorState.CLOSE_NOW); + } + if (!getErrorState().isError()) { if (attach != null) { attach.setComet(comet); if (comet) { @@ -137,19 +139,19 @@ public class Http11NioProcessor extends } } } catch (InterruptedIOException e) { - error = true; + setErrorState(ErrorState.CLOSE_NOW); } catch (Throwable t) { ExceptionUtils.handleThrowable(t); - log.error(sm.getString("http11processor.request.process"), t); // 500 - Internal Server Error response.setStatus(500); - adapter.log(request, response, 0); - error = true; + setErrorState(ErrorState.CLOSE_NOW); + log.error(sm.getString("http11processor.request.process"), t); + getAdapter().log(request, response, 0); } rp.setStage(org.apache.coyote.Constants.STAGE_ENDED); - if (error || status==SocketStatus.STOP) { + if (getErrorState().isError() || status==SocketStatus.STOP) { return SocketState.CLOSED; } else if (!comet) { if (keepAlive) { @@ -168,7 +170,7 @@ public class Http11NioProcessor extends @Override protected void resetTimeouts() { final NioEndpoint.KeyAttachment attach = (NioEndpoint.KeyAttachment)socketWrapper.getSocket().getAttachment(false); - if (!error && attach != null && + if (!getErrorState().isError() && attach != null && asyncStateMachine.isAsyncDispatching()) { long soTimeout = endpoint.getSoTimeout(); @@ -231,8 +233,8 @@ public class Http11NioProcessor extends if (endpoint.isPaused()) { // 503 - Service unavailable response.setStatus(503); - adapter.log(request, response, 0); - error = true; + setErrorState(ErrorState.CLOSE_CLEAN); + getAdapter().log(request, response, 0); } else { return true; } @@ -268,11 +270,10 @@ public class Http11NioProcessor extends @Override - protected boolean breakKeepAliveLoop( - SocketWrapper<NioChannel> socketWrapper) { + protected boolean breakKeepAliveLoop(SocketWrapper<NioChannel> socketWrapper) { openSocket = keepAlive; // Do sendfile as needed: add socket to sendfile and end - if (sendfileData != null && !error) { + if (sendfileData != null && !getErrorState().isError()) { ((KeyAttachment) socketWrapper).setSendfileData(sendfileData); sendfileData.keepAlive = keepAlive; SelectionKey key = socketWrapper.getSocket().getIOChannel().keyFor( @@ -286,7 +287,7 @@ public class Http11NioProcessor extends if (log.isDebugEnabled()) { log.debug(sm.getString("http11processor.sendfile.error")); } - error = true; + setErrorState(ErrorState.CLOSE_NOW); } return true; } @@ -303,7 +304,6 @@ public class Http11NioProcessor extends // ----------------------------------------------------- ActionHook Methods - /** * Send an action to the connector. * --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org