Author: markt
Date: Mon Dec 15 11:41:49 2014
New Revision: 1645626
URL: http://svn.apache.org/r1645626
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=57324
If it is known that the connection is going to be closed when committing the
response, send the connection: close header.
Modified:
tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java
tomcat/trunk/test/org/apache/catalina/startup/TomcatBaseTest.java
tomcat/trunk/test/org/apache/coyote/http11/TestAbstractHttp11Processor.java
Modified:
tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java?rev=1645626&r1=1645625&r2=1645626&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java
(original)
+++ tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java Mon
Dec 15 11:41:49 2014
@@ -1098,16 +1098,11 @@ public abstract class AbstractHttp11Proc
// input. This way uploading a 100GB file doesn't tie up
the
// thread if the servlet has rejected it.
getInputBuffer().setSwallowInput(false);
- } else if (expectation &&
- (response.getStatus() < 200 || response.getStatus() >
299)) {
- // Client sent Expect: 100-continue but received a
- // non-2xx final response. Disable keep-alive (if enabled)
- // to ensure that the connection is closed. Some clients
may
- // still send the body, some may send the next request.
- // No way to differentiate, so close the connection to
- // force the client to send the next request.
- getInputBuffer().setSwallowInput(false);
- keepAlive = 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();
}
endRequest();
}
@@ -1169,6 +1164,20 @@ public abstract class AbstractHttp11Proc
}
+ private void checkExpectationAndResponseStatus() {
+ if (expectation && (response.getStatus() < 200 || response.getStatus()
> 299)) {
+ // Client sent Expect: 100-continue but received a
+ // non-2xx final response. Disable keep-alive (if enabled)
+ // to ensure that the connection is closed. Some clients may
+ // still send the body, some may send the next request.
+ // No way to differentiate, so close the connection to
+ // force the client to send the next request.
+ getInputBuffer().setSwallowInput(false);
+ keepAlive = false;
+ }
+ }
+
+
/**
* After reading the request headers, we have to setup the request filters.
*/
@@ -1493,6 +1502,10 @@ public abstract class AbstractHttp11Proc
keepAlive = false;
}
+ // This may disabled keep-alive to check before working out the
+ // Connection header.
+ checkExpectationAndResponseStatus();
+
// If we know that the request is bad this early, add the
// Connection: close header.
keepAlive = keepAlive && !statusDropsConnection(statusCode);
Modified: tomcat/trunk/test/org/apache/catalina/startup/TomcatBaseTest.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/startup/TomcatBaseTest.java?rev=1645626&r1=1645625&r2=1645626&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/startup/TomcatBaseTest.java (original)
+++ tomcat/trunk/test/org/apache/catalina/startup/TomcatBaseTest.java Mon Dec
15 11:41:49 2014
@@ -565,6 +565,37 @@ public abstract class TomcatBaseTest ext
}
+ /**
+ * Servlet that simply echos the request body back as the response body.
+ */
+ public static class EchoBodyServlet extends HttpServlet {
+
+ private static final long serialVersionUID = 1L;
+
+ @Override
+ protected void doGet(HttpServletRequest req, HttpServletResponse resp)
+ throws ServletException, IOException {
+ // NO-OP - No body to echo
+ }
+
+ @Override
+ protected void doPost(HttpServletRequest req, HttpServletResponse resp)
+ throws ServletException, IOException {
+ // Beware of clients that try to send the whole request body before
+ // reading any of the response. They may cause this test to lock
up.
+ byte[] buffer = new byte[8096];
+ int read = 0;
+ try (InputStream is = req.getInputStream();
+ OutputStream os = resp.getOutputStream()) {
+ while (read > -1) {
+ os.write(buffer, 0, read);
+ read = is.read(buffer);
+ }
+ }
+ }
+ }
+
+
/*
* Wrapper for getting the response.
*/
Modified:
tomcat/trunk/test/org/apache/coyote/http11/TestAbstractHttp11Processor.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/coyote/http11/TestAbstractHttp11Processor.java?rev=1645626&r1=1645625&r2=1645626&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/coyote/http11/TestAbstractHttp11Processor.java
(original)
+++ tomcat/trunk/test/org/apache/coyote/http11/TestAbstractHttp11Processor.java
Mon Dec 15 11:41:49 2014
@@ -23,8 +23,10 @@ import java.io.Writer;
import java.net.Socket;
import java.nio.CharBuffer;
import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
+import java.util.Locale;
import java.util.Map;
import java.util.concurrent.CountDownLatch;
@@ -49,6 +51,8 @@ import org.apache.catalina.startup.Tomca
import org.apache.catalina.startup.TomcatBaseTest;
import org.apache.tomcat.util.buf.B2CConverter;
import org.apache.tomcat.util.buf.ByteChunk;
+import org.apache.tomcat.util.descriptor.web.SecurityCollection;
+import org.apache.tomcat.util.descriptor.web.SecurityConstraint;
public class TestAbstractHttp11Processor extends TomcatBaseTest {
@@ -536,6 +540,59 @@ public class TestAbstractHttp11Processor
}
}
+
+ // https://issues.apache.org/bugzilla/show_bug.cgi?id=57324
+ @Test
+ public void testNon2xxResponseWithExpectation() throws Exception {
+ doTestNon2xxResponseAndExpectation(true);
+ }
+
+ @Test
+ public void testNon2xxResponseWithoutExpectation() throws Exception {
+ doTestNon2xxResponseAndExpectation(false);
+ }
+
+ private void doTestNon2xxResponseAndExpectation(boolean useExpectation)
throws Exception {
+ Tomcat tomcat = getTomcatInstance();
+
+ // No file system docBase required
+ Context ctx = tomcat.addContext("", null);
+
+ Tomcat.addServlet(ctx, "echo", new EchoBodyServlet());
+ ctx.addServletMapping("/echo", "echo");
+
+ SecurityCollection collection = new SecurityCollection("All", "");
+ collection.addPattern("/*");
+ SecurityConstraint constraint = new SecurityConstraint();
+ constraint.addAuthRole("Any");
+ constraint.addCollection(collection);
+ ctx.addConstraint(constraint);
+
+ tomcat.start();
+
+ byte[] requestBody = "HelloWorld".getBytes(StandardCharsets.UTF_8);
+ Map<String,List<String>> reqHeaders = null;
+ if (useExpectation) {
+ reqHeaders = new HashMap<>();
+ List<String> expectation = new ArrayList<>();
+ expectation.add("100-continue");
+ reqHeaders.put("Expect", expectation);
+ }
+ ByteChunk responseBody = new ByteChunk();
+ Map<String,List<String>> responseHeaders = new HashMap<>();
+ int rc = postUrl(requestBody, "http://localhost:"; + getPort() +
"/echo",
+ responseBody, reqHeaders, responseHeaders);
+
+ Assert.assertEquals(HttpServletResponse.SC_FORBIDDEN, rc);
+ List<String> connectionHeaders = responseHeaders.get("Connection");
+ if (useExpectation) {
+ Assert.assertEquals(1, connectionHeaders.size());
+ Assert.assertEquals("close",
connectionHeaders.get(0).toLowerCase(Locale.ENGLISH));
+ } else {
+ Assert.assertNull(connectionHeaders);
+ }
+ }
+
private static class Bug55772Servlet extends HttpServlet {
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
