Author: markt
Date: Thu Jan 31 11:24:37 2013
New Revision: 1440912
URL: http://svn.apache.org/viewvc?rev=1440912&view=rev
Log:
Align NIO with BIO and APR and include leading blank lines when counting input
for HTTP header size limit
Modified:
tomcat/tc7.0.x/trunk/ (props changed)
tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/InternalNioInputBuffer.java
tomcat/tc7.0.x/trunk/test/org/apache/coyote/http11/TestInternalInputBuffer.java
tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml
Propchange: tomcat/tc7.0.x/trunk/
------------------------------------------------------------------------------
Merged /tomcat/trunk:r1440911
Modified:
tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/InternalNioInputBuffer.java
URL:
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/InternalNioInputBuffer.java?rev=1440912&r1=1440911&r2=1440912&view=diff
==============================================================================
---
tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/InternalNioInputBuffer.java
(original)
+++
tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/InternalNioInputBuffer.java
Thu Jan 31 11:24:37 2013
@@ -145,7 +145,8 @@ public class InternalNioInputBuffer exte
/**
- * Maximum allowed size of the HTTP request line plus headers.
+ * Maximum allowed size of the HTTP request line plus headers plus any
+ * leading blank lines.
*/
private final int headerBufferSize;
@@ -154,18 +155,6 @@ public class InternalNioInputBuffer exte
*/
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;
-
// --------------------------------------------------------- Public Methods
@@ -234,22 +223,15 @@ public class InternalNioInputBuffer exte
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;
+ if (!fill(true, false)) {
+ return false;
+ }
}
chr = buf[pos++];
} while ((chr == Constants.CR) || (chr == Constants.LF));
pos--;
- 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 (log.isDebugEnabled()) {
@@ -481,7 +463,7 @@ public class InternalNioInputBuffer exte
// 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 (pos - skipBlankLinesBytes > headerBufferSize
+ if (pos > headerBufferSize
|| buf.length - pos < socketReadBufferSize) {
throw new IllegalArgumentException(
sm.getString("iib.requestheadertoolarge.error"));
@@ -768,8 +750,7 @@ public class InternalNioInputBuffer exte
socketReadBufferSize =
socket.getBufHandler().getReadBuffer().capacity();
- int bufLength = skipBlankLinesSize + headerBufferSize
- + socketReadBufferSize;
+ int bufLength = headerBufferSize + socketReadBufferSize;
if (buf == null || buf.length < bufLength) {
buf = new byte[bufLength];
}
@@ -795,7 +776,7 @@ public class InternalNioInputBuffer exte
if (parsingHeader) {
- if (lastValid == buf.length) {
+ if (lastValid > headerBufferSize) {
throw new IllegalArgumentException
(sm.getString("iib.requestheadertoolarge.error"));
}
Modified:
tomcat/tc7.0.x/trunk/test/org/apache/coyote/http11/TestInternalInputBuffer.java
URL:
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/test/org/apache/coyote/http11/TestInternalInputBuffer.java?rev=1440912&r1=1440911&r2=1440912&view=diff
==============================================================================
---
tomcat/tc7.0.x/trunk/test/org/apache/coyote/http11/TestInternalInputBuffer.java
(original)
+++
tomcat/tc7.0.x/trunk/test/org/apache/coyote/http11/TestInternalInputBuffer.java
Thu Jan 31 11:24:37 2013
@@ -27,12 +27,14 @@ import javax.servlet.http.HttpServletReq
import javax.servlet.http.HttpServletResponse;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import org.junit.Test;
import org.apache.catalina.Context;
import org.apache.catalina.startup.SimpleHttpClient;
+import org.apache.catalina.startup.TesterServlet;
import org.apache.catalina.startup.Tomcat;
import org.apache.catalina.startup.TomcatBaseTest;
@@ -316,4 +318,97 @@ public class TestInternalInputBuffer ext
}
}
}
+
+
+ /**
+ * Test case for new lines at the start of a request. RFC2616
+ * does not permit any, but Tomcat is tolerant of them if they are present.
+ */
+ @Test
+ public void testNewLines() {
+
+ NewLinesClient client = new NewLinesClient(10);
+
+ client.doRequest();
+ assertTrue(client.isResponse200());
+ assertTrue(client.isResponseBodyOK());
+ }
+
+
+ /**
+ * Test case for new lines at the start of a request. RFC2616
+ * does not permit any, but Tomcat is tolerant of them if they are present.
+ */
+ @Test
+ public void testNewLinesExcessive() {
+
+ NewLinesClient client = new NewLinesClient(10000);
+
+ client.doRequest();
+ assertTrue(client.isResponse400());
+ assertFalse(client.isResponseBodyOK());
+ }
+
+
+ private class NewLinesClient extends SimpleHttpClient {
+
+ private final String newLines;
+
+ private NewLinesClient(int count) {
+ StringBuilder sb = new StringBuilder(count * 2);
+ for (int i = 0; i < count; i++) {
+ sb.append(CRLF);
+ }
+ newLines = sb.toString();
+ }
+
+ private Exception doRequest() {
+
+ Tomcat tomcat = getTomcatInstance();
+
+ Context root = tomcat.addContext("", TEMP_DIR);
+ Tomcat.addServlet(root, "test", new TesterServlet());
+ root.addServletMapping("/test", "test");
+
+ try {
+ tomcat.start();
+ setPort(tomcat.getConnector().getLocalPort());
+
+ // Open connection
+ connect();
+
+ String[] request = new String[1];
+ request[0] =
+ newLines +
+ "GET http://localhost:8080/test HTTP/1.1" + CRLF +
+ "X-Bug48839: abcd" + CRLF +
+ "\tefgh" + CRLF +
+ "Connection: close" + CRLF +
+ CRLF;
+
+ setRequest(request);
+ processRequest(); // blocks until response has been read
+
+ // Close the connection
+ disconnect();
+ } catch (Exception e) {
+ return e;
+ }
+ return null;
+ }
+
+ @Override
+ public boolean isResponseBodyOK() {
+ if (getResponseBody() == null) {
+ return false;
+ }
+ if (!getResponseBody().contains("OK")) {
+ return false;
+ }
+ return true;
+ }
+
+ }
+
+
}
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=1440912&r1=1440911&r2=1440912&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Thu Jan 31 11:24:37 2013
@@ -111,6 +111,10 @@
are supported - new behaviour is to warn and explicitly enable no
options. (timw)
</fix>
+ <fix>
+ Align NIO HTTP connector with other HTTP connectors and include leading
+ blank lines when determining the size of the HTTP headers. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Jasper">
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
