Here is the promised patch for the ChunkedInputFilter, please review
and comment,
Filip
-------------------------------------------------------------------------------------
Index: java/org/apache/coyote/http11/Constants.java
===================================================================
--- java/org/apache/coyote/http11/Constants.java (revision 417384)
+++ java/org/apache/coyote/http11/Constants.java (working copy)
@@ -83,8 +83,14 @@
* COLON.
*/
public static final byte COLON = (byte) ':';
+ + /**
+ * SEMI_COLON.
+ */
+ public static final byte SEMI_COLON = (byte) ';';
+
/**
* 'A'.
*/
Index: java/org/apache/coyote/http11/filters/ChunkedInputFilter.java
===================================================================
--- java/org/apache/coyote/http11/filters/ChunkedInputFilter.java
(revision 417384)
+++ java/org/apache/coyote/http11/filters/ChunkedInputFilter.java
(working copy)
@@ -27,9 +27,11 @@
import org.apache.coyote.http11.InputFilter;
/**
- * Chunked input filter.
+ * Chunked input filter. Parses chunked data according to
+ * <a
href="http://www.w3.org/Protocols/rfc2616/rfc2616-sec3.html#sec3.6.1";>http://www.w3.org/Protocols/rfc2616/rfc2616-sec3.html#sec3.6.1</a><br>
*
* @author Remy Maucherat
+ * @author Filip Hanik
*/
public class ChunkedInputFilter implements InputFilter {
@@ -127,7 +129,7 @@
if (remaining <= 0) {
if (!parseChunkHeader()) {
- throw new IOException("Invalid chunk");
+ throw new IOException("Invalid chunk header");
}
if (endChunk) {
parseEndChunk();
@@ -234,6 +236,12 @@
/**
* Parse the header of a chunk.
+ * A chunk header can look like
+ * A10CRLF
+ * F23;ignore this dataCRLF
+ * The letters before CRLF or the trailer mark, must be valid hex
digits,
+ * we should not parse
+ * F23IAMGONNAMESSTHISUP34CRLF
*/
protected boolean parseChunkHeader()
throws IOException {
@@ -241,6 +249,7 @@
int result = 0;
boolean eol = false;
boolean readDigit = false;
+ boolean trailer = false;
while (!eol) {
@@ -252,11 +261,18 @@
if (buf[pos] == Constants.CR) {
} else if (buf[pos] == Constants.LF) {
eol = true;
- } else {
+ } else if (buf[pos] == Constants.SEMI_COLON) {
+ trailer = true;
+ } else if (!trailer) {
+ //don't read data after the trailer
if (HexUtils.DEC[buf[pos]] != -1) {
readDigit = true;
result *= 16;
result += HexUtils.DEC[buf[pos]];
+ } else {
+ //we shouldn't allow invalid, non hex characters
+ //in the chunked header
+ return false;
}
}
-------------------------------------------------------------------------------------
Filip Hanik - Dev Lists wrote:
Remy Maucherat wrote:
Filip Hanik - Dev Lists wrote:
gents, I know, back again, and totally bastardizing the HTTP protocol,
I have a test request that looks like this:
----------------------------------
POST /comet HTTP/1.1CRLF
User-Agent: Jakarta Commons-HttpClient/3.0.1CRLF
Host: 127.0.0.1:8080CRLF
Transfer-Encoding: chunkedCRLF
CRLF
10CRLF
test data test 1CRLF
---------------------------------------
If I keep sending up
---------------------------------------
10CRLF
test data test 1CRLF
---------------------------------------
we are all happy, however, if I bastardize the next request and
send up the same request twice, so the 2nd request of the HTTP
request above becomes the body of the first chunked request, then
suddenly the chunked input filter will set its remaining bytes to
over 50k remaining bytes, and everything gets streamed into the
servlet.
The ChunkedInputFilter is written to handle correct chunks very
well, but when incorrect chunked bodies come through, it passes all
the content through, instead of throwing an error.
If no one objects, I would like to add in some error checking, even
though it is the clients responsibility to provide correct data,
the web server should not let incorrect data through as that could
be a semi http buffer overflow :)
The only thing which determines if a chunk is correct is if the
chunk length is valid and readable. The parseChunkHeader method
includes the ability to return false and throw an IOE if an invalid
header is received.
yes, and there is where the bug lies, it doesn't throw an IOException
when it gets an invalid header, I'll fix it and submit the patch for
review so that we can get it taken care of.
Filip
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
------------------------------------------------------------------------
Index: java/org/apache/coyote/http11/Constants.java
===================================================================
--- java/org/apache/coyote/http11/Constants.java (revision 417384)
+++ java/org/apache/coyote/http11/Constants.java (working copy)
@@ -83,8 +83,14 @@
* COLON.
*/
public static final byte COLON = (byte) ':';
+
+ /**
+ * SEMI_COLON.
+ */
+ public static final byte SEMI_COLON = (byte) ';';
+
/**
* 'A'.
*/
Index: java/org/apache/coyote/http11/filters/ChunkedInputFilter.java
===================================================================
--- java/org/apache/coyote/http11/filters/ChunkedInputFilter.java
(revision 417384)
+++ java/org/apache/coyote/http11/filters/ChunkedInputFilter.java
(working copy)
@@ -27,9 +27,11 @@
import org.apache.coyote.http11.InputFilter;
/**
- * Chunked input filter.
+ * Chunked input filter. Parses chunked data according to
+ * <a
href="http://www.w3.org/Protocols/rfc2616/rfc2616-sec3.html#sec3.6.1";>http://www.w3.org/Protocols/rfc2616/rfc2616-sec3.html#sec3.6.1</a><br>
*
* @author Remy Maucherat
+ * @author Filip Hanik
*/
public class ChunkedInputFilter implements InputFilter {
@@ -127,7 +129,7 @@
if (remaining <= 0) {
if (!parseChunkHeader()) {
- throw new IOException("Invalid chunk");
+ throw new IOException("Invalid chunk header");
}
if (endChunk) {
parseEndChunk();
@@ -234,6 +236,12 @@
/**
* Parse the header of a chunk.
+ * A chunk header can look like
+ * A10CRLF
+ * F23;ignore this dataCRLF
+ * The letters before CRLF or the trailer mark, must be valid hex digits,
+ * we should not parse
+ * F23IAMGONNAMESSTHISUP34CRLF
*/
protected boolean parseChunkHeader()
throws IOException {
@@ -241,6 +249,7 @@
int result = 0;
boolean eol = false;
boolean readDigit = false;
+ boolean trailer = false;
while (!eol) {
@@ -252,11 +261,18 @@
if (buf[pos] == Constants.CR) {
} else if (buf[pos] == Constants.LF) {
eol = true;
- } else {
+ } else if (buf[pos] == Constants.SEMI_COLON) {
+ trailer = true;
+ } else if (!trailer) {
+ //don't read data after the trailer
if (HexUtils.DEC[buf[pos]] != -1) {
readDigit = true;
result *= 16;
result += HexUtils.DEC[buf[pos]];
+ } else {
+ //we shouldn't allow invalid, non hex characters
+ //in the chunked header
+ return false;
}
}
------------------------------------------------------------------------
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
