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

Reply via email to