tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kkoli...@apache.org
Subject svn commit: r1066313 - in /tomcat/tc6.0.x/trunk: STATUS.txt java/org/apache/coyote/http11/InternalNioInputBuffer.java webapps/docs/changelog.xml
Date Wed, 02 Feb 2011 02:56:40 GMT
Author: kkolinko
Date: Wed Feb  2 02:56:39 2011
New Revision: 1066313

URL: http://svn.apache.org/viewvc?rev=1066313&view=rev
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=50631
InternalNioInputBuffer should honor maxHttpHeadSize

Modified:
    tomcat/tc6.0.x/trunk/STATUS.txt
    tomcat/tc6.0.x/trunk/java/org/apache/coyote/http11/InternalNioInputBuffer.java
    tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml

Modified: tomcat/tc6.0.x/trunk/STATUS.txt
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=1066313&r1=1066312&r2=1066313&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/STATUS.txt (original)
+++ tomcat/tc6.0.x/trunk/STATUS.txt Wed Feb  2 02:56:39 2011
@@ -119,13 +119,6 @@ PATCHES PROPOSED TO BACKPORT:
      concerns against it.
    markt: Happy to exclude those changes
 
-* Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=50631
-  InternalNioInputBuffer should honor maxHttpHeadSize
-  (backport of r1065939)
-  http://people.apache.org/~kkolinko/patches/2011-02-01_tc6_50631.patch
-  +1: kkolinko, markt, funkman
-  -1:
-
 * Improve HTTP specification compliance
   http://svn.apache.org/viewvc?rev=1066244&view=rev
   +1: kkolinko, rjung, markt, funkman

Modified: tomcat/tc6.0.x/trunk/java/org/apache/coyote/http11/InternalNioInputBuffer.java
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/coyote/http11/InternalNioInputBuffer.java?rev=1066313&r1=1066312&r2=1066313&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/coyote/http11/InternalNioInputBuffer.java (original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/coyote/http11/InternalNioInputBuffer.java Wed Feb
 2 02:56:39 2011
@@ -41,6 +41,11 @@ import org.apache.tomcat.util.net.NioEnd
  */
 public class InternalNioInputBuffer implements InputBuffer {
 
+    /**
+     * Logger.
+     */
+    private static final org.apache.juli.logging.Log log =
+        org.apache.juli.logging.LogFactory.getLog(InternalNioInputBuffer.class);
 
     // -------------------------------------------------------------- Constants
 
@@ -57,12 +62,7 @@ public class InternalNioInputBuffer impl
         this.request = request;
         headers = request.getMimeHeaders();
 
-        buf = new byte[headerBufferSize];
-//        if (headerBufferSize < (8 * 1024)) {
-//            bbuf = ByteBuffer.allocateDirect(6 * 1500);
-//        } else {
-//            bbuf = ByteBuffer.allocateDirect((headerBufferSize / 1500 + 1) * 1500);
-//        }
+        this.headerBufferSize = headerBufferSize;
 
         inputStreamInputBuffer = new SocketInputBuffer();
 
@@ -189,6 +189,28 @@ public class InternalNioInputBuffer impl
     protected int lastActiveFilter;
 
 
+    /**
+     * Maximum allowed size of the HTTP request line plus headers.
+     */
+    private final int headerBufferSize;
+
+    /**
+     * Known size of the NioChannel read buffer.
+     */
+    private int socketReadBufferSize;
+
+    /**
+     * Additional size we allocate to the buffer to be more effective when
+     * skipping empty lines that may precede the request.
+     */
+    private static final int skipBlankLinesSize = 1024;
+
+    /**
+     * How many bytes in the buffer are occupied by skipped blank lines that
+     * precede the request.
+     */
+    private int skipBlankLinesBytes;
+
     // ------------------------------------------------------------- Properties
 
 
@@ -197,6 +219,12 @@ public class InternalNioInputBuffer impl
      */
     public void setSocket(NioChannel socket) {
         this.socket = socket;
+        socketReadBufferSize = socket.getBufHandler().getReadBuffer().capacity();
+        int bufLength = skipBlankLinesSize + headerBufferSize
+                + socketReadBufferSize;
+        if (buf == null || buf.length < bufLength) {
+            buf = new byte[bufLength];
+        }
     }
     
     /**
@@ -421,25 +449,23 @@ public class InternalNioInputBuffer impl
                     if (useAvailableDataOnly) {
                         return false;
                     }
+                    // Ignore bytes that were read
+                    pos = lastValid = 0;
                     // Do a simple read with a short timeout
                     if ( readSocket(true, false)==0 ) return false;
                 }
                 chr = buf[pos++];
             } while ((chr == Constants.CR) || (chr == Constants.LF));
             pos--;
-            parsingRequestLineStart = pos;
-            parsingRequestLinePhase = 1;
-        } 
-        if ( parsingRequestLinePhase == 1 ) {
-            // Mark the current buffer position
-            
-            if (pos >= lastValid) {
-                if (useAvailableDataOnly) {
-                    return false;
-                }
-                // Do a simple read with a short timeout
-                if ( readSocket(true, false)==0 ) return false;
+            if (pos >= skipBlankLinesSize) {
+                // Move data, to have enough space for further reading
+                // of headers and body
+                System.arraycopy(buf, pos, buf, 0, lastValid - pos);
+                lastValid -= pos;
+                pos = 0;
             }
+            skipBlankLinesBytes = pos;
+            parsingRequestLineStart = pos;
             parsingRequestLinePhase = 2;
         }
         if ( parsingRequestLinePhase == 2 ) {
@@ -583,6 +609,13 @@ public class InternalNioInputBuffer impl
     
     private void expand(int newsize) {
         if ( newsize > buf.length ) {
+            if (parsingHeader) {
+                throw new IllegalArgumentException(
+                        sm.getString("iib.requestheadertoolarge.error"));
+            }
+            // Should not happen
+            log.warn("Expanding buffer size. Old size: " + buf.length
+                    + ", new size: " + newsize, new Exception());
             byte[] tmp = new byte[newsize];
             System.arraycopy(buf,0,tmp,0,buf.length);
             buf = tmp;
@@ -644,6 +677,19 @@ public class InternalNioInputBuffer impl
         if (status == HeaderParseStatus.DONE) {
             parsingHeader = false;
             end = pos;
+            // Checking that
+            // (1) Headers plus request line size does not exceed its limit
+            // (2) There are enough bytes to avoid expanding the buffer when
+            // reading body
+            // Technically, (2) is technical limitation, (1) is logical
+            // limitation to enforce the meaning of headerBufferSize
+            // From the way how buf is allocated and how blank lines are being
+            // read, it should be enough to check (1) only.
+            if (end - skipBlankLinesBytes > headerBufferSize
+                    || buf.length - end < socketReadBufferSize) {
+                throw new IllegalArgumentException(
+                        sm.getString("iib.requestheadertoolarge.error"));
+            }
             return true;
         } else {
             return false;
@@ -894,16 +940,7 @@ public class InternalNioInputBuffer impl
             // Do a simple read with a short timeout
             read = readSocket(timeout,block)>0;
         } else {
-
-            if (buf.length - end < 4500) {
-                // In this case, the request header was really large, so we allocate a 
-                // brand new one; the old one will get GCed when subsequent requests
-                // clear all references
-                buf = new byte[buf.length];
-                end = 0;
-            }
-            pos = end;
-            lastValid = pos;
+            lastValid = pos = end;
             // Do a simple read with a short timeout
             read = readSocket(timeout, block)>0;
         }

Modified: tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml?rev=1066313&r1=1066312&r2=1066313&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml Wed Feb  2 02:56:39 2011
@@ -59,6 +59,10 @@
         it more robust. (mturk/kkolinko)
       </fix>
       <fix>
+        <bug>50631</bug>: InternalNioInputBuffer should honor
+        <code>maxHttpHeadSize</code>. (kkolinko)
+      </fix>
+      <fix>
         <bug>50651</bug>: Fix NPE in InternalNioOutputBuffer.recycle().
         (kkolinko)
       </fix>



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Mime
View raw message