[Bug 64649] XSLT transformation - document('') doesn't return stylesheet

2020-08-06 Thread bugzilla
https://bz.apache.org/bugzilla/show_bug.cgi?id=64649

Stefan Pöschel  changed:

   What|Removed |Added

  Component|Standard Taglib |XTags Taglib

-- 
You are receiving this mail because:
You are the assignee for the bug.
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



[Bug 64614] tomcat doesn't work with JSSE FIPS-compliant with NSS

2020-08-06 Thread bugzilla
https://bz.apache.org/bugzilla/show_bug.cgi?id=64614

--- Comment #14 from jfclere  ---
https://github.com/apache/tomcat/pull/334 as the best I can get ;-)

-- 
You are receiving this mail because:
You are the assignee for the bug.
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



[GitHub] [tomcat] jfclere opened a new pull request #334: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=64614

2020-08-06 Thread GitBox


jfclere opened a new pull request #334:
URL: https://github.com/apache/tomcat/pull/334


   I was not able to convince the JVM in "FIPS mode" to use an alias, if there 
are several key/cert in the keystore it will use the latest one, therefore the 
following patch.



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



Publishing EOL dates on whichversion?

2020-08-06 Thread Christopher Schultz
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

All,

I'm wondering if we shouldn't add EOL dates to the "which version" page.

The table on that page is very busy, but I think it would help to know:

1. When a currently-supported version will be EOL'd (e.g. 7.0.x)
2. When a superseded version has been EOL'd (e.g. 6.0.x)

We might be able to shrink the table horizontally a little by
shrinkking / abbreviating some of the column headers. For example
replace "Authentication (JASPIC) Spec" with "JASPIC". Maybe drop
"Spec" from all headers. Maybe shorten "Latest Released Version" to
"Latest" and "Supported Java Versions" to "Java". We could use a
stylesheet to add those longer headers for printing, and we could use
"title"s to expand the headers if you float a mouse/tap a finger on
the headings to get more information. We could also put a key below
the table.

WDYT?

- -chris
-BEGIN PGP SIGNATURE-
Comment: Using GnuPG with Thunderbird - https://www.enigmail.net/

iQIzBAEBCAAdFiEEMmKgYcQvxMe7tcJcHPApP6U8pFgFAl8sIa8ACgkQHPApP6U8
pFhjkA/8DLMqIHgF130aGL2YXWuG5OKl/WHh1NFUHhcWHg1zu4zosaMeqExcxFdM
20kE44WjbcD0kvAEeAvR6ejOLm1S7lfXzCs41ZnJI6v4wyhEfp+n9gfYESF3kZqU
5v4IhD0XBspp60iom2BVggF61qu2ZIE9BCD+zv5ikELni4+psg1T7WkTomtIj8Id
W0A+QGMnsYyAGZpqxPRM3agn/T2A+pbsQKFTfqX8KEot+m00PrEy5HRjeYCRpm4Y
OtlHrPLhrOutS7M3K8b191BPf7I55pGEfsq7rSHnpD0H+KM4ur9v6tHtWNutwVTY
3Y2Fs357Cckr+jU0YpBEDn/2jmHioiShMYNmchalkvbXCiCESzgzhr5oazU7tp9j
et7zFy3XlJ1e0fJ3vq/LQnu6HqCwPRPaF27h8hyVrLSzatrPb9Cb2/AGr3SaHonK
9YmpziI1MR4i358y3cxeVIaTMal6MDAjRn8fIfQxxm3k5PZiHb+aMNd4Mion11xH
SqVAep9FyV9V0AbikoazTzUApPBosiD0onioq3o6ApSVfdaa+shh2jmI2JFzRGkf
Unpb3xGRye3EZZVjwXCDw0QJ2UVx65gM7k5W0xvUr0IOmSjJVLBpi1Z1Zv7ijNN0
nj8x3UUTNrABK8PniUIZ+xafoRFlPv4RDGcf6laCdks5R5y31+8=
=8Ax9
-END PGP SIGNATURE-

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



buildbot success in on tomcat-trunk

2020-08-06 Thread buildbot
The Buildbot has detected a restored build on builder tomcat-trunk while 
building tomcat. Full details are available at:
https://ci.apache.org/builders/tomcat-trunk/builds/5330

Buildbot URL: https://ci.apache.org/

Buildslave for this Build: asf946_ubuntu

Build Reason: The AnyBranchScheduler scheduler named 'on-tomcat-commit' 
triggered this build
Build Source Stamp: [branch master] ec5e948652e3bc4a3ea084501fe56f1939297952
Blamelist: Jean-Frederic Clere 

Build succeeded!

Sincerely,
 -The Buildbot




-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



[GitHub] [tomcat] jfclere merged pull request #309: Allow recursive substitution of properties.

2020-08-06 Thread GitBox


jfclere merged pull request #309:
URL: https://github.com/apache/tomcat/pull/309


   



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



[tomcat] branch master updated: Allow recursive substitution of properties. Add tests and use indexOf("${") instead indexOf('$').

2020-08-06 Thread jfclere
This is an automated email from the ASF dual-hosted git repository.

jfclere 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 e7896ee  Allow recursive substitution of properties. Add tests and use 
indexOf("${") instead indexOf('$').
 new ec5e948  Merge pull request #309 from jfclere/trunk
e7896ee is described below

commit e7896ee44a3c9f2cff4b93bca9ae6112917f6c88
Author: Jean-Frederic Clere 
AuthorDate: Fri Jun 26 09:50:09 2020 +0200

Allow recursive substitution of properties.
Add tests and use indexOf("${") instead indexOf('$').
---
 java/org/apache/tomcat/util/IntrospectionUtils.java | 21 +++--
 .../apache/tomcat/util/TestIntrospectionUtils.java  | 12 
 2 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/java/org/apache/tomcat/util/IntrospectionUtils.java 
b/java/org/apache/tomcat/util/IntrospectionUtils.java
index 78ab66f..9f12323 100644
--- a/java/org/apache/tomcat/util/IntrospectionUtils.java
+++ b/java/org/apache/tomcat/util/IntrospectionUtils.java
@@ -285,8 +285,17 @@ public final class IntrospectionUtils {
 public static String replaceProperties(String value,
 Hashtable staticProp, PropertySource dynamicProp[],
 ClassLoader classLoader) {
+return replaceProperties(value, staticProp, dynamicProp, 
classLoader, 0);
+}
 
-if (value.indexOf('$') < 0) {
+private static String replaceProperties(String value,
+Hashtable staticProp, PropertySource dynamicProp[],
+ClassLoader classLoader, int iterationCount) {
+if (value.indexOf("${") < 0) {
+return value;
+}
+if (iterationCount >=20) {
+log.warn("System property failed to update and remains [" + value 
+ "]");
 return value;
 }
 StringBuilder sb = new StringBuilder();
@@ -332,7 +341,15 @@ public final class IntrospectionUtils {
 }
 if (prev < value.length())
 sb.append(value.substring(prev));
-return sb.toString();
+String newval = sb.toString();
+if (newval.indexOf("${") < 0) {
+return newval;
+}
+if (newval.equals(value))
+return value;
+if (log.isDebugEnabled())
+log.debug("IntrospectionUtils.replaceProperties iter on: " + 
newval);
+return replaceProperties(newval, staticProp, dynamicProp, classLoader, 
iterationCount+1);
 }
 
 private static String getProperty(String name, Hashtable 
staticProp,
diff --git a/test/org/apache/tomcat/util/TestIntrospectionUtils.java 
b/test/org/apache/tomcat/util/TestIntrospectionUtils.java
index ed9fe39..73ea86a 100644
--- a/test/org/apache/tomcat/util/TestIntrospectionUtils.java
+++ b/test/org/apache/tomcat/util/TestIntrospectionUtils.java
@@ -143,5 +143,17 @@ public class TestIntrospectionUtils {
 
 Assert.assertEquals("abc${normal}xyz", 
IntrospectionUtils.replaceProperties(
 "abc${normal}xyz", properties, null, null));
+
+properties.setProperty("my.ajp.port", "8009");
+properties.setProperty("tomcat.ajp.port", "${my.ajp.port}");
+Assert.assertEquals("8009", IntrospectionUtils.replaceProperties(
+"${tomcat.ajp.port}", properties, null, null));
+
+}
+@Test
+public void testReplacePropertiesRecursively() {
+Properties properties = new Properties();
+properties.setProperty("replaceMe", "something ${replaceMe}");
+IntrospectionUtils.replaceProperties("${replaceMe}", properties, null, 
null);
 }
 }


-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



[GitHub] [tomcat] kelthuzadx opened a new pull request #333: Support INDEX.LIST in jar file to speed up resource finding

2020-08-06 Thread GitBox


kelthuzadx opened a new pull request #333:
URL: https://github.com/apache/tomcat/pull/333


   `jdk.internal.util.jar.JarIndex` was introduced by JDK1.3, it's an old but 
useful technique. If third-party JAR files contain INDEX.LIST file(generated 
via `jar -i` command), these JAR files can be identified as loading into JVM. 
Class loader such as BuiltinClassLoader,URLClassLoader and subclasses of 
URLClassLoader can speed up their resource searching with the help of 
INDEX.LIST since it maps resource names to corresponding JAR names. 
   
   The latest version of Tomcat does not aware of `INDEX.LIST`, when there are 
so many JAR files in `WEB-INF/lib`,  Tom cat have to search in all JAR files 
sequentially, which is obviously slow. This patch helps Tomcat to be aware of 
`INDEX.LIST`, speeds up the search of resources, especially when there are many 
JAR files in `WEB-INF/lib`



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



Re: Failing Travis CI build and Smoke Test / JDK8 ubuntu-latest

2020-08-06 Thread jean-frederic clere

On 05/08/2020 14:35, Martin Grigorov wrote:

Hi,

On Wed, Aug 5, 2020 at 10:22 AM jean-frederic clere > wrote:


Hi,

The Travis CI build seems to fail on timout regularly should we
increase
the timeout? Or investigate the problem?

Smoke Test / JDK8 ubuntu-latest fails on tls3 tests should we make the
test conditional?


TravisCI or GitHub Actions ?


GitHub Actions.



TravisCI is used for non-x86_64 architectures and they are known to run 
slow because those architectures are experimental on Travis and run on 
less powerful VMs. So, timeouts happen more often :-/



-- 
Cheers


Jean-Frederic

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org

For additional commands, e-mail: dev-h...@tomcat.apache.org





--
Cheers

Jean-Frederic

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



[Bug 64649] New: XSLT transformation - document('') doesn't return stylesheet

2020-08-06 Thread bugzilla
https://bz.apache.org/bugzilla/show_bug.cgi?id=64649

Bug ID: 64649
   Summary: XSLT transformation - document('') doesn't return
stylesheet
   Product: Taglibs
   Version: nightly
  Hardware: PC
OS: Linux
Status: NEW
  Severity: normal
  Priority: P2
 Component: Standard Taglib
  Assignee: dev@tomcat.apache.org
  Reporter: spoesc...@irt.de
  Target Milestone: ---

Created attachment 37384
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=37384=edit
Minimal test case JSP

According to § 12.1 of the W3C XSLT 1.0 spec, "document("") refers to the root
node of the stylesheet".

However using JSP and the standard taglib, an empty sequence is returned in
such case instead (please see attached a minimal example). I tested with v1.2.5
and also compiled the last SVN revision (as bug 60950 seems to be related to
this issue).

When the affected XSLT is processed with a standalone version of Xalan (2.7.2),
the document function works as expected though.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



[GitHub] [tomcat] martin-g commented on a change in pull request #330: BZ-64644 throw an Idle Session Event on idle timeout

2020-08-06 Thread GitBox


martin-g commented on a change in pull request #330:
URL: https://github.com/apache/tomcat/pull/330#discussion_r466241414



##
File path: java/org/apache/tomcat/websocket/WsSession.java
##
@@ -809,20 +825,36 @@ protected void updateLastActive() {
 lastActive = System.currentTimeMillis();
 }
 
+protected void updateLastActiveRead() {
+   lastActiveRead = System.currentTimeMillis();
+}
 
 protected void checkExpiration() {
 long timeout = maxIdleTimeout;
 if (timeout < 1) {
 return;
 }
+long currentTime = System.currentTimeMillis();
+boolean isWriteTimeout =  currentTime - lastActive > timeout;
+boolean isReadTimeout = currentTime - lastActiveRead > timeout;
 
-if (System.currentTimeMillis() - lastActive > timeout) {
-String msg = sm.getString("wsSession.timeout", getId());
-if (log.isDebugEnabled()) {
-log.debug(msg);
-}
-doClose(new CloseReason(CloseCodes.GOING_AWAY, msg),
-new CloseReason(CloseCodes.CLOSED_ABNORMALLY, msg));
+if (isWriteTimeout || isReadTimeout) {
+   if(closeOnIdle) {
+String msg = sm.getString("wsSession.timeout", getId());
+if (log.isDebugEnabled()) {
+log.debug(msg);
+}
+doClose(new CloseReason(CloseCodes.GOING_AWAY, msg),
+new CloseReason(CloseCodes.CLOSED_ABNORMALLY, msg));
+   } else {
+   String msg = sm.getString("wsSession.timeout", getId());

Review comment:
   indentation is off

##
File path: java/javax/websocket/OnIdleSession.java
##
@@ -0,0 +1,12 @@
+package javax.websocket;

Review comment:
   I'm afraid this is not part of WebSocket specification: 
https://www.jcp.org/en/jsr/detail?id=356 and 
https://download.oracle.com/otndocs/jcp/websocket-1_1-mrel-eval-spec/index.html
   So you cannot add it to `javax.websocket` package

##
File path: java/javax/websocket/Session.java
##
@@ -77,6 +77,19 @@
  */
 void setMaxIdleTimeout(long timeout);
 
+/**
+ * Get the flag to decide whether the session will be closed if idle Time 
is passed
+ * @return true if the session is to be closed on expire
+ * false if the session will remain open and an IdleSession event 
is thrown instead
+ */
+boolean getCloseOnIdleTimeout();
+
+/**
+ * Get the flag to decide whether the session will be closed if idle Time 
is passed
+ * @param clsoeOnIdleTimeout- true for closing the session on idle timeout
+ *false for keeping the session open and 
throwing IdleSession event instead.
+ */
+void setCloseOnIdleTimeout(boolean clsoeOnIdleTimeout);

Review comment:
   typo in the parameter name

##
File path: java/org/apache/tomcat/websocket/WsSession.java
##
@@ -98,6 +99,8 @@
 private volatile int maxTextMessageBufferSize = 
Constants.DEFAULT_BUFFER_SIZE;
 private volatile long maxIdleTimeout = 0;
 private volatile long lastActive = System.currentTimeMillis();
+private volatile long lastActiveRead = System.currentTimeMillis();

Review comment:
   `lastRead` ?

##
File path: java/org/apache/tomcat/websocket/WsSession.java
##
@@ -98,6 +99,8 @@
 private volatile int maxTextMessageBufferSize = 
Constants.DEFAULT_BUFFER_SIZE;
 private volatile long maxIdleTimeout = 0;
 private volatile long lastActive = System.currentTimeMillis();

Review comment:
   Probably this one should be renamed to `lastWrite` ?!





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



[GitHub] [tomcat] malaysf commented on a change in pull request #332: Support sending the 100 continue response when the servlet reads the …

2020-08-06 Thread GitBox


malaysf commented on a change in pull request #332:
URL: https://github.com/apache/tomcat/pull/332#discussion_r466203459



##
File path: test/org/apache/catalina/core/TestStandardContextValve.java
##
@@ -182,4 +184,66 @@ 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
+final Tomcat tomcat = getTomcatInstance();
+
+final Connector connector = tomcat.getConnector();
+connector.setProperty("continueHandlingResponsePolicy", "IMMEDIATELY");
+
+test100Continue();
+}
+
+@Test
+public void test100ContinueSentImmediately() throws Exception {
+final Tomcat tomcat = getTomcatInstance();
+
+final Connector connector = tomcat.getConnector();
+connector.setProperty("continueHandlingResponsePolicy", "IMMEDIATELY");
+
+test100Continue();
+}
+
+@Test
+public void test100ContinueSentOnRequestContentRead() throws Exception {
+final Tomcat tomcat = getTomcatInstance();
+
+final Connector connector = tomcat.getConnector();
+connector.setProperty("continueHandlingResponsePolicy", 
"ON_REQUEST_CONTENT_READ");
+
+test100Continue();
+}
+
+public void test100Continue() throws Exception {

Review comment:
   I've reworked the test, please let me know what you think of if you have 
other ideas on how to test this change.





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



[GitHub] [tomcat] malaysf commented on a change in pull request #332: Support sending the 100 continue response when the servlet reads the …

2020-08-06 Thread GitBox


malaysf commented on a change in pull request #332:
URL: https://github.com/apache/tomcat/pull/332#discussion_r466202972



##
File path: java/org/apache/coyote/ContinueHandlingResponsePolicy.java
##
@@ -0,0 +1,37 @@
+/*
+ *  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;
+
+/**
+ * Enum defining policies on responding to '100-continue' expectations.
+ */
+public enum ContinueHandlingResponsePolicy {
+/**
+ * Tomcat will automatically and always send the 100 intermediate response

Review comment:
   I've updated the comment





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



[GitHub] [tomcat] malaysf commented on a change in pull request #332: Support sending the 100 continue response when the servlet reads the …

2020-08-06 Thread GitBox


malaysf commented on a change in pull request #332:
URL: https://github.com/apache/tomcat/pull/332#discussion_r466179556



##
File path: test/org/apache/catalina/core/TestStandardContextValve.java
##
@@ -182,4 +184,66 @@ 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
+final Tomcat tomcat = getTomcatInstance();
+
+final Connector connector = tomcat.getConnector();
+connector.setProperty("continueHandlingResponsePolicy", "IMMEDIATELY");
+
+test100Continue();
+}
+
+@Test
+public void test100ContinueSentImmediately() throws Exception {
+final Tomcat tomcat = getTomcatInstance();
+
+final Connector connector = tomcat.getConnector();
+connector.setProperty("continueHandlingResponsePolicy", "IMMEDIATELY");
+
+test100Continue();
+}
+
+@Test
+public void test100ContinueSentOnRequestContentRead() throws Exception {
+final Tomcat tomcat = getTomcatInstance();
+
+final Connector connector = tomcat.getConnector();
+connector.setProperty("continueHandlingResponsePolicy", 
"ON_REQUEST_CONTENT_READ");
+
+test100Continue();
+}
+
+public void test100Continue() throws Exception {

Review comment:
   I struggled a bit with writing the tests, in both cases a 100 continue 
response is expected. I'll give testing some more thought.





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



[GitHub] [tomcat] malaysf commented on a change in pull request #332: Support sending the 100 continue response when the servlet reads the …

2020-08-06 Thread GitBox


malaysf commented on a change in pull request #332:
URL: https://github.com/apache/tomcat/pull/332#discussion_r466178352



##
File path: java/org/apache/coyote/ContinueHandlingResponsePolicy.java
##
@@ -0,0 +1,37 @@
+/*
+ *  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;
+
+/**
+ * Enum defining policies on responding to '100-continue' expectations.
+ */
+public enum ContinueHandlingResponsePolicy {
+/**
+ * Tomcat will automatically and always send the 100 intermediate response
+ *
+ * This is the default behavior
+ */
+IMMEDIATELY,
+
+/**
+ * Send the 100 intermediate response only when the servlet attempts to
+ * read the request's body. This allows the servlet to process the
+ * request headers and possibly respond before asking for the request

Review comment:
   I will clarity the comment





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



[GitHub] [tomcat] malaysf commented on a change in pull request #332: Support sending the 100 continue response when the servlet reads the …

2020-08-06 Thread GitBox


malaysf commented on a change in pull request #332:
URL: https://github.com/apache/tomcat/pull/332#discussion_r466174698



##
File path: java/org/apache/catalina/connector/Response.java
##
@@ -1197,16 +1197,12 @@ public String encodeUrl(String url) {
 public void sendAcknowledgement()
 throws IOException {
 
-if (isCommitted()) {

Review comment:
   The `isCommitted` check moved into `Coyote.Request` so the behavior 
should be the same.





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



[GitHub] [tomcat] malaysf commented on a change in pull request #332: Support sending the 100 continue response when the servlet reads the …

2020-08-06 Thread GitBox


malaysf commented on a change in pull request #332:
URL: https://github.com/apache/tomcat/pull/332#discussion_r466173884



##
File path: java/org/apache/coyote/Request.java
##
@@ -709,4 +718,13 @@ private static String getCharsetFromContentType(String 
contentType) {
 
 return encoding.trim();
 }
+
+/**
+ * set whether to acknowledge the request when the request body is
+ * first read from
+ * @param acknowledgeOnFirstRead the value to set
+ */
+public void setAcknowledgeOnRequestBodyRead(boolean 
acknowledgeOnFirstRead) {

Review comment:
   thanks, missed this one while renaming





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



[GitHub] [tomcat] malaysf commented on a change in pull request #332: Support sending the 100 continue response when the servlet reads the …

2020-08-06 Thread GitBox


malaysf commented on a change in pull request #332:
URL: https://github.com/apache/tomcat/pull/332#discussion_r466173546



##
File path: java/org/apache/coyote/AbstractProtocol.java
##
@@ -262,6 +267,18 @@ public void setConnectionLinger(int connectionLinger) {
 endpoint.setConnectionLinger(connectionLinger);
 }
 
+//  HTTP specific 
properties
+// - queried by 
StandardContextValve
+public void setContinueHandlingResponsePolicy(String value) {
+continueHandlingResponsePolicy = 
Enum.valueOf(ContinueHandlingResponsePolicy.class, value);
+}
+
+
+//todo(malay) move to AbstractProtocol

Review comment:
   sorry, that was a note to myself that I missed removing





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



[GitHub] [tomcat] michael-o commented on a change in pull request #332: Support sending the 100 continue response when the servlet reads the …

2020-08-06 Thread GitBox


michael-o commented on a change in pull request #332:
URL: https://github.com/apache/tomcat/pull/332#discussion_r466167274



##
File path: java/org/apache/coyote/ContinueHandlingResponsePolicy.java
##
@@ -0,0 +1,37 @@
+/*
+ *  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;
+
+/**
+ * Enum defining policies on responding to '100-continue' expectations.
+ */
+public enum ContinueHandlingResponsePolicy {
+/**
+ * Tomcat will automatically and always send the 100 intermediate response

Review comment:
   It will not always send 100, only on the case the request is eligible 
for 100.

##
File path: java/org/apache/coyote/ContinueHandlingResponsePolicy.java
##
@@ -0,0 +1,37 @@
+/*
+ *  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;
+
+/**
+ * Enum defining policies on responding to '100-continue' expectations.
+ */
+public enum ContinueHandlingResponsePolicy {
+/**
+ * Tomcat will automatically and always send the 100 intermediate response
+ *
+ * This is the default behavior
+ */
+IMMEDIATELY,
+
+/**
+ * Send the 100 intermediate response only when the servlet attempts to
+ * read the request's body. This allows the servlet to process the
+ * request headers and possibly respond before asking for the request

Review comment:
   The text is confusing. You are saying: "attempt to read the request 
body", but in the next sentence you say "before asking for the request body". 
So does one have to invoke `request#getInputStream()`?

##
File path: java/org/apache/catalina/connector/Response.java
##
@@ -1197,16 +1197,12 @@ public String encodeUrl(String url) {
 public void sendAcknowledgement()
 throws IOException {
 
-if (isCommitted()) {

Review comment:
   The move to `sendAcknowledgement()` changes the behavior, doesn't it?

##
File path: java/org/apache/coyote/AbstractProtocol.java
##
@@ -262,6 +267,18 @@ public void setConnectionLinger(int connectionLinger) {
 endpoint.setConnectionLinger(connectionLinger);
 }
 
+//  HTTP specific 
properties
+// - queried by 
StandardContextValve
+public void setContinueHandlingResponsePolicy(String value) {
+continueHandlingResponsePolicy = 
Enum.valueOf(ContinueHandlingResponsePolicy.class, value);
+}
+
+
+//todo(malay) move to AbstractProtocol

Review comment:
   What does `malay` mean? Like Malay language?

##
File path: java/org/apache/coyote/Request.java
##
@@ -709,4 +718,13 @@ private static String getCharsetFromContentType(String 
contentType) {
 
 return encoding.trim();
 }
+
+/**
+ * set whether to acknowledge the request when the request body is
+ * first read from
+ * @param acknowledgeOnFirstRead the value to set
+ */
+public void setAcknowledgeOnRequestBodyRead(boolean 
acknowledgeOnFirstRead) {

Review comment:
   Why does the arg has a different name than the instane field?

##
File path: test/org/apache/catalina/core/TestStandardContextValve.java
##
@@ -182,4 +184,66 @@ public void requestDestroyed(ServletRequestEvent sre) {
 }
 
 }
+
+@Test
+public void test100ContinueDefaultPolicy() throws Exception {
+// the