buildbot success in on tomcat-trunk

[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/4754

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] 9d7cb5468fbf2df4709c222b472bd86a26c9d4b6
Blamelist: remm 

Build succeeded!

Sincerely,
 -The Buildbot




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

[tomcat] branch master updated: Fix test

[remm]

This is an automated email from the ASF dual-hosted git repository.

remm 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 9d7cb54  Fix test
9d7cb54 is described below

commit 9d7cb5468fbf2df4709c222b472bd86a26c9d4b6
Author: remm 
AuthorDate: Tue Nov 19 23:25:09 2019 +0100

Fix test
---
 test/org/apache/catalina/filters/TestCsrfPreventionFilter.java | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/org/apache/catalina/filters/TestCsrfPreventionFilter.java 
b/test/org/apache/catalina/filters/TestCsrfPreventionFilter.java
index 6d0c81d..1e74313 100644
--- a/test/org/apache/catalina/filters/TestCsrfPreventionFilter.java
+++ b/test/org/apache/catalina/filters/TestCsrfPreventionFilter.java
@@ -37,7 +37,7 @@ public class TestCsrfPreventionFilter extends TomcatBaseTest {
 
 private final HttpServletResponse wrapper =
 new CsrfPreventionFilter.CsrfResponseWrapper(
-new NonEncodingResponse(), "TESTNONCE");
+new NonEncodingResponse(), 
Constants.CSRF_NONCE_SESSION_ATTR_NAME, "TESTNONCE");
 
 @Test
 public void testAddNonceNoQueryNoAnchor() throws Exception {


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

[Bug 63859] AJP cping/cpong mode failing on Tomcat 9.x

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=63859

--- Comment #19 from Mark Thomas  ---
Thanks for those logs. The Tomcat debug logs were useful. I'm now reasonably
sure Tomcat is closing the connection because of an IOException. However, the
logs don't show the exception. I have extended the debug logging to include
this exception in 8.5.x and 9.0.x.

To save you building from source (I wasn't sure whether you were set up for
that) I've uploaded the latest build here:
http://people.apache.org/~markt/dev/apache-tomcat-9.0.30-dev.tar.gz

Usual caveats apply. This isn't an official release. Use it at your own risk
and if your server catches fire it isn't our fault ;)

I really do appreciate your willingness to help track this down. Thanks again.

-- 
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

[tomcat] branch master updated: Refactor endpoint close/destroySocket API

[remm]

This is an automated email from the ASF dual-hosted git repository.

remm 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 a8388e7  Refactor endpoint close/destroySocket API
a8388e7 is described below

commit a8388e72ec1fe27d55d7b5a2b3aa01032ebb18d5
Author: remm 
AuthorDate: Tue Nov 19 23:18:57 2019 +0100

Refactor endpoint close/destroySocket API

These APIs exist mostly for APR, but is actually useful for cleanup for
errors which occur during setSocketOptions.
The API is refactored to the following behavior which likely corresponds
to what was originally intended:
- closeSocket: close the socket through the wrapper using the connection
to look it up in the map
- destroySocket: low level close when no wrapper exists, this happens
when accepting while the connector is not started, or setSocketOptions
has an unexpected error before the wrapper is created
For NIOx, ensure that setSocketOptions errors call destorySocket if an
error occurs before the wrapper is associated with the connection
(closeSocket will do nothing in that case).
For APR, refactor to use the wrapper close in all cases (except the use
in the acceptor, like for the other connectors), the poller will then
call the low level destroy once that close is done.
---
 .../apache/tomcat/util/net/AbstractEndpoint.java   | 26 --
 java/org/apache/tomcat/util/net/AprEndpoint.java   | 96 +-
 java/org/apache/tomcat/util/net/Nio2Endpoint.java  | 30 ---
 java/org/apache/tomcat/util/net/NioEndpoint.java   | 38 -
 .../apache/tomcat/util/net/SocketWrapperBase.java  |  1 -
 webapps/docs/changelog.xml |  8 ++
 6 files changed, 100 insertions(+), 99 deletions(-)

diff --git a/java/org/apache/tomcat/util/net/AbstractEndpoint.java 
b/java/org/apache/tomcat/util/net/AbstractEndpoint.java
index 8f216c4..12b8a25 100644
--- a/java/org/apache/tomcat/util/net/AbstractEndpoint.java
+++ b/java/org/apache/tomcat/util/net/AbstractEndpoint.java
@@ -189,9 +189,9 @@ public abstract class AbstractEndpoint {
 private ObjectName oname = null;
 
 /**
- * Map holding all current connections keyed with the sockets (for APR).
+ * Map holding all current connections keyed with the sockets.
  */
-protected Map> connections = new 
ConcurrentHashMap<>();
+protected Map> connections = new 
ConcurrentHashMap<>();
 
 /**
  * Get a set with the current open connections.
@@ -1355,14 +1355,24 @@ public abstract class AbstractEndpoint {
 
 /**
  * Close the socket when the connection has to be immediately closed when
- * an error occurs while configuring the accepted socket, allocating
- * a wrapper for the socket, or trying to dispatch it for processing.
+ * an error occurs while configuring the accepted socket or trying to
+ * dispatch it for processing. The wrapper associated with the socket will
+ * be used for the close.
  * @param socket The newly accepted socket
  */
-protected abstract void closeSocket(U socket);
-
-protected void destroySocket(U socket) {
-closeSocket(socket);
+protected void closeSocket(U socket) {
+SocketWrapperBase socketWrapper = connections.get(socket);
+if (socketWrapper != null) {
+socketWrapper.close();
+}
 }
+
+/**
+ * Close the socket. This is used when the connector is not in a state
+ * which allows processing the socket, or if there was an error which
+ * prevented the allocation of the socket wrapper.
+ * @param socket The newly accepted socket
+ */
+protected abstract void destroySocket(U socket);
 }
 
diff --git a/java/org/apache/tomcat/util/net/AprEndpoint.java 
b/java/org/apache/tomcat/util/net/AprEndpoint.java
index 146a816..31b811b 100644
--- a/java/org/apache/tomcat/util/net/AprEndpoint.java
+++ b/java/org/apache/tomcat/util/net/AprEndpoint.java
@@ -695,8 +695,6 @@ public class AprEndpoint extends 
AbstractEndpoint implements SNICallB
 // the pool and its queue are full
 log.error(sm.getString("endpoint.process.fail"), t);
 }
-// Note: doing connections.remove is not needed since closeSocket will
-// be called, which then calls close on the wrapper
 return false;
 }
 
@@ -744,43 +742,19 @@ public class AprEndpoint extends 
AbstractEndpoint implements SNICallB
 }
 
 
-@Override
-protected void closeSocket(Long socket) {
-closeSocket(socket.longValue());
-}
-
-
-private void closeSocket(long socket) {
-// Once this is called, the mapping from socket to wrapper will no
-// longer be required.
-SocketWrapperBase wrapper = 
connections.remove(Long.valueOf(socket));
-if (wrapper != null) {
-// Cast to avoid having to 

[tomcat] branch 8.5.x updated: Additional debug logging to investigate bug 63859

[markt]

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 e92da8e  Additional debug logging to investigate bug 63859
e92da8e is described below

commit e92da8e26313e12b91112b9a083efb5f9045cfab
Author: Mark Thomas 
AuthorDate: Tue Nov 19 22:07:59 2019 +

Additional debug logging to investigate bug 63859
---
 java/org/apache/coyote/ajp/AjpProcessor.java | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/java/org/apache/coyote/ajp/AjpProcessor.java 
b/java/org/apache/coyote/ajp/AjpProcessor.java
index 81ab0b7..c827455 100644
--- a/java/org/apache/coyote/ajp/AjpProcessor.java
+++ b/java/org/apache/coyote/ajp/AjpProcessor.java
@@ -426,6 +426,9 @@ public class AjpProcessor extends AbstractProcessor {
 socketWrapper.write(true, pongMessageArray, 0, 
pongMessageArray.length);
 socketWrapper.flush(true);
 } catch (IOException e) {
+if (getLog().isDebugEnabled()) {
+getLog().debug("Pong message failed", e);
+}
 setErrorState(ErrorState.CLOSE_CONNECTION_NOW, e);
 }
 recycle();


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

[tomcat] branch master updated: Additional debug logging to investigate bug 63859

[markt]

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 203057c  Additional debug logging to investigate bug 63859
203057c is described below

commit 203057cd1fdab6896b0b0718ca8cee1caed22bbc
Author: Mark Thomas 
AuthorDate: Tue Nov 19 22:07:59 2019 +

Additional debug logging to investigate bug 63859
---
 java/org/apache/coyote/ajp/AjpProcessor.java | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/java/org/apache/coyote/ajp/AjpProcessor.java 
b/java/org/apache/coyote/ajp/AjpProcessor.java
index 280a340..a3e628d 100644
--- a/java/org/apache/coyote/ajp/AjpProcessor.java
+++ b/java/org/apache/coyote/ajp/AjpProcessor.java
@@ -343,6 +343,9 @@ public class AjpProcessor extends AbstractProcessor {
 socketWrapper.write(true, pongMessageArray, 0, 
pongMessageArray.length);
 socketWrapper.flush(true);
 } catch (IOException e) {
+if (getLog().isDebugEnabled()) {
+getLog().debug("Pong message failed", e);
+}
 setErrorState(ErrorState.CLOSE_CONNECTION_NOW, e);
 }
 recycle();


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

Re: [VOTE] Release Apache Tomcat 8.5.49

[Mark Thomas]

On 19/11/2019 19:29, Rémy Maucherat wrote:
> On Tue, Nov 19, 2019 at 5:58 PM Mark Thomas  > wrote:
> 
> On 19/11/2019 00:44, Konstantin Kolinko wrote:



> > I think the single pollset change should not be backported to
> Tomcat 7.
> > I am OK with it being backported to Tomcat 8.5.
> 
> In favour of back-porting:
> - It keeps the code (more) aligned so future maintenance is easier
> - It is more obvious where previous APR fixes have not been back-ported
>   (as well as being easier to back-port them). I can see a handful of
>   fixes it would be worth back-porting
> - Windows XP / Server 2003 are out of support. I think it would be
>   unusual for a user to be keeping Tomcat up to date but not the OS.
> - sendfile is already using a larger pollset size and we have had no
>   complaints (that I recall)
> 
> I've just finished back-porting the single pollset change. I want to
> backport the additional fixes I can now see and then run the unit tests.
> 
> I'd like to then commit these changes but I can do it via a PR if folks
> want a chance to review them first.
> 
> 
> +1 for attempting it, if you're comfortable about it it should be fine.

FYI, the single pollset changes back-ported easily along with one other
fix. The other fixes were not as clean so I decided to leave them for now.

Mark

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

Re: [VOTE] Release Apache Tomcat 8.5.49

[Rémy Maucherat]

On Tue, Nov 19, 2019 at 5:58 PM Mark Thomas  wrote:

> On 19/11/2019 00:44, Konstantin Kolinko wrote:
> > вт, 19 нояб. 2019 г. в 01:42, Mark Thomas :
> >>
> >> On 18/11/2019 22:01, Rémy Maucherat wrote:
> >>> On Mon, Nov 18, 2019 at 10:22 PM Mark Thomas  >>> > wrote:
> >>
> >>> Is porting the multipoller removal to 7.0 really doable ?
> >>
> >> I don't know. I haven't looked at the code yet. I do have an alternative
> >> fix I tested that should work if back-porting proves tricky.
> >>
> >>> It could be running on older Windows platforms maybe.
> >>
> >> That would only be an issue of the versions of Windows were so old they
> >> were no longer supported by Microsoft. Users in those circumstances are
> >> unlikely to be updating to the latest 7.0.x release anyway. Therefore
> >> that isn't something I'm particularly worrying about. (But I am happy to
> >> be over-ruled if the consensus is not to back-port to 7.0.x.)
> >
> > Searching my mailbox and archives at https://tomcat.markmail.org/,
> >
> > a) Tomcat 7.0.96 running on Windows XP  was mentioned recently (August
> 2019)
> > https://bz.apache.org/bugzilla/show_bug.cgi?id=63625#c16
> >
> > b) Tomcat 8.5 was never mentioned running on Windows XP or on Windows
> 2003.
> > There was one thread about Tomcat 8.5.3 when a client was using
> > Windows XP, but Tomcat was running on Ubuntu.
> > There was one thread in year 2017 about posting an application from an
> > old server running Windows 2003 and Tomcat 5 to Tomcat 8.5.
> > https://markmail.org/thread/qvmzmmjz2dqohbk5
> >
> > I think the single pollset change should not be backported to Tomcat 7.
> > I am OK with it being backported to Tomcat 8.5.
>
> In favour of back-porting:
> - It keeps the code (more) aligned so future maintenance is easier
> - It is more obvious where previous APR fixes have not been back-ported
>   (as well as being easier to back-port them). I can see a handful of
>   fixes it would be worth back-porting
> - Windows XP / Server 2003 are out of support. I think it would be
>   unusual for a user to be keeping Tomcat up to date but not the OS.
> - sendfile is already using a larger pollset size and we have had no
>   complaints (that I recall)
>
> I've just finished back-porting the single pollset change. I want to
> backport the additional fixes I can now see and then run the unit tests.
>
> I'd like to then commit these changes but I can do it via a PR if folks
> want a chance to review them first.
>

+1 for attempting it, if you're comfortable about it it should be fine.

Rémy

buildbot failure in on tomcat-trunk

[buildbot]

The Buildbot has detected a new failure on builder tomcat-trunk while building 
tomcat. Full details are available at:
https://ci.apache.org/builders/tomcat-trunk/builds/4751

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] 1f5b578669cd016d599d711f48d28e53573c72d1
Blamelist: Christopher Schultz 

BUILD FAILED: failed compile_1

Sincerely,
 -The Buildbot




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

[tomcat] branch master updated: Adjust changelog to reflect which releases actually contain which improvements to the CSRF prevention filter.

[schultz]

This is an automated email from the ASF dual-hosted git repository.

schultz 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 1f5b578  Adjust changelog to reflect which releases actually contain 
which improvements to the CSRF prevention filter.
1f5b578 is described below

commit 1f5b578669cd016d599d711f48d28e53573c72d1
Author: Christopher Schultz 
AuthorDate: Tue Nov 19 13:03:14 2019 -0500

Adjust changelog to reflect which releases actually contain which
improvements to the CSRF prevention filter.
---
 webapps/docs/changelog.xml | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 84377f6..3f70beb 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -52,9 +52,8 @@
 example in the JSP section of the examples web application. (markt)
   
   
-Improvements to CsrfPreventionFilter including additional
-logging, making the latest nonce available in the request attributes,
-and allowing the CSRF nonce request parameter name to be customized.
+Improvements to CsrfPreventionFilter: additional logging, allow the
+CSRF nonce request parameter name to be customized.
 (schultz)
   
 
@@ -66,6 +65,12 @@
   
Refactor JMX remote RMI registry creation. (remm)
   
+  
+Improvement to CsrfPreventionFilter: expose the latest available nonce
+as a request attribute; expose the expected nonce request parameter
+name as a context attribute.
+(schultz)
+  
 
   
   


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

[tomcat] branch master updated: Move initialization of CSRF REST nonce header name context attribute into the RestCsrfPreventionFilter where it belongs.

[schultz]

This is an automated email from the ASF dual-hosted git repository.

schultz 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 f651d87  Move initialization of CSRF REST nonce header name context 
attribute into the RestCsrfPreventionFilter where it belongs.
f651d87 is described below

commit f651d87638fe53f7f03a6b52f0570f38cd03
Author: Christopher Schultz 
AuthorDate: Tue Nov 19 12:57:23 2019 -0500

Move initialization of CSRF REST nonce header name context attribute into 
the RestCsrfPreventionFilter where it belongs.
---
 java/org/apache/catalina/filters/CsrfPreventionFilter.java   |  5 -
 .../apache/catalina/filters/RestCsrfPreventionFilter.java| 12 
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/java/org/apache/catalina/filters/CsrfPreventionFilter.java 
b/java/org/apache/catalina/filters/CsrfPreventionFilter.java
index 8a09cfb..621cd6d 100644
--- a/java/org/apache/catalina/filters/CsrfPreventionFilter.java
+++ b/java/org/apache/catalina/filters/CsrfPreventionFilter.java
@@ -104,11 +104,6 @@ public class CsrfPreventionFilter extends 
CsrfPreventionFilterBase {
 filterConfig.getServletContext().setAttribute(
 Constants.CSRF_NONCE_REQUEST_PARAM_NAME_KEY,
 nonceRequestParameterName);
-
-// Put the expected request header name into the application scope
-filterConfig.getServletContext().setAttribute(
-Constants.CSRF_REST_NONCE_HEADER_NAME_KEY,
-Constants.CSRF_REST_NONCE_HEADER_NAME);
 }
 
 @Override
diff --git a/java/org/apache/catalina/filters/RestCsrfPreventionFilter.java 
b/java/org/apache/catalina/filters/RestCsrfPreventionFilter.java
index 649464b..44f5da1 100644
--- a/java/org/apache/catalina/filters/RestCsrfPreventionFilter.java
+++ b/java/org/apache/catalina/filters/RestCsrfPreventionFilter.java
@@ -25,6 +25,7 @@ import java.util.function.Predicate;
 import java.util.regex.Pattern;
 
 import javax.servlet.FilterChain;
+import javax.servlet.FilterConfig;
 import javax.servlet.ServletException;
 import javax.servlet.ServletRequest;
 import javax.servlet.ServletResponse;
@@ -89,6 +90,17 @@ public class RestCsrfPreventionFilter extends 
CsrfPreventionFilterBase {
 private String pathsDelimiter = ",";
 
 @Override
+public void init(FilterConfig filterConfig) throws ServletException {
+// Set the parameters
+super.init(filterConfig);
+
+// Put the expected request header name into the application scope
+filterConfig.getServletContext().setAttribute(
+Constants.CSRF_REST_NONCE_HEADER_NAME_KEY,
+Constants.CSRF_REST_NONCE_HEADER_NAME);
+}
+
+@Override
 public void doFilter(ServletRequest request, ServletResponse response, 
FilterChain chain)
 throws IOException, ServletException {
 


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

Re: [tomcat] branch master updated: Add missing changelog for CSRF prevention filter changes.

[Christopher Schultz]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

Mark,

On 11/19/19 12:41, Mark Thomas wrote:
>> All,
>> 
>> This claims that these changes are being added in 9.0.30 which is
>> only partially correct. Some of these changes were made in 9.0.29
>> which is currently under release-vote.
>> 
>> I wasn't sure how everyone felt about me changing the changelog
>> for a release that is already kind of "fixed". I"m happy to do
>> any of the following:
>> 
>> a. Ignore the oversight; leave the items in the changelog for
>> 9.0.30 b. Split the items into 9.0.29 and 9.0.30 to reflect
>> reality c. Make a note in 9.0.30 changelog that some items were
>> really done in 9.0.29
> 
> I very much prefer b). I don't see the harm in updating a
> changelog after a release to reflect reality. It helps future users
> figure out what fix is in what version.

ACK

I also prefer (b) so I will proceed.

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

iQIzBAEBCAAdFiEEMmKgYcQvxMe7tcJcHPApP6U8pFgFAl3ULU0ACgkQHPApP6U8
pFiuARAApe7I3Q7FbnXAcYWtv9iWAGyGa4sULVMpDq8dJkpneCvCLqsXkQnx+yZl
dYfTKMwpoYHnXh+JeymWdIII/qfYHMMc8QvJ0xykNPJ7vB7hbE3qYkmn1dXwOjC0
3xxvvlqRxLRLiHPeU4jMpogblxhap/+fPQjRBxKWx+W1n3n65+Sb+G3jGxRspYrZ
dzJ9ifx0VEY8ol41eVu/oZbBtFnC7vk8K80Jt2PNo9tul65tB9twC8cL0olMQT1g
sJKmWUJQclpOgfNNshhy84Jzsk3gcVBtGjRviLXiPP8Rdj6V2PqkyxZGX5u7/ZoT
oYt9rYzaKI/zdDuHfTa2b2DIbju1sthDxWdWrThf1MCjf6kAqSw+3zV1oFvoK4At
4vi5/fmqVbhYb2WCs0f5lUCk6V2QhTtuekb3j16M158hqxYXoqLY4oI+UYxEpz8a
EjAzHW7iyxbmABxGRbY4aVOLXLfm+WEntL4W2ekJ6F93r2BaRqac5+8qMk4PEsEt
P9upNXXFknzo1YxPvJfzvlU2ZRbXOCk8F9aop+eQSgo//YwJueag6gr1oZK+4noQ
XnFv+lDaRKfhJP0/U+MBsSVdIe1o8a2drVBYbfVBE2+lmZpBrVq+0eF1e/mH8w6A
AXqWhMZAThsAcWsbSSI95a4bX3Gn9v05ICituj4BuAVh6qowOps=
=QHUv
-END PGP SIGNATURE-

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

[tomcat] branch master updated: Allow customization of the CSRF prevention filter's request parameter name.

[schultz]

This is an automated email from the ASF dual-hosted git repository.

schultz 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 707e194  Allow customization of the CSRF prevention filter's request 
parameter name.
707e194 is described below

commit 707e1949cb4a9b7dc1430a41e28b8c72675dcced
Author: Christopher Schultz 
AuthorDate: Tue Nov 19 12:54:45 2019 -0500

Allow customization of the CSRF prevention filter's request parameter name.
---
 .../catalina/filters/CsrfPreventionFilter.java | 24 +-
 webapps/docs/changelog.xml |  5 +++--
 2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/java/org/apache/catalina/filters/CsrfPreventionFilter.java 
b/java/org/apache/catalina/filters/CsrfPreventionFilter.java
index 8aace6b..8a09cfb 100644
--- a/java/org/apache/catalina/filters/CsrfPreventionFilter.java
+++ b/java/org/apache/catalina/filters/CsrfPreventionFilter.java
@@ -53,6 +53,8 @@ public class CsrfPreventionFilter extends 
CsrfPreventionFilterBase {
 
 private int nonceCacheSize = 5;
 
+private String nonceRequestParameterName = 
Constants.CSRF_NONCE_REQUEST_PARAM;
+
 /**
  * Entry points are URLs that will not be tested for the presence of a 
valid
  * nonce. They are used to provide a way to navigate back to a protected
@@ -83,6 +85,16 @@ public class CsrfPreventionFilter extends 
CsrfPreventionFilterBase {
 this.nonceCacheSize = nonceCacheSize;
 }
 
+/**
+ * Sets the request parameter name to use for CSRF nonces.
+ *
+ * @param parameterName The request parameter name to use
+ *for CSRF nonces.
+ */
+public void setNonceRequestParameterName(String parameterName) {
+this.nonceRequestParameterName = parameterName;
+}
+
 @Override
 public void init(FilterConfig filterConfig) throws ServletException {
 // Set the parameters
@@ -91,7 +103,7 @@ public class CsrfPreventionFilter extends 
CsrfPreventionFilterBase {
 // Put the expected request parameter name into the application scope
 filterConfig.getServletContext().setAttribute(
 Constants.CSRF_NONCE_REQUEST_PARAM_NAME_KEY,
-Constants.CSRF_NONCE_REQUEST_PARAM);
+nonceRequestParameterName);
 
 // Put the expected request header name into the application scope
 filterConfig.getServletContext().setAttribute(
@@ -131,7 +143,7 @@ public class CsrfPreventionFilter extends 
CsrfPreventionFilterBase {
 
 if (!skipNonceCheck) {
 String previousNonce =
-req.getParameter(Constants.CSRF_NONCE_REQUEST_PARAM);
+req.getParameter(nonceRequestParameterName);
 
 if(previousNonce == null) {
 if(log.isDebugEnabled()) {
@@ -196,7 +208,7 @@ public class CsrfPreventionFilter extends 
CsrfPreventionFilterBase {
 // requiring the use of response.encodeURL.
 request.setAttribute(Constants.CSRF_NONCE_REQUEST_ATTR_NAME, 
newNonce);
 
-wResponse = new CsrfResponseWrapper(res, newNonce);
+wResponse = new CsrfResponseWrapper(res, 
nonceRequestParameterName, newNonce);
 } else {
 wResponse = response;
 }
@@ -208,10 +220,12 @@ public class CsrfPreventionFilter extends 
CsrfPreventionFilterBase {
 protected static class CsrfResponseWrapper
 extends HttpServletResponseWrapper {
 
+private final String nonceRequestParameterName;
 private final String nonce;
 
-public CsrfResponseWrapper(HttpServletResponse response, String nonce) 
{
+public CsrfResponseWrapper(HttpServletResponse response, String 
nonceRequestParameterName, String nonce) {
 super(response);
+this.nonceRequestParameterName = nonceRequestParameterName;
 this.nonce = nonce;
 }
 
@@ -266,7 +280,7 @@ public class CsrfPreventionFilter extends 
CsrfPreventionFilterBase {
 } else {
 sb.append('?');
 }
-sb.append(Constants.CSRF_NONCE_REQUEST_PARAM);
+sb.append(nonceRequestParameterName);
 sb.append('=');
 sb.append(nonce);
 sb.append(anchor);
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 42dbde9..84377f6 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -53,8 +53,9 @@
   
   
 Improvements to CsrfPreventionFilter including additional
-logging and making the latest nonce available in the request
-attributes. (schultz)
+logging, making the latest nonce available in the request attributes,
+and allowing the CSRF nonce request parameter name to be customized.
+(schultz)
   
 
   

buildbot success in on tomcat-trunk

[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/4749

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] cab2a8eaf142e80aee92d287b464bf8540828b1a
Blamelist: Christopher Schultz 

Build succeeded!

Sincerely,
 -The Buildbot




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

Re: [tomcat] branch master updated: Add missing changelog for CSRF prevention filter changes.

[Mark Thomas]

> All,
> 
> This claims that these changes are being added in 9.0.30 which is only
> partially correct. Some of these changes were made in 9.0.29 which is
> currently under release-vote.
> 
> I wasn't sure how everyone felt about me changing the changelog for a
> release that is already kind of "fixed". I"m happy to do any of the
> following:
> 
> a. Ignore the oversight; leave the items in the changelog for 9.0.30
> b. Split the items into 9.0.29 and 9.0.30 to reflect reality
> c. Make a note in 9.0.30 changelog that some items were really done in
> 9.0.29

I very much prefer b). I don't see the harm in updating a changelog
after a release to reflect reality. It helps future users figure out
what fix is in what version.

Mark

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

Re: [tomcat] branch master updated: Add missing changelog for CSRF prevention filter changes.

[Christopher Schultz]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

All,

This claims that these changes are being added in 9.0.30 which is only
partially correct. Some of these changes were made in 9.0.29 which is
currently under release-vote.

I wasn't sure how everyone felt about me changing the changelog for a
release that is already kind of "fixed". I"m happy to do any of the
following:

a. Ignore the oversight; leave the items in the changelog for 9.0.30
b. Split the items into 9.0.29 and 9.0.30 to reflect reality
c. Make a note in 9.0.30 changelog that some items were really done in
9.0.29

The same will be true for the 8.5.x release. I have not yet
back-ported these changes and want to make sure that I do things
"properly".

Thanks,
- -chris

On 11/19/19 12:32, schu...@apache.org wrote:
> This is an automated email from the ASF dual-hosted git
> repository.
> 
> schultz 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 decb12b  Add missing changelog for CSRF prevention filter
> changes. decb12b is described below
> 
> commit decb12be68b2fc93284b0b8cc44fb53a16110eb0 Author: Christopher
> Schultz  AuthorDate: Tue Nov 19
> 12:31:56 2019 -0500
> 
> Add missing changelog for CSRF prevention filter changes. --- 
> webapps/docs/changelog.xml | 5 + 1 file changed, 5
> insertions(+)
> 
> diff --git a/webapps/docs/changelog.xml
> b/webapps/docs/changelog.xml index a0915f9..42dbde9 100644 ---
> a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@
> -51,6 +51,11 @@ Fix the broken re-try link on the error page for
> the FORM authentication example in the JSP section of the examples
> web application. (markt)  +   +Improvements
> to CsrfPreventionFilter including additional +logging and
> making the latest nonce available in the request +
> attributes. (schultz) + 
> 
> 
> 
> -
>
> 
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
> 
-BEGIN PGP SIGNATURE-
Comment: Using GnuPG with Thunderbird - https://www.enigmail.net/

iQIzBAEBCAAdFiEEMmKgYcQvxMe7tcJcHPApP6U8pFgFAl3UJ9wACgkQHPApP6U8
pFh4GA/9H/bJjB+bWDYxMtYIpWJ34E3A7Bm4fcUOXCQKdcrkoNSPLKWpvM6CXrlc
UA4j10vkCj9urwQQDCNdoQwhANDb7PRu85iV5usrqC1Eic+8Clp+JZeV1zk4MFEc
5zTXUPE1U2nXyfphwK0oKAXbXZJYtv4H8XMToHoCRegOFjpmPinW/UnzFZZ+rfSI
60/QTuCVe4PQ5sjYTcc2S/V3kRYFgFO80GI3fm38MLUGSOfZQJLS6nFqhKgV0112
vxbBrRQqrYTLOcgCzSNhHhlWbe1i4mwUblGbAKCamg9bHYIiOje798fKfKcRT2zA
jkuKjako7C/A7Cr22iiRZgWwtSS4qn97SVNW5KeKJvzEbVb5MmPG5b782glL/u1F
NCica+XZhCeO/jes3CC3Vs/fPJ/ZwwZgDVUAXGSZjZZ+VYOzoEnva1IbcTIZ0g7a
uMDtzHxG1QxSHLWE6bmAQvBHop2bc5X5y0C88MaLKTiKGYnENpsRm+RXwYXqR6tY
/Wm0yK16cep9JvLhtLpKJa1HvDIpHFBCp2aXSmb9BbGD2JBkscTY1JM9ZaBZSiTB
oYPcQChlmxkzbmc559ufi1GoDDtlPFxxKpg7HYhTjBn/6A3U0DN6US0A3DM2pXMk
emmEFy7PRTq38D8HIc9JfYE77e90KNOn3STBUtboC2EYgQ7sl6M=
=C2Dq
-END PGP SIGNATURE-

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

[tomcat] branch master updated: Add missing changelog for CSRF prevention filter changes.

[schultz]

This is an automated email from the ASF dual-hosted git repository.

schultz 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 decb12b  Add missing changelog for CSRF prevention filter changes.
decb12b is described below

commit decb12be68b2fc93284b0b8cc44fb53a16110eb0
Author: Christopher Schultz 
AuthorDate: Tue Nov 19 12:31:56 2019 -0500

Add missing changelog for CSRF prevention filter changes.
---
 webapps/docs/changelog.xml | 5 +
 1 file changed, 5 insertions(+)

diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index a0915f9..42dbde9 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -51,6 +51,11 @@
 Fix the broken re-try link on the error page for the FORM 
authentication
 example in the JSP section of the examples web application. (markt)
   
+  
+Improvements to CsrfPreventionFilter including additional
+logging and making the latest nonce available in the request
+attributes. (schultz)
+  
 
   
 


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

[tomcat] branch master updated: Post-review from kkolonko: fix typo in constant name, push initialization down into subclass.

[schultz]

This is an automated email from the ASF dual-hosted git repository.

schultz 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 cab2a8e  Post-review from kkolonko: fix typo in constant name, push 
initialization down into subclass.
cab2a8e is described below

commit cab2a8eaf142e80aee92d287b464bf8540828b1a
Author: Christopher Schultz 
AuthorDate: Tue Nov 19 12:25:32 2019 -0500

Post-review from kkolonko: fix typo in constant name, push initialization 
down into subclass.
---
 java/org/apache/catalina/filters/Constants.java |  2 +-
 .../apache/catalina/filters/CsrfPreventionFilter.java   | 17 +
 .../catalina/filters/CsrfPreventionFilterBase.java  | 10 --
 3 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/java/org/apache/catalina/filters/Constants.java 
b/java/org/apache/catalina/filters/Constants.java
index 87dd6c4..ab550b1 100644
--- a/java/org/apache/catalina/filters/Constants.java
+++ b/java/org/apache/catalina/filters/Constants.java
@@ -72,6 +72,6 @@ public final class Constants {
  * The servlet context attribute key under which the
  * CSRF REST header name can be found.
  */
-public static final String CSRF_REST_NONCE_HEDAER_NAME_KEY =
+public static final String CSRF_REST_NONCE_HEADER_NAME_KEY =
 "org.apache.catalina.filters.CSRF_REST_NONCE_HEADER_NAME";
 }
diff --git a/java/org/apache/catalina/filters/CsrfPreventionFilter.java 
b/java/org/apache/catalina/filters/CsrfPreventionFilter.java
index 369745b..8aace6b 100644
--- a/java/org/apache/catalina/filters/CsrfPreventionFilter.java
+++ b/java/org/apache/catalina/filters/CsrfPreventionFilter.java
@@ -24,6 +24,7 @@ import java.util.Map;
 import java.util.Set;
 
 import javax.servlet.FilterChain;
+import javax.servlet.FilterConfig;
 import javax.servlet.ServletException;
 import javax.servlet.ServletRequest;
 import javax.servlet.ServletResponse;
@@ -83,6 +84,22 @@ public class CsrfPreventionFilter extends 
CsrfPreventionFilterBase {
 }
 
 @Override
+public void init(FilterConfig filterConfig) throws ServletException {
+// Set the parameters
+super.init(filterConfig);
+
+// Put the expected request parameter name into the application scope
+filterConfig.getServletContext().setAttribute(
+Constants.CSRF_NONCE_REQUEST_PARAM_NAME_KEY,
+Constants.CSRF_NONCE_REQUEST_PARAM);
+
+// Put the expected request header name into the application scope
+filterConfig.getServletContext().setAttribute(
+Constants.CSRF_REST_NONCE_HEADER_NAME_KEY,
+Constants.CSRF_REST_NONCE_HEADER_NAME);
+}
+
+@Override
 public void doFilter(ServletRequest request, ServletResponse response,
 FilterChain chain) throws IOException, ServletException {
 
diff --git a/java/org/apache/catalina/filters/CsrfPreventionFilterBase.java 
b/java/org/apache/catalina/filters/CsrfPreventionFilterBase.java
index 8d401af..c0083f0 100644
--- a/java/org/apache/catalina/filters/CsrfPreventionFilterBase.java
+++ b/java/org/apache/catalina/filters/CsrfPreventionFilterBase.java
@@ -78,16 +78,6 @@ public abstract class CsrfPreventionFilterBase extends 
FilterBase {
 // Set the parameters
 super.init(filterConfig);
 
-// Put the expected request parameter name into the application scope
-filterConfig.getServletContext().setAttribute(
-Constants.CSRF_NONCE_REQUEST_PARAM_NAME_KEY,
-Constants.CSRF_NONCE_REQUEST_PARAM);
-
-// Put the expected request header name into the application scope
-filterConfig.getServletContext().setAttribute(
-Constants.CSRF_REST_NONCE_HEDAER_NAME_KEY,
-Constants.CSRF_REST_NONCE_HEADER_NAME);
-
 try {
 Class clazz = Class.forName(randomClass);
 randomSource = (Random) clazz.getConstructor().newInstance();


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

[tomcat] branch master updated: Add logging to CSRF prevention listener.

[schultz]

This is an automated email from the ASF dual-hosted git repository.

schultz 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 a783e4e  Add logging to CSRF prevention listener.
a783e4e is described below

commit a783e4e7ff4c532e67d9dee826cf562b78147818
Author: Christopher Schultz 
AuthorDate: Sat Nov 16 11:40:47 2019 -0500

Add logging to CSRF prevention listener.
---
 .../catalina/filters/CsrfPreventionFilter.java | 50 +-
 1 file changed, 48 insertions(+), 2 deletions(-)

diff --git a/java/org/apache/catalina/filters/CsrfPreventionFilter.java 
b/java/org/apache/catalina/filters/CsrfPreventionFilter.java
index d94cdec..369745b 100644
--- a/java/org/apache/catalina/filters/CsrfPreventionFilter.java
+++ b/java/org/apache/catalina/filters/CsrfPreventionFilter.java
@@ -32,6 +32,9 @@ import javax.servlet.http.HttpServletResponse;
 import javax.servlet.http.HttpServletResponseWrapper;
 import javax.servlet.http.HttpSession;
 
+import org.apache.juli.logging.Log;
+import org.apache.juli.logging.LogFactory;
+
 /**
  * Provides basic CSRF protection for a web application. The filter assumes
  * that:
@@ -43,6 +46,7 @@ import javax.servlet.http.HttpSession;
  * 
  */
 public class CsrfPreventionFilter extends CsrfPreventionFilterBase {
+private final Log log = LogFactory.getLog(CsrfPreventionFilter.class);
 
 private final Set entryPoints = new HashSet<>();
 
@@ -94,6 +98,10 @@ public class CsrfPreventionFilter extends 
CsrfPreventionFilterBase {
 
 if (Constants.METHOD_GET.equals(req.getMethod())
 && entryPoints.contains(getRequestedPath(req))) {
+if(log.isTraceEnabled()) {
+log.trace("Skipping CSRF nonce-check for GET request to 
entry point " + getRequestedPath(req));
+}
+
 skipNonceCheck = true;
 }
 
@@ -108,16 +116,54 @@ public class CsrfPreventionFilter extends 
CsrfPreventionFilterBase {
 String previousNonce =
 req.getParameter(Constants.CSRF_NONCE_REQUEST_PARAM);
 
-if (nonceCache == null || previousNonce == null ||
-!nonceCache.contains(previousNonce)) {
+if(previousNonce == null) {
+if(log.isDebugEnabled()) {
+log.debug("Rejecting request for " + 
getRequestedPath(req)
+  + ", session "
+  + (null == session ? "(none)" : 
session.getId())
+  + " with no CSRF nonce found in request");
+}
+
+res.sendError(getDenyStatus());
+return;
+} else if(nonceCache == null) {
+if(log.isDebugEnabled()) {
+log.debug("Rejecting request for " + 
getRequestedPath(req)
+  + ", session "
+  + (null == session ? "(none)" : 
session.getId())
+  + " due to empty / missing nonce cache");
+}
+
 res.sendError(getDenyStatus());
 return;
+} else if(!nonceCache.contains(previousNonce)) {
+if(log.isDebugEnabled()) {
+log.debug("Rejecting request for " + 
getRequestedPath(req)
+  + ", session "
+  + (null == session ? "(none)" : 
session.getId())
+  + " due to invalid nonce " + previousNonce);
+}
+
+res.sendError(getDenyStatus());
+return;
+}
+if(log.isTraceEnabled()) {
+log.trace("Allowing request to " + getRequestedPath(req)
+   + " with valid CSRF nonce " + previousNonce);
 }
 }
 
 if (nonceCache == null) {
+if(log.isDebugEnabled()) {
+log.debug("Creating new CSRF nonce cache with size=" + 
nonceCacheSize + " for session " + (null == session ? "(will create)" : 
session.getId()));
+}
+
 nonceCache = new LruCache<>(nonceCacheSize);
 if (session == null) {
+if(log.isDebugEnabled()) {
+ log.debug("Creating new session to store CSRF nonce 
cache");
+}
+
 session = req.getSession(true);
 }
 session.setAttribute(


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

Re: [tomcat] branch master updated: Improve CSRF prevention filter by exposing the request's current nonce to the request.

[Christopher Schultz]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

Konstantin,

Thanks for the review.

On 11/16/19 16:12, Konstantin Kolinko wrote:
> сб, 16 нояб. 2019 г. в 18:55, :
> 
>> 
>> This is an automated email from the ASF dual-hosted git
>> repository.
>> 
>> schultz 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 ff687b5  Improve CSRF prevention filter by exposing the
>> request's current nonce to the request. ff687b5 is described
>> below
>> 
>> commit ff687b5524b2b78c449fe32e8c520da86068cd1a Author:
>> Christopher Schultz  AuthorDate:
>> Sat Nov 16 10:54:36 2019 -0500
>> 
>> Improve CSRF prevention filter by exposing the request's current
>> nonce to the request. --- 
>> java/org/apache/catalina/filters/Constants.java| 33
>> ++ 
>> .../catalina/filters/CsrfPreventionFilter.java |  5  
>> .../catalina/filters/CsrfPreventionFilterBase.java | 10 +++ 3
>> files changed, 48 insertions(+)
>> 
>> diff --git a/java/org/apache/catalina/filters/Constants.java
>> b/java/org/apache/catalina/filters/Constants.java index
>> 4fe41cd..87dd6c4 100644 ---
>> a/java/org/apache/catalina/filters/Constants.java +++
>> b/java/org/apache/catalina/filters/Constants.java @@ -25,12
>> +25,34 @@ package org.apache.catalina.filters; */ public final
>> class Constants {
>> 
>> +/** + * The session attribute key under which the CSRF
>> nonce + * cache will be stored. + */ public static final
>> String CSRF_NONCE_SESSION_ATTR_NAME = 
>> "org.apache.catalina.filters.CSRF_NONCE";
>> 
>> +/** + * The request attribute key under which the
>> current + * requests's CSRF nonce can be found. + */ +
>> public static final String CSRF_NONCE_REQUEST_ATTR_NAME = +
>> "org.apache.catalina.filters.CSRF_REQUEST_NONCE"; + +/** +
>> * The name of the request parameter which carries CSRF nonces +
>> * from the client to the server for validation. + */ public
>> static final String CSRF_NONCE_REQUEST_PARAM = 
>> "org.apache.catalina.filters.CSRF_NONCE";
>> 
>> +/** + * The servlet context attribute key under which
>> the + * CSRF request parameter name can be found. + */ +
>> public static final String CSRF_NONCE_REQUEST_PARAM_NAME_KEY = +
>> "org.apache.catalina.filters.CSRF_NONCE_PARAM_NAME"; + public
>> static final String METHOD_GET = "GET";
>> 
>> public static final String CSRF_REST_NONCE_HEADER_NAME =
>> "X-CSRF-Token"; @@ -39,6 +61,17 @@ public final class Constants
>> {
>> 
>> public static final String CSRF_REST_NONCE_HEADER_REQUIRED_VALUE
>> = "Required";
>> 
>> +/** + * The session attribute key under which the CSRF
>> REST nonce + * cache will be stored. + */ public static
>> final String CSRF_REST_NONCE_SESSION_ATTR_NAME = 
>> "org.apache.catalina.filters.CSRF_REST_NONCE"; + +/** + *
>> The servlet context attribute key under which the + * CSRF
>> REST header name can be found. + */ +public static final
>> String CSRF_REST_NONCE_HEDAER_NAME_KEY =
> 
> 1) There is a typo in the name of the constant above,
> s/HEDAER/HEADER/.

ACK I will fix this.

> 2) This commit is not mentioned in changelog.

I will also be fixing this. I have a handful of changes for the CSRF
prevention filter and I was going to file them all under
"improvements", instead of individually. I can add that changelog
entry sooner rather than later.

>> +
>> "org.apache.catalina.filters.CSRF_REST_NONCE_HEADER_NAME"; } diff
>> --git
>> a/java/org/apache/catalina/filters/CsrfPreventionFilter.java
>> b/java/org/apache/catalina/filters/CsrfPreventionFilter.java 
>> index fff5c2f..d94cdec 100644 ---
>> a/java/org/apache/catalina/filters/CsrfPreventionFilter.java +++
>> b/java/org/apache/catalina/filters/CsrfPreventionFilter.java @@
>> -128,6 +128,11 @@ public class CsrfPreventionFilter extends
>> CsrfPreventionFilterBase {
>> 
>> nonceCache.add(newNonce);
>> 
>> +// Take this request's nonce and put it into the
>> request +// attributes so pages can make direct use
>> of it, rather than +// requiring the use of
>> response.encodeURL. +
>> request.setAttribute(Constants.CSRF_NONCE_REQUEST_ATTR_NAME,
>> newNonce); + wResponse = new CsrfResponseWrapper(res, newNonce); 
>> } else { wResponse = response; diff --git
>> a/java/org/apache/catalina/filters/CsrfPreventionFilterBase.java
>> b/java/org/apache/catalina/filters/CsrfPreventionFilterBase.java 
>> index c0083f0..8d401af 100644 ---
>> a/java/org/apache/catalina/filters/CsrfPreventionFilterBase.java 
>> +++
>> b/java/org/apache/catalina/filters/CsrfPreventionFilterBase.java 
>> @@ -78,6 +78,16 @@ public abstract class CsrfPreventionFilterBase
>> extends FilterBase { // Set the parameters 
>> super.init(filterConfig);
>> 
>> +// Put the expected request parameter name into the
>> application scope +
>> 

Re: [VOTE] Release Apache Tomcat 8.5.49

[Mark Thomas]

On 19/11/2019 00:44, Konstantin Kolinko wrote:
> вт, 19 нояб. 2019 г. в 01:42, Mark Thomas :
>>
>> On 18/11/2019 22:01, Rémy Maucherat wrote:
>>> On Mon, Nov 18, 2019 at 10:22 PM Mark Thomas >> > wrote:
>>
>>> Is porting the multipoller removal to 7.0 really doable ?
>>
>> I don't know. I haven't looked at the code yet. I do have an alternative
>> fix I tested that should work if back-porting proves tricky.
>>
>>> It could be running on older Windows platforms maybe.
>>
>> That would only be an issue of the versions of Windows were so old they
>> were no longer supported by Microsoft. Users in those circumstances are
>> unlikely to be updating to the latest 7.0.x release anyway. Therefore
>> that isn't something I'm particularly worrying about. (But I am happy to
>> be over-ruled if the consensus is not to back-port to 7.0.x.)
> 
> Searching my mailbox and archives at https://tomcat.markmail.org/,
> 
> a) Tomcat 7.0.96 running on Windows XP  was mentioned recently (August 2019)
> https://bz.apache.org/bugzilla/show_bug.cgi?id=63625#c16
> 
> b) Tomcat 8.5 was never mentioned running on Windows XP or on Windows 2003.
> There was one thread about Tomcat 8.5.3 when a client was using
> Windows XP, but Tomcat was running on Ubuntu.
> There was one thread in year 2017 about posting an application from an
> old server running Windows 2003 and Tomcat 5 to Tomcat 8.5.
> https://markmail.org/thread/qvmzmmjz2dqohbk5
> 
> I think the single pollset change should not be backported to Tomcat 7.
> I am OK with it being backported to Tomcat 8.5.

In favour of back-porting:
- It keeps the code (more) aligned so future maintenance is easier
- It is more obvious where previous APR fixes have not been back-ported
  (as well as being easier to back-port them). I can see a handful of
  fixes it would be worth back-porting
- Windows XP / Server 2003 are out of support. I think it would be
  unusual for a user to be keeping Tomcat up to date but not the OS.
- sendfile is already using a larger pollset size and we have had no
  complaints (that I recall)

I've just finished back-porting the single pollset change. I want to
backport the additional fixes I can now see and then run the unit tests.

I'd like to then commit these changes but I can do it via a PR if folks
want a chance to review them first.

Mark

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

buildbot failure in on tomcat-trunk

[buildbot]

The Buildbot has detected a new failure on builder tomcat-trunk while building 
tomcat. Full details are available at:
https://ci.apache.org/builders/tomcat-trunk/builds/4748

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] f403a56c296859a0d3bb375c128030f617a1f8fd
Blamelist: Mark Thomas 

BUILD FAILED: failed compile_1

Sincerely,
 -The Buildbot




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

[tomcat] branch 8.5.x updated: Fix broken link.

[markt]

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 16d6a19  Fix broken link.
16d6a19 is described below

commit 16d6a19d293a7a1422823796f0eddd1dc77f3102
Author: Mark Thomas 
AuthorDate: Tue Nov 19 16:01:51 2019 +

Fix broken link.

It didn't work if you requested /jsp/security/protected
---
 webapps/docs/changelog.xml| 8 
 webapps/examples/jsp/security/protected/error.jsp | 4 ++--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index bf86274..f00d418 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -54,6 +54,14 @@
   
 
   
+  
+
+  
+Fix the broken re-try link on the error page for the FORM 
authentication
+example in the JSP section of the examples web application. (markt)
+  
+
+  
 
 
   
diff --git a/webapps/examples/jsp/security/protected/error.jsp 
b/webapps/examples/jsp/security/protected/error.jsp
index 3ef5743..29616af 100644
--- a/webapps/examples/jsp/security/protected/error.jsp
+++ b/webapps/examples/jsp/security/protected/error.jsp
@@ -19,7 +19,7 @@
 Error Page For Examples
 
 
-Invalid username and/or password, please try
-again.
+Invalid user name and/or password, please try
+again.
 
 


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

[tomcat] branch master updated: Fix broken link.

[markt]

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 f403a56  Fix broken link.
f403a56 is described below

commit f403a56c296859a0d3bb375c128030f617a1f8fd
Author: Mark Thomas 
AuthorDate: Tue Nov 19 16:01:51 2019 +

Fix broken link.

It didn't work if you requested /jsp/security/protected
---
 webapps/docs/changelog.xml| 8 
 webapps/examples/jsp/security/protected/error.jsp | 4 ++--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 7fabc56..a0915f9 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -45,6 +45,14 @@
   issues do not "pop up" wrt. others).
 -->
 
+  
+
+  
+Fix the broken re-try link on the error page for the FORM 
authentication
+example in the JSP section of the examples web application. (markt)
+  
+
+  
 
 
   
diff --git a/webapps/examples/jsp/security/protected/error.jsp 
b/webapps/examples/jsp/security/protected/error.jsp
index 3ef5743..29616af 100644
--- a/webapps/examples/jsp/security/protected/error.jsp
+++ b/webapps/examples/jsp/security/protected/error.jsp
@@ -19,7 +19,7 @@
 Error Page For Examples
 
 
-Invalid username and/or password, please try
-again.
+Invalid user name and/or password, please try
+again.
 
 


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

[Bug 63932] Content compression breaks contract of ETag

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=63932

--- Comment #14 from Michael Osipov  ---
(In reply to Remy Maucherat from comment #12)
> Doing cosmetic configuration changes like this is not a good idea. When the
> subelements of Connector were introduced, it was out of necessity to
> implement SNI, not to beautify. It caused a lot of bugs and still causes a
> significant amount of mess in the code (which will only be fixed in the next
> major release).

This is likely true, it was just an idea to consider more flexibility.
Eventually moving to context.xml instead of sitting on connector level.

-- 
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 63932] Content compression breaks contract of ETag

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=63932

--- Comment #13 from Michael Osipov  ---
(In reply to Konstantin Kolinko from comment #11)
> (In reply to Michael Osipov from comment #8)
> > 
> > I get the feeling that compression configuration must be moved sooner or
> > later to a subelement  beneath a connector.
> 
> Enabling compression globally like that may make one vulnerable to BREACH
> exploit. Maybe controlling this feature from within a web application is a
> way to go. (E.g. like sendfile feature can be used by DefaultServlet).

I don't understand this?! Transparent compression is already on the Connector?
All I am saying is to move those three attributes into a subelement.

Maybe it would be better to move this completely to a valve?! Then you will
have full control from the webapp.

> https://en.wikipedia.org/wiki/BREACH
> 
> > WDYT about adding a suffix and removing it on the fly like mod_deflate 
> > should do?
> 
> I do not have a clue what you are talking about here.

The proposal from mod_deflate was to transform

ETag: "..." to "...-gzip", -br, etc. When the client presents the ETag
"...-gzip" the compressor would remove the '-gzip" and the application would
not notice that at all.

-- 
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

Re: [VOTE] Release Apache Tomcat 9.0.29

[jean-frederic clere]

On 16/11/2019 19:56, Mark Thomas wrote:

The proposed Apache Tomcat 9.0.29 release is now available for voting.

The major changes compared to the 9.0.27 release are:

- Improvements to Async error handling

- Stricter processing of HTTP headers when looking for specific token
   values

- Fix various issues that could lead to modification to a JSP not being
   reflected in the served page

Along with lots of other bug fixes and improvements.

For full details, see the changelog:
https://ci.apache.org/projects/tomcat/tomcat9/docs/changelog.html

It can be obtained from:
https://dist.apache.org/repos/dist/dev/tomcat/tomcat-9/v9.0.29/
The Maven staging repo is:
https://repository.apache.org/content/repositories/orgapachetomcat-1236/
The tag is:
https://github.com/apache/tomcat/tree/9.0.29


The proposed 9.0.29 release is:
[ ] Broken - do not release
[X] Stable - go ahead and release as 9.0.29


Tested on fedora 31 with jdk8 and tomcat-native-1.2.23



-
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 63859] AJP cping/cpong mode failing on Tomcat 9.x

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=63859

--- Comment #18 from Aurelien Pernoud  ---
Of course I didn't mention that but the catalina logs is the one from
tom_tst01_srv09 !

-- 
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 63859] AJP cping/cpong mode failing on Tomcat 9.x

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=63859

Aurelien Pernoud  changed:

   What|Removed |Added

 Status|NEEDINFO|NEW

--- Comment #17 from Aurelien Pernoud  ---
Hi,

I've reproduced the bug with mode CI after one day... do definitely related to
CI I think :)

I uploaded a new zip in your dropboy with :

mod_jk.log.20191119 from both nodes
You0'll see both of them have some "(tom_tst01_srv09) timeout in reply cpong
after 1 ms.", not at the same timing.

(I limited this log to the timing of the errors between 9 and 11 UTC) to avoid
sending a huge file)


catalina logs in FINE mode 
(org.apache.coyote.ajp.level = FINE
org.apache.tomcat.util.net.level = FINE)

Since startup and for the day.

Let me know if I can help in any way

-- 
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 63932] Content compression breaks contract of ETag

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=63932

--- Comment #12 from Remy Maucherat  ---
Doing cosmetic configuration changes like this is not a good idea. When the
subelements of Connector were introduced, it was out of necessity to implement
SNI, not to beautify. It caused a lot of bugs and still causes a significant
amount of mess in the code (which will only be fixed in the next major
release).

-- 
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 63932] Content compression breaks contract of ETag

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=63932

--- Comment #11 from Konstantin Kolinko  ---
(In reply to Michael Osipov from comment #8)
> 
> I get the feeling that compression configuration must be moved sooner or
> later to a subelement  beneath a connector.

Enabling compression globally like that may make one vulnerable to BREACH
exploit. Maybe controlling this feature from within a web application is a way
to go. (E.g. like sendfile feature can be used by DefaultServlet).

https://en.wikipedia.org/wiki/BREACH

> WDYT about adding a suffix and removing it on the fly like mod_deflate should 
> do?

I do not have a clue what you are talking about here.

-- 
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

[tomcat] branch master updated: Polish. Align with 8.5.x.

[markt]

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 913e941  Polish. Align with 8.5.x.
913e941 is described below

commit 913e941b2daeed074c2cf8f2381b1ba7c48e0ca7
Author: Mark Thomas 
AuthorDate: Tue Nov 19 13:25:01 2019 +

Polish. Align with 8.5.x.
---
 java/org/apache/tomcat/util/net/AprEndpoint.java | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/java/org/apache/tomcat/util/net/AprEndpoint.java 
b/java/org/apache/tomcat/util/net/AprEndpoint.java
index 1d40f29..146a816 100644
--- a/java/org/apache/tomcat/util/net/AprEndpoint.java
+++ b/java/org/apache/tomcat/util/net/AprEndpoint.java
@@ -1060,7 +1060,7 @@ public class AprEndpoint extends 
AbstractEndpoint implements SNICallB
  *  for a maximum of two events (read and write) at any one
  *  time.
  *
- * Therefore size is actual poller size *4.
+ * Therefore size is poller size *4.
  */
 desc = new long[pollerSize * 4];
 connectionCount.set(0);
@@ -1112,7 +1112,7 @@ public class AprEndpoint extends 
AbstractEndpoint implements SNICallB
 while (info != null) {
 // Make sure we aren't trying add the socket as well as close 
it
 addList.remove(info.socket);
-// Make sure the  socket isn't in the poller before we close it
+// Make sure the socket isn't in the poller before we close it
 removeFromPoller(info.socket);
 // Poller isn't running at this point so use destroySocket()
 // directly
@@ -1123,7 +1123,7 @@ public class AprEndpoint extends 
AbstractEndpoint implements SNICallB
 // Close all sockets in the add queue
 info = addList.get();
 while (info != null) {
-// Make sure the  socket isn't in the poller before we close it
+// Make sure the socket isn't in the poller before we close it
 removeFromPoller(info.socket);
 // Poller isn't running at this point so use destroySocket()
 // directly
@@ -1998,7 +1998,6 @@ public class AprEndpoint extends 
AbstractEndpoint implements SNICallB
 }
 }
 }
-
 }
 
 


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

[tomcat] branch 8.5.x updated (a3cc1b7 -> 8120030)

[markt]

This is an automated email from the ASF dual-hosted git repository.

markt pushed a change to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git.


from a3cc1b7  Increment version for next dev cycle
 new 4508e70  Refactor APR Poller to remove use of multiple pollsets
 new 8120030  Remove extra space

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 java/org/apache/tomcat/util/net/AprEndpoint.java | 413 +--
 webapps/docs/changelog.xml   |   9 +
 2 files changed, 167 insertions(+), 255 deletions(-)


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

[tomcat] 02/02: Remove extra space

[markt]

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

commit 81200306c3a20a1b38c9c3795b23efae5b8edff3
Author: Mark Thomas 
AuthorDate: Tue Nov 19 13:24:38 2019 +

Remove extra space
---
 java/org/apache/tomcat/util/net/AprEndpoint.java | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/java/org/apache/tomcat/util/net/AprEndpoint.java 
b/java/org/apache/tomcat/util/net/AprEndpoint.java
index 4821279..460b075 100644
--- a/java/org/apache/tomcat/util/net/AprEndpoint.java
+++ b/java/org/apache/tomcat/util/net/AprEndpoint.java
@@ -1178,7 +1178,7 @@ public class AprEndpoint extends AbstractEndpoint 
implements SNICallBack {
 while (info != null) {
 // Make sure we aren't trying add the socket as well as close 
it
 addList.remove(info.socket);
-// Make sure the  socket isn't in the poller before we close it
+// Make sure the socket isn't in the poller before we close it
 removeFromPoller(info.socket);
 // Poller isn't running at this point so use destroySocket()
 // directly
@@ -1189,7 +1189,7 @@ public class AprEndpoint extends AbstractEndpoint 
implements SNICallBack {
 // Close all sockets in the add queue
 info = addList.get();
 while (info != null) {
-// Make sure the  socket isn't in the poller before we close it
+// Make sure the socket isn't in the poller before we close it
 removeFromPoller(info.socket);
 // Poller isn't running at this point so use destroySocket()
 // directly


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

[tomcat] 01/02: Refactor APR Poller to remove use of multiple pollsets

[markt]

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

commit 4508e70e4fcf542c6071ef77c13cd7141abb9bf4
Author: Mark Thomas 
AuthorDate: Tue Nov 19 13:18:36 2019 +

Refactor APR Poller to remove use of multiple pollsets
---
 java/org/apache/tomcat/util/net/AprEndpoint.java | 409 +--
 webapps/docs/changelog.xml   |   9 +
 2 files changed, 165 insertions(+), 253 deletions(-)

diff --git a/java/org/apache/tomcat/util/net/AprEndpoint.java 
b/java/org/apache/tomcat/util/net/AprEndpoint.java
index 5af9fe0..4821279 100644
--- a/java/org/apache/tomcat/util/net/AprEndpoint.java
+++ b/java/org/apache/tomcat/util/net/AprEndpoint.java
@@ -1061,36 +1061,14 @@ public class AprEndpoint extends AbstractEndpoint 
implements SNICallBack {
 public class Poller implements Runnable {
 
 /**
- * Pointers to the pollers.
+ * Pointer to the poller.
  */
-private long[] pollers = null;
+private long aprPoller;
 
 /**
  * Actual poller size.
  */
-private int actualPollerSize = 0;
-
-/**
- * Amount of spots left in the poller.
- */
-private int[] pollerSpace = null;
-
-/**
- * Amount of low level pollers in use by this poller.
- */
-private int pollerCount;
-
-/**
- * Timeout value for the poll call.
- */
-private int pollerTime;
-
-/**
- * Variable poller timeout that adjusts depending on how many poll sets
- * are in use so that the total poll time across all poll sets remains
- * equal to pollTime.
- */
-private int nextPollerTime;
+private int pollerSize = 0;
 
 /**
  * Root pool.
@@ -1144,55 +1122,18 @@ public class AprEndpoint extends AbstractEndpoint 
implements SNICallBack {
 private volatile boolean pollerRunning = true;
 
 /**
- * Create the poller. With some versions of APR, the maximum poller 
size
- * will be 62 (recompiling APR is necessary to remove this limitation).
+ * Create the poller.
  */
 protected synchronized void init() {
 
 pool = Pool.create(serverSockPool);
-
-// Single poller by default
-int defaultPollerSize = getMaxConnections();
-
-if ((OS.IS_WIN32 || OS.IS_WIN64) && (defaultPollerSize > 1024)) {
-// The maximum per poller to get reasonable performance is 1024
-// Adjust poller size so that it won't reach the limit. This is
-// a limitation of XP / Server 2003 that has been fixed in
-// Vista / Server 2008 onwards.
-actualPollerSize = 1024;
-} else {
-actualPollerSize = defaultPollerSize;
-}
-
-timeouts = new SocketTimeouts(defaultPollerSize);
+pollerSize = getMaxConnections();
+timeouts = new SocketTimeouts(pollerSize);
 
 // At the moment, setting the timeout is useless, but it could get
 // used again as the normal poller could be faster using maintain.
 // It might not be worth bothering though.
-long pollset = allocatePoller(actualPollerSize, pool, -1);
-if (pollset == 0 && actualPollerSize > 1024) {
-actualPollerSize = 1024;
-pollset = allocatePoller(actualPollerSize, pool, -1);
-}
-if (pollset == 0) {
-actualPollerSize = 62;
-pollset = allocatePoller(actualPollerSize, pool, -1);
-}
-
-pollerCount = defaultPollerSize / actualPollerSize;
-pollerTime = pollTime / pollerCount;
-nextPollerTime = pollerTime;
-
-pollers = new long[pollerCount];
-pollers[0] = pollset;
-for (int i = 1; i < pollerCount; i++) {
-pollers[i] = allocatePoller(actualPollerSize, pool, -1);
-}
-
-pollerSpace = new int[pollerCount];
-for (int i = 0; i < pollerCount; i++) {
-pollerSpace[i] = actualPollerSize;
-}
+aprPoller = allocatePoller(pollerSize, pool, -1);
 
 /*
  * x2 - One descriptor for the socket, one for the event(s).
@@ -1201,12 +1142,12 @@ public class AprEndpoint extends AbstractEndpoint 
implements SNICallBack {
  *  for a maximum of two events (read and write) at any one
  *  time.
  *
- * Therefore size is actual poller size *4.
+ * Therefore size is poller size *4.
  */
-desc = new long[actualPollerSize * 4];
+desc = new long[pollerSize * 4];
 connectionCount.set(0);
-addList = new 

[Bug 63625] Unable to start Tomcat 7.0.96 (stop by 0xc0000005)

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=63625

--- Comment #27 from Tony Yan  ---
Thank you very much for such quick response, Mark. I will download the Commons
Daemon 1.2.1. And thanks all the people for the contributions in this fix.

-- 
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 63939] CORS filter incorrectly implements same/local origin check

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=63939

Michael Osipov  changed:

   What|Removed |Added

 CC||micha...@apache.org

-- 
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 63939] New: CORS filter incorrectly implements same/local origin check

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=63939

Bug ID: 63939
   Summary: CORS filter incorrectly implements same/local origin
check
   Product: Tomcat 9
   Version: 9.0.x
  Hardware: All
OS: All
Status: NEW
  Severity: major
  Priority: P2
 Component: Catalina
  Assignee: dev@tomcat.apache.org
  Reporter: micha...@apache.org
  Target Milestone: -

I believe
org.apache.catalina.filters.CorsFilter.isLocalOrigin(HttpServletRequest,
String) has two bugs:

One note upfront, I believe this method should be renamed to isSameOrigin() to
use the same term as with the Fetch Standard.

Bug 1: When Origin contains a standard port Tomcat does not take that into
account and omits the default port from target, .e.g.,
> curl -X OPTIONS -H "Origin: https://fqdn:443;
vs. 
> curl -X OPTIONS -H "Origin: https://fqdn;

both result in different responses.

The root cause is here:
https://github.com/apache/tomcat/blob/master/java/org/apache/catalina/filters/CorsFilter.java#L656-L663

As far as I understand https://url.spec.whatwg.org/#concept-url-port, the
default port for the specific protocol has to be used within the comparison.

Bug 2: at the very end "origin.equalsIgnoreCase(target.toString());" is
performed while isOriginAllowed() performs a case-sensitive comparision as
documented here: 
https://www.w3.org/TR/access-control/#resource-preflight-requests
This seems to be inconsistent.

-- 
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 63931] The remote endpoint was in state [TEXT_FULL_WRITING] which is an invalid state

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=63931

--- Comment #8 from Saurav Singh  ---
Hey Mark! Sorry if my tone of comment #6 sounds something different which
wasn't meant to be its just being impatient.

When i raised the bug with Apche i didn't think of cost involved; its apache
foundation decision to go open source or go license i respect that.

Bug was raised in seek to understand what's goffy thing going on in production
when incoming messages rate increases. and to be very honest no IOException
being thrown any where.

Thanks for your help with free of cost. Have a good one.

-- 
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 63938] CORS filter adds headers to non-CORS request

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=63938

Michael Osipov  changed:

   What|Removed |Added

 CC||micha...@apache.org

-- 
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 63931] The remote endpoint was in state [TEXT_FULL_WRITING] which is an invalid state

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=63931

--- Comment #7 from Mark Thomas  ---
>From the tone of comment #6 you appear to have forgotten that everyone here is
a volunteer, providing you with help and assistance (at no cost) to use the
software that you have been given (also at no cost). 

What makes this worse is that it has already been explained to you what is
going wrong. An explanation that was provided within 2 hours of the issue being
opened.

Let me try again with more words.

Something goes wrong. This is probably a timeout due to congestion at the
client and/or network and/or Tomcat because one or more of those components
can't handle the message volume.

At the point something goes wrong Tomcat will:
- throw an IOException that the application will see (based on the stack trace
you provided)
- close the session, triggering an onClose() event for the Endpoint that the
application should handle

If the application ignores the Exception and the onClose() event and continues
to try and write messages you will see the IllegalStateException reported in
the original report.

That the application ignores / swallows / fails to handle the Exception is an
application issue, not a Tomcat issue. This is something that needs to be fixed
in the application, not in Tomcat.

That the application ignores / swallows / fails to handle the onClose() event
is an application issue, not a Tomcat issue. This is something that needs to be
fixed in the application, not in Tomcat. You may wish to enquire with the
Spring team what the application needs to do to ensure that it is notified when
the onClose() event is triggered.

The only area where Tomcat may be able to do more (and why this issue is still
open rather than being resolved as INVALID) is preventing applications writing
messages after a close frame has been written to the client. Even if we are
able to improve that, the only difference you are likely to see is a difference
exception if the application tries to write a message after the close frame has
been sent.

-- 
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 63938] New: CORS filter adds headers to non-CORS request

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=63938

Bug ID: 63938
   Summary: CORS filter adds headers to non-CORS request
   Product: Tomcat 9
   Version: 9.0.x
  Hardware: All
OS: All
Status: NEW
  Severity: normal
  Priority: P2
 Component: Catalina
  Assignee: dev@tomcat.apache.org
  Reporter: micha...@apache.org
  Target Milestone: -

When the CorsFilter identifies a request as NOT_CORS, #handleNonCORS() still
calls #addStandardHeaders() and invokes filterChain.

While is not particularly wrong, the identified request is has no "Origin"
header and still serving those standard reponse headers is a waste of bytes w/o
any value to the client. One caveat I see is that a local origin request is
identified as NOT_CORS for some reason altough an "Origin" header has been
provided.

-- 
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

Re: Possible optimization for class loading?

[Mark Thomas]

On 17/11/2019 16:34, Rainer Jung wrote:
> Am 17.11.2019 um 17:21 schrieb Konstantin Kolinko:



>> Looking into AbstractFileResourceSet.file() from the above stacktrace,
>> some check are already there, e.g. "mustExist" flag. I see that
>> DirResourceSet.getResource(DirResourceSet.java:101) calls file(...,
>> false), so "mustExist" is false. It is just a first glance, I have not
>> dug further yet.
> 
> This is correct. A question is, whether all of the checks and
> normalizations in the default DirResourceSet are need for a resource
> with path starting with WEB-INF/classes, if WEB-INF/classes does not
> exist. AbstractFileResourceSet in my case does return a non-null File
> object after the (expensive) checks, which then is wrapped by
> DirResourceSet into an EmptyResource because the file does not exist.
> 
> Later is is found in the jar files, so the EmptyResource isn't used.
> 
> If the class does not get found in one of the jars, one would probably
> then have to create an appropriate EmptyResource to return.

I think there is scope for some optimisation here. I only took a quick
look but I couldn't see a way to do it without some changes to the API
(I'm fine with changes but was hoping to be able to do it without).

Mark

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

Re: Bundling of localized messages

[Mark Thomas]

> Is there a particular reason (a) the translations are in a separate
> JAR file - i.e. not in catalina.jar and (b) they are separated by
> language? Perhaps the thought was that, knowing that you don't need
> e.g. Korean means you can remove the JAR file from some kind of
> bare-ones distribution.

Essentially, yes. The intention was to make it easy to use the default
English text if users didn't need / didn't want the translations.

> For SSI, at least, I think it makes sense to bundle all the i18n files
> together in the same JAR file. I'm using the  macro to build my
> JAR file which explicitly  LocalStrings_*.properties from
> whatever JAR file is being built. I'd need to change the way that the
> SSI JAR file is built in order to achieve this. But before doing so, I
> wanted to get consensus on what to do with these files.

Have you removed all the other dependencies? FastHttpDateFormat,
StringManager, JULI, IOTools etc?

Mark

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

[Bug 63932] Content compression breaks contract of ETag

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=63932

--- Comment #10 from Michael Osipov  ---
(In reply to Remy Maucherat from comment #9)
> (In reply to Mark Thomas from comment #5)
> > Where things get "interesting" is when resources set their own, strong ETag.
> > It looks to me that the simplest solution would be for the container
> > provided compression to check for the presence of a strong ETag and, if it
> > finds one, prepend the weakness indicator to the ETag if it is going to
> > compress the resource.
> 
> +1, but this is all academic and I think nobody cares overall.

Not really. If you see that how long it took to discuss the HTTPd ticket, I
would be inclined to disable compression when the strong validator is turned
into a weak one w/o my knowledge.

-- 
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 63937] New: CORS preflight request not possible on authenticated endpoints

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=63937

Bug ID: 63937
   Summary: CORS preflight request not possible on authenticated
endpoints
   Product: Tomcat 9
   Version: 9.0.x
  Hardware: All
OS: All
Status: NEW
  Severity: normal
  Priority: P2
 Component: Catalina
  Assignee: dev@tomcat.apache.org
  Reporter: micha...@apache.org
  Target Milestone: -

Consider the following web.xml snippet:

> 
>   apiCorsFilter
>   org.apache.catalina.filters.CorsFilter
>   
>   cors.allowed.origins
>   https://...
>   
>   
>   cors.exposed.headers
>   
> Correlation-Id,Content-Length,Content-Disposition,Location
>   
>   
>   cors.support.credentials
>   true
>   
> 
> 
>   apiCorsFilter
>   /api/*
> 
> 
> 
>   
>   Authenticated REST 
> Services
>   /api/*
>   
>   
>   ...
>   
> 

A CORS preflight will fail with 401 because a configured authenticator will
kick in before the CORS filter.

According to https://fetch.spec.whatwg.org/#http-responses and
https://fetch.spec.whatwg.org/#ref-for-credentials%E2%91%A5 regardless of the
browser config with fetch/XHR API and credentials in "include" mode, all
preflight requests are anonymous by default.

You may Google for "cors preflight bypass authentication". The solutions on the
web, by omitting OPTIONS with  as in
https://developer.ibm.com/answers/questions/405007/is-it-possible-to-exclude-http-options-method-from/,
are incomplete and pose a security risk.
Consider that a client issues a regular OPTIONS request not related to CORS and
no credentials are passed you will not able to properly serve the "Allow"
header if you enforce some kind of ACLs on your resources.

My proposal is to add a boolean property to AuthenticatorBase which will detect
a CORS preflight request and bypass authentication, but all other OPTIONS
requests will require authentication as before.

-- 
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 63937] CORS preflight request not possible on authenticated endpoints

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=63937

Michael Osipov  changed:

   What|Removed |Added

 CC||micha...@apache.org

-- 
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 63932] Content compression breaks contract of ETag

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=63932

--- Comment #9 from Remy Maucherat  ---
(In reply to Mark Thomas from comment #5)
> Where things get "interesting" is when resources set their own, strong ETag.
> It looks to me that the simplest solution would be for the container
> provided compression to check for the presence of a strong ETag and, if it
> finds one, prepend the weakness indicator to the ETag if it is going to
> compress the resource.

+1, but this is all academic and I think nobody cares overall.

-- 
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 63932] Content compression breaks contract of ETag

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=63932

Michael Osipov  changed:

   What|Removed |Added

 CC||micha...@apache.org

-- 
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 63932] Content compression breaks contract of ETag

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=63932

--- Comment #8 from Michael Osipov  ---
(In reply to Konstantin Kolinko from comment #6)
> (In reply to Mark Thomas from comment #5)
> > Please take care, as Julian did, to be specific about whether you are
> > talking about weak or strong validators.
> > 
> > RFC 7232 states (section 2.1)
> > [...]
> > 
> > So the Default servlet (that only ever sets a weak ETag) is fine. As is the
> > WebDAV servlet.
> 
> +1
> 
> > Where things get "interesting" is when resources set their own, strong ETag.
> > It looks to me that the simplest solution would be for the container
> > provided compression to check for the presence of a strong ETag and, if it
> > finds one, prepend the weakness indicator to the ETag if it is going to
> > compress the resource.
> 
> Interesting idea. But I think such feature has to be documented. A server
> administrator should be aware that changing a Connector setting will change
> behaviour of a web application in that way as well. (Changing strong ETag to
> a weak one may affect performance of caches.)
> 
> Another possible solution is to do not compress a response if a strong ETag
> is encountered. The same what we do when a response is already compressed.
> 
> https://github.com/apache/tomcat/blob/9.0.29/java/org/apache/coyote/
> CompressionConfig.java#L203

That sounds like an option. WDYT about adding a suffix and removing it on the
fly like mod_deflate should do?


I get the feeling that compression configuration must be moved sooner or later
to a subelement  beneath a connector.

-- 
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 63932] Content compression breaks contract of ETag

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=63932

--- Comment #7 from Michael Osipov  ---
(In reply to Mark Thomas from comment #5)
> Please take care, as Julian did, to be specific about whether you are
> talking about weak or strong validators.
> 
> RFC 7232 states (section 2.1)
> 
> Likewise, a validator is weak if it is shared by two or more
> representations of a given resource at the same time, unless those
> representations have identical representation data.  For example, if
> the origin server sends the same validator for a representation with
> a gzip content coding applied as it does for a representation with no
> content coding, then that validator is weak.
> 
> 
> So the Default servlet (that only ever sets a weak ETag) is fine. As is the
> WebDAV servlet.

I agree. Is there a possibility that a custom resource is added to the context
with issues strong validators?

> Where things get "interesting" is when resources set their own, strong ETag.
> It looks to me that the simplest solution would be for the container
> provided compression to check for the presence of a strong ETag and, if it
> finds one, prepend the weakness indicator to the ETag if it is going to
> compress the resource.

That would be wrong, imho. The servlet expects the ETag to retain strong and
not to be modified in terms of comparison strength.

-- 
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 63625] Unable to start Tomcat 7.0.96 (stop by 0xc0000005)

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=63625

--- Comment #26 from Mark Thomas  ---
(In reply to Tony Yan from comment #25)
> Does it mean that Tomcat 7.0.96 32bit is not working because the Tomcat7.exe
> has defect?

Yes.

You can download Commons Daemon 1.2.1 and rename the 32-bit version of
prunsrv.exe to Tomcat7.exe and use that with Tomcat 7.0.96.

-- 
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