Author: markt
Date: Tue Mar 3 09:14:17 2015
New Revision: 1663562
URL: http://svn.apache.org/r1663562
Log:
Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=57621
When an async request completes, need to ensure that any unread input is
swallowed.
Modified:
tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java
tomcat/trunk/java/org/apache/coyote/ActionCode.java
tomcat/trunk/java/org/apache/coyote/ajp/AjpProcessor.java
tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java
tomcat/trunk/test/org/apache/catalina/startup/SimpleHttpClient.java
tomcat/trunk/test/org/apache/coyote/http11/TestHttp11Processor.java
Modified: tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java?rev=1663562&r1=1663561&r2=1663562&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java (original)
+++ tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java Tue Mar 3
09:14:17 2015
@@ -107,11 +107,15 @@ public class AsyncContextImpl implements
context.unbind(Globals.IS_SECURITY_ENABLED, oldCL);
}
- // The application doesn't know it has to stop writing until it
receives
- // the complete event so the response has to be closed after firing the
- // event.
+ // The application doesn't know it has to stop read and/or writing
until
+ // it receives the complete event so the request and response have to
be
+ // closed after firing the event.
try {
+ // First of all ensure that any data written to the response is
+ // written to the I/O layer.
request.getResponse().finishResponse();
+ // Close the request and the response.
+ request.getCoyoteRequest().action(ActionCode.END_REQUEST, null);
} catch (Throwable t) {
ExceptionUtils.handleThrowable(t);
// Catch this here and allow async context complete to continue
Modified: tomcat/trunk/java/org/apache/coyote/ActionCode.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/ActionCode.java?rev=1663562&r1=1663561&r2=1663562&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/ActionCode.java (original)
+++ tomcat/trunk/java/org/apache/coyote/ActionCode.java Tue Mar 3 09:14:17 2015
@@ -228,5 +228,11 @@ public enum ActionCode {
* when the non-blocking listeners are configured on a thread where the
* processing wasn't triggered by a read or write event on the socket.
*/
- DISPATCH_EXECUTE
+ DISPATCH_EXECUTE,
+
+ /**
+ * Trigger end of request processing (remaining input swallowed, write any
+ * remaining parts of the response etc.).
+ */
+ END_REQUEST
}
Modified: tomcat/trunk/java/org/apache/coyote/ajp/AjpProcessor.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/ajp/AjpProcessor.java?rev=1663562&r1=1663561&r2=1663562&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/ajp/AjpProcessor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/ajp/AjpProcessor.java Tue Mar 3
09:14:17 2015
@@ -600,6 +600,9 @@ public class AjpProcessor extends Abstra
setErrorState(ErrorState.CLOSE_NOW, null);
break;
}
+ case END_REQUEST: {
+ // NO-OP for AJP
+ }
}
}
Modified: tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java?rev=1663562&r1=1663561&r2=1663562&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java Tue Mar 3
09:14:17 2015
@@ -974,6 +974,9 @@ public class Http11Processor extends Abs
}
break;
}
+ case END_REQUEST: {
+ endRequest();
+ }
}
}
@@ -1128,22 +1131,12 @@ public class Http11Processor extends Abs
// Finish the handling of the request
rp.setStage(org.apache.coyote.Constants.STAGE_ENDINPUT);
-
if (!isAsync()) {
- if (getErrorState().isError()) {
- // If we know we are closing the connection, don't drain
- // input. This way uploading a 100GB file doesn't tie up
the
- // thread if the servlet has rejected it.
- inputBuffer.setSwallowInput(false);
- } else {
- // Need to check this again here in case the response was
- // committed before the error that requires the connection
- // to be closed occurred.
- checkExpectationAndResponseStatus();
- }
+ // If this is an async request then the request ends when it
has
+ // been completed. The AsyncContext is responsible for calling
+ // endRequest() in that case.
endRequest();
}
-
rp.setStage(org.apache.coyote.Constants.STAGE_ENDOUTPUT);
// If there was an error, make sure the request is counted as
@@ -1807,7 +1800,23 @@ public class Http11Processor extends Abs
}
+ /*
+ * No more input will be passed to the application. Remaining input will be
+ * swallowed or the connection dropped depending on the error and
+ * expectation status.
+ */
private void endRequest() {
+ if (getErrorState().isError()) {
+ // If we know we are closing the connection, don't drain
+ // input. This way uploading a 100GB file doesn't tie up the
+ // thread if the servlet has rejected it.
+ inputBuffer.setSwallowInput(false);
+ } else {
+ // Need to check this again here in case the response was
+ // committed before the error that requires the connection
+ // to be closed occurred.
+ checkExpectationAndResponseStatus();
+ }
// Finish the handling of the request
if (getErrorState().isIoAllowed()) {
Modified: tomcat/trunk/test/org/apache/catalina/startup/SimpleHttpClient.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/startup/SimpleHttpClient.java?rev=1663562&r1=1663561&r2=1663562&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/startup/SimpleHttpClient.java
(original)
+++ tomcat/trunk/test/org/apache/catalina/startup/SimpleHttpClient.java Tue Mar
3 09:14:17 2015
@@ -385,6 +385,10 @@ public abstract class SimpleHttpClient {
useContinue = false;
+ resetResponse();
+ }
+
+ public void resetResponse() {
responseLine = null;
responseHeaders = new ArrayList<>();
responseBody = null;
Modified: tomcat/trunk/test/org/apache/coyote/http11/TestHttp11Processor.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/coyote/http11/TestHttp11Processor.java?rev=1663562&r1=1663561&r2=1663562&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/coyote/http11/TestHttp11Processor.java
(original)
+++ tomcat/trunk/test/org/apache/coyote/http11/TestHttp11Processor.java Tue Mar
3 09:14:17 2015
@@ -45,6 +45,7 @@ import org.junit.Assert;
import org.junit.Test;
import org.apache.catalina.Context;
+import org.apache.catalina.Wrapper;
import org.apache.catalina.startup.SimpleHttpClient;
import org.apache.catalina.startup.TesterServlet;
import org.apache.catalina.startup.Tomcat;
@@ -701,4 +702,103 @@ public class TestHttp11Processor extends
return getResponseBody().contains("test - data");
}
}
+
+
+ /*
+ * Partially read chunked input is not swallowed when it is read during
+ * async processing.
+ */
+ @Test
+ public void testBug57621() throws Exception {
+
+ Tomcat tomcat = getTomcatInstance();
+ Context root = tomcat.addContext("", null);
+ Wrapper w = Tomcat.addServlet(root, "Bug57621", new Bug57621Servlet());
+ w.setAsyncSupported(true);
+ root.addServletMapping("/test", "Bug57621");
+
+ tomcat.start();
+
+ Bug57621Client client = new Bug57621Client();
+ client.setPort(tomcat.getConnector().getLocalPort());
+
+ client.setUseContentLength(true);
+
+ client.connect();
+
+ client.doRequest();
+ assertTrue(client.getResponseLine(), client.isResponse200());
+ assertTrue(client.isResponseBodyOK());
+
+ // Do the request again to ensure that the remaining body was swallowed
+ client.resetResponse();
+ client.processRequest();
+ assertTrue(client.isResponse200());
+ assertTrue(client.isResponseBodyOK());
+
+ client.disconnect();
+ }
+
+
+ private static class Bug57621Servlet extends HttpServlet {
+
+ private static final long serialVersionUID = 1L;
+
+ @Override
+ protected void doPut(HttpServletRequest req, HttpServletResponse resp)
+ throws ServletException, IOException {
+ AsyncContext ac = req.startAsync();
+ ac.start(new Runnable() {
+ @Override
+ public void run() {
+ resp.setContentType("text/plain");
+ resp.setCharacterEncoding("UTF-8");
+ try {
+ resp.getWriter().print("OK");
+ } catch (IOException e) {
+ // Should never happen. Test will fail if it does.
+ }
+ ac.complete();
+ }
+ });
+ }
+ }
+
+
+ private class Bug57621Client extends SimpleHttpClient {
+
+ private Exception doRequest() {
+ try {
+ String[] request = new String[2];
+ request[0] =
+ "PUT http://localhost:8080/test HTTP/1.1" + CRLF +
+ "Transfer-encoding: chunked" + CRLF +
+ CRLF +
+ "2" + CRLF +
+ "OK";
+
+ request[1] =
+ CRLF +
+ "0" + CRLF +
+ CRLF;
+
+ setRequest(request);
+ processRequest(); // blocks until response has been read
+ } catch (Exception e) {
+ return e;
+ }
+ return null;
+ }
+
+ @Override
+ public boolean isResponseBodyOK() {
+ if (getResponseBody() == null) {
+ return false;
+ }
+ if (!getResponseBody().contains("OK")) {
+ return false;
+ }
+ return true;
+ }
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
