Author: markt
Date: Mon Dec 15 11:54:24 2014
New Revision: 1645627
URL: http://svn.apache.org/r1645627
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/tc8.0.x/trunk/ (props changed)
tomcat/tc8.0.x/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java
tomcat/tc8.0.x/trunk/test/org/apache/catalina/startup/TomcatBaseTest.java
tomcat/tc8.0.x/trunk/test/org/apache/coyote/http11/TestAbstractHttp11Processor.java
tomcat/tc8.0.x/trunk/webapps/docs/changelog.xml
Propchange: tomcat/tc8.0.x/trunk/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Mon Dec 15 11:54:24 2014
@@ -1 +1 @@
-/tomcat/trunk:1636524,1637156,1637176,1637188,1637331,1637684,1637695,1638720-1638725,1639653,1640010,1640083-1640084,1640088,1640275,1640322,1640347,1640361,1640365,1640403,1640410,1640652,1640655-1640658,1640688,1640700-1640883,1640903,1640976,1640978,1641000,1641026,1641038-1641039,1641051-1641052,1641058,1641064,1641300,1641369,1641374,1641380,1641486,1641634,1641656-1641692,1641704,1641707-1641718,1641720-1641722,1641735,1641981,1642233,1642280,1642554,1642564,1642595,1642606,1642668,1642679,1642697,1642699,1642766,1643002,1643045,1643054-1643055,1643066,1643121,1643128,1643206,1643209-1643210,1643216,1643249,1643270,1643283,1643309-1643310,1643323,1643365-1643366,1643370-1643371,1643465,1643474,1643536,1643570,1643634,1643649,1643651,1643654,1643675,1643731,1643733-1643734,1643761,1643766,1643814,1643937,1643963,1644017,1644169,1644201-1644203,1644321,1644323,1644516,1644523,1644529,1644535,1644730,1644768,1644784-1644785,1644790,1644793,1644815,1644884,1644886,1644890,1644892
,1644910,1644924,1644929-1644930,1644935,1644989,1645011,1645247,1645355,1645357,1645455,1645465,1645469,1645471,1645473,1645475,1645487
+/tomcat/trunk:1636524,1637156,1637176,1637188,1637331,1637684,1637695,1638720-1638725,1639653,1640010,1640083-1640084,1640088,1640275,1640322,1640347,1640361,1640365,1640403,1640410,1640652,1640655-1640658,1640688,1640700-1640883,1640903,1640976,1640978,1641000,1641026,1641038-1641039,1641051-1641052,1641058,1641064,1641300,1641369,1641374,1641380,1641486,1641634,1641656-1641692,1641704,1641707-1641718,1641720-1641722,1641735,1641981,1642233,1642280,1642554,1642564,1642595,1642606,1642668,1642679,1642697,1642699,1642766,1643002,1643045,1643054-1643055,1643066,1643121,1643128,1643206,1643209-1643210,1643216,1643249,1643270,1643283,1643309-1643310,1643323,1643365-1643366,1643370-1643371,1643465,1643474,1643536,1643570,1643634,1643649,1643651,1643654,1643675,1643731,1643733-1643734,1643761,1643766,1643814,1643937,1643963,1644017,1644169,1644201-1644203,1644321,1644323,1644516,1644523,1644529,1644535,1644730,1644768,1644784-1644785,1644790,1644793,1644815,1644884,1644886,1644890,1644892
,1644910,1644924,1644929-1644930,1644935,1644989,1645011,1645247,1645355,1645357,1645455,1645465,1645469,1645471,1645473,1645475,1645487,1645626
Modified:
tomcat/tc8.0.x/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java
URL:
http://svn.apache.org/viewvc/tomcat/tc8.0.x/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java?rev=1645627&r1=1645626&r2=1645627&view=diff
==============================================================================
---
tomcat/tc8.0.x/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java
(original)
+++
tomcat/tc8.0.x/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java
Mon Dec 15 11:54:24 2014
@@ -1129,16 +1129,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();
}
@@ -1200,6 +1195,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.
*/
@@ -1524,6 +1533,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/tc8.0.x/trunk/test/org/apache/catalina/startup/TomcatBaseTest.java
URL:
http://svn.apache.org/viewvc/tomcat/tc8.0.x/trunk/test/org/apache/catalina/startup/TomcatBaseTest.java?rev=1645627&r1=1645626&r2=1645627&view=diff
==============================================================================
--- tomcat/tc8.0.x/trunk/test/org/apache/catalina/startup/TomcatBaseTest.java
(original)
+++ tomcat/tc8.0.x/trunk/test/org/apache/catalina/startup/TomcatBaseTest.java
Mon Dec 15 11:54:24 2014
@@ -250,6 +250,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.
*/
@@ -407,6 +438,10 @@ public abstract class TomcatBaseTest ext
os.write(next);
os.flush();
}
+ } catch (IOException ioe) {
+ // Failed to write the request body. Server may have closed the
+ // connection.
+ ioe.printStackTrace();
}
int rc = connection.getResponseCode();
Modified:
tomcat/tc8.0.x/trunk/test/org/apache/coyote/http11/TestAbstractHttp11Processor.java
URL:
http://svn.apache.org/viewvc/tomcat/tc8.0.x/trunk/test/org/apache/coyote/http11/TestAbstractHttp11Processor.java?rev=1645627&r1=1645626&r2=1645627&view=diff
==============================================================================
---
tomcat/tc8.0.x/trunk/test/org/apache/coyote/http11/TestAbstractHttp11Processor.java
(original)
+++
tomcat/tc8.0.x/trunk/test/org/apache/coyote/http11/TestAbstractHttp11Processor.java
Mon Dec 15 11:54:24 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 {
Modified: tomcat/tc8.0.x/trunk/webapps/docs/changelog.xml
URL:
http://svn.apache.org/viewvc/tomcat/tc8.0.x/trunk/webapps/docs/changelog.xml?rev=1645627&r1=1645626&r2=1645627&view=diff
==============================================================================
--- tomcat/tc8.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc8.0.x/trunk/webapps/docs/changelog.xml Mon Dec 15 11:54:24 2014
@@ -195,6 +195,13 @@
and the NIO connector. (markt)
</fix>
<fix>
+ <bug>57324</bug>: If the client uses <code>Expect: 100-continue</code>
+ and Tomcat responds with a non-2xx response code, Tomcat also closes
the
+ connection. If Tomcat knows the connection is going to be closed when
+ committing the response, Tomcat will now also send the
+ <code>Connection: close</code> response header. (markt)
+ </fix>
+ <fix>
<bug>57347</bug>: AJP response contains wrong status reason phrase
(rjung)
</fix>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
