You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2014/03/17 15:17:19 UTC
svn commit: r1578392 - in /tomcat/trunk:
java/org/apache/coyote/ajp/AbstractAjpProcessor.java
test/org/apache/coyote/ajp/TestAbstractAjpProcessor.java
webapps/docs/changelog.xml
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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org