This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/master by this push:
new 17d9e68 Manually merge #332 - BZ 57661 - Vary when 100 response code
is sent
17d9e68 is described below
commit 17d9e688fab3b138d35c80c840763c4422e7dbfa
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 reques 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 | 40 ++++-
webapps/docs/changelog.xml | 7 +
webapps/docs/config/http.xml | 15 ++
20 files changed, 553 insertions(+), 58 deletions(-)
diff --git a/java/org/apache/catalina/authenticator/FormAuthenticator.java
b/java/org/apache/catalina/authenticator/FormAuthenticator.java
index c53f49c..ed2caf6 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 ebd94f5..5e277fa 100644
--- a/java/org/apache/catalina/connector/Response.java
+++ b/java/org/apache/catalina/connector/Response.java
@@ -50,6 +50,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;
@@ -1193,9 +1194,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;
@@ -1206,7 +1224,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 ef95ea2..33dc8dc 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 01b71c0..f3b87ac 100644
--- a/java/org/apache/coyote/AbstractProcessor.java
+++ b/java/org/apache/coyote/AbstractProcessor.java
@@ -390,7 +390,7 @@ public abstract class AbstractProcessor extends
AbstractProcessorLight implement
break;
}
case ACK: {
- ack();
+ ack((ContinueResponseTiming) param);
break;
}
case CLIENT_FLUSH: {
@@ -722,7 +722,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 6acb6be..83960cb 100644
--- a/java/org/apache/coyote/LocalStrings.properties
+++ b/java/org/apache/coyote/LocalStrings.properties
@@ -53,6 +53,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 a2af283..f9dac68 100644
--- a/java/org/apache/coyote/Request.java
+++ b/java/org/apache/coyote/Request.java
@@ -549,6 +549,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 c97f9b7..ae7bc5a 100644
--- a/java/org/apache/coyote/ajp/AjpProcessor.java
+++ b/java/org/apache/coyote/ajp/AjpProcessor.java
@@ -37,6 +37,7 @@ import jakarta.servlet.http.HttpServletResponse;
import org.apache.coyote.AbstractProcessor;
import org.apache.coyote.ActionCode;
import org.apache.coyote.Adapter;
+import org.apache.coyote.ContinueResponseTiming;
import org.apache.coyote.ErrorState;
import org.apache.coyote.InputBuffer;
import org.apache.coyote.OutputBuffer;
@@ -1064,7 +1065,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 35141a1..561ae8e 100644
--- a/java/org/apache/coyote/http11/AbstractHttp11Protocol.java
+++ b/java/org/apache/coyote/http11/AbstractHttp11Protocol.java
@@ -31,6 +31,7 @@ import jakarta.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 c369837..906e8c9 100644
--- a/java/org/apache/coyote/http11/Http11OutputBuffer.java
+++ b/java/org/apache/coyote/http11/Http11OutputBuffer.java
@@ -52,6 +52,9 @@ public class Http11OutputBuffer implements HttpOutputBuffer {
protected final Response response;
+ private volatile boolean ackSent = false;
+
+
/**
* Finished flag.
*/
@@ -272,6 +275,7 @@ public class Http11OutputBuffer implements HttpOutputBuffer
{
// Reset pointers
headerBuffer.position(0).limit(headerBuffer.capacity());
lastActiveFilter = -1;
+ ackSent = false;
responseFinished = false;
byteCount = 0;
}
@@ -283,7 +287,11 @@ public class Http11OutputBuffer implements
HttpOutputBuffer {
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;
socketWrapper.write(isBlocking(), Constants.ACK_BYTES, 0,
Constants.ACK_BYTES.length);
if (flushBuffer(true)) {
throw new IOException(sm.getString("iob.failedwrite.ack"));
diff --git a/java/org/apache/coyote/http11/Http11Processor.java
b/java/org/apache/coyote/http11/Http11Processor.java
index 00469e1..37bb97b 100644
--- a/java/org/apache/coyote/http11/Http11Processor.java
+++ b/java/org/apache/coyote/http11/Http11Processor.java
@@ -30,6 +30,7 @@ import jakarta.servlet.http.HttpServletResponse;
import org.apache.coyote.AbstractProcessor;
import org.apache.coyote.ActionCode;
import org.apache.coyote.Adapter;
+import org.apache.coyote.ContinueResponseTiming;
import org.apache.coyote.ErrorState;
import org.apache.coyote.Request;
import org.apache.coyote.RequestInfo;
@@ -1160,15 +1161,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(null);
+ }
+
+
+ @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 56cb8e1..a93955b 100644
--- a/java/org/apache/coyote/http2/Http2Protocol.java
+++ b/java/org/apache/coyote/http2/Http2Protocol.java
@@ -21,6 +21,7 @@ import java.util.Enumeration;
import org.apache.coyote.AbstractProtocol;
import org.apache.coyote.Adapter;
+import org.apache.coyote.ContinueResponseTiming;
import org.apache.coyote.Processor;
import org.apache.coyote.Request;
import org.apache.coyote.Response;
@@ -318,6 +319,11 @@ public class Http2Protocol implements UpgradeProtocol {
}
+ public ContinueResponseTiming getContinueResponseTimingInternal() {
+ return http11Protocol.getContinueResponseTimingInternal();
+ }
+
+
public AbstractProtocol<?> getHttp11Protocol() {
return this.http11Protocol;
}
diff --git a/java/org/apache/coyote/http2/StreamProcessor.java
b/java/org/apache/coyote/http2/StreamProcessor.java
index 2e40f9c..7eab7ee 100644
--- a/java/org/apache/coyote/http2/StreamProcessor.java
+++ b/java/org/apache/coyote/http2/StreamProcessor.java
@@ -24,6 +24,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;
@@ -211,12 +212,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 46c30f2..12879e1 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 jakarta.servlet.ServletException;
import jakarta.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 b14fc90..dbada46 100644
--- a/test/org/apache/coyote/http2/TestHttp2Section_8_1.java
+++ b/test/org/apache/coyote/http2/TestHttp2Section_8_1.java
@@ -23,6 +23,9 @@ 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;
import org.apache.coyote.http11.AbstractHttp11Protocol;
/**
@@ -97,7 +100,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];
@@ -106,7 +142,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 55b2d82..d6a65a2 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -109,6 +109,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>
Remove deprecated <code>CookieProcessor.generateHeader</code> method.
(remm)
diff --git a/webapps/docs/config/http.xml b/webapps/docs/config/http.xml
index 5e447e9..1cca00f 100644
--- a/webapps/docs/config/http.xml
+++ b/webapps/docs/config/http.xml
@@ -91,6 +91,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
