malaysf commented on a change in pull request #332:
URL: https://github.com/apache/tomcat/pull/332#discussion_r477099224
##########
File path: test/org/apache/catalina/core/TestStandardContextValve.java
##########
@@ -182,4 +186,123 @@ public void requestDestroyed(ServletRequestEvent sre) {
}
}
+
+ @Test
+ public void test100ContinueDefaultPolicy() throws Exception {
+ // the default policy is IMMEDIATELY
+ // This test verifies that we get proper 100 Continue responses
+ // when the continueHandlingResponsePolicy property is not set
+ test100Continue(ContinueHandlingResponsePolicy.IMMEDIATELY);
+ }
+
+ @Test
+ public void test100ContinueSentImmediately() throws Exception {
+ final Tomcat tomcat = getTomcatInstance();
+
+ final Connector connector = tomcat.getConnector();
+ connector.setProperty("continueHandlingResponsePolicy", "immediately");
+
+ test100Continue(ContinueHandlingResponsePolicy.IMMEDIATELY);
+ }
+
+ @Test
+ public void test100ContinueSentOnRequestContentRead() throws Exception {
+ final Tomcat tomcat = getTomcatInstance();
+
+ final Connector connector = tomcat.getConnector();
+ final String policyString =
ContinueHandlingResponsePolicy.ON_REQUEST_BODY_READ.toString()
+ .toLowerCase(Locale.ENGLISH);
+ connector.setProperty("continueHandlingResponsePolicy", policyString);
+
+ test100Continue(ContinueHandlingResponsePolicy.ON_REQUEST_BODY_READ);
+ }
+
+ public void test100Continue(ContinueHandlingResponsePolicy expectedPolicy)
throws Exception {
+ final Tomcat tomcat = getTomcatInstance();
+
+ // No file system docBase required
+ final Context ctx = tomcat.addContext("", null);
+
+ // configure the servlet to wait 1 second before reading the request
body
+ Tomcat.addServlet(ctx, "echo", new DelayingEchoBodyServlet(1000));
+ 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();
+
+ // time how long it takes to send the request headers and get the
+ // 100 continue response
+ final long startTime = System.currentTimeMillis();
+ client.doRequestHeaders();
+ final long endTime = System.currentTimeMillis();
+
+ final long duration = endTime - startTime;
+
+ if(expectedPolicy == ContinueHandlingResponsePolicy.IMMEDIATELY) {
+ // the 100 response should be received immediately while
+ // the servlet will wait 1 second before responding. 500 ms
+ // should be enough time to allow for any slowness that may
+ // occur but still differentiate from the 1 second or more
+ // expected delay by the ON_REQUEST_BODY_READ policy.
+ Assert.assertTrue(duration < 500);
Review comment:
@martin-g I've reworked the interface and tests to differentiate the two
policies without requiring checking the time elapsed. Basically, it can't be
determined which policy was used simply by making a request and checking the
response, so I also added a unit test for Coyote Request directly that mocks
out classes to check that `sendAcknowledgement` is only called when expected.
Can you please take a look at the latest version?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
