Improve handling of too large packets in AJP connectors
authormarkt <markt@13f79535-47bb-0310-9956-ffa450edef68>
Wed, 4 May 2011 18:32:05 +0000 (18:32 +0000)
committermarkt <markt@13f79535-47bb-0310-9956-ffa450edef68>
Wed, 4 May 2011 18:32:05 +0000 (18:32 +0000)
- Explicitly check the packet size rather than waiting to see if it fails
- Provide a better debug message when it does fail
- Once we know we have a bad request, don't try to process it

git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@1099556 13f79535-47bb-0310-9956-ffa450edef68

java/org/apache/coyote/ajp/AjpAprProcessor.java
java/org/apache/coyote/ajp/AjpProcessor.java
java/org/apache/coyote/ajp/LocalStrings.properties
webapps/docs/changelog.xml

index f3b9e8c..434b2f2 100644 (file)
@@ -263,17 +263,19 @@ public class AjpAprProcessor extends AbstractAjpProcessor {
                 error = true;
             }
 
-            // Setting up filters, and parse some request headers
-            rp.setStage(org.apache.coyote.Constants.STAGE_PREPARE);
-            try {
-                prepareRequest();
-            } catch (Throwable t) {
-                ExceptionUtils.handleThrowable(t);
-                log.debug(sm.getString("ajpprocessor.request.prepare"), t);
-                // 400 - Internal Server Error
-                response.setStatus(400);
-                adapter.log(request, response, 0);
-                error = true;
+            if (!error) {
+                // Setting up filters, and parse some request headers
+                rp.setStage(org.apache.coyote.Constants.STAGE_PREPARE);
+                try {
+                    prepareRequest();
+                } catch (Throwable t) {
+                    ExceptionUtils.handleThrowable(t);
+                    log.debug(sm.getString("ajpprocessor.request.prepare"), t);
+                    // 400 - Internal Server Error
+                    response.setStatus(400);
+                    adapter.log(request, response, 0);
+                    error = true;
+                }
             }
 
             // Process the request in the adapter
@@ -621,6 +623,14 @@ public class AjpAprProcessor extends AbstractAjpProcessor {
             return true;
         }
         else {
+            if (messageLength > message.getBuffer().length) {
+                // Message too long for the buffer
+                // Need to trigger a 400 response
+                throw new IllegalArgumentException(sm.getString(
+                        "ajpprocessor.header.tooLong",
+                        Integer.valueOf(messageLength),
+                        Integer.valueOf(message.getBuffer().length)));
+            }
             read(messageLength);
             inputBuffer.get(message.getBuffer(), headerLength, messageLength);
             return true;
index c4023d2..10355d6 100644 (file)
@@ -274,17 +274,19 @@ public class AjpProcessor extends AbstractAjpProcessor {
                 error = true;
             }
 
-            // Setting up filters, and parse some request headers
-            rp.setStage(org.apache.coyote.Constants.STAGE_PREPARE);
-            try {
-                prepareRequest();
-            } catch (Throwable t) {
-                ExceptionUtils.handleThrowable(t);
-                log.debug(sm.getString("ajpprocessor.request.prepare"), t);
-                // 400 - Internal Server Error
-                response.setStatus(400);
-                adapter.log(request, response, 0);
-                error = true;
+            if (!error) {
+                // Setting up filters, and parse some request headers
+                rp.setStage(org.apache.coyote.Constants.STAGE_PREPARE);
+                try {
+                    prepareRequest();
+                } catch (Throwable t) {
+                    ExceptionUtils.handleThrowable(t);
+                    log.debug(sm.getString("ajpprocessor.request.prepare"), t);
+                    // 400 - Internal Server Error
+                    response.setStatus(400);
+                    adapter.log(request, response, 0);
+                    error = true;
+                }
             }
 
             if (endpoint.isPaused()) {
@@ -570,6 +572,14 @@ public class AjpProcessor extends AbstractAjpProcessor {
             return true;
         }
         else {
+            if (messageLength > buf.length) {
+                // Message too long for the buffer
+                // Need to trigger a 400 response
+                throw new IllegalArgumentException(sm.getString(
+                        "ajpprocessor.header.tooLong",
+                        Integer.valueOf(messageLength),
+                        Integer.valueOf(buf.length)));
+            }
             read(buf, headerLength, messageLength);
             return true;
         }
index 3025aa8..62d230e 100644 (file)
@@ -34,6 +34,7 @@ ajpprotocol.request.register=Error registering request processor in JMX
 ajpprocessor.failedflush=Failed to flush AJP message
 ajpprocessor.failedsend=Failed to send AJP message
 ajpprocessor.header.error=Header message parsing failed
+ajpprocessor.header.tooLong=Header message of length [{0}] received but the packetSize is only [{1}]
 ajpprocessor.request.prepare=Error preparing request
 ajpprocessor.request.process=Error processing request
 ajpprocessor.certs.fail=Certificate conversion failed
index 560bf19..7792e81 100644 (file)
         handshake fails with the HTTP-APR connector. Patch provided by Mike
         Glazer. (markt)
       </fix>
+      <fix>
+        Improve handling in AJP connectors of the case where too large a AJP
+        packet is received. (markt) 
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Jasper">