This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/8.5.x by this push:
new 32db2a6 Manually merge #332 - BZ 57661 - Vary when 100 response code
is sent
32db2a6 is described below
commit 32db2a6fa62a15a77682f6a193528f061e98235e
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Sat Sep 5 16:57:42 2020 +0100
Manually merge #332 - BZ 57661 - Vary when 100 response code is sent
Allows the user to control when the 100 response is sent when the
request contains an expectation. Options are immediately (no change) or
on first read of the request body.
---
.../catalina/authenticator/FormAuthenticator.java | 5 +-
java/org/apache/catalina/connector/Response.java | 24 ++-
.../apache/catalina/core/StandardContextValve.java | 3 +-
java/org/apache/coyote/AbstractProcessor.java | 14 +-
java/org/apache/coyote/ContinueResponseTiming.java | 88 +++++++++++
java/org/apache/coyote/LocalStrings.properties | 2 +
java/org/apache/coyote/Request.java | 4 +
java/org/apache/coyote/ajp/AjpProcessor.java | 3 +-
.../coyote/http11/AbstractHttp11Protocol.java | 13 ++
.../apache/coyote/http11/Http11OutputBuffer.java | 10 +-
java/org/apache/coyote/http11/Http11Processor.java | 30 ++--
java/org/apache/coyote/http2/Http2Protocol.java | 6 +
java/org/apache/coyote/http2/StreamProcessor.java | 18 ++-
.../catalina/core/TestStandardContextValve.java | 77 ++++++++++
.../apache/catalina/startup/ExpectationClient.java | 51 +++++++
test/org/apache/coyote/TestRequest.java | 168 +++++++++++++++++++++
.../coyote/http11/TestHttp11OutputBuffer.java | 33 +---
.../apache/coyote/http2/TestHttp2Section_8_1.java | 41 ++++-
webapps/docs/changelog.xml | 7 +
webapps/docs/config/http.xml | 15 ++
20 files changed, 554 insertions(+), 58 deletions(-)
diff --git a/java/org/apache/catalina/authenticator/FormAuthenticator.java
b/java/org/apache/catalina/authenticator/FormAuthenticator.java
index 1d4257d..1dbe658 100644
--- a/java/org/apache/catalina/authenticator/FormAuthenticator.java
+++ b/java/org/apache/catalina/authenticator/FormAuthenticator.java
@@ -33,6 +33,7 @@ import org.apache.catalina.Session;
import org.apache.catalina.connector.Request;
import org.apache.catalina.connector.Response;
import org.apache.coyote.ActionCode;
+import org.apache.coyote.ContinueResponseTiming;
import org.apache.juli.logging.Log;
import org.apache.juli.logging.LogFactory;
import org.apache.tomcat.util.ExceptionUtils;
@@ -230,7 +231,7 @@ public class FormAuthenticator
// Yes -- Acknowledge the request, validate the specified credentials
// and redirect to the error page if they are not correct
- request.getResponse().sendAcknowledgement();
+
request.getResponse().sendAcknowledgement(ContinueResponseTiming.ALWAYS);
Realm realm = context.getRealm();
if (characterEncoding != null) {
request.setCharacterEncoding(characterEncoding);
@@ -670,7 +671,7 @@ public class FormAuthenticator
}
// May need to acknowledge a 100-continue expectation
- request.getResponse().sendAcknowledgement();
+
request.getResponse().sendAcknowledgement(ContinueResponseTiming.ALWAYS);
int maxSavePostSize = request.getConnector().getMaxSavePostSize();
if (maxSavePostSize != 0) {
diff --git a/java/org/apache/catalina/connector/Response.java
b/java/org/apache/catalina/connector/Response.java
index 0140895..4cac2a9 100644
--- a/java/org/apache/catalina/connector/Response.java
+++ b/java/org/apache/catalina/connector/Response.java
@@ -47,6 +47,7 @@ import org.apache.catalina.Session;
import org.apache.catalina.security.SecurityUtil;
import org.apache.catalina.util.SessionConfig;
import org.apache.coyote.ActionCode;
+import org.apache.coyote.ContinueResponseTiming;
import org.apache.juli.logging.Log;
import org.apache.juli.logging.LogFactory;
import org.apache.tomcat.util.buf.CharChunk;
@@ -1205,9 +1206,26 @@ public class Response implements HttpServletResponse {
* Send an acknowledgement of a request.
*
* @exception IOException if an input/output error occurs
+ *
+ * @deprecated Unused. Will be removed in Tomcat 10.
+ * Use {@link #sendAcknowledgement(ContinueResponseTiming)}.
*/
- public void sendAcknowledgement()
- throws IOException {
+ @Deprecated
+ public void sendAcknowledgement() throws IOException {
+ sendAcknowledgement(ContinueResponseTiming.ALWAYS);
+ }
+
+
+ /**
+ * Send an acknowledgement of a request.
+ *
+ * @param continueResponseTiming Indicates when the request for the ACK
+ * originated so it can be compared with the
+ * configured timing for ACK responses.
+ *
+ * @exception IOException if an input/output error occurs
+ */
+ public void sendAcknowledgement(ContinueResponseTiming
continueResponseTiming) throws IOException {
if (isCommitted()) {
return;
@@ -1218,7 +1236,7 @@ public class Response implements HttpServletResponse {
return;
}
- getCoyoteResponse().action(ActionCode.ACK, null);
+ getCoyoteResponse().action(ActionCode.ACK, continueResponseTiming);
}
diff --git a/java/org/apache/catalina/core/StandardContextValve.java
b/java/org/apache/catalina/core/StandardContextValve.java
index 020a2f7..01d0b03 100644
--- a/java/org/apache/catalina/core/StandardContextValve.java
+++ b/java/org/apache/catalina/core/StandardContextValve.java
@@ -26,6 +26,7 @@ import org.apache.catalina.Wrapper;
import org.apache.catalina.connector.Request;
import org.apache.catalina.connector.Response;
import org.apache.catalina.valves.ValveBase;
+import org.apache.coyote.ContinueResponseTiming;
import org.apache.tomcat.util.buf.MessageBytes;
import org.apache.tomcat.util.res.StringManager;
@@ -81,7 +82,7 @@ final class StandardContextValve extends ValveBase {
// Acknowledge the request
try {
- response.sendAcknowledgement();
+ response.sendAcknowledgement(ContinueResponseTiming.IMMEDIATELY);
} catch (IOException ioe) {
container.getLogger().error(sm.getString(
"standardContextValve.acknowledgeException"), ioe);
diff --git a/java/org/apache/coyote/AbstractProcessor.java
b/java/org/apache/coyote/AbstractProcessor.java
index abd73da..b06a8da 100644
--- a/java/org/apache/coyote/AbstractProcessor.java
+++ b/java/org/apache/coyote/AbstractProcessor.java
@@ -395,7 +395,7 @@ public abstract class AbstractProcessor extends
AbstractProcessorLight implement
break;
}
case ACK: {
- ack();
+ ack((ContinueResponseTiming) param);
break;
}
case CLIENT_FLUSH: {
@@ -715,7 +715,17 @@ public abstract class AbstractProcessor extends
AbstractProcessorLight implement
protected abstract void finishResponse() throws IOException;
- protected abstract void ack();
+ /**
+ * @deprecated Unused. This will be removed in Tomcat 10 onwards. Use
+ * @{link {@link #ack(ContinueResponseTiming)}.
+ */
+ @Deprecated
+ protected void ack() {
+ ack(ContinueResponseTiming.ALWAYS);
+ }
+
+
+ protected abstract void ack(ContinueResponseTiming continueResponseTiming);
protected abstract void flush() throws IOException;
diff --git a/java/org/apache/coyote/ContinueResponseTiming.java
b/java/org/apache/coyote/ContinueResponseTiming.java
new file mode 100644
index 0000000..b3672fb
--- /dev/null
+++ b/java/org/apache/coyote/ContinueResponseTiming.java
@@ -0,0 +1,88 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.coyote;
+
+import org.apache.tomcat.util.res.StringManager;
+
+/**
+ * Defines timing options for responding to requests that contain a
+ * '100-continue' expectations.
+ *
+ *
+ */
+public enum ContinueResponseTiming {
+
+ /**
+ * Tomcat will automatically send the 100 intermediate response before
+ * sending the request to the servlet.
+ */
+ IMMEDIATELY("immediately"),
+
+ /**
+ * Send the 100 intermediate response only when the servlet attempts to
+ * read the request's body by either:
+ * <ul>
+ * <li>calling read on the InputStream returned by
+ * HttpServletRequest.getInputStream</li>
+ * <li>calling read on the BufferedReader returned by
+ * HttpServletRequest.getReader</li>
+ * </ul>
+ * This allows the servlet to process the request headers and possibly
+ * respond before reading the request body.
+ */
+ ON_REQUEST_BODY_READ("onRead"),
+
+
+ /**
+ * Internal use only. Used to indicate that the 100 intermediate response
+ * should be sent if possible regardless of the current configuration.
+ */
+ ALWAYS("always");
+
+
+ private static final StringManager sm =
StringManager.getManager(ContinueResponseTiming.class);
+
+ public static ContinueResponseTiming fromString(String value) {
+ /*
+ * Do this for two reasons:
+ * - Not all of the Enum values are intended to be used in
configuration
+ * - the naming convention for Enum constants and configuration values
+ * - is not consistent
+ */
+ if (IMMEDIATELY.toString().equalsIgnoreCase(value)) {
+ return IMMEDIATELY;
+ } else if (ON_REQUEST_BODY_READ.toString().equalsIgnoreCase(value)) {
+ return ContinueResponseTiming.ON_REQUEST_BODY_READ;
+ } else {
+ throw new
IllegalArgumentException(sm.getString("continueResponseTiming.invalid", value));
+ }
+ }
+
+
+ private final String configValue;
+
+
+ private ContinueResponseTiming(String configValue) {
+ this.configValue = configValue;
+ }
+
+
+ @Override
+ public String toString() {
+ return configValue;
+ }
+}
diff --git a/java/org/apache/coyote/LocalStrings.properties
b/java/org/apache/coyote/LocalStrings.properties
index c5e97c0..1aeb314 100644
--- a/java/org/apache/coyote/LocalStrings.properties
+++ b/java/org/apache/coyote/LocalStrings.properties
@@ -56,6 +56,8 @@ asyncStateMachine.invalidAsyncState=Calling [{0}] is not
valid for a request wit
compressionConfig.ContentEncodingParseFail=Failed to parse Content-Encoding
header when checking to see if compression was already in use
+continueResponseTiming.invalid=The value [{0}] is not a valid configuration
option for continueResponseTiming
+
request.notAsync=It is only valid to switch to non-blocking IO within async
processing or HTTP upgrade processing
request.nullReadListener=The listener passed to setReadListener() may not be
null
request.readListenerSet=The non-blocking read listener has already been set
diff --git a/java/org/apache/coyote/Request.java
b/java/org/apache/coyote/Request.java
index 1ebba0e..e932694 100644
--- a/java/org/apache/coyote/Request.java
+++ b/java/org/apache/coyote/Request.java
@@ -578,6 +578,10 @@ public final class Request {
* @throws IOException If an I/O error occurs during the copy
*/
public int doRead(ApplicationBufferHandler handler) throws IOException {
+ if (getBytesRead() == 0 && !response.isCommitted()) {
+ action(ActionCode.ACK,
ContinueResponseTiming.ON_REQUEST_BODY_READ);
+ }
+
int n = inputBuffer.doRead(handler);
if (n > 0) {
bytesRead+=n;
diff --git a/java/org/apache/coyote/ajp/AjpProcessor.java
b/java/org/apache/coyote/ajp/AjpProcessor.java
index 4a297f5..74bf27e 100644
--- a/java/org/apache/coyote/ajp/AjpProcessor.java
+++ b/java/org/apache/coyote/ajp/AjpProcessor.java
@@ -34,6 +34,7 @@ import javax.servlet.http.HttpServletResponse;
import org.apache.coyote.AbstractProcessor;
import org.apache.coyote.ActionCode;
+import org.apache.coyote.ContinueResponseTiming;
import org.apache.coyote.ErrorState;
import org.apache.coyote.InputBuffer;
import org.apache.coyote.OutputBuffer;
@@ -1166,7 +1167,7 @@ public class AjpProcessor extends AbstractProcessor {
@Override
- protected final void ack() {
+ protected final void ack(ContinueResponseTiming continueResponseTiming) {
// NO-OP for AJP
}
diff --git a/java/org/apache/coyote/http11/AbstractHttp11Protocol.java
b/java/org/apache/coyote/http11/AbstractHttp11Protocol.java
index 9417ea9..d28e4fc 100644
--- a/java/org/apache/coyote/http11/AbstractHttp11Protocol.java
+++ b/java/org/apache/coyote/http11/AbstractHttp11Protocol.java
@@ -30,6 +30,7 @@ import javax.servlet.http.HttpUpgradeHandler;
import org.apache.coyote.AbstractProtocol;
import org.apache.coyote.CompressionConfig;
+import org.apache.coyote.ContinueResponseTiming;
import org.apache.coyote.Processor;
import org.apache.coyote.Request;
import org.apache.coyote.Response;
@@ -95,6 +96,18 @@ public abstract class AbstractHttp11Protocol<S> extends
AbstractProtocol<S> {
// ------------------------------------------------ HTTP specific
properties
// ------------------------------------------ managed in the
ProtocolHandler
+ private ContinueResponseTiming continueResponseTiming =
ContinueResponseTiming.IMMEDIATELY;
+ public String getContinueResponseTiming() {
+ return continueResponseTiming.toString();
+ }
+ public void setContinueResponseTiming(String continueResponseTiming) {
+ this.continueResponseTiming =
ContinueResponseTiming.fromString(continueResponseTiming);
+ }
+ public ContinueResponseTiming getContinueResponseTimingInternal() {
+ return continueResponseTiming;
+ }
+
+
private boolean useKeepAliveResponseHeader = true;
public boolean getUseKeepAliveResponseHeader() {
return useKeepAliveResponseHeader;
diff --git a/java/org/apache/coyote/http11/Http11OutputBuffer.java
b/java/org/apache/coyote/http11/Http11OutputBuffer.java
index 3e6c20f..287e226 100644
--- a/java/org/apache/coyote/http11/Http11OutputBuffer.java
+++ b/java/org/apache/coyote/http11/Http11OutputBuffer.java
@@ -51,6 +51,9 @@ public class Http11OutputBuffer implements HttpOutputBuffer {
protected Response response;
+ private volatile boolean ackSent = false;
+
+
/**
* Finished flag.
*/
@@ -307,6 +310,7 @@ public class Http11OutputBuffer implements HttpOutputBuffer
{
// Reset pointers
headerBuffer.position(0).limit(headerBuffer.capacity());
lastActiveFilter = -1;
+ ackSent = false;
responseFinished = false;
byteCount = 0;
}
@@ -319,7 +323,11 @@ public class Http11OutputBuffer implements
HttpOutputBuffer {
@SuppressWarnings("deprecation")
public void sendAck() throws IOException {
- if (!response.isCommitted()) {
+ // It possible that the protocol configuration is changed between the
+ // request being received and the first read of the body. That could
led
+ // to multiple calls to this method so ensure the ACK is only sent
once.
+ if (!response.isCommitted() && !ackSent) {
+ ackSent = true;
if (sendReasonPhrase) {
socketWrapper.write(isBlocking(), Constants.ACK_BYTES_REASON,
0, Constants.ACK_BYTES_REASON.length);
} else {
diff --git a/java/org/apache/coyote/http11/Http11Processor.java
b/java/org/apache/coyote/http11/Http11Processor.java
index 0ba6597..ce346da 100644
--- a/java/org/apache/coyote/http11/Http11Processor.java
+++ b/java/org/apache/coyote/http11/Http11Processor.java
@@ -29,6 +29,7 @@ import javax.servlet.http.HttpServletResponse;
import org.apache.coyote.AbstractProcessor;
import org.apache.coyote.ActionCode;
+import org.apache.coyote.ContinueResponseTiming;
import org.apache.coyote.ErrorState;
import org.apache.coyote.Request;
import org.apache.coyote.RequestInfo;
@@ -1397,15 +1398,26 @@ public class Http11Processor extends AbstractProcessor {
@Override
protected final void ack() {
- // Acknowledge request
- // Send a 100 status back if it makes sense (response not committed
- // yet, and client specified an expectation for 100-continue)
- if (!response.isCommitted() && request.hasExpectation()) {
- inputBuffer.setSwallowInput(true);
- try {
- outputBuffer.sendAck();
- } catch (IOException e) {
- setErrorState(ErrorState.CLOSE_CONNECTION_NOW, e);
+ ack(ContinueResponseTiming.ALWAYS);
+ }
+
+
+ @Override
+ protected final void ack(ContinueResponseTiming continueResponseTiming) {
+ // Only try and send the ACK for ALWAYS or if the timing of the request
+ // to send the ACK matches the current configuration.
+ if (continueResponseTiming == ContinueResponseTiming.ALWAYS ||
+ continueResponseTiming ==
protocol.getContinueResponseTimingInternal()) {
+ // Acknowledge request
+ // Send a 100 status back if it makes sense (response not committed
+ // yet, and client specified an expectation for 100-continue)
+ if (!response.isCommitted() && request.hasExpectation()) {
+ inputBuffer.setSwallowInput(true);
+ try {
+ outputBuffer.sendAck();
+ } catch (IOException e) {
+ setErrorState(ErrorState.CLOSE_CONNECTION_NOW, e);
+ }
}
}
}
diff --git a/java/org/apache/coyote/http2/Http2Protocol.java
b/java/org/apache/coyote/http2/Http2Protocol.java
index a2ac7c2..50748a2 100644
--- a/java/org/apache/coyote/http2/Http2Protocol.java
+++ b/java/org/apache/coyote/http2/Http2Protocol.java
@@ -29,6 +29,7 @@ import java.util.regex.Pattern;
import org.apache.coyote.Adapter;
import org.apache.coyote.CompressionConfig;
+import org.apache.coyote.ContinueResponseTiming;
import org.apache.coyote.Processor;
import org.apache.coyote.Request;
import org.apache.coyote.Response;
@@ -414,6 +415,11 @@ public class Http2Protocol implements UpgradeProtocol {
}
+ public ContinueResponseTiming getContinueResponseTimingInternal() {
+ return http11Protocol.getContinueResponseTimingInternal();
+ }
+
+
public AbstractHttp11Protocol<?> getHttp11Protocol() {
return this.http11Protocol;
}
diff --git a/java/org/apache/coyote/http2/StreamProcessor.java
b/java/org/apache/coyote/http2/StreamProcessor.java
index a6f677a..2c77a64 100644
--- a/java/org/apache/coyote/http2/StreamProcessor.java
+++ b/java/org/apache/coyote/http2/StreamProcessor.java
@@ -23,6 +23,7 @@ import org.apache.coyote.AbstractProcessor;
import org.apache.coyote.ActionCode;
import org.apache.coyote.Adapter;
import org.apache.coyote.ContainerThreadMarker;
+import org.apache.coyote.ContinueResponseTiming;
import org.apache.coyote.ErrorState;
import org.apache.coyote.Request;
import org.apache.coyote.Response;
@@ -185,12 +186,17 @@ class StreamProcessor extends AbstractProcessor {
@Override
- protected final void ack() {
- if (!response.isCommitted() && request.hasExpectation()) {
- try {
- stream.writeAck();
- } catch (IOException ioe) {
- setErrorState(ErrorState.CLOSE_CONNECTION_NOW, ioe);
+ protected final void ack(ContinueResponseTiming continueResponseTiming) {
+ // Only try and send the ACK for ALWAYS or if the timing of the request
+ // to send the ACK matches the current configuration.
+ if (continueResponseTiming == ContinueResponseTiming.ALWAYS ||
+ continueResponseTiming ==
handler.getProtocol().getContinueResponseTimingInternal()) {
+ if (!response.isCommitted() && request.hasExpectation()) {
+ try {
+ stream.writeAck();
+ } catch (IOException ioe) {
+ setErrorState(ErrorState.CLOSE_CONNECTION_NOW, ioe);
+ }
}
}
}
diff --git a/test/org/apache/catalina/core/TestStandardContextValve.java
b/test/org/apache/catalina/core/TestStandardContextValve.java
index 96b2a8b..8974d58 100644
--- a/test/org/apache/catalina/core/TestStandardContextValve.java
+++ b/test/org/apache/catalina/core/TestStandardContextValve.java
@@ -17,6 +17,7 @@
package org.apache.catalina.core;
import java.io.IOException;
+import java.util.Locale;
import javax.servlet.ServletException;
import javax.servlet.ServletRequestEvent;
@@ -29,9 +30,12 @@ import org.junit.Assert;
import org.junit.Test;
import org.apache.catalina.Context;
+import org.apache.catalina.connector.Connector;
import org.apache.catalina.connector.Response;
+import org.apache.catalina.startup.ExpectationClient;
import org.apache.catalina.startup.Tomcat;
import org.apache.catalina.startup.TomcatBaseTest;
+import org.apache.coyote.ContinueResponseTiming;
import org.apache.tomcat.util.buf.ByteChunk;
import org.apache.tomcat.util.descriptor.web.ErrorPage;
@@ -83,6 +87,7 @@ public class TestStandardContextValve extends TomcatBaseTest {
Assert.assertEquals("InitErrorDestroy", trace.toString());
}
+
@Test
public void testBug51653b() throws Exception {
// Set up a container
@@ -133,6 +138,7 @@ public class TestStandardContextValve extends
TomcatBaseTest {
Assert.assertEquals("InitErrorDestroy", trace.toString());
}
+
private static class Bug51653ErrorTrigger extends HttpServlet {
private static final long serialVersionUID = 1L;
@@ -143,6 +149,7 @@ public class TestStandardContextValve extends
TomcatBaseTest {
}
}
+
private static class Bug51653ErrorPage extends HttpServlet {
private static final long serialVersionUID = 1L;
@@ -162,6 +169,7 @@ public class TestStandardContextValve extends
TomcatBaseTest {
}
}
+
private static class Bug51653RequestListener
implements ServletRequestListener {
@@ -182,4 +190,73 @@ public class TestStandardContextValve extends
TomcatBaseTest {
}
}
+
+
+ @Test
+ public void test100ContinueDefault() throws Exception {
+ // The default setting is IMMEDIATELY
+ // This test verifies that we get proper 100 Continue responses
+ // when the continueResponseTiming property is not set
+ test100Continue();
+ }
+
+
+ @Test
+ public void test100ContinueSentImmediately() throws Exception {
+ final Tomcat tomcat = getTomcatInstance();
+
+ final Connector connector = tomcat.getConnector();
+ connector.setProperty("continueResponseTiming", "immediately");
+
+ test100Continue();
+ }
+
+
+ @Test
+ public void test100ContinueSentOnRequestContentRead() throws Exception {
+ final Tomcat tomcat = getTomcatInstance();
+
+ final Connector connector = tomcat.getConnector();
+ final String policyString =
ContinueResponseTiming.ON_REQUEST_BODY_READ.toString().toLowerCase(Locale.ENGLISH);
+ connector.setProperty("continueResponseTiming", policyString);
+
+ test100Continue();
+ }
+
+
+ private void test100Continue() throws Exception {
+ // Makes a request that expects a 100 Continue response and verifies
+ // that the 100 Continue response is received. This does not check
+ // that the correct ContinueResponseTiming was used, just
+ // that a 100 Continue response is received. The unit tests for
+ // Request verify that the various settings are correctly implemented.
+
+ final Tomcat tomcat = getTomcatInstance();
+
+ // No file system docBase required
+ final Context ctx = tomcat.addContext("", null);
+
+ Tomcat.addServlet(ctx, "echo", new EchoBodyServlet());
+ ctx.addServletMappingDecoded("/echo", "echo");
+
+ tomcat.start();
+
+ final ExpectationClient client = new ExpectationClient();
+
+ client.setPort(tomcat.getConnector().getLocalPort());
+ // Expected content doesn't end with a CR-LF so if it isn't chunked
make
+ // sure the content length is used as reading it line-by-line will fail
+ // since there is no "line".
+ client.setUseContentLength(true);
+
+ client.connect();
+
+ client.doRequestHeaders();
+
+ Assert.assertTrue(client.isResponse100());
+
+ client.doRequestBody();
+ Assert.assertTrue(client.isResponse200());
+ Assert.assertTrue(client.isResponseBodyOK());
+ }
}
diff --git a/test/org/apache/catalina/startup/ExpectationClient.java
b/test/org/apache/catalina/startup/ExpectationClient.java
new file mode 100644
index 0000000..ef7722b
--- /dev/null
+++ b/test/org/apache/catalina/startup/ExpectationClient.java
@@ -0,0 +1,51 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.catalina.startup;
+
+/**
+ * Simple http client used for testing requests with "Expect" headers.
+ */
+public class ExpectationClient extends SimpleHttpClient {
+
+ private static final String BODY = "foo=bar";
+
+ public void doRequestHeaders() throws Exception {
+ StringBuilder requestHeaders = new StringBuilder();
+ requestHeaders.append("POST /echo HTTP/1.1").append(CRLF);
+ requestHeaders.append("Host: localhost").append(CRLF);
+ requestHeaders.append("Expect: 100-continue").append(CRLF);
+ requestHeaders.append("Content-Type:
application/x-www-form-urlencoded").append(CRLF);
+ String len = Integer.toString(BODY.length());
+ requestHeaders.append("Content-length: ").append(len).append(CRLF);
+ requestHeaders.append(CRLF);
+
+ setRequest(new String[] {requestHeaders.toString()});
+
+ processRequest(false);
+ }
+
+ public void doRequestBody() throws Exception {
+ setRequest(new String[] { BODY });
+
+ processRequest(true);
+ }
+
+ @Override
+ public boolean isResponseBodyOK() {
+ return BODY.equals(getResponseBody());
+ }
+}
diff --git a/test/org/apache/coyote/TestRequest.java
b/test/org/apache/coyote/TestRequest.java
new file mode 100644
index 0000000..8ccdf55
--- /dev/null
+++ b/test/org/apache/coyote/TestRequest.java
@@ -0,0 +1,168 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.coyote;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import org.apache.coyote.http11.Constants;
+import org.apache.coyote.http11.Http11NioProtocol;
+import org.apache.coyote.http11.Http11Processor;
+import org.apache.tomcat.util.net.ApplicationBufferHandler;
+import org.apache.tomcat.util.net.SocketBufferHandler;
+import org.apache.tomcat.util.net.SocketWrapperBase;
+import org.easymock.EasyMock;
+
+public class TestRequest {
+
+ private final Http11NioProtocol protocol = new Http11NioProtocol();
+ private final Http11Processor processor = new Http11Processor(protocol,
null);
+ private final Request request = processor.getRequest();
+ private final Response response = request.getResponse();
+ private final SocketWrapperBase<?> socketWrapper =
EasyMock.createNiceMock(SocketWrapperBase.class);
+
+
+ @Before
+ public void setupTest() {
+ // Set up the socket wrapper
+ EasyMock.expect(socketWrapper.getSocketBufferHandler()).andReturn(new
SocketBufferHandler(0, 0, false));
+ EasyMock.replay(socketWrapper);
+ // Cast to make the method visible
+ ((AbstractProcessor) processor).setSocketWrapper(socketWrapper);
+ }
+
+
+ @Test
+ public void test100ContinueExpectationImmediately() throws IOException {
+ // Tests that response.sendAcknowledgement is only called when
+ // request.setContinueHandlingResponsePolicy is called.
+
+ request.setExpectation(true);
+
+ // Configure the mock to verify that a network write is made.
+ configureMockForOneAckowledgementWrite(socketWrapper);
+
+
protocol.setContinueResponseTiming(ContinueResponseTiming.IMMEDIATELY.toString());
+ response.action(ActionCode.ACK, ContinueResponseTiming.IMMEDIATELY);
+
+ // Verify that acknowledgement is written to network.
+ EasyMock.verify(socketWrapper);
+
+ // Configure the mock to verify that no network write is made.
+ configureMockForNoAckowledgementWrite(socketWrapper);
+
+ request.doRead(new DoNothingApplicationBufferHandler());
+
+ // Verify that acknowledgement is not written to network.
+ EasyMock.verify(socketWrapper);
+ }
+
+
+ @Test
+ public void test100ContinueExpectationOnRequestBodyRead() throws
IOException {
+ // Tests that response.sendAcknowledgement is only called when
+ // request.doRead is called.
+
+ request.setExpectation(true);
+
+ // Configure the mock to verify that no network write is made.
+ configureMockForNoAckowledgementWrite(socketWrapper);
+
+
protocol.setContinueResponseTiming(ContinueResponseTiming.ON_REQUEST_BODY_READ.toString());
+ response.action(ActionCode.ACK, ContinueResponseTiming.IMMEDIATELY);
+
+ // Verify that no acknowledgement is written to network.
+ EasyMock.verify(socketWrapper);
+
+ // Configure the mock to verify that a network write is made.
+ configureMockForOneAckowledgementWrite(socketWrapper);
+
+ request.doRead(new DoNothingApplicationBufferHandler());
+
+ // Verify that acknowledgement is written to network.
+ EasyMock.verify(socketWrapper);
+ }
+
+
+ @Test
+ public void testNoExpectationWithOnRequestBodyReadPolicy() throws
IOException {
+ // When expectation is false, sendAcknowledgement must never be called.
+
+ request.setExpectation(false);
+
+ // Configure the mock to verify that no network write is made.
+ configureMockForNoAckowledgementWrite(socketWrapper);
+
+
protocol.setContinueResponseTiming(ContinueResponseTiming.ON_REQUEST_BODY_READ.toString());
+ request.doRead(new DoNothingApplicationBufferHandler());
+
+ // Verify that no acknowledgement is written to network.
+ EasyMock.verify(socketWrapper);
+ }
+
+
+ @Test
+ public void testNoExpectationWithOnRequestImmediately() {
+ // When expectation is false, sendAcknowledgement must never be called.
+
+ request.setExpectation(false);
+
+ // Configure the mock to verify that no network write is made.
+ configureMockForNoAckowledgementWrite(socketWrapper);
+
+
protocol.setContinueResponseTiming(ContinueResponseTiming.IMMEDIATELY.toString());
+ response.action(ActionCode.ACK, ContinueResponseTiming.IMMEDIATELY);
+
+ // Verify that no acknowledgement is written to network.
+ EasyMock.verify(socketWrapper);
+ }
+
+
+ private class DoNothingApplicationBufferHandler implements
ApplicationBufferHandler {
+ @Override
+ public void setByteBuffer(ByteBuffer buffer) {
+
+ }
+
+ @Override
+ public ByteBuffer getByteBuffer() {
+ return null;
+ }
+
+ @Override
+ public void expand(int size) {
+
+ }
+ }
+
+
+ private void configureMockForOneAckowledgementWrite(SocketWrapperBase<?>
socketWrapper) throws IOException {
+ EasyMock.reset(socketWrapper);
+ socketWrapper.write(true, Constants.ACK_BYTES, 0,
Constants.ACK_BYTES.length);
+ EasyMock.expectLastCall().once();
+ EasyMock.replay(socketWrapper);
+ }
+
+
+ private void configureMockForNoAckowledgementWrite(SocketWrapperBase<?>
socketWrapper) {
+ EasyMock.reset(socketWrapper);
+ EasyMock.replay(socketWrapper);
+ }
+}
diff --git a/test/org/apache/coyote/http11/TestHttp11OutputBuffer.java
b/test/org/apache/coyote/http11/TestHttp11OutputBuffer.java
index 3e2a652..affb5d3 100644
--- a/test/org/apache/coyote/http11/TestHttp11OutputBuffer.java
+++ b/test/org/apache/coyote/http11/TestHttp11OutputBuffer.java
@@ -20,7 +20,7 @@ import org.junit.Assert;
import org.junit.Test;
import org.apache.catalina.Context;
-import org.apache.catalina.startup.SimpleHttpClient;
+import org.apache.catalina.startup.ExpectationClient;
import org.apache.catalina.startup.Tomcat;
import org.apache.catalina.startup.TomcatBaseTest;
@@ -55,35 +55,4 @@ public class TestHttp11OutputBuffer extends TomcatBaseTest {
Assert.assertTrue(client.isResponse200());
Assert.assertTrue(client.isResponseBodyOK());
}
-
- private static class ExpectationClient extends SimpleHttpClient {
-
- private static final String BODY = "foo=bar";
-
- public void doRequestHeaders() throws Exception {
- StringBuilder requestHeaders = new StringBuilder();
- requestHeaders.append("POST /echo HTTP/1.1").append(CRLF);
- requestHeaders.append("Host: localhost").append(CRLF);
- requestHeaders.append("Expect: 100-continue").append(CRLF);
- requestHeaders.append("Content-Type:
application/x-www-form-urlencoded").append(CRLF);
- String len = Integer.toString(BODY.length());
- requestHeaders.append("Content-length: ").append(len).append(CRLF);
- requestHeaders.append(CRLF);
-
- setRequest(new String[] {requestHeaders.toString()});
-
- processRequest(false);
- }
-
- public void doRequestBody() throws Exception {
- setRequest(new String[] { BODY });
-
- processRequest(true);
- }
-
- @Override
- public boolean isResponseBodyOK() {
- return BODY.equals(getResponseBody());
- }
- }
}
diff --git a/test/org/apache/coyote/http2/TestHttp2Section_8_1.java
b/test/org/apache/coyote/http2/TestHttp2Section_8_1.java
index cd44280..83a831f 100644
--- a/test/org/apache/coyote/http2/TestHttp2Section_8_1.java
+++ b/test/org/apache/coyote/http2/TestHttp2Section_8_1.java
@@ -23,6 +23,10 @@ import java.util.List;
import org.junit.Assert;
import org.junit.Test;
+import org.apache.catalina.connector.Connector;
+import org.apache.catalina.startup.Tomcat;
+import org.apache.coyote.ContinueResponseTiming;
+
/**
* Unit tests for Section 8.1 of
* <a href="https://tools.ietf.org/html/rfc7540";>RFC 7540</a>.
@@ -95,7 +99,40 @@ public class TestHttp2Section_8_1 extends Http2TestBase {
@Test
+ public void testSendAckWithDefaultPolicy() throws Exception {
+ testSendAck();
+ }
+
+
+ @Test
+ public void testSendAckWithImmediatelyPolicy() throws Exception {
+ setContinueHandlingResponsePolicy(ContinueResponseTiming.IMMEDIATELY);
+ testSendAck();
+ }
+
+
+ @Test
+ public void testSendAckWithOnRequestBodyReadPolicy() throws Exception {
+
setContinueHandlingResponsePolicy(ContinueResponseTiming.ON_REQUEST_BODY_READ);
+ testSendAck();
+ }
+
+
+ public void setContinueHandlingResponsePolicy(ContinueResponseTiming
policy) throws Exception {
+ final Tomcat tomcat = getTomcatInstance();
+
+ final Connector connector = tomcat.getConnector();
+ connector.setProperty("continueHandlingResponsePolicy",
policy.toString());
+ }
+
+
+ @Test
public void testSendAck() throws Exception {
+ // makes a request that expects a 100 Continue response and verifies
+ // that the 100 Continue response is received. This does not check
+ // that the correct ContinueHandlingResponsePolicy was followed, just
+ // that a 100 Continue response is received. The unit tests for
+ // Request verify that the various policies are implemented.
http2Connect();
byte[] headersFrameHeader = new byte[9];
@@ -104,7 +141,9 @@ public class TestHttp2Section_8_1 extends Http2TestBase {
ByteBuffer dataPayload = ByteBuffer.allocate(256);
buildPostRequest(headersFrameHeader, headersPayload, true,
- dataFrameHeader, dataPayload, null, 3);
+ null, -1, "/simple",
+ dataFrameHeader, dataPayload, null,
+ null, null, 3);
// Write the headers
writeFrame(headersFrameHeader, headersPayload);
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 7211544..0301083 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -99,6 +99,13 @@
</subsection>
<subsection name="Coyote">
<changelog>
+ <add>
+ <bug>57661</bug>: For requests containing the
+ <code>Expect: 100-continue</code> header, add optional support to delay
+ sending an intermediate 100 status response until the servlet reads the
+ request body, allowing the servlet the opportunity to respond without
+ asking for the request body. Based on a pull request by malaysf.
(markt)
+ </add>
<fix>
Refactor the implementation of
<code>ServletInputStream.available()</code> to provide a more accurate
diff --git a/webapps/docs/config/http.xml b/webapps/docs/config/http.xml
index e8616e5..2538c20 100644
--- a/webapps/docs/config/http.xml
+++ b/webapps/docs/config/http.xml
@@ -85,6 +85,21 @@
30000 (30 seconds).</p>
</attribute>
+ <attribute name="continueResponseTiming" required="false">
+ <p>When to respond with a <code>100</code> intermediate response code to
a
+ request containing an <code>Expect: 100-continue</code> header.
+ The following values may used:
+ <ul>
+ <li><code>immediately</code> - an intermediate 100 status response
+ will be returned as soon as practical</li>
+ <li><code>onRead</code> - an intermediate 100 status
+ response will be returned only when the Servlet reads the request body,
+ allowing the servlet to inspect the headers and possibly respond
+ before the user agent sends a possibly large request body.</li>
+ </ul>
+ </p>
+ </attribute>
+
<attribute name="defaultSSLHostConfigName" required="false">
<p>The name of the default <strong>SSLHostConfig</strong> that will be
used for secure connections (if this connector is configured for secure
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
