Author: markt Date: Mon Mar 17 14:17:19 2014 New Revision: 1578392 URL: http://svn.apache.org/r1578392 Log: Correct regression introduced in 8.0.0-RC2 as part of the Servlet 3.1 non-blocking IO support that broke handling of requests with an explicit content length of zero.
Modified: tomcat/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java tomcat/trunk/test/org/apache/coyote/ajp/TestAbstractAjpProcessor.java tomcat/trunk/webapps/docs/changelog.xml Modified: tomcat/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java?rev=1578392&r1=1578391&r2=1578392&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java (original) +++ tomcat/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java Mon Mar 17 14:17:19 2014 @@ -242,13 +242,6 @@ public abstract class AbstractAjpProcess /** - * Is a body present for the current request? This is determined by the - * presence of the content-length header with a non-zero value. - */ - private boolean bodyPresent = false; - - - /** * Indicates that a 'get body chunk' message has been sent but the body * chunk has not yet been received. */ @@ -902,7 +895,6 @@ public abstract class AbstractAjpProcess // Recycle Request object first = true; endOfStream = false; - bodyPresent = false; waitingForBodyMessage = false; empty = true; replay = false; @@ -975,12 +967,10 @@ public abstract class AbstractAjpProcess } waitingForBodyMessage = false; - first = false; // No data received. if (bodyMessage.getLen() == 0) { // just the header - // Don't mark 'end of stream' for the first chunk. return false; } int blen = bodyMessage.peekInt(); @@ -1061,9 +1051,8 @@ public abstract class AbstractAjpProcess * @return true if there is more data, false if not. */ protected boolean refillReadBuffer(boolean block) throws IOException { - // If the server returns an empty packet, assume that that end of - // the stream has been reached (yuck -- fix protocol??). - // FORM support + // When using replay (e.g. after FORM auth) all the data to read has + // been buffered so there is no opportunity to refill the buffer. if (replay) { endOfStream = true; // we've read everything there is } @@ -1071,14 +1060,30 @@ public abstract class AbstractAjpProcess return false; } + if (first) { + first = false; + long contentLength = request.getContentLengthLong(); + // - When content length > 0, AJP sends the first body message + // automatically. + // - When content length == 0, AJP does not send a body message. + // - When content length is unknown, AJP does not send the first + // body message automatically. + if (contentLength > 0) { + waitingForBodyMessage = true; + } else if (contentLength == 0) { + endOfStream = true; + return false; + } + } + // Request more data immediately - if (!first && !waitingForBodyMessage) { + if (!waitingForBodyMessage) { output(getBodyMessageArray, 0, getBodyMessageArray.length, true); waitingForBodyMessage = true; } boolean moreData = receive(block); - if (!moreData && ((first && !bodyPresent) || (!first && !waitingForBodyMessage))) { + if (!moreData && !waitingForBodyMessage) { endOfStream = true; } return moreData; @@ -1160,9 +1165,6 @@ public abstract class AbstractAjpProcess // Set the content-length header for the request request.setContentLength(cl); } - if (cl != 0) { - bodyPresent = true; - } } else if (hId == Constants.SC_REQ_CONTENT_TYPE || (hId == -1 && tmpMB.equalsIgnoreCase("Content-Type"))) { // just read the content-type header, so set it @@ -1519,8 +1521,8 @@ public abstract class AbstractAjpProcess finished = true; // Swallow the unread body packet if present - if (first && request.getContentLengthLong() > 0 || waitingForBodyMessage) { - receive(true); + if (waitingForBodyMessage || first && request.getContentLengthLong() > 0) { + refillReadBuffer(true); } // Add the end message @@ -1539,7 +1541,7 @@ public abstract class AbstractAjpProcess if (empty) { try { refillReadBuffer(false); - } catch (IOException e) { + } catch (IOException timeout) { // Not ideal. This will indicate that data is available // which should trigger a read which in turn will trigger // another IOException and that one can be thrown. @@ -1682,12 +1684,7 @@ public abstract class AbstractAjpProcess if (endOfStream) { return -1; } - if (first && req.getContentLengthLong() > 0) { - // Handle special first-body-chunk - if (!receive(true)) { - return 0; - } - } else if (empty) { + if (empty) { if (!refillReadBuffer(true)) { return -1; } Modified: tomcat/trunk/test/org/apache/coyote/ajp/TestAbstractAjpProcessor.java URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/coyote/ajp/TestAbstractAjpProcessor.java?rev=1578392&r1=1578391&r2=1578392&view=diff ============================================================================== --- tomcat/trunk/test/org/apache/coyote/ajp/TestAbstractAjpProcessor.java (original) +++ tomcat/trunk/test/org/apache/coyote/ajp/TestAbstractAjpProcessor.java Mon Mar 17 14:17:19 2014 @@ -18,6 +18,11 @@ package org.apache.coyote.ajp; import java.io.File; import java.io.IOException; +import java.io.InputStream; +import java.io.PrintWriter; +import java.util.ArrayList; +import java.util.Iterator; +import java.util.List; import javax.servlet.ServletException; import javax.servlet.http.HttpServlet; @@ -201,6 +206,78 @@ public class TestAbstractAjpProcessor ex } + @Test + public void testZeroLengthRequestBodyGetA() throws Exception { + doTestZeroLengthRequestBody(2, true); + } + + @Test + public void testZeroLengthRequestBodyGetB() throws Exception { + doTestZeroLengthRequestBody(2, false); + } + + @Test + public void testZeroLengthRequestBodyPostA() throws Exception { + doTestZeroLengthRequestBody(4, true); + } + + @Test + public void testZeroLengthRequestBodyPostB() throws Exception { + doTestZeroLengthRequestBody(4, false); + } + + private void doTestZeroLengthRequestBody(int method, boolean callAvailable) + throws Exception { + + Tomcat tomcat = getTomcatInstance(); + + // Must have a real docBase - just use temp + Context ctx = tomcat.addContext("", System.getProperty("java.io.tmpdir")); + ReadBodyServlet servlet = new ReadBodyServlet(callAvailable); + Tomcat.addServlet(ctx, "ReadBody", servlet); + ctx.addServletMapping("/", "ReadBody"); + + tomcat.start(); + + SimpleAjpClient ajpClient = new SimpleAjpClient(); + ajpClient.setPort(getPort()); + ajpClient.connect(); + + validateCpong(ajpClient.cping()); + + TesterAjpMessage forwardMessage = ajpClient.createForwardMessage("/", method); + forwardMessage.addHeader(0xA008, "0"); + forwardMessage.end(); + + TesterAjpMessage responseHeaders = + ajpClient.sendMessage(forwardMessage, null); + + // Expect 3 messages: headers, body, end + validateResponseHeaders(responseHeaders, 200); + validateResponseBody(ajpClient.readMessage(), + "Request Body length in bytes: 0"); + validateResponseEnd(ajpClient.readMessage(), true); + + // Double check the connection is still open + validateCpong(ajpClient.cping()); + + ajpClient.disconnect(); + + if (callAvailable) { + boolean success = true; + Iterator<Integer> itAvailable = servlet.availableList.iterator(); + Iterator<Integer> itRead = servlet.readList.iterator(); + while (success && itAvailable.hasNext()) { + success = ((itRead.next().intValue() > 0) == (itAvailable.next().intValue() > 0)); + } + if (!success) { + Assert.fail("available() and read() results do not match.\nAvailable: " + + servlet.availableList + "\nRead: " + servlet.readList); + } + } + } + + /** * Process response header packet and checks the status. Any other data is * ignored. @@ -305,4 +382,64 @@ public class TestAbstractAjpProcessor ex resp.getWriter().print("Body not permitted for 304 response"); } } + + + private static class ReadBodyServlet extends HttpServlet { + + private static final long serialVersionUID = 1L; + + private final boolean callAvailable; + final List<Integer> availableList; + final List<Integer> readList; + + public ReadBodyServlet(boolean callAvailable) { + this.callAvailable = callAvailable; + this.availableList = callAvailable ? new ArrayList<Integer>() : null; + this.readList = callAvailable ? new ArrayList<Integer>() : null; + } + + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) + throws ServletException, IOException { + doRequest(req, resp, false); + } + + @Override + protected void doPost(HttpServletRequest req, HttpServletResponse resp) + throws ServletException, IOException { + doRequest(req, resp, true); + } + + private void doRequest(HttpServletRequest request, HttpServletResponse response, + boolean isPost) throws IOException { + + long readCount = 0; + + try (InputStream s = request.getInputStream()) { + byte[] buf = new byte[4096]; + int read; + do { + if (callAvailable) { + int available = s.available(); + read = s.read(buf); + availableList.add(Integer.valueOf(available)); + readList.add(Integer.valueOf(read)); + } else { + read = s.read(buf); + } + if (read > 0) { + readCount += read; + } + } while (read > 0); + } + + response.setContentType("text/plain"); + response.setCharacterEncoding("UTF-8"); + + try (PrintWriter w = response.getWriter()) { + w.println("Method: " + (isPost ? "POST" : "GET") + ". Reading request body..."); + w.println("Request Body length in bytes: " + readCount); + } + } + } } Modified: tomcat/trunk/webapps/docs/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1578392&r1=1578391&r2=1578392&view=diff ============================================================================== --- tomcat/trunk/webapps/docs/changelog.xml (original) +++ tomcat/trunk/webapps/docs/changelog.xml Mon Mar 17 14:17:19 2014 @@ -154,6 +154,11 @@ and use a bit shift instead of a multiplication as it is marginally faster. (markt/kkolinko) </fix> + <fix> + Correct regression introduced in 8.0.0-RC2 as part of the Servlet 3.1 + non-blocking IO support that broke handling of requests with an explicit + content length of zero. (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