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: [email protected]
For additional commands, e-mail: [email protected]
