Bug 50631

Summary: InternalNioInputBuffer should honor maxHttpHeadSize
Product: Tomcat 6 Reporter: yuesong.c
Component: ConnectorsAssignee: Tomcat Developers Mailing List <dev>
Severity: normal    
Priority: P2    
Version: 6.0.29   
Target Milestone: default   
Hardware: PC   
OS: All   

Description yuesong.c 2011-01-21 13:23:52 UTC
InternalNioInputBuffer automatically expands its buffer while reading in request line, effectively ignoring the maxHttpHeaderSize setting.
Comment 1 Konstantin Kolinko 2011-01-27 15:26:07 UTC
For reference: this issue was created from the following thread on users@:

There is the following code in InternalNioInputBuffer#fill(boolean, boolean) both in 6.0.31 and in 7.0.6:

      if (parsingHeader) {
            if (lastValid == buf.length) {
                throw new IllegalArgumentException

and it should take care of overflows. But there is also InternalNioInputBuffer#expand(int) which is called from #readSocket().

I thought of the below patch (for trunk, not tested), but I see another problem: I do not see InternalNioInputBuffer.buf being recycled (shrunk) after its growth. I suspect that it should not be allowed to grow even when it is body of the request that we are reading now. In InternalAprInputBuffer.fill() and InternalInputBuffer.fill() the buffer can be reallocated, but the new buffer will be of the same size as the old one.

Thus instead of the patch below I suspect that InternalNioInputBuffer.expand() should fail unconditionally when ( newsize > buf.length ).

Index: java/org/apache/coyote/http11/InternalNioInputBuffer.java
--- java/org/apache/coyote/http11/InternalNioInputBuffer.java	(revision 1064206)
+++ java/org/apache/coyote/http11/InternalNioInputBuffer.java	(working copy)
@@ -370,6 +370,10 @@
     private void expand(int newsize) {
         if ( newsize > buf.length ) {
+            if (parsingHeader) {
+                throw new IllegalArgumentException(
+                        sm.getString("iib.requestheadertoolarge.error"));
+            }
             byte[] tmp = new byte[newsize];
             buf = tmp;
Comment 2 Konstantin Kolinko 2011-02-01 21:58:16 UTC
Fixed in trunk and in 6.0.
Will be in 7.0.7, 6.0.32.