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: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org