Author: markt
Date: Mon Mar 17 12:34:59 2014
New Revision: 1578341
URL: http://svn.apache.org/r1578341
Log:
Improve processing of chuck size from chunked headers. Avoid overflow and use a
bit shift instead of a multiplication as it is marginally faster.
Modified:
tomcat/tc7.0.x/trunk/ (props changed)
tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java
tomcat/tc7.0.x/trunk/test/org/apache/coyote/http11/filters/TestChunkedInputFilter.java
tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml
Propchange: tomcat/tc7.0.x/trunk/
------------------------------------------------------------------------------
Merged /tomcat/trunk:r1578329,1578337
Modified:
tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java
URL:
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java?rev=1578341&r1=1578340&r2=1578341&view=diff
==============================================================================
---
tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java
(original)
+++
tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java
Mon Mar 17 12:34:59 2014
@@ -320,7 +320,7 @@ public class ChunkedInputFilter implemen
int result = 0;
boolean eol = false;
- boolean readDigit = false;
+ int readDigit = 0;
boolean extension = false;
while (!eol) {
@@ -342,10 +342,9 @@ public class ChunkedInputFilter implemen
} else if (!extension) {
//don't read data after the trailer
int charValue = HexUtils.getDec(buf[pos]);
- if (charValue != -1) {
- readDigit = true;
- result *= 16;
- result += charValue;
+ if (charValue != -1 && readDigit < 8) {
+ readDigit++;
+ result = (result << 4) | charValue;
} else {
//we shouldn't allow invalid, non hex characters
//in the chunked header
@@ -368,7 +367,7 @@ public class ChunkedInputFilter implemen
}
- if (!readDigit)
+ if (readDigit == 0 || result < 0)
return false;
if (result == 0)
Modified:
tomcat/tc7.0.x/trunk/test/org/apache/coyote/http11/filters/TestChunkedInputFilter.java
URL:
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/test/org/apache/coyote/http11/filters/TestChunkedInputFilter.java?rev=1578341&r1=1578340&r2=1578341&view=diff
==============================================================================
---
tomcat/tc7.0.x/trunk/test/org/apache/coyote/http11/filters/TestChunkedInputFilter.java
(original)
+++
tomcat/tc7.0.x/trunk/test/org/apache/coyote/http11/filters/TestChunkedInputFilter.java
Mon Mar 17 12:34:59 2014
@@ -105,7 +105,7 @@ public class TestChunkedInputFilter exte
Context ctx =
tomcat.addContext("", System.getProperty("java.io.tmpdir"));
- EchoHeaderServlet servlet = new EchoHeaderServlet();
+ EchoHeaderServlet servlet = new EchoHeaderServlet(expectPass);
Tomcat.addServlet(ctx, "servlet", servlet);
ctx.addServletMapping("/", "servlet");
@@ -169,7 +169,7 @@ public class TestChunkedInputFilter exte
Context ctx =
tomcat.addContext("", System.getProperty("java.io.tmpdir"));
- Tomcat.addServlet(ctx, "servlet", new EchoHeaderServlet());
+ Tomcat.addServlet(ctx, "servlet", new EchoHeaderServlet(false));
ctx.addServletMapping("/", "servlet");
// Limit the size of the trailing header
@@ -233,7 +233,7 @@ public class TestChunkedInputFilter exte
Context ctx =
tomcat.addContext("", System.getProperty("java.io.tmpdir"));
- Tomcat.addServlet(ctx, "servlet", new EchoHeaderServlet());
+ Tomcat.addServlet(ctx, "servlet", new EchoHeaderServlet(ok));
ctx.addServletMapping("/", "servlet");
tomcat.start();
@@ -282,7 +282,7 @@ public class TestChunkedInputFilter exte
Context ctx =
tomcat.addContext("", System.getProperty("java.io.tmpdir"));
- Tomcat.addServlet(ctx, "servlet", new EchoHeaderServlet());
+ Tomcat.addServlet(ctx, "servlet", new EchoHeaderServlet(true));
ctx.addServletMapping("/", "servlet");
tomcat.start();
@@ -311,11 +311,136 @@ public class TestChunkedInputFilter exte
assertEquals("nullnull7nullnull", client.getResponseBody());
}
+ @Test
+ public void testChunkSizeZero() throws Exception {
+ doTestChunkSize(true, true, "", 10, 0);
+ }
+
+ @Test
+ public void testChunkSizeAbsent() throws Exception {
+ doTestChunkSize(false, false, SimpleHttpClient.CRLF, 10, 0);
+ }
+
+ @Test
+ public void testChunkSizeTwentyFive() throws Exception {
+ doTestChunkSize(true, true, "19" + SimpleHttpClient.CRLF
+ + "Hello World!Hello World!!" + SimpleHttpClient.CRLF, 40, 25);
+ }
+
+ @Test
+ public void testChunkSizeEightDigit() throws Exception {
+ doTestChunkSize(true, true, "0000000C" + SimpleHttpClient.CRLF
+ + "Hello World!" + SimpleHttpClient.CRLF, 20, 12);
+ }
+
+ @Test
+ public void testChunkSizeNineDigit() throws Exception {
+ doTestChunkSize(false, false, "00000000C" + SimpleHttpClient.CRLF
+ + "Hello World!" + SimpleHttpClient.CRLF, 20, 12);
+ }
+
+ @Test
+ public void testChunkSizeLong() throws Exception {
+ doTestChunkSize(true, false, "7fFFffFF" + SimpleHttpClient.CRLF
+ + "Hello World!" + SimpleHttpClient.CRLF, 10, 10);
+ }
+
+ @Test
+ public void testChunkSizeIntegerMinValue() throws Exception {
+ doTestChunkSize(false, false, "80000000" + SimpleHttpClient.CRLF
+ + "Hello World!" + SimpleHttpClient.CRLF, 10, 10);
+ }
+
+ @Test
+ public void testChunkSizeMinusOne() throws Exception {
+ doTestChunkSize(false, false, "ffffffff" + SimpleHttpClient.CRLF
+ + "Hello World!" + SimpleHttpClient.CRLF, 10, 10);
+ }
+
+ /**
+ * @param expectPass
+ * If the servlet is expected to process the request
+ * @param expectReadWholeBody
+ * If the servlet is expected to fully read the body and
reliably
+ * deliver a response
+ * @param chunks
+ * Text of chunks
+ * @param readLimit
+ * Do not read more than this many bytes
+ * @param expectReadCount
+ * Expected count of read bytes
+ * @throws Exception
+ * Unexpected
+ */
+ private void doTestChunkSize(boolean expectPass,
+ boolean expectReadWholeBody, String chunks, int readLimit,
+ int expectReadCount) throws Exception {
+ // Setup Tomcat instance
+ Tomcat tomcat = getTomcatInstance();
+
+ // Must have a real docBase - just use temp
+ Context ctx = tomcat.addContext("",
+ System.getProperty("java.io.tmpdir"));
+
+ BodyReadServlet servlet = new BodyReadServlet(expectPass, readLimit);
+ Tomcat.addServlet(ctx, "servlet", servlet);
+ ctx.addServletMapping("/", "servlet");
+
+ tomcat.start();
+
+ String request = "POST /echo-params.jsp HTTP/1.1"
+ + SimpleHttpClient.CRLF + "Host: any" + SimpleHttpClient.CRLF
+ + "Transfer-encoding: chunked" + SimpleHttpClient.CRLF
+ + "Content-Type: text/plain" + SimpleHttpClient.CRLF;
+ if (expectPass) {
+ request += "Connection: close" + SimpleHttpClient.CRLF;
+ }
+ request += SimpleHttpClient.CRLF + chunks + "0" + SimpleHttpClient.CRLF
+ + SimpleHttpClient.CRLF;
+
+ TrailerClient client = new TrailerClient(tomcat.getConnector()
+ .getLocalPort());
+ client.setRequest(new String[] { request });
+
+ Exception processException = null;
+ client.connect();
+ try {
+ client.processRequest();
+ } catch (Exception e) {
+ // Socket was probably closed before client had a chance to read
+ // response
+ processException = e;
+ }
+ if (expectPass) {
+ if (expectReadWholeBody) {
+ assertNull(processException);
+ }
+ if (processException == null) {
+ assertTrue(client.getResponseLine(), client.isResponse200());
+ assertEquals(String.valueOf(expectReadCount),
+ client.getResponseBody());
+ }
+ assertEquals(expectReadCount, servlet.getCountRead());
+ } else {
+ if (processException == null) {
+ assertTrue(client.getResponseLine(), client.isResponse500());
+ }
+ assertEquals(0, servlet.getCountRead());
+ assertTrue(servlet.getExceptionDuringRead());
+ }
+ }
+
private static class EchoHeaderServlet extends HttpServlet {
private static final long serialVersionUID = 1L;
private boolean exceptionDuringRead = false;
+ private final boolean expectPass;
+
+ public EchoHeaderServlet(boolean expectPass) {
+ this.expectPass = expectPass;
+ }
+
@Override
protected void doPost(HttpServletRequest req, HttpServletResponse resp)
throws ServletException, IOException {
@@ -334,6 +459,11 @@ public class TestChunkedInputFilter exte
}
} catch (IOException ioe) {
exceptionDuringRead = true;
+ if (!expectPass) { // as expected
+ log(ioe.toString());
+
resp.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
+ return;
+ }
throw ioe;
}
@@ -358,6 +488,53 @@ public class TestChunkedInputFilter exte
}
}
+ private static class BodyReadServlet extends HttpServlet {
+ private static final long serialVersionUID = 1L;
+
+ private boolean exceptionDuringRead = false;
+ private int countRead = 0;
+ private final boolean expectPass;
+ private final int readLimit;
+
+ public BodyReadServlet(boolean expectPass, int readLimit) {
+ this.expectPass = expectPass;
+ this.readLimit = readLimit;
+ }
+
+ @Override
+ protected void doPost(HttpServletRequest req, HttpServletResponse resp)
+ throws ServletException, IOException {
+ resp.setContentType("text/plain");
+ PrintWriter pw = resp.getWriter();
+
+ // Read the body - quick and dirty
+ InputStream is = req.getInputStream();
+ try {
+ while (is.read() > -1 && countRead < readLimit) {
+ countRead++;
+ }
+ } catch (IOException ioe) {
+ exceptionDuringRead = true;
+ if (!expectPass) { // as expected
+ log(ioe.toString());
+
resp.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
+ return;
+ }
+ throw ioe;
+ }
+
+ pw.write(Integer.valueOf(countRead).toString());
+ }
+
+ public boolean getExceptionDuringRead() {
+ return exceptionDuringRead;
+ }
+
+ public int getCountRead() {
+ return countRead;
+ }
+ }
+
private static class TrailerClient extends SimpleHttpClient {
public TrailerClient(int port) {
Modified: tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml
URL:
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml?rev=1578341&r1=1578340&r2=1578341&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Mon Mar 17 12:34:59 2014
@@ -109,6 +109,11 @@
<bug>56213</bug>: Reduce garbage collection when the NIO connector is
under heavy load. (markt)
</fix>
+ <fix>
+ Improve processing of chuck size from chunked headers. Avoid overflow
+ and use a bit shift instead of a multiplication as it is marginally
+ faster. (markt/kkolinko)
+ </fix>
</changelog>
</subsection>
<subsection name="Jasper">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
