[GitHub] [tomcat] rmaucher commented on a diff in pull request #607: Added RateLimitFilter

2023-03-30 Thread via GitHub


rmaucher commented on code in PR #607:
URL: https://github.com/apache/tomcat/pull/607#discussion_r1152944003


##
java/org/apache/catalina/filters/RateLimitFilter.java:
##
@@ -0,0 +1,230 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.catalina.filters;
+
+import jakarta.servlet.FilterChain;
+import jakarta.servlet.FilterConfig;
+import jakarta.servlet.GenericFilter;
+import jakarta.servlet.ServletException;
+import jakarta.servlet.ServletRequest;
+import jakarta.servlet.ServletResponse;
+import jakarta.servlet.http.HttpServletResponse;
+import org.apache.catalina.util.TimeBucketCounter;
+import org.apache.juli.logging.Log;
+import org.apache.juli.logging.LogFactory;
+import org.apache.tomcat.util.res.StringManager;
+
+import java.io.IOException;
+
+public class RateLimitFilter extends GenericFilter {
+
+/**
+ * default duration in seconds
+ */
+public static final int DEFAULT_BUCKET_DURATION = 60;
+
+/**
+ * default number of requests per duration
+ */
+public static final int DEFAULT_BUCKET_REQUESTS = 300;
+
+/**
+ * default value for enforce
+ */
+public static final boolean DEFAULT_ENFORCE = true;
+
+/**
+ * default status code to return if requests per duration exceeded
+ */
+public static final int DEFAULT_STATUS_CODE = 429;
+
+/**
+ * default status message to return if requests per duration exceeded
+ */
+public static final String DEFAULT_STATUS_MESSAGE = "Too many requests";
+
+/**
+ * request attribute that will contain the number of requests per duration
+ */
+public static final String RATE_LIMIT_ATTRIBUTE_COUNT = 
"org.apache.catalina.filters.RateLimitFilter.Count";
+
+/**
+ * filter init-param to set the bucket duration in seconds
+ */
+public static final String PARAM_BUCKET_DURATION = 
"ratelimit.bucket.duration";
+
+/**
+ * filter init-param to set the bucket number of requests
+ */
+public static final String PARAM_BUCKET_REQUESTS = 
"ratelimit.bucket.requests";
+
+/**
+ * filter init-param to set the enforce flag
+ */
+public static final String PARAM_ENFORCE = "ratelimit.enforce";
+
+/**
+ * filter init-param to set a custom status code if requests per duration 
exceeded
+ */
+public static final String PARAM_STATUS_CODE = "ratelimit.status.code";
+
+/**
+ * filter init-param to set a custom status message if requests per 
duration exceeded
+ */
+public static final String PARAM_STATUS_MESSAGE = 
"ratelimit.status.message";
+
+TimeBucketCounter bucketCounter;
+
+private int actualRequests;
+
+private int bucketRequests = DEFAULT_BUCKET_REQUESTS;
+
+private int bucketDuration = DEFAULT_BUCKET_DURATION;
+
+private boolean enforce = DEFAULT_ENFORCE;
+private int statusCode = DEFAULT_STATUS_CODE;
+
+private String statusMessage = DEFAULT_STATUS_MESSAGE;
+
+private transient Log log = LogFactory.getLog(RateLimitFilter.class);
+
+private static final StringManager sm = 
StringManager.getManager(RateLimitFilter.class);
+
+/**
+ * @return the actual maximum allowed requests per time bucket
+ */
+public int getActualRequests() {
+return actualRequests;
+}
+
+/**
+ * @return the actual duration of a time bucket in milliseconds
+ */
+public int getActualDurationInSeconds() {
+return bucketCounter.getActualDuration() / 1000;
+}
+
+@Override
+public void init() throws ServletException {
+
+FilterConfig config = getFilterConfig();
+
+String param;
+param = config.getInitParameter(PARAM_BUCKET_DURATION);
+if (param != null)
+bucketDuration = Integer.parseInt(param);
+
+param = config.getInitParameter(PARAM_BUCKET_REQUESTS);
+if (param != null)
+bucketRequests = Integer.parseInt(param);
+
+param = config.getInitParameter(PARAM_ENFORCE);
+if (param != null)
+enforce = Boolean.parseBoolean(param);
+
+param = config.getInitParameter(PARAM_STATUS_CODE);
+if (param != null)
+statusCode = Integer.parseInt(param);

Session serialization: clustering vs cross-restart persistance

2023-03-30 Thread Christopher Schultz

All,

Yes, I could read the code, but I was wondering if the (Session)Manager 
configuration attributes sessionAttributeNameFilter and 
sessionAttributeValueClassNameFilter are expected to apply to both 
clustering AND cross-restart persistence, or only clustering.


The documentation[1] says that these attributes configure "which session 
attributes will be *distributed*" (emphasis mine). Is this intended to 
only apply to "distribution" (i.e. Clustering) or does it affect all 
types of serialization.


If it only affects Clustering, I would suggest that we should change it 
to apply to all types of serialization (since these attributes exist to 
provide some security controls which should apply in any context). If it 
affects both, I think we should update the documentation.


Thanks,
-chris

[1] 
https://tomcat.apache.org/tomcat-9.0-doc/config/manager.html#Standard_Implementation 
and also PersistentManager, which for some reason does not have an entry 
in the TOC on that page.


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



Re: Session serialization: clustering vs cross-restart persistance

2023-03-30 Thread Christopher Schultz

All,

On 3/30/23 10:02, Christopher Schultz wrote:

All,

Yes, I could read the code, but I was wondering if the (Session)Manager 
configuration attributes sessionAttributeNameFilter and 
sessionAttributeValueClassNameFilter are expected to apply to both 
clustering AND cross-restart persistence, or only clustering.


The documentation[1] says that these attributes configure "which session 
attributes will be *distributed*" (emphasis mine). Is this intended to 
only apply to "distribution" (i.e. Clustering) or does it affect all 
types of serialization.


If it only affects Clustering, I would suggest that we should change it 
to apply to all types of serialization (since these attributes exist to 
provide some security controls which should apply in any context). If it 
affects both, I think we should update the documentation.


Thanks,
-chris

[1] 
https://tomcat.apache.org/tomcat-9.0-doc/config/manager.html#Standard_Implementation and also PersistentManager, which for some reason does not have an entry in the TOC on that page.


After checking the code, it appears that all types of persistence 
(clustering, persisting-to-file across restarts, writing to a Store) use 
StandardSession.doWriteObject and StandardSession.doReadObject (or their 
overridden implementations in DeltaSession), all of which ultimately 
check both of these filters.


I would like to update the documentation to make it clear that these 
filters apply to all uses of serialization.


-chris

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



Re: Session serialization: clustering vs cross-restart persistance

2023-03-30 Thread Mark Thomas

On 30/03/2023 15:11, Christopher Schultz wrote:

All,

On 3/30/23 10:02, Christopher Schultz wrote:

All,

Yes, I could read the code, but I was wondering if the 
(Session)Manager configuration attributes sessionAttributeNameFilter 
and sessionAttributeValueClassNameFilter are expected to apply to both 
clustering AND cross-restart persistence, or only clustering.


The documentation[1] says that these attributes configure "which 
session attributes will be *distributed*" (emphasis mine). Is this 
intended to only apply to "distribution" (i.e. Clustering) or does it 
affect all types of serialization.


If it only affects Clustering, I would suggest that we should change 
it to apply to all types of serialization (since these attributes 
exist to provide some security controls which should apply in any 
context). If it affects both, I think we should update the documentation.


Thanks,
-chris

[1] 
https://tomcat.apache.org/tomcat-9.0-doc/config/manager.html#Standard_Implementation and also PersistentManager, which for some reason does not have an entry in the TOC on that page.


After checking the code, it appears that all types of persistence 
(clustering, persisting-to-file across restarts, writing to a Store) use 
StandardSession.doWriteObject and StandardSession.doReadObject (or their 
overridden implementations in DeltaSession), all of which ultimately 
check both of these filters.


I would like to update the documentation to make it clear that these 
filters apply to all uses of serialization.


+1

Mark

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



[Bug 66548] New: Tomcat does not validate value of Sec-Websocket-Key header

2023-03-30 Thread bugzilla
https://bz.apache.org/bugzilla/show_bug.cgi?id=66548

Bug ID: 66548
   Summary: Tomcat does not validate value of Sec-Websocket-Key
header
   Product: Tomcat 9
   Version: 9.0.73
  Hardware: All
OS: All
Status: NEW
  Severity: normal
  Priority: P2
 Component: WebSocket
  Assignee: dev@tomcat.apache.org
  Reporter: dan.r...@workday.com
  Target Milestone: -

In the websocket RFC (https://www.rfc-editor.org/rfc/rfc6455#section-4.1) we
read:

The request MUST include a header field with the name
|Sec-WebSocket-Key|.  The value of this header field MUST be a
nonce consisting of a randomly selected 16-byte value that has
been base64-encoded (see Section 4 of [RFC4648]).  The nonce
MUST be selected randomly for each connection.

Tomcat appears to accept any value for Sec-WebSocket-Key - even if it's not a
base64 string, and even if it's not the correct length.

I don't think this causes any functional or security issues, but since the
WebSocket spec is worded pretty strongly ("MUST"), I think it would make sense
for Tomcat to throw an exception if the Sec-WebSocket-Key header does not meet
this requirement.

-- 
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 main updated: Make it clear that session-attribute name and class filters apply to all types of serialization, not just clustering.

2023-03-30 Thread schultz
This is an automated email from the ASF dual-hosted git repository.

schultz pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/main by this push:
 new 3ab4c0052e Make it clear that session-attribute name and class filters 
apply to all types of serialization, not just clustering.
3ab4c0052e is described below

commit 3ab4c0052e13a8faa479ed8f738cec4737948340
Author: Christopher Schultz 
AuthorDate: Thu Mar 30 15:54:20 2023 -0400

Make it clear that session-attribute name and class filters apply to all 
types of serialization, not just clustering.
---
 webapps/docs/config/manager.xml | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/webapps/docs/config/manager.xml b/webapps/docs/config/manager.xml
index 1b7e0b9169..27ea0fa29a 100644
--- a/webapps/docs/config/manager.xml
+++ b/webapps/docs/config/manager.xml
@@ -196,7 +196,8 @@
 
   
 A regular expression used to filter which session attributes will be
-distributed. An attribute will only be distributed if its name matches
+serialized for clustering/replication, or storage to any persistent 
store.
+An attribute will only be serialized if its name matches
 this pattern. If the pattern is zero length or null, all
 attributes are eligible for distribution. The pattern is anchored so 
the
 session attribute name must fully match the pattern. As an example, the
@@ -208,7 +209,8 @@
 
   
 A regular expression used to filter which session attributes will be
-distributed. An attribute will only be distributed if the 
implementation
+serialized for clustering/replication, or storage to any persistent 
store.
+An attribute will only be serialized if the implementation
 class name of the value matches this pattern. If the pattern is zero
 length or null, all attributes are eligible for
 distribution. The pattern is anchored so the fully qualified class name
@@ -331,7 +333,8 @@
 
   
 A regular expression used to filter which session attributes will be
-distributed. An attribute will only be distributed if its name matches
+serialized for clustering/replication, or storage to any persistent 
store.
+An attribute will only be serialized if its name matches
 this pattern. If the pattern is zero length or null, all
 attributes are eligible for distribution. The pattern is anchored so 
the
 session attribute name must fully match the pattern. As an example, the
@@ -343,7 +346,8 @@
 
   
 A regular expression used to filter which session attributes will be
-distributed. An attribute will only be distributed if the 
implementation
+serialized for clustering/replication, or storage to any persistent 
store.
+An attribute will only be serialized if the implementation
 class name of the value matches this pattern. If the pattern is zero
 length or null, all attributes are eligible for
 distribution. The pattern is anchored so the fully qualified class name


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



[tomcat] branch 10.1.x updated: Make it clear that session-attribute name and class filters apply to all types of serialization, not just clustering.

2023-03-30 Thread schultz
This is an automated email from the ASF dual-hosted git repository.

schultz pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/10.1.x by this push:
 new bbcbc5ef62 Make it clear that session-attribute name and class filters 
apply to all types of serialization, not just clustering.
bbcbc5ef62 is described below

commit bbcbc5ef621124890c7fb2f5ce98d001cbd375b6
Author: Christopher Schultz 
AuthorDate: Thu Mar 30 15:54:20 2023 -0400

Make it clear that session-attribute name and class filters apply to all 
types of serialization, not just clustering.
---
 webapps/docs/config/manager.xml | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/webapps/docs/config/manager.xml b/webapps/docs/config/manager.xml
index 93489f8f9c..ed7ae2b83e 100644
--- a/webapps/docs/config/manager.xml
+++ b/webapps/docs/config/manager.xml
@@ -196,7 +196,8 @@
 
   
 A regular expression used to filter which session attributes will be
-distributed. An attribute will only be distributed if its name matches
+serialized for clustering/replication, or storage to any persistent 
store.
+An attribute will only be serialized if its name matches
 this pattern. If the pattern is zero length or null, all
 attributes are eligible for distribution. The pattern is anchored so 
the
 session attribute name must fully match the pattern. As an example, the
@@ -208,7 +209,8 @@
 
   
 A regular expression used to filter which session attributes will be
-distributed. An attribute will only be distributed if the 
implementation
+serialized for clustering/replication, or storage to any persistent 
store.
+An attribute will only be serialized if the implementation
 class name of the value matches this pattern. If the pattern is zero
 length or null, all attributes are eligible for
 distribution. The pattern is anchored so the fully qualified class name
@@ -334,7 +336,8 @@
 
   
 A regular expression used to filter which session attributes will be
-distributed. An attribute will only be distributed if its name matches
+serialized for clustering/replication, or storage to any persistent 
store.
+An attribute will only be serialized if its name matches
 this pattern. If the pattern is zero length or null, all
 attributes are eligible for distribution. The pattern is anchored so 
the
 session attribute name must fully match the pattern. As an example, the
@@ -346,7 +349,8 @@
 
   
 A regular expression used to filter which session attributes will be
-distributed. An attribute will only be distributed if the 
implementation
+serialized for clustering/replication, or storage to any persistent 
store.
+An attribute will only be serialized if the implementation
 class name of the value matches this pattern. If the pattern is zero
 length or null, all attributes are eligible for
 distribution. The pattern is anchored so the fully qualified class name


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



[tomcat] branch 9.0.x updated: Make it clear that session-attribute name and class filters apply to all types of serialization, not just clustering.

2023-03-30 Thread schultz
This is an automated email from the ASF dual-hosted git repository.

schultz pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/9.0.x by this push:
 new 3d75bb000a Make it clear that session-attribute name and class filters 
apply to all types of serialization, not just clustering.
3d75bb000a is described below

commit 3d75bb000af2ec03f74b3da29d44bb45ac213161
Author: Christopher Schultz 
AuthorDate: Thu Mar 30 15:54:20 2023 -0400

Make it clear that session-attribute name and class filters apply to all 
types of serialization, not just clustering.
---
 webapps/docs/config/manager.xml | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/webapps/docs/config/manager.xml b/webapps/docs/config/manager.xml
index b1fc609ebd..5abbd4c99f 100644
--- a/webapps/docs/config/manager.xml
+++ b/webapps/docs/config/manager.xml
@@ -187,7 +187,8 @@
 
   
 A regular expression used to filter which session attributes will be
-distributed. An attribute will only be distributed if its name matches
+serialized for clustering/replication, or storage to any persistent 
store.
+An attribute will only be serialized if its name matches
 this pattern. If the pattern is zero length or null, all
 attributes are eligible for distribution. The pattern is anchored so 
the
 session attribute name must fully match the pattern. As an example, the
@@ -199,7 +200,8 @@
 
   
 A regular expression used to filter which session attributes will be
-distributed. An attribute will only be distributed if the 
implementation
+serialized for clustering/replication, or storage to any persistent 
store.
+An attribute will only be serialized if the implementation
 class name of the value matches this pattern. If the pattern is zero
 length or null, all attributes are eligible for
 distribution. The pattern is anchored so the fully qualified class name
@@ -344,7 +346,8 @@
 
   
 A regular expression used to filter which session attributes will be
-distributed. An attribute will only be distributed if its name matches
+serialized for clustering/replication, or storage to any persistent 
store.
+An attribute will only be serialized if its name matches
 this pattern. If the pattern is zero length or null, all
 attributes are eligible for distribution. The pattern is anchored so 
the
 session attribute name must fully match the pattern. As an example, the
@@ -356,7 +359,8 @@
 
   
 A regular expression used to filter which session attributes will be
-distributed. An attribute will only be distributed if the 
implementation
+serialized for clustering/replication, or storage to any persistent 
store.
+An attribute will only be serialized if the implementation
 class name of the value matches this pattern. If the pattern is zero
 length or null, all attributes are eligible for
 distribution. The pattern is anchored so the fully qualified class name


-
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: Make it clear that session-attribute name and class filters apply to all types of serialization, not just clustering.

2023-03-30 Thread schultz
This is an automated email from the ASF dual-hosted git repository.

schultz 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 5ad12fc31a Make it clear that session-attribute name and class filters 
apply to all types of serialization, not just clustering.
5ad12fc31a is described below

commit 5ad12fc31a5acb92d4efad94ad8ba421f838f8a0
Author: Christopher Schultz 
AuthorDate: Thu Mar 30 15:54:20 2023 -0400

Make it clear that session-attribute name and class filters apply to all 
types of serialization, not just clustering.
---
 webapps/docs/config/manager.xml | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/webapps/docs/config/manager.xml b/webapps/docs/config/manager.xml
index 1890656e5b..13bd1b470f 100644
--- a/webapps/docs/config/manager.xml
+++ b/webapps/docs/config/manager.xml
@@ -170,7 +170,8 @@
 
   
 A regular expression used to filter which session attributes will be
-distributed. An attribute will only be distributed if its name matches
+serialized for clustering/replication, or storage to any persistent 
store.
+An attribute will only be serialized if its name matches
 this pattern. If the pattern is zero length or null, all
 attributes are eligible for distribution. The pattern is anchored so 
the
 session attribute name must fully match the pattern. As an example, the
@@ -182,7 +183,8 @@
 
   
 A regular expression used to filter which session attributes will be
-distributed. An attribute will only be distributed if the 
implementation
+serialized for clustering/replication, or storage to any persistent 
store.
+An attribute will only be serialized if the implementation
 class name of the value matches this pattern. If the pattern is zero
 length or null, all attributes are eligible for
 distribution. The pattern is anchored so the fully qualified class name
@@ -327,7 +329,8 @@
 
   
 A regular expression used to filter which session attributes will be
-distributed. An attribute will only be distributed if its name matches
+serialized for clustering/replication, or storage to any persistent 
store.
+An attribute will only be serialized if its name matches
 this pattern. If the pattern is zero length or null, all
 attributes are eligible for distribution. The pattern is anchored so 
the
 session attribute name must fully match the pattern. As an example, the
@@ -339,7 +342,8 @@
 
   
 A regular expression used to filter which session attributes will be
-distributed. An attribute will only be distributed if the 
implementation
+serialized for clustering/replication, or storage to any persistent 
store.
+An attribute will only be serialized if the implementation
 class name of the value matches this pattern. If the pattern is zero
 length or null, all attributes are eligible for
 distribution. The pattern is anchored so the fully qualified class name


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



[Bug 66548] Tomcat does not validate value of Sec-Websocket-Key header

2023-03-30 Thread bugzilla
https://bz.apache.org/bugzilla/show_bug.cgi?id=66548

--- Comment #1 from Christopher Schultz  ---
Seems reasonable.

Care you provide a patch/PR?

-- 
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] ChristopherSchultz commented on a diff in pull request #607: Added RateLimitFilter

2023-03-30 Thread via GitHub


ChristopherSchultz commented on code in PR #607:
URL: https://github.com/apache/tomcat/pull/607#discussion_r1153742361


##
java/org/apache/catalina/filters/RateLimitFilter.java:
##
@@ -0,0 +1,230 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.catalina.filters;
+
+import jakarta.servlet.FilterChain;
+import jakarta.servlet.FilterConfig;
+import jakarta.servlet.GenericFilter;
+import jakarta.servlet.ServletException;
+import jakarta.servlet.ServletRequest;
+import jakarta.servlet.ServletResponse;
+import jakarta.servlet.http.HttpServletResponse;
+import org.apache.catalina.util.TimeBucketCounter;
+import org.apache.juli.logging.Log;
+import org.apache.juli.logging.LogFactory;
+import org.apache.tomcat.util.res.StringManager;
+
+import java.io.IOException;
+
+public class RateLimitFilter extends GenericFilter {
+
+/**
+ * default duration in seconds
+ */
+public static final int DEFAULT_BUCKET_DURATION = 60;
+
+/**
+ * default number of requests per duration
+ */
+public static final int DEFAULT_BUCKET_REQUESTS = 300;
+
+/**
+ * default value for enforce
+ */
+public static final boolean DEFAULT_ENFORCE = true;
+
+/**
+ * default status code to return if requests per duration exceeded
+ */
+public static final int DEFAULT_STATUS_CODE = 429;
+
+/**
+ * default status message to return if requests per duration exceeded
+ */
+public static final String DEFAULT_STATUS_MESSAGE = "Too many requests";
+
+/**
+ * request attribute that will contain the number of requests per duration
+ */
+public static final String RATE_LIMIT_ATTRIBUTE_COUNT = 
"org.apache.catalina.filters.RateLimitFilter.Count";
+
+/**
+ * filter init-param to set the bucket duration in seconds
+ */
+public static final String PARAM_BUCKET_DURATION = 
"ratelimit.bucket.duration";
+
+/**
+ * filter init-param to set the bucket number of requests
+ */
+public static final String PARAM_BUCKET_REQUESTS = 
"ratelimit.bucket.requests";
+
+/**
+ * filter init-param to set the enforce flag
+ */
+public static final String PARAM_ENFORCE = "ratelimit.enforce";
+
+/**
+ * filter init-param to set a custom status code if requests per duration 
exceeded
+ */
+public static final String PARAM_STATUS_CODE = "ratelimit.status.code";
+
+/**
+ * filter init-param to set a custom status message if requests per 
duration exceeded
+ */
+public static final String PARAM_STATUS_MESSAGE = 
"ratelimit.status.message";
+
+TimeBucketCounter bucketCounter;
+
+private int actualRequests;
+
+private int bucketRequests = DEFAULT_BUCKET_REQUESTS;
+
+private int bucketDuration = DEFAULT_BUCKET_DURATION;
+
+private boolean enforce = DEFAULT_ENFORCE;
+private int statusCode = DEFAULT_STATUS_CODE;
+
+private String statusMessage = DEFAULT_STATUS_MESSAGE;
+
+private transient Log log = LogFactory.getLog(RateLimitFilter.class);
+
+private static final StringManager sm = 
StringManager.getManager(RateLimitFilter.class);
+
+/**
+ * @return the actual maximum allowed requests per time bucket
+ */
+public int getActualRequests() {
+return actualRequests;
+}
+
+/**
+ * @return the actual duration of a time bucket in milliseconds
+ */
+public int getActualDurationInSeconds() {
+return bucketCounter.getActualDuration() / 1000;
+}
+
+@Override
+public void init() throws ServletException {
+
+FilterConfig config = getFilterConfig();
+
+String param;
+param = config.getInitParameter(PARAM_BUCKET_DURATION);
+if (param != null)
+bucketDuration = Integer.parseInt(param);
+
+param = config.getInitParameter(PARAM_BUCKET_REQUESTS);
+if (param != null)
+bucketRequests = Integer.parseInt(param);
+
+param = config.getInitParameter(PARAM_ENFORCE);
+if (param != null)
+enforce = Boolean.parseBoolean(param);
+
+param = config.getInitParameter(PARAM_STATUS_CODE);
+if (param != null)
+statusCode = Integer.parseIn

[GitHub] [tomcat] ChristopherSchultz commented on a diff in pull request #607: Added RateLimitFilter

2023-03-30 Thread via GitHub


ChristopherSchultz commented on code in PR #607:
URL: https://github.com/apache/tomcat/pull/607#discussion_r1153744130


##
java/org/apache/catalina/util/TimeBucketCounter.java:
##
@@ -0,0 +1,217 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.catalina.util;
+
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.atomic.AtomicInteger;
+
+/**
+ * this class maintains a thread safe hash map that has timestamp-based buckets
+ * followed by a string for a key, and a counter for a value. each time the
+ * increment() method is called it adds the key if it does not exist, 
increments
+ * its value and returns it.
+ *
+ * a maintenance thread cleans up keys that are prefixed by previous timestamp
+ * buckets.
+ */
+public class TimeBucketCounter {
+
+/**
+ * Map to hold the buckets
+ */
+private final ConcurrentHashMap map = new 
ConcurrentHashMap<>();
+
+/**
+ * Milliseconds bucket size as a Power of 2 for bit shift math, e.g.
+ * 16 for 65_536ms which is about 1:05 minute
+ */
+private final int numBits;
+
+/**
+ * ratio of actual duration to config duration
+ */
+private final double ratio;
+
+/**
+ * flag for the maintenance thread
+ */
+volatile boolean isRunning = false;
+
+/**
+ *
+ * @param bucketDuration duration in seconds, e.g. for 1 minute pass 60
+ */
+public TimeBucketCounter(int bucketDuration) {
+
+int durationMillis = bucketDuration * 1000;
+
+int bits = 0;
+int pof2 = nextPowerOf2(durationMillis);
+int bitCheck = pof2;
+while (bitCheck > 1) {
+bitCheck = pof2 >> ++bits;
+}
+
+this.numBits = bits;
+
+this.ratio = ratioToPowerOf2(durationMillis);
+
+int cleanupsPerBucketDuration = (durationMillis >= 60_000) ? 6 : 3;
+Thread mt = new MaintenanceThread(durationMillis / 
cleanupsPerBucketDuration);
+mt.start();
+}
+
+/**
+ * increments the counter for the passed identifier in the current time
+ * bucket and returns the new value
+ *
+ * @param identifier an identifier for which we want to maintain count, 
e.g. IP Address
+ * @return the count within the current time bucket
+ */
+public final int increment(String identifier) {
+String key = getCurrentBucketPrefix() + "-" + identifier;
+AtomicInteger ai = map.computeIfAbsent(key, v -> new AtomicInteger());
+return ai.incrementAndGet();
+}
+
+/**
+ * calculates the current time bucket prefix by shifting bits for fast
+ * division, e.g. shift 16 bits is the same as dividing by 65,536 which is
+ * about 1:05m
+ */
+public final int getCurrentBucketPrefix() {
+return (int) (System.currentTimeMillis() >> this.numBits);
+}
+
+/**
+ *
+ * @return
+ */
+public int getNumBits() {
+return numBits;
+}
+
+/**
+ * the actual duration may differ from the configured duration because
+ * it is set to the next power of 2 value in order to perform very fast
+ * bit shift arithmetic
+ *
+ * @return the actual bucket duration in milliseconds
+ */
+public int getActualDuration() {
+return (int) Math.pow(2, getNumBits());
+}
+
+/**
+ * returns the ratio between the configured duration param and the
+ * actual duration which will be set to the next power of 2.  we then
+ * multiply the configured requests param by the same ratio in order
+ * to compensate for the added time, if any
+ *
+ * @return the ratio, e.g. 1.092 if the actual duration is 65_536 for
+ * the configured duration of 60_000
+ */
+public double getRatio() {
+return ratio;
+}
+
+/**
+ * returns the ratio to the next power of 2 so that we can adjust the value
+ *
+ * @param value
+ * @return
+ */
+static double ratioToPowerOf2(int value) {
+double nextPO2 = nextPowerOf2(value);
+return Math.round((1000 * nextPO2 / value)) / 1000d;
+}
+
+/**
+ * returns the next power of 2 given a value, e.g. 256 for 250,
+ * or 10

[GitHub] [tomcat] isapir commented on a diff in pull request #607: Added RateLimitFilter

2023-03-30 Thread via GitHub


isapir commented on code in PR #607:
URL: https://github.com/apache/tomcat/pull/607#discussion_r1153762530


##
java/org/apache/catalina/filters/RateLimitFilter.java:
##
@@ -0,0 +1,230 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.catalina.filters;
+
+import jakarta.servlet.FilterChain;
+import jakarta.servlet.FilterConfig;
+import jakarta.servlet.GenericFilter;
+import jakarta.servlet.ServletException;
+import jakarta.servlet.ServletRequest;
+import jakarta.servlet.ServletResponse;
+import jakarta.servlet.http.HttpServletResponse;
+import org.apache.catalina.util.TimeBucketCounter;
+import org.apache.juli.logging.Log;
+import org.apache.juli.logging.LogFactory;
+import org.apache.tomcat.util.res.StringManager;
+
+import java.io.IOException;
+
+public class RateLimitFilter extends GenericFilter {
+
+/**
+ * default duration in seconds
+ */
+public static final int DEFAULT_BUCKET_DURATION = 60;
+
+/**
+ * default number of requests per duration
+ */
+public static final int DEFAULT_BUCKET_REQUESTS = 300;
+
+/**
+ * default value for enforce
+ */
+public static final boolean DEFAULT_ENFORCE = true;
+
+/**
+ * default status code to return if requests per duration exceeded
+ */
+public static final int DEFAULT_STATUS_CODE = 429;
+
+/**
+ * default status message to return if requests per duration exceeded
+ */
+public static final String DEFAULT_STATUS_MESSAGE = "Too many requests";
+
+/**
+ * request attribute that will contain the number of requests per duration
+ */
+public static final String RATE_LIMIT_ATTRIBUTE_COUNT = 
"org.apache.catalina.filters.RateLimitFilter.Count";
+
+/**
+ * filter init-param to set the bucket duration in seconds
+ */
+public static final String PARAM_BUCKET_DURATION = 
"ratelimit.bucket.duration";
+
+/**
+ * filter init-param to set the bucket number of requests
+ */
+public static final String PARAM_BUCKET_REQUESTS = 
"ratelimit.bucket.requests";
+
+/**
+ * filter init-param to set the enforce flag
+ */
+public static final String PARAM_ENFORCE = "ratelimit.enforce";
+
+/**
+ * filter init-param to set a custom status code if requests per duration 
exceeded
+ */
+public static final String PARAM_STATUS_CODE = "ratelimit.status.code";
+
+/**
+ * filter init-param to set a custom status message if requests per 
duration exceeded
+ */
+public static final String PARAM_STATUS_MESSAGE = 
"ratelimit.status.message";
+
+TimeBucketCounter bucketCounter;
+
+private int actualRequests;
+
+private int bucketRequests = DEFAULT_BUCKET_REQUESTS;
+
+private int bucketDuration = DEFAULT_BUCKET_DURATION;
+
+private boolean enforce = DEFAULT_ENFORCE;
+private int statusCode = DEFAULT_STATUS_CODE;
+
+private String statusMessage = DEFAULT_STATUS_MESSAGE;
+
+private transient Log log = LogFactory.getLog(RateLimitFilter.class);
+
+private static final StringManager sm = 
StringManager.getManager(RateLimitFilter.class);
+
+/**
+ * @return the actual maximum allowed requests per time bucket
+ */
+public int getActualRequests() {
+return actualRequests;
+}
+
+/**
+ * @return the actual duration of a time bucket in milliseconds
+ */
+public int getActualDurationInSeconds() {
+return bucketCounter.getActualDuration() / 1000;
+}
+
+@Override
+public void init() throws ServletException {
+
+FilterConfig config = getFilterConfig();
+
+String param;
+param = config.getInitParameter(PARAM_BUCKET_DURATION);
+if (param != null)
+bucketDuration = Integer.parseInt(param);
+
+param = config.getInitParameter(PARAM_BUCKET_REQUESTS);
+if (param != null)
+bucketRequests = Integer.parseInt(param);
+
+param = config.getInitParameter(PARAM_ENFORCE);
+if (param != null)
+enforce = Boolean.parseBoolean(param);
+
+param = config.getInitParameter(PARAM_STATUS_CODE);
+if (param != null)
+statusCode = Integer.parseInt(param);
+

[GitHub] [tomcat] isapir commented on a diff in pull request #607: Added RateLimitFilter

2023-03-30 Thread via GitHub


isapir commented on code in PR #607:
URL: https://github.com/apache/tomcat/pull/607#discussion_r1153771643


##
java/org/apache/catalina/util/TimeBucketCounter.java:
##
@@ -0,0 +1,217 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.catalina.util;
+
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.atomic.AtomicInteger;
+
+/**
+ * this class maintains a thread safe hash map that has timestamp-based buckets
+ * followed by a string for a key, and a counter for a value. each time the
+ * increment() method is called it adds the key if it does not exist, 
increments
+ * its value and returns it.
+ *
+ * a maintenance thread cleans up keys that are prefixed by previous timestamp
+ * buckets.
+ */
+public class TimeBucketCounter {
+
+/**
+ * Map to hold the buckets
+ */
+private final ConcurrentHashMap map = new 
ConcurrentHashMap<>();
+
+/**
+ * Milliseconds bucket size as a Power of 2 for bit shift math, e.g.
+ * 16 for 65_536ms which is about 1:05 minute
+ */
+private final int numBits;
+
+/**
+ * ratio of actual duration to config duration
+ */
+private final double ratio;
+
+/**
+ * flag for the maintenance thread
+ */
+volatile boolean isRunning = false;
+
+/**
+ *
+ * @param bucketDuration duration in seconds, e.g. for 1 minute pass 60
+ */
+public TimeBucketCounter(int bucketDuration) {
+
+int durationMillis = bucketDuration * 1000;
+
+int bits = 0;
+int pof2 = nextPowerOf2(durationMillis);
+int bitCheck = pof2;
+while (bitCheck > 1) {
+bitCheck = pof2 >> ++bits;
+}
+
+this.numBits = bits;
+
+this.ratio = ratioToPowerOf2(durationMillis);
+
+int cleanupsPerBucketDuration = (durationMillis >= 60_000) ? 6 : 3;
+Thread mt = new MaintenanceThread(durationMillis / 
cleanupsPerBucketDuration);
+mt.start();
+}
+
+/**
+ * increments the counter for the passed identifier in the current time
+ * bucket and returns the new value
+ *
+ * @param identifier an identifier for which we want to maintain count, 
e.g. IP Address
+ * @return the count within the current time bucket
+ */
+public final int increment(String identifier) {
+String key = getCurrentBucketPrefix() + "-" + identifier;
+AtomicInteger ai = map.computeIfAbsent(key, v -> new AtomicInteger());
+return ai.incrementAndGet();
+}
+
+/**
+ * calculates the current time bucket prefix by shifting bits for fast
+ * division, e.g. shift 16 bits is the same as dividing by 65,536 which is
+ * about 1:05m
+ */
+public final int getCurrentBucketPrefix() {
+return (int) (System.currentTimeMillis() >> this.numBits);
+}
+
+/**
+ *
+ * @return
+ */
+public int getNumBits() {
+return numBits;
+}
+
+/**
+ * the actual duration may differ from the configured duration because
+ * it is set to the next power of 2 value in order to perform very fast
+ * bit shift arithmetic
+ *
+ * @return the actual bucket duration in milliseconds
+ */
+public int getActualDuration() {
+return (int) Math.pow(2, getNumBits());
+}
+
+/**
+ * returns the ratio between the configured duration param and the
+ * actual duration which will be set to the next power of 2.  we then
+ * multiply the configured requests param by the same ratio in order
+ * to compensate for the added time, if any
+ *
+ * @return the ratio, e.g. 1.092 if the actual duration is 65_536 for
+ * the configured duration of 60_000
+ */
+public double getRatio() {
+return ratio;
+}
+
+/**
+ * returns the ratio to the next power of 2 so that we can adjust the value
+ *
+ * @param value
+ * @return
+ */
+static double ratioToPowerOf2(int value) {
+double nextPO2 = nextPowerOf2(value);
+return Math.round((1000 * nextPO2 / value)) / 1000d;
+}
+
+/**
+ * returns the next power of 2 given a value, e.g. 256 for 250,
+ * or 1024, for 1000

[tomcat] branch main updated: Code clean-up. No functional change.

2023-03-30 Thread markt
This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/main by this push:
 new a5962eaa50 Code clean-up. No functional change.
a5962eaa50 is described below

commit a5962eaa50c1f1d0af945af574ffe3160536f7da
Author: Mark Thomas 
AuthorDate: Thu Mar 30 23:31:37 2023 +0100

Code clean-up. No functional change.
---
 .../catalina/valves/rewrite/TestResolverSSL.java   |  12 +-
 .../catalina/valves/rewrite/TestRewriteValve.java  | 220 +
 .../catalina/valves/rewrite/TesterRewriteMapA.java |   2 +-
 3 files changed, 101 insertions(+), 133 deletions(-)

diff --git a/test/org/apache/catalina/valves/rewrite/TestResolverSSL.java 
b/test/org/apache/catalina/valves/rewrite/TestResolverSSL.java
index e426dacb51..6b7de8ef2c 100644
--- a/test/org/apache/catalina/valves/rewrite/TestResolverSSL.java
+++ b/test/org/apache/catalina/valves/rewrite/TestResolverSSL.java
@@ -49,10 +49,9 @@ public class TestResolverSSL extends TomcatBaseTest {
 @Parameterized.Parameters(name = "{0}")
 public static Collection parameters() {
 List parameterSets = new ArrayList<>();
-parameterSets.add(new Object[] {
-"JSSE", Boolean.FALSE, 
"org.apache.tomcat.util.net.jsse.JSSEImplementation"});
-parameterSets.add(new Object[] {
-"OpenSSL", Boolean.TRUE, 
"org.apache.tomcat.util.net.openssl.OpenSSLImplementation"});
+parameterSets.add(new Object[] { "JSSE", Boolean.FALSE, 
"org.apache.tomcat.util.net.jsse.JSSEImplementation" });
+parameterSets.add(
+new Object[] { "OpenSSL", Boolean.TRUE, 
"org.apache.tomcat.util.net.openssl.OpenSSLImplementation" });
 
 return parameterSets;
 }
@@ -84,6 +83,7 @@ public class TestResolverSSL extends TomcatBaseTest {
 Assert.assertTrue(res.toString().indexOf("OK") > 0);
 }
 
+//@formatter:off
 // List from https://httpd.apache.org/docs/2.4/mod/mod_ssl.html#envvars
 private static final String[] keys = {
 "HTTPS",
@@ -141,12 +141,12 @@ public class TestResolverSSL extends TomcatBaseTest {
 "SSL_SRP_USER",
 "SSL_SRP_USERINFO",
 "SSL_TLS_SNI" };
+//@formatter:on
 
 public static class ResolverTestValve extends ValveBase {
 
 @Override
-public void invoke(Request request, Response response)
-throws IOException, ServletException {
+public void invoke(Request request, Response response) throws 
IOException, ServletException {
 PrintWriter writer = response.getWriter();
 Resolver resolver = new ResolverImpl(request);
 for (String key : keys) {
diff --git a/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java 
b/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java
index 6a85de082d..d7eb88fe80 100644
--- a/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java
+++ b/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java
@@ -126,26 +126,22 @@ public class TestRewriteValve extends TomcatBaseTest {
 
 @Test
 public void testRewriteMap08() throws Exception {
-doTestRewrite("RewriteMap lc int:tolower\n" +
-"RewriteRule ^(.*) ${lc:$1}", "/C/AaA", "/c/aaa");
+doTestRewrite("RewriteMap lc int:tolower\n" + "RewriteRule ^(.*) 
${lc:$1}", "/C/AaA", "/c/aaa");
 }
 
 @Test
 public void testRewriteMap09() throws Exception {
-doTestRewrite("RewriteMap lc int:toupper\n" +
-"RewriteRule ^(.*) ${lc:$1}", "/w/aAa", "/W/AAA");
+doTestRewrite("RewriteMap lc int:toupper\n" + "RewriteRule ^(.*) 
${lc:$1}", "/w/aAa", "/W/AAA");
 }
 
 @Test
 public void testRewriteMap10() throws Exception {
-doTestRewrite("RewriteMap lc int:escape\n" +
-"RewriteRule ^(.*) ${lc:$1}", "/c/a%20aa", "/c/a%2520aa");
+doTestRewrite("RewriteMap lc int:escape\n" + "RewriteRule ^(.*) 
${lc:$1}", "/c/a%20aa", "/c/a%2520aa");
 }
 
 @Test
 public void testRewriteMap11() throws Exception {
-doTestRewrite("RewriteMap lc int:unescape\n" +
-"RewriteRule ^(.*) ${lc:$1}", "/c/a%2520aa", "/c/a%20aa");
+doTestRewrite("RewriteMap lc int:unescape\n" + "RewriteRule ^(.*) 
${lc:$1}", "/c/a%2520aa", "/c/a%20aa");
 }
 
 
@@ -158,86 +154,86 @@ public class TestRewriteValve extends TomcatBaseTest {
 @Test
 public void testRewriteMap12() throws Exception {
 doTestRewrite("RewriteMap mapb txt:" + getTestConfDirectory() + 
"TesterRewriteMapB.txt\n" +
-"RewriteRule /b/(.*).html$ /c/${mapb:$1}", "/b/a.html", "/c/aa");
+"RewriteRule /b/(.*).html$ /c/${mapb:$1}", "/b/a.html", 
"/c/aa");
 }
 
 @Test
 public void testRewriteMap13() throws Exception {
 doTestRewrite("RewriteMap mapb txt:" + getTestConfDire

[tomcat] branch 10.1.x updated: Code clean-up. No functional change.

2023-03-30 Thread markt
This is an automated email from the ASF dual-hosted git repository.

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


The following commit(s) were added to refs/heads/10.1.x by this push:
 new 8f492ec2a2 Code clean-up. No functional change.
8f492ec2a2 is described below

commit 8f492ec2a296dbd20eea2f0bd084ba50737a045d
Author: Mark Thomas 
AuthorDate: Thu Mar 30 23:33:13 2023 +0100

Code clean-up. No functional change.
---
 .../catalina/valves/rewrite/TestResolverSSL.java   |  12 +-
 .../catalina/valves/rewrite/TestRewriteValve.java  | 220 +
 .../catalina/valves/rewrite/TesterRewriteMapA.java |   2 +-
 3 files changed, 101 insertions(+), 133 deletions(-)

diff --git a/test/org/apache/catalina/valves/rewrite/TestResolverSSL.java 
b/test/org/apache/catalina/valves/rewrite/TestResolverSSL.java
index e426dacb51..6b7de8ef2c 100644
--- a/test/org/apache/catalina/valves/rewrite/TestResolverSSL.java
+++ b/test/org/apache/catalina/valves/rewrite/TestResolverSSL.java
@@ -49,10 +49,9 @@ public class TestResolverSSL extends TomcatBaseTest {
 @Parameterized.Parameters(name = "{0}")
 public static Collection parameters() {
 List parameterSets = new ArrayList<>();
-parameterSets.add(new Object[] {
-"JSSE", Boolean.FALSE, 
"org.apache.tomcat.util.net.jsse.JSSEImplementation"});
-parameterSets.add(new Object[] {
-"OpenSSL", Boolean.TRUE, 
"org.apache.tomcat.util.net.openssl.OpenSSLImplementation"});
+parameterSets.add(new Object[] { "JSSE", Boolean.FALSE, 
"org.apache.tomcat.util.net.jsse.JSSEImplementation" });
+parameterSets.add(
+new Object[] { "OpenSSL", Boolean.TRUE, 
"org.apache.tomcat.util.net.openssl.OpenSSLImplementation" });
 
 return parameterSets;
 }
@@ -84,6 +83,7 @@ public class TestResolverSSL extends TomcatBaseTest {
 Assert.assertTrue(res.toString().indexOf("OK") > 0);
 }
 
+//@formatter:off
 // List from https://httpd.apache.org/docs/2.4/mod/mod_ssl.html#envvars
 private static final String[] keys = {
 "HTTPS",
@@ -141,12 +141,12 @@ public class TestResolverSSL extends TomcatBaseTest {
 "SSL_SRP_USER",
 "SSL_SRP_USERINFO",
 "SSL_TLS_SNI" };
+//@formatter:on
 
 public static class ResolverTestValve extends ValveBase {
 
 @Override
-public void invoke(Request request, Response response)
-throws IOException, ServletException {
+public void invoke(Request request, Response response) throws 
IOException, ServletException {
 PrintWriter writer = response.getWriter();
 Resolver resolver = new ResolverImpl(request);
 for (String key : keys) {
diff --git a/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java 
b/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java
index 6a85de082d..d7eb88fe80 100644
--- a/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java
+++ b/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java
@@ -126,26 +126,22 @@ public class TestRewriteValve extends TomcatBaseTest {
 
 @Test
 public void testRewriteMap08() throws Exception {
-doTestRewrite("RewriteMap lc int:tolower\n" +
-"RewriteRule ^(.*) ${lc:$1}", "/C/AaA", "/c/aaa");
+doTestRewrite("RewriteMap lc int:tolower\n" + "RewriteRule ^(.*) 
${lc:$1}", "/C/AaA", "/c/aaa");
 }
 
 @Test
 public void testRewriteMap09() throws Exception {
-doTestRewrite("RewriteMap lc int:toupper\n" +
-"RewriteRule ^(.*) ${lc:$1}", "/w/aAa", "/W/AAA");
+doTestRewrite("RewriteMap lc int:toupper\n" + "RewriteRule ^(.*) 
${lc:$1}", "/w/aAa", "/W/AAA");
 }
 
 @Test
 public void testRewriteMap10() throws Exception {
-doTestRewrite("RewriteMap lc int:escape\n" +
-"RewriteRule ^(.*) ${lc:$1}", "/c/a%20aa", "/c/a%2520aa");
+doTestRewrite("RewriteMap lc int:escape\n" + "RewriteRule ^(.*) 
${lc:$1}", "/c/a%20aa", "/c/a%2520aa");
 }
 
 @Test
 public void testRewriteMap11() throws Exception {
-doTestRewrite("RewriteMap lc int:unescape\n" +
-"RewriteRule ^(.*) ${lc:$1}", "/c/a%2520aa", "/c/a%20aa");
+doTestRewrite("RewriteMap lc int:unescape\n" + "RewriteRule ^(.*) 
${lc:$1}", "/c/a%2520aa", "/c/a%20aa");
 }
 
 
@@ -158,86 +154,86 @@ public class TestRewriteValve extends TomcatBaseTest {
 @Test
 public void testRewriteMap12() throws Exception {
 doTestRewrite("RewriteMap mapb txt:" + getTestConfDirectory() + 
"TesterRewriteMapB.txt\n" +
-"RewriteRule /b/(.*).html$ /c/${mapb:$1}", "/b/a.html", "/c/aa");
+"RewriteRule /b/(.*).html$ /c/${mapb:$1}", "/b/a.html", 
"/c/aa");
 }
 
 @Test
 public void testRewriteMap13() throws Exception {
 doTestRewrite("RewriteMap mapb txt:" + getTestConf

[tomcat] branch 9.0.x updated: Code clean-up. No functional change.

2023-03-30 Thread markt
This is an automated email from the ASF dual-hosted git repository.

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


The following commit(s) were added to refs/heads/9.0.x by this push:
 new 9ad44a4cb5 Code clean-up. No functional change.
9ad44a4cb5 is described below

commit 9ad44a4cb5e2a6e08d721da07bff896a0e053fc9
Author: Mark Thomas 
AuthorDate: Thu Mar 30 23:34:21 2023 +0100

Code clean-up. No functional change.
---
 .../catalina/valves/rewrite/TestResolverSSL.java   |   5 +-
 .../catalina/valves/rewrite/TestRewriteValve.java  | 220 +
 .../catalina/valves/rewrite/TesterRewriteMapA.java |   2 +-
 3 files changed, 98 insertions(+), 129 deletions(-)

diff --git a/test/org/apache/catalina/valves/rewrite/TestResolverSSL.java 
b/test/org/apache/catalina/valves/rewrite/TestResolverSSL.java
index 14198bd2bd..641b55eebf 100644
--- a/test/org/apache/catalina/valves/rewrite/TestResolverSSL.java
+++ b/test/org/apache/catalina/valves/rewrite/TestResolverSSL.java
@@ -53,6 +53,7 @@ public class TestResolverSSL extends TomcatBaseTest {
 Assert.assertTrue(res.toString().indexOf("OK") > 0);
 }
 
+//@formatter:off
 // List from https://httpd.apache.org/docs/2.4/mod/mod_ssl.html#envvars
 private static final String[] keys = {
 "HTTPS",
@@ -110,12 +111,12 @@ public class TestResolverSSL extends TomcatBaseTest {
 "SSL_SRP_USER",
 "SSL_SRP_USERINFO",
 "SSL_TLS_SNI" };
+//@formatter:on
 
 public static class ResolverTestValve extends ValveBase {
 
 @Override
-public void invoke(Request request, Response response)
-throws IOException, ServletException {
+public void invoke(Request request, Response response) throws 
IOException, ServletException {
 PrintWriter writer = response.getWriter();
 Resolver resolver = new ResolverImpl(request);
 for (String key : keys) {
diff --git a/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java 
b/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java
index e558394cec..6049d3e9fc 100644
--- a/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java
+++ b/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java
@@ -127,26 +127,22 @@ public class TestRewriteValve extends TomcatBaseTest {
 
 @Test
 public void testRewriteMap08() throws Exception {
-doTestRewrite("RewriteMap lc int:tolower\n" +
-"RewriteRule ^(.*) ${lc:$1}", "/C/AaA", "/c/aaa");
+doTestRewrite("RewriteMap lc int:tolower\n" + "RewriteRule ^(.*) 
${lc:$1}", "/C/AaA", "/c/aaa");
 }
 
 @Test
 public void testRewriteMap09() throws Exception {
-doTestRewrite("RewriteMap lc int:toupper\n" +
-"RewriteRule ^(.*) ${lc:$1}", "/w/aAa", "/W/AAA");
+doTestRewrite("RewriteMap lc int:toupper\n" + "RewriteRule ^(.*) 
${lc:$1}", "/w/aAa", "/W/AAA");
 }
 
 @Test
 public void testRewriteMap10() throws Exception {
-doTestRewrite("RewriteMap lc int:escape\n" +
-"RewriteRule ^(.*) ${lc:$1}", "/c/a%20aa", "/c/a%2520aa");
+doTestRewrite("RewriteMap lc int:escape\n" + "RewriteRule ^(.*) 
${lc:$1}", "/c/a%20aa", "/c/a%2520aa");
 }
 
 @Test
 public void testRewriteMap11() throws Exception {
-doTestRewrite("RewriteMap lc int:unescape\n" +
-"RewriteRule ^(.*) ${lc:$1}", "/c/a%2520aa", "/c/a%20aa");
+doTestRewrite("RewriteMap lc int:unescape\n" + "RewriteRule ^(.*) 
${lc:$1}", "/c/a%2520aa", "/c/a%20aa");
 }
 
 
@@ -159,86 +155,86 @@ public class TestRewriteValve extends TomcatBaseTest {
 @Test
 public void testRewriteMap12() throws Exception {
 doTestRewrite("RewriteMap mapb txt:" + getTestConfDirectory() + 
"TesterRewriteMapB.txt\n" +
-"RewriteRule /b/(.*).html$ /c/${mapb:$1}", "/b/a.html", "/c/aa");
+"RewriteRule /b/(.*).html$ /c/${mapb:$1}", "/b/a.html", 
"/c/aa");
 }
 
 @Test
 public void testRewriteMap13() throws Exception {
 doTestRewrite("RewriteMap mapb txt:" + getTestConfDirectory() + 
"TesterRewriteMapB.txt\n" +
-"RewriteRule /b/(.*).html$ /c/${mapb:$1|dd}", "/b/x.html", 
"/c/dd");
+"RewriteRule /b/(.*).html$ /c/${mapb:$1|dd}", "/b/x.html", 
"/c/dd");
 }
 
 // BZ 62667
 @Test
 public void testRewriteMap14() throws Exception {
 doTestRewrite("RewriteMap mapb txt:" + getTestConfDirectory() + 
"TesterRewriteMapB.txt\n" +
-"RewriteRule /b/(.*).html$ /c/${mapb:$1|d$1d}", "/b/x.html", 
"/c/dxd");
+"RewriteRule /b/(.*).html$ /c/${mapb:$1|d$1d}", "/b/x.html", 
"/c/dxd");
 }
 
 @Test
 public void testRewriteMap15() throws Exception {
 doTestRewrite("RewriteMap mapb txt:" + getTestConfDirectory() + 
"TesterRewriteMapB.txt\n" +
-"RewriteRule /b/(.*).html$ /c/${mapb:a$1

[tomcat] branch 8.5.x updated: Code clean-up. No functional change.

2023-03-30 Thread 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 f76d39093f Code clean-up. No functional change.
f76d39093f is described below

commit f76d39093f4d62619eaab5272924364f53fbdeef
Author: Mark Thomas 
AuthorDate: Thu Mar 30 23:35:16 2023 +0100

Code clean-up. No functional change.
---
 .../catalina/valves/rewrite/TestResolverSSL.java   |   5 +-
 .../catalina/valves/rewrite/TestRewriteValve.java  | 220 +
 .../catalina/valves/rewrite/TesterRewriteMapA.java |   2 +-
 3 files changed, 98 insertions(+), 129 deletions(-)

diff --git a/test/org/apache/catalina/valves/rewrite/TestResolverSSL.java 
b/test/org/apache/catalina/valves/rewrite/TestResolverSSL.java
index a1ac1f6a29..e54f49103e 100644
--- a/test/org/apache/catalina/valves/rewrite/TestResolverSSL.java
+++ b/test/org/apache/catalina/valves/rewrite/TestResolverSSL.java
@@ -57,6 +57,7 @@ public class TestResolverSSL extends TomcatBaseTest {
 Assert.assertTrue(res.toString().indexOf("OK") > 0);
 }
 
+//@formatter:off
 // List from https://httpd.apache.org/docs/2.4/mod/mod_ssl.html#envvars
 private static final String[] keys = {
 "HTTPS",
@@ -114,12 +115,12 @@ public class TestResolverSSL extends TomcatBaseTest {
 "SSL_SRP_USER",
 "SSL_SRP_USERINFO",
 "SSL_TLS_SNI" };
+//@formatter:on
 
 public static class ResolverTestValve extends ValveBase {
 
 @Override
-public void invoke(Request request, Response response)
-throws IOException, ServletException {
+public void invoke(Request request, Response response) throws 
IOException, ServletException {
 PrintWriter writer = response.getWriter();
 Resolver resolver = new ResolverImpl(request);
 for (String key : keys) {
diff --git a/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java 
b/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java
index c6ecd486af..cf58cf05fb 100644
--- a/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java
+++ b/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java
@@ -121,26 +121,22 @@ public class TestRewriteValve extends TomcatBaseTest {
 
 @Test
 public void testRewriteMap08() throws Exception {
-doTestRewrite("RewriteMap lc int:tolower\n" +
-"RewriteRule ^(.*) ${lc:$1}", "/C/AaA", "/c/aaa");
+doTestRewrite("RewriteMap lc int:tolower\n" + "RewriteRule ^(.*) 
${lc:$1}", "/C/AaA", "/c/aaa");
 }
 
 @Test
 public void testRewriteMap09() throws Exception {
-doTestRewrite("RewriteMap lc int:toupper\n" +
-"RewriteRule ^(.*) ${lc:$1}", "/w/aAa", "/W/AAA");
+doTestRewrite("RewriteMap lc int:toupper\n" + "RewriteRule ^(.*) 
${lc:$1}", "/w/aAa", "/W/AAA");
 }
 
 @Test
 public void testRewriteMap10() throws Exception {
-doTestRewrite("RewriteMap lc int:escape\n" +
-"RewriteRule ^(.*) ${lc:$1}", "/c/a%20aa", "/c/a%2520aa");
+doTestRewrite("RewriteMap lc int:escape\n" + "RewriteRule ^(.*) 
${lc:$1}", "/c/a%20aa", "/c/a%2520aa");
 }
 
 @Test
 public void testRewriteMap11() throws Exception {
-doTestRewrite("RewriteMap lc int:unescape\n" +
-"RewriteRule ^(.*) ${lc:$1}", "/c/a%2520aa", "/c/a%20aa");
+doTestRewrite("RewriteMap lc int:unescape\n" + "RewriteRule ^(.*) 
${lc:$1}", "/c/a%2520aa", "/c/a%20aa");
 }
 
 
@@ -153,86 +149,86 @@ public class TestRewriteValve extends TomcatBaseTest {
 @Test
 public void testRewriteMap12() throws Exception {
 doTestRewrite("RewriteMap mapb txt:" + getTestConfDirectory() + 
"TesterRewriteMapB.txt\n" +
-"RewriteRule /b/(.*).html$ /c/${mapb:$1}", "/b/a.html", "/c/aa");
+"RewriteRule /b/(.*).html$ /c/${mapb:$1}", "/b/a.html", 
"/c/aa");
 }
 
 @Test
 public void testRewriteMap13() throws Exception {
 doTestRewrite("RewriteMap mapb txt:" + getTestConfDirectory() + 
"TesterRewriteMapB.txt\n" +
-"RewriteRule /b/(.*).html$ /c/${mapb:$1|dd}", "/b/x.html", 
"/c/dd");
+"RewriteRule /b/(.*).html$ /c/${mapb:$1|dd}", "/b/x.html", 
"/c/dd");
 }
 
 // BZ 62667
 @Test
 public void testRewriteMap14() throws Exception {
 doTestRewrite("RewriteMap mapb txt:" + getTestConfDirectory() + 
"TesterRewriteMapB.txt\n" +
-"RewriteRule /b/(.*).html$ /c/${mapb:$1|d$1d}", "/b/x.html", 
"/c/dxd");
+"RewriteRule /b/(.*).html$ /c/${mapb:$1|d$1d}", "/b/x.html", 
"/c/dxd");
 }
 
 @Test
 public void testRewriteMap15() throws Exception {
 doTestRewrite("RewriteMap mapb txt:" + getTestConfDirectory() + 
"TesterRewriteMapB.txt\n" +
-"RewriteRule /b/(.*).html$ /c/${mapb:a$1

[tomcat] branch main updated: Revert "Refactor AmbiguousBean test after seeing failures in CI"

2023-03-30 Thread markt
This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/main by this push:
 new 2cd3c6b620 Revert "Refactor AmbiguousBean test after seeing failures 
in CI"
2cd3c6b620 is described below

commit 2cd3c6b620e9485694ed42e66896fa7f07885602
Author: Mark Thomas 
AuthorDate: Fri Mar 31 00:09:16 2023 +0100

Revert "Refactor AmbiguousBean test after seeing failures in CI"

This reverts commit ffb5ef2de0f161b2d4c1ca92452e478ef4fcc13b.

JavaBeans has a fixed algorithm when mehtods appear to be
ambiguous. This needs to be replicated in the standalone support
---
 test/jakarta/el/TestBeanSupport.java  | 18 +++
 test/jakarta/el/TestBeanSupportAmbiguous.java | 77 ---
 2 files changed, 18 insertions(+), 77 deletions(-)

diff --git a/test/jakarta/el/TestBeanSupport.java 
b/test/jakarta/el/TestBeanSupport.java
index 1f87d37bec..0807a90dce 100644
--- a/test/jakarta/el/TestBeanSupport.java
+++ b/test/jakarta/el/TestBeanSupport.java
@@ -74,6 +74,15 @@ public class TestBeanSupport extends ELBaseTest {
 doTest(MismatchBean.class, "value", TypeA.class, TypeA.class, null);
 }
 
+/*
+ * The first setter found "wins".
+ */
+@Test
+public void testAmbiguousBean() {
+doTest(AmbiguousBean.class, "value", TypeA.class, null, TypeA.class);
+}
+
+
 private void doTest(Class clazz, String propertyName, Class type, 
Class typeGet, Class typeSet) {
 BeanProperties beanProperties = 
BeanSupport.getInstance().getBeanProperties(clazz);
 BeanProperty beanProperty = 
beanProperties.properties.get(propertyName);
@@ -213,6 +222,15 @@ public class TestBeanSupport extends ELBaseTest {
 }
 
 
+public static class AmbiguousBean {
+public void setValue(@SuppressWarnings("unused") TypeA value) {
+}
+
+public void setValue(@SuppressWarnings("unused") String value) {
+}
+}
+
+
 public static class TypeA {
 }
 
diff --git a/test/jakarta/el/TestBeanSupportAmbiguous.java 
b/test/jakarta/el/TestBeanSupportAmbiguous.java
deleted file mode 100644
index 2f19020063..00
--- a/test/jakarta/el/TestBeanSupportAmbiguous.java
+++ /dev/null
@@ -1,77 +0,0 @@
-/*
- * 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 jakarta.el;
-
-import jakarta.el.BeanELResolver.BeanProperties;
-import jakarta.el.BeanELResolver.BeanProperty;
-import jakarta.el.TestBeanSupport.TypeA;
-
-import org.junit.Assert;
-import org.junit.Test;
-
-public class TestBeanSupportAmbiguous {
-
-/*
- * Different JREs appear to call different methods on AmbiguousBean. This 
test ensures that the behaviour is
- * consistent across BeanSupport implementations within any single JRE,
- */
-@Test
-public void testAmbiguousBean() {
-// Disable caching so we can switch implementations within a JVM 
instance.
-System.setProperty("jakarta.el.BeanSupport.doNotCacheInstance", 
"true");
-
-// First test the stand-alone type
-System.setProperty("jakarta.el.BeanSupport.useStandalone", 
Boolean.TRUE.toString());
-Class standaloneType = doTest();
-
-// Then test the full JavaBeans implementation
-System.setProperty("jakarta.el.BeanSupport.useStandalone", 
Boolean.FALSE.toString());
-Class javaBeansType = doTest();
-
-Assert.assertEquals(standaloneType, javaBeansType);
-}
-
-
-private Class doTest() {
-BeanProperties beanProperties = 
BeanSupport.getInstance().getBeanProperties(AmbiguousBean.class);
-BeanProperty beanProperty = beanProperties.properties.get("value");
-
-// Property type
-Assert.assertNotNull(beanProperty);
-Class propertyType = beanProperty.getPropertyType();
-Assert.assertNotNull(propertyType);
-
-// There is no getter
-Assert.assertNull(beanProperty.getReadMethod());
-
-// Check setter
-Assert.assertEquals(void.class, 
beanProperty.getWriteMethod().getReturnType());
-Assert.assertEquals(1, 
beanProperty.getWriteMethod(

[tomcat] branch main updated: Address CI failures for potentially ambiguous bean methods.

2023-03-30 Thread markt
This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/main by this push:
 new 7690caf7dc Address CI failures for potentially ambiguous bean methods.
7690caf7dc is described below

commit 7690caf7dcc5a4c4e9306f38c3d181aa0880f416
Author: Mark Thomas 
AuthorDate: Fri Mar 31 00:36:06 2023 +0100

Address CI failures for potentially ambiguous bean methods.
---
 java/jakarta/el/BeanSupportStandalone.java | 22 ++
 test/jakarta/el/TestBeanSupport.java   | 23 +--
 2 files changed, 39 insertions(+), 6 deletions(-)

diff --git a/java/jakarta/el/BeanSupportStandalone.java 
b/java/jakarta/el/BeanSupportStandalone.java
index a2531451d2..0414261359 100644
--- a/java/jakarta/el/BeanSupportStandalone.java
+++ b/java/jakarta/el/BeanSupportStandalone.java
@@ -19,6 +19,7 @@ package jakarta.el;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
 import java.util.ArrayList;
+import java.util.Comparator;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -32,6 +33,15 @@ import jakarta.el.BeanELResolver.BeanProperty;
  */
 class BeanSupportStandalone extends BeanSupport {
 
+/*
+ * The full JavaBeans implementation has a much more detailed definition 
of method order that applies to an entire
+ * class. When ordering write methods for a single property, a much 
simpler comparator can be used because it is
+ * known that the method names are the same, the return parameters are 
both void and the methods only have a single
+ * parameter.
+ */
+private static final Comparator WRITE_METHOD_COMPARATOR =
+Comparator.comparing(m -> m.getParameterTypes()[0].getName());
+
 @Override
 BeanProperties getBeanProperties(Class type) {
 return new BeanPropertiesStandalone(type);
@@ -133,6 +143,9 @@ class BeanSupportStandalone extends BeanSupport {
 if (readMethod != null) {
 type = readMethod.getReturnType();
 } else {
+if (writeMethods.size() > 1) {
+writeMethods.sort(WRITE_METHOD_COMPARATOR);
+}
 type = writeMethods.get(0).getParameterTypes()[0];
 }
 for (Method candidate : writeMethods) {
@@ -214,4 +227,13 @@ class BeanSupportStandalone extends BeanSupport {
 return writeMethod;
 }
 }
+
+
+static final class WriteMethodComparator implements Comparator {
+
+@Override
+public int compare(Method m1, Method m2) {
+return 0;
+}
+}
 }
diff --git a/test/jakarta/el/TestBeanSupport.java 
b/test/jakarta/el/TestBeanSupport.java
index 0807a90dce..e5185a530f 100644
--- a/test/jakarta/el/TestBeanSupport.java
+++ b/test/jakarta/el/TestBeanSupport.java
@@ -74,12 +74,14 @@ public class TestBeanSupport extends ELBaseTest {
 doTest(MismatchBean.class, "value", TypeA.class, TypeA.class, null);
 }
 
-/*
- * The first setter found "wins".
- */
 @Test
-public void testAmbiguousBean() {
-doTest(AmbiguousBean.class, "value", TypeA.class, null, TypeA.class);
+public void testAmbiguousBean01() {
+doTest(AmbiguousBean01.class, "value", TypeA.class, null, TypeA.class);
+}
+
+@Test
+public void testAmbiguousBean02() {
+doTest(AmbiguousBean02.class, "value", TypeA.class, null, TypeA.class);
 }
 
 
@@ -222,7 +224,7 @@ public class TestBeanSupport extends ELBaseTest {
 }
 
 
-public static class AmbiguousBean {
+public static class AmbiguousBean01 {
 public void setValue(@SuppressWarnings("unused") TypeA value) {
 }
 
@@ -231,6 +233,15 @@ public class TestBeanSupport extends ELBaseTest {
 }
 
 
+public static class AmbiguousBean02 {
+public void setValue(@SuppressWarnings("unused") String value) {
+}
+
+public void setValue(@SuppressWarnings("unused") TypeA value) {
+}
+}
+
+
 public static class TypeA {
 }
 


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



[Bug 66548] Tomcat does not validate value of Sec-Websocket-Key header

2023-03-30 Thread bugzilla
https://bz.apache.org/bugzilla/show_bug.cgi?id=66548

--- Comment #2 from Mark Thomas  ---
Throwing an exception isn't appropriate here. Just returning SC_BAD_REQUEST is
sufficient.

I'll note that RFC 6455 also states:

"It is not necessary for the server to base64-decode the |Sec-WebSocket-Key|
value."

Which begs the question exactly how far should the server go to validate this
value? Possible tests:
a) length of 24 characters
b) ends with "=="
c) characters 0 to 21 are valid for use in base64

Or just decode and check the length despite RFC 6455 saying it is unnecessary.

I think you either do a) + b) or do the full decode. c)

-- 
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 66548] Tomcat does not validate value of Sec-Websocket-Key header

2023-03-30 Thread bugzilla
https://bz.apache.org/bugzilla/show_bug.cgi?id=66548

--- Comment #3 from Mark Thomas  ---
Sorry, comment was posted while incomplete. Continuing...

The changes required for c) are such that it would be simpler just to do the
decode.

I'd lean towards the a) + b) approach but have no objection to the decode.

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