Author: markt Date: Fri Aug 30 09:50:30 2013 New Revision: 1518926 URL: http://svn.apache.org/r1518926 Log: Review action code usage across HTTP and AJP processors - remove the unused POST_REQUEST action - align ordering between AJP and HTTP for easier comparison - add missing actions to AJP for implementation / review (see TODOs) - document known NO-OPs for AJP - ensure trying to use comet over AJP triggers an error
Modified: tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java tomcat/trunk/java/org/apache/coyote/ActionCode.java tomcat/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java tomcat/trunk/java/org/apache/coyote/ajp/LocalStrings.properties tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java 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=1518926&r1=1518925&r2=1518926&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java (original) +++ tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java Fri Aug 30 09:50:30 2013 @@ -425,7 +425,6 @@ public class CoyoteAdapter implements Ad if (!request.isAsync() && !comet) { request.finishRequest(); response.finishResponse(); - req.action(ActionCode.POST_REQUEST , null); request.getMappingData().context.logAccess( request, response, System.currentTimeMillis() - req.getStartTime(), @@ -561,7 +560,6 @@ public class CoyoteAdapter implements Ad System.currentTimeMillis() - req.getStartTime(), false); } - req.action(ActionCode.POST_REQUEST , null); } } catch (IOException e) { Modified: tomcat/trunk/java/org/apache/coyote/ActionCode.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/ActionCode.java?rev=1518926&r1=1518925&r2=1518926&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/ActionCode.java (original) +++ tomcat/trunk/java/org/apache/coyote/ActionCode.java Fri Aug 30 09:50:30 2013 @@ -41,12 +41,6 @@ public enum ActionCode { RESET, /** - * Hook called after request, but before recycling. Can be used for logging, - * to update counters, custom cleanup - the request is still visible - */ - POST_REQUEST, - - /** * Hook called if swallowing request input should be disabled. * Example: Cancel a large file upload. * Modified: tomcat/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java?rev=1518926&r1=1518925&r2=1518926&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java (original) +++ tomcat/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java Fri Aug 30 09:50:30 2013 @@ -318,7 +318,18 @@ public abstract class AbstractAjpProcess @Override public final void action(ActionCode actionCode, Object param) { - if (actionCode == ActionCode.COMMIT) { + if (actionCode == ActionCode.CLOSE) { + // End the processing of the current request, and stop any further + // transactions with the client + + try { + finish(); + } catch (IOException e) { + // Set error flag + error = true; + } + + } else if (actionCode == ActionCode.COMMIT) { if (response.isCommitted()) return; @@ -338,6 +349,9 @@ public abstract class AbstractAjpProcess error = true; } + } else if (actionCode == ActionCode.ACK) { + // NO_OP for AJP + } else if (actionCode == ActionCode.CLIENT_FLUSH) { if (!response.isCommitted()) { @@ -363,17 +377,10 @@ public abstract class AbstractAjpProcess // make sure we are closing the connection error = true; - } else if (actionCode == ActionCode.CLOSE) { - // Close - // End the processing of the current request, and stop any further - // transactions with the client - - try { - finish(); - } catch (IOException e) { - // Set error flag - error = true; - } + } else if (actionCode == ActionCode.RESET) { + // NO-OP + // TODO Check if this is really a NO-OP for AJP or if something + // needs to be done here } else if (actionCode == ActionCode.REQ_SSL_ATTRIBUTE ) { @@ -416,6 +423,10 @@ public abstract class AbstractAjpProcess request.setAttribute(SSLSupport.CERTIFICATE_KEY, jsseCerts); } + } else if (actionCode == ActionCode.REQ_SSL_CERTIFICATE) { + // NO-OP. Can't force a new SSL handshake with the client when using + // AJP as the reverse proxy controls that connection. + } else if (actionCode == ActionCode.REQ_HOST_ATTRIBUTE) { // Get remote host name using a DNS resolution @@ -428,11 +439,31 @@ public abstract class AbstractAjpProcess } } + } else if (actionCode == ActionCode.REQ_HOST_ADDR_ATTRIBUTE) { + // NO-OP + // TODO Check if this is really a NO-OP for AJP or if something + // needs to be done here + + } else if (actionCode == ActionCode.REQ_LOCAL_NAME_ATTRIBUTE) { + // NO-OP + // TODO Check if this is really a NO-OP for AJP or if something + // needs to be done here + } else if (actionCode == ActionCode.REQ_LOCAL_ADDR_ATTRIBUTE) { // Copy from local name for now, which should simply be an address request.localAddr().setString(request.localName().toString()); + } else if (actionCode == ActionCode.REQ_REMOTEPORT_ATTRIBUTE) { + // NO-OP + // TODO Check if this is really a NO-OP for AJP or if something + // needs to be done here + + } else if (actionCode == ActionCode.REQ_LOCALPORT_ATTRIBUTE) { + // NO-OP + // TODO Check if this is really a NO-OP for AJP or if something + // needs to be done here + } else if (actionCode == ActionCode.REQ_SET_BODY_REPLAY) { // Set the given bytes as the content @@ -470,6 +501,39 @@ public abstract class AbstractAjpProcess // HTTP connections only. Unsupported for AJP. throw new UnsupportedOperationException( sm.getString("ajpprocessor.httpupgrade.notsupported")); + + } else if (actionCode == ActionCode.COMET_BEGIN) { + // HTTP connections only. Unsupported for AJP. + throw new UnsupportedOperationException( + sm.getString("ajpprocessor.comet.notsupported")); + + } else if (actionCode == ActionCode.COMET_END) { + // HTTP connections only. Unsupported for AJP. + throw new UnsupportedOperationException( + sm.getString("ajpprocessor.comet.notsupported")); + + } else if (actionCode == ActionCode.COMET_CLOSE) { + // HTTP connections only. Unsupported for AJP. + throw new UnsupportedOperationException( + sm.getString("ajpprocessor.comet.notsupported")); + + } else if (actionCode == ActionCode.COMET_SETTIMEOUT) { + // HTTP connections only. Unsupported for AJP. + throw new UnsupportedOperationException( + sm.getString("ajpprocessor.comet.notsupported")); + + } else if (actionCode == ActionCode.AVAILABLE) { + // TODO + } else if (actionCode == ActionCode.NB_WRITE_INTEREST) { + // TODO + } else if (actionCode == ActionCode.NB_READ_INTEREST) { + // TODO + } else if (actionCode == ActionCode.REQUEST_BODY_FULLY_READ) { + // TODO + } else if (actionCode == ActionCode.DISPATCH_READ) { + // TODO + } else if (actionCode == ActionCode.DISPATCH_WRITE) { + // TODO } else { actionInternal(actionCode, param); } Modified: tomcat/trunk/java/org/apache/coyote/ajp/LocalStrings.properties URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/ajp/LocalStrings.properties?rev=1518926&r1=1518925&r2=1518926&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/ajp/LocalStrings.properties (original) +++ tomcat/trunk/java/org/apache/coyote/ajp/LocalStrings.properties Fri Aug 30 09:50:30 2013 @@ -15,6 +15,7 @@ ajpnioprotocol.releaseStart=Iterating through our connections to release a socket channel [{0}] ajpnioprotocol.releaseEnd=Done iterating through our connections to release a socket channel [{0}] released [{1}] +ajpprocessor.comet.notsupported=Comet is not supported by the AJP protocol ajpprocessor.failedread=Socket read failed ajpprocessor.failedsend=Failed to send AJP message ajpprocessor.header.error=Header message parsing failed Modified: tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java?rev=1518926&r1=1518925&r2=1518926&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java (original) +++ tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java Fri Aug 30 09:50:30 2013 @@ -810,6 +810,10 @@ public abstract class AbstractHttp11Proc ((AtomicBoolean) param).set(asyncStateMachine.isAsyncTimingOut()); } else if (actionCode == ActionCode.ASYNC_IS_ERROR) { ((AtomicBoolean) param).set(asyncStateMachine.isAsyncError()); + } else if (actionCode == ActionCode.UPGRADE) { + httpUpgradeHandler = (HttpUpgradeHandler) param; + // Stop further HTTP output + getOutputBuffer().finished = true; } else if (actionCode == ActionCode.AVAILABLE) { request.setAvailable(inputBuffer.available()); } else if (actionCode == ActionCode.NB_WRITE_INTEREST) { @@ -822,10 +826,6 @@ public abstract class AbstractHttp11Proc } } else if (actionCode == ActionCode.NB_READ_INTEREST) { registerForEvent(true, false); - } else if (actionCode == ActionCode.UPGRADE) { - httpUpgradeHandler = (HttpUpgradeHandler) param; - // Stop further HTTP output - getOutputBuffer().finished = true; } else if (actionCode == ActionCode.REQUEST_BODY_FULLY_READ) { AtomicBoolean result = (AtomicBoolean) param; result.set(getInputBuffer().isFinished()); --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org