[GitHub] [tomcat-jakartaee-migration] markt-asf closed issue #44: Does this work tool work at the source or binary level?

[via GitHub]

markt-asf closed issue #44: Does this work tool work at the source or binary 
level?
URL: https://github.com/apache/tomcat-jakartaee-migration/issues/44


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


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

[Bug 66513] Primary Key Violation using PersistentManager + PersistentValves +

[bugzilla]

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

--- Comment #4 from Vincent Liautaud  ---
Great thanks...

-- 
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: PersistentManager, PersistentValve, and DataSource/JDBCStore can cause PK violations

[Romain Manni-Bucau]

Hi Chris,

Seems doing it only there will get back to the issue the synchronization
was introduced (there are other synchronized(session) for "local" instance).
However you hit a real point, the instance does not have to be stable, only
its equals/hashCode could be considered stable so guess the idea would be
to add to the session *manager* a getLock() method (don't think a RWLock is
needed there due to current usage) and use it instead of synchronized.
The lock eviction should get some kind of delay for its own eviction and
not be evicted directly with the session to work.
A trivial impl could be to use a Map<$sessionId, $lock> and add a default
delay of 0 when purely in memory and a few seconds when a remote manager is
used (this logic belonging to the manager).

That said it stays a workaround and a better protocol handling that in a
cluster can be a more robust (but more complex) solution so not sure which
compromise is the best.

Romain Manni-Bucau
@rmannibucau  |  Blog
 | Old Blog
 | Github  |
LinkedIn  | Book



Le mer. 8 mars 2023 à 04:43, Han Li  a écrit :

>
>
> > On Mar 8, 2023, at 07:29, Christopher Schultz <
> ch...@christopherschultz.net> wrote:
> >
> > All,
> >
> > Please see https://bz.apache.org/bugzilla/show_bug.cgi?id=66513 for
> reference.
> >
> > It appears that the synchronization used by the PersistentManager can
> cause problems when used with the PersistentValve and DataSource/JDBCStore.
> >
> > The problem is that PersistentManager assumes that the Session object
> can be used as a synchronization monitor to load/update the session in the
> Store. The DataSource/JDBCStore implementation uses an INSERT to create a
> new session, and a DELETE-then-INSERT to re-write the session data in the
> db.
> >
> > When two requests arrive simultaneously, thread scheduling can cause
> DELETE-then-DELETE-then-INSERT-then-INSERT which causes a duplicate PK
> violation.
> >
> > If the PersistentValve were not in use, the in-memory Session would be
> stable. PersistentValve re-loads the Session from the Store on ever
> request, rendering the PersistentManager's synchronized(session) attempt to
> protect things useless.
> >
> > I think a simple way to fix this might be to change:
> >
> > // PersistentManager.java:478~
> >if(session != null) {
> >synchronized(session){
> >session = super.findSession(session.getIdInternal());
> >if(session != null){
> >   // To keep any external calling code from messing up
> the
> >   // concurrency.
> >   session.access();
> >   session.endAccess();
> >}
> >}
> >}
> >
> > to this:
> >
> >if(session != null) {
> >sessionId = String.intern(sessionId);
> >synchronized(sessionId){
> >session = super.findSession(session.getIdInternal());
> >if(session != null){
> >   // To keep any external calling code from messing up
> the
> >   // concurrency.
> >   session.access();
> >   session.endAccess();
> >}
> >}
> >}
> >
> > This swaps the Session object for the sessionId as the synchronization
> monitor. We use String.intern to ensure that we always have the same exact
> object, even across sessions, request, etc.
> -1
>
> This method does seem very simple and solves this problem, but it’s not as
> good as you might think, see below:
> https://shipilev.net/jvm/anatomy-quarks/10-string-intern/ <
> https://shipilev.net/jvm/anatomy-quarks/10-string-intern/>
>
> So I don’t think it should be the preferred option.
>
> Han
>
> >
> > This is *a* way to solve this problem. There are other ways.
> >
> > Another way is also a TODO in the DataSourceRealm code which suggests
> using UPDATE for sessions that already exist. That is probably worth
> implementing, and it would fix this particular issue.
> >
> > Note that it is essentially impossible to prevent thread scheduling,
> requests to other members of the cluster, etc. to prevent data-loss from
> the session and this BZ isn't asking us to fix that. It's only asking that
> a single Tomcat node with PersistentValve enabled doesn't cause thee
> duplicate PK violations for some pretty basic usages.
> >
> > -chris
> >
> > -
> > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> > For additional commands, e-mail: dev-h...@tomcat.apache.org
> >
>
>

[tomcat] branch main updated: Update DBCP to f131286 (2023-03-08, 2.10.0-SNAPSHOT)

[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 182824c8af Update DBCP to f131286 (2023-03-08, 2.10.0-SNAPSHOT)
182824c8af is described below

commit 182824c8af7934eef3ad8b9b486075459cc13c93
Author: Mark Thomas 
AuthorDate: Wed Mar 8 14:26:34 2023 +

Update DBCP to f131286 (2023-03-08, 2.10.0-SNAPSHOT)

This corrects a regression introduced in 11.0.0-M2
---
 MERGE.txt  |  2 +-
 .../apache/tomcat/dbcp/dbcp2/AbandonedTrace.java   | 10 +---
 .../tomcat/dbcp/dbcp2/DelegatingConnection.java| 13 +++
 .../dbcp/dbcp2/DelegatingDatabaseMetaData.java |  1 -
 java/org/apache/tomcat/dbcp/dbcp2/PStmtKey.java|  2 +-
 .../dbcp/dbcp2/PoolableCallableStatement.java  |  4 +---
 .../dbcp/dbcp2/PoolablePreparedStatement.java  |  4 +---
 .../dbcp/dbcp2/cpdsadapter/DriverAdapterCPDS.java  |  4 ++--
 .../dbcp2/cpdsadapter/PooledConnectionImpl.java| 27 --
 .../dbcp2/datasources/InstanceKeyDataSource.java   |  1 -
 webapps/docs/changelog.xml |  5 
 11 files changed, 41 insertions(+), 32 deletions(-)

diff --git a/MERGE.txt b/MERGE.txt
index 63075f061d..5f98d88f84 100644
--- a/MERGE.txt
+++ b/MERGE.txt
@@ -77,4 +77,4 @@ Sub-tree
 src/main/java/org/apache/commons/dbcp2
 src/main/resources/org/apache/commons/dbcp2
 The SHA1 ID / tag for the most recent commit to be merged to Tomcat is:
-f13128604536e78bb1b45b44f74128e93cfbb7cc (2023-01-03)
+b1e0c86d101aa43029625eb191aaee4306911702 (2023-03-08)
diff --git a/java/org/apache/tomcat/dbcp/dbcp2/AbandonedTrace.java 
b/java/org/apache/tomcat/dbcp/dbcp2/AbandonedTrace.java
index a094f16d31..e57b4108a1 100644
--- a/java/org/apache/tomcat/dbcp/dbcp2/AbandonedTrace.java
+++ b/java/org/apache/tomcat/dbcp/dbcp2/AbandonedTrace.java
@@ -37,6 +37,12 @@ import org.apache.tomcat.dbcp.pool2.TrackedUse;
  */
 public class AbandonedTrace implements TrackedUse, AutoCloseable {
 
+static void add(AbandonedTrace receiver, AbandonedTrace trace) {
+if (receiver != null) {
+receiver.addTrace(trace);
+}
+}
+
 /** A list of objects created by children of this object. */
 private final List> traceList = new 
ArrayList<>();
 
@@ -152,9 +158,7 @@ public class AbandonedTrace implements TrackedUse, 
AutoCloseable {
  *AbandonedTrace parent object.
  */
 private void init(final AbandonedTrace parent) {
-if (parent != null) {
-parent.addTrace(this);
-}
+AbandonedTrace.add(parent, this);
 }
 
 /**
diff --git a/java/org/apache/tomcat/dbcp/dbcp2/DelegatingConnection.java 
b/java/org/apache/tomcat/dbcp/dbcp2/DelegatingConnection.java
index 6394875366..a9e620d3b7 100644
--- a/java/org/apache/tomcat/dbcp/dbcp2/DelegatingConnection.java
+++ b/java/org/apache/tomcat/dbcp/dbcp2/DelegatingConnection.java
@@ -41,6 +41,8 @@ import java.util.Map;
 import java.util.Properties;
 import java.util.concurrent.Executor;
 
+import org.apache.tomcat.dbcp.dbcp2.managed.ManagedConnection;
+
 /**
  * A base delegating implementation of {@link Connection}.
  * 
@@ -78,7 +80,7 @@ public class DelegatingConnection 
extends AbandonedTrace i
 /**
  * Creates a wrapper for the Connection which traces this Connection in 
the AbandonedObjectPool.
  *
- * @param connection the {@link Connection} to delegate all calls to.
+ * @param connection the {@link Connection} to delegate all calls to, may 
be null (see {@link ManagedConnection}).
  */
 public DelegatingConnection(final C connection) {
 this.connection = connection;
@@ -104,11 +106,12 @@ public class DelegatingConnection 
extends AbandonedTrace i
 protected void checkOpen() throws SQLException {
 if (closed) {
 if (null != connection) {
-String label = "";
+String label;
 try {
 label = connection.toString();
-} catch (final Exception ignored) {
-// ignore, leave label empty
+} catch (final Exception e) {
+// leave label empty
+label = "";
 }
 throw new SQLException("Connection " + label + " is closed.");
 }
@@ -889,7 +892,7 @@ public class DelegatingConnection 
extends AbandonedTrace i
  * Sets my delegate.
  *
  * @param connection
- *my delegate.
+ *my delegate, may be null.
  */
 public void setDelegate(final C connection) {
 this.connection = connection;
diff --git a/java/org/apache/tomcat/dbcp/dbcp2/DelegatingDatabaseMetaData.java 
b/java/org/apache/tomcat/dbcp/dbcp2/DelegatingDatabaseMetaData.java
index 00f816cbd2..fa9bfaa34e 1006

[tomcat] branch 10.1.x updated: Update DBCP to f131286 (2023-03-08, 2.10.0-SNAPSHOT)

[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 41818a5841 Update DBCP to f131286 (2023-03-08, 2.10.0-SNAPSHOT)
41818a5841 is described below

commit 41818a5841b2e23f4579c8038c101abc893d3d03
Author: Mark Thomas 
AuthorDate: Wed Mar 8 14:26:34 2023 +

Update DBCP to f131286 (2023-03-08, 2.10.0-SNAPSHOT)

This corrects a regression introduced in 10.1.5
---
 MERGE.txt  |  2 +-
 .../apache/tomcat/dbcp/dbcp2/AbandonedTrace.java   | 10 +---
 .../apache/tomcat/dbcp/dbcp2/DataSourceMXBean.java |  7 --
 .../tomcat/dbcp/dbcp2/DelegatingConnection.java| 13 +++
 .../dbcp/dbcp2/DelegatingDatabaseMetaData.java |  1 -
 java/org/apache/tomcat/dbcp/dbcp2/PStmtKey.java|  2 +-
 .../dbcp/dbcp2/PoolableCallableStatement.java  |  4 +---
 .../dbcp/dbcp2/PoolablePreparedStatement.java  |  4 +---
 .../dbcp/dbcp2/cpdsadapter/DriverAdapterCPDS.java  |  4 ++--
 .../dbcp2/cpdsadapter/PooledConnectionImpl.java| 27 --
 .../dbcp2/datasources/InstanceKeyDataSource.java   |  1 -
 webapps/docs/changelog.xml |  5 
 12 files changed, 41 insertions(+), 39 deletions(-)

diff --git a/MERGE.txt b/MERGE.txt
index 63075f061d..5f98d88f84 100644
--- a/MERGE.txt
+++ b/MERGE.txt
@@ -77,4 +77,4 @@ Sub-tree
 src/main/java/org/apache/commons/dbcp2
 src/main/resources/org/apache/commons/dbcp2
 The SHA1 ID / tag for the most recent commit to be merged to Tomcat is:
-f13128604536e78bb1b45b44f74128e93cfbb7cc (2023-01-03)
+b1e0c86d101aa43029625eb191aaee4306911702 (2023-03-08)
diff --git a/java/org/apache/tomcat/dbcp/dbcp2/AbandonedTrace.java 
b/java/org/apache/tomcat/dbcp/dbcp2/AbandonedTrace.java
index a094f16d31..e57b4108a1 100644
--- a/java/org/apache/tomcat/dbcp/dbcp2/AbandonedTrace.java
+++ b/java/org/apache/tomcat/dbcp/dbcp2/AbandonedTrace.java
@@ -37,6 +37,12 @@ import org.apache.tomcat.dbcp.pool2.TrackedUse;
  */
 public class AbandonedTrace implements TrackedUse, AutoCloseable {
 
+static void add(AbandonedTrace receiver, AbandonedTrace trace) {
+if (receiver != null) {
+receiver.addTrace(trace);
+}
+}
+
 /** A list of objects created by children of this object. */
 private final List> traceList = new 
ArrayList<>();
 
@@ -152,9 +158,7 @@ public class AbandonedTrace implements TrackedUse, 
AutoCloseable {
  *AbandonedTrace parent object.
  */
 private void init(final AbandonedTrace parent) {
-if (parent != null) {
-parent.addTrace(this);
-}
+AbandonedTrace.add(parent, this);
 }
 
 /**
diff --git a/java/org/apache/tomcat/dbcp/dbcp2/DataSourceMXBean.java 
b/java/org/apache/tomcat/dbcp/dbcp2/DataSourceMXBean.java
index f6dbef1dd8..7315c5889b 100644
--- a/java/org/apache/tomcat/dbcp/dbcp2/DataSourceMXBean.java
+++ b/java/org/apache/tomcat/dbcp/dbcp2/DataSourceMXBean.java
@@ -142,7 +142,6 @@ public interface DataSourceMXBean {
  *
  * @return {@link BasicDataSource#getMaxConnLifetimeMillis()}.
  */
-@SuppressWarnings("javadoc")
 long getMaxConnLifetimeMillis();
 
 /**
@@ -171,7 +170,6 @@ public interface DataSourceMXBean {
  *
  * @return {@link BasicDataSource#getMaxWaitMillis()}.
  */
-@SuppressWarnings("javadoc")
 long getMaxWaitMillis();
 
 /**
@@ -179,7 +177,6 @@ public interface DataSourceMXBean {
  *
  * @return {@link BasicDataSource#getMinEvictableIdleTimeMillis()}.
  */
-@SuppressWarnings("javadoc")
 long getMinEvictableIdleTimeMillis();
 
 /**
@@ -229,7 +226,6 @@ public interface DataSourceMXBean {
  *
  * @return {@link BasicDataSource#getRemoveAbandonedTimeout()}.
  */
-@SuppressWarnings("javadoc")
 int getRemoveAbandonedTimeout();
 
 /**
@@ -237,7 +233,6 @@ public interface DataSourceMXBean {
  *
  * @return {@link BasicDataSource#getSoftMinEvictableIdleTimeMillis()}.
  */
-@SuppressWarnings("javadoc")
 long getSoftMinEvictableIdleTimeMillis();
 
 /**
@@ -266,7 +261,6 @@ public interface DataSourceMXBean {
  *
  * @return {@link BasicDataSource#getTimeBetweenEvictionRunsMillis()}.
  */
-@SuppressWarnings("javadoc")
 long getTimeBetweenEvictionRunsMillis();
 
 /**
@@ -295,7 +289,6 @@ public interface DataSourceMXBean {
  *
  * @return {@link BasicDataSource#getValidationQueryTimeout()}.
  */
-@SuppressWarnings("javadoc")
 int getValidationQueryTimeout();
 
 /**
diff --git a/java/org/apache/tomcat/dbcp/dbcp2/DelegatingConnection.java 
b/java/org/apache/tomcat/dbcp/dbcp2/DelegatingConnection.java
index 6394875366..a9e620d3b7 100644
--- a/java/org/apache/tomcat/dbcp/dbcp2/DelegatingConnection.java
+++ b/java/org/apache/tomcat/dbcp/d

[tomcat] branch 9.0.x updated: Update DBCP to f131286 (2023-03-08, 2.10.0-SNAPSHOT)

[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 7a173495d5 Update DBCP to f131286 (2023-03-08, 2.10.0-SNAPSHOT)
7a173495d5 is described below

commit 7a173495d5e5acb542059e00a9f824af026d697a
Author: Mark Thomas 
AuthorDate: Wed Mar 8 14:26:34 2023 +

Update DBCP to f131286 (2023-03-08, 2.10.0-SNAPSHOT)

This corrects a regression introduced in 10.1.5
---
 MERGE.txt  |  2 +-
 .../apache/tomcat/dbcp/dbcp2/AbandonedTrace.java   | 10 +---
 .../tomcat/dbcp/dbcp2/DelegatingConnection.java| 13 +++
 .../dbcp/dbcp2/DelegatingDatabaseMetaData.java |  1 -
 java/org/apache/tomcat/dbcp/dbcp2/PStmtKey.java|  2 +-
 .../dbcp/dbcp2/PoolableCallableStatement.java  |  4 +---
 .../dbcp/dbcp2/PoolablePreparedStatement.java  |  4 +---
 .../dbcp/dbcp2/cpdsadapter/DriverAdapterCPDS.java  |  4 ++--
 .../dbcp2/cpdsadapter/PooledConnectionImpl.java| 27 --
 .../dbcp2/datasources/InstanceKeyDataSource.java   |  1 -
 webapps/docs/changelog.xml |  5 
 11 files changed, 41 insertions(+), 32 deletions(-)

diff --git a/MERGE.txt b/MERGE.txt
index 82df920672..8f24ba74ea 100644
--- a/MERGE.txt
+++ b/MERGE.txt
@@ -72,4 +72,4 @@ Sub-tree
 src/main/java/org/apache/commons/dbcp2
 src/main/resources/org/apache/commons/dbcp2
 The SHA1 ID / tag for the most recent commit to be merged to Tomcat is:
-f13128604536e78bb1b45b44f74128e93cfbb7cc (2023-01-03)
+b1e0c86d101aa43029625eb191aaee4306911702 (2023-03-08)
diff --git a/java/org/apache/tomcat/dbcp/dbcp2/AbandonedTrace.java 
b/java/org/apache/tomcat/dbcp/dbcp2/AbandonedTrace.java
index a094f16d31..e57b4108a1 100644
--- a/java/org/apache/tomcat/dbcp/dbcp2/AbandonedTrace.java
+++ b/java/org/apache/tomcat/dbcp/dbcp2/AbandonedTrace.java
@@ -37,6 +37,12 @@ import org.apache.tomcat.dbcp.pool2.TrackedUse;
  */
 public class AbandonedTrace implements TrackedUse, AutoCloseable {
 
+static void add(AbandonedTrace receiver, AbandonedTrace trace) {
+if (receiver != null) {
+receiver.addTrace(trace);
+}
+}
+
 /** A list of objects created by children of this object. */
 private final List> traceList = new 
ArrayList<>();
 
@@ -152,9 +158,7 @@ public class AbandonedTrace implements TrackedUse, 
AutoCloseable {
  *AbandonedTrace parent object.
  */
 private void init(final AbandonedTrace parent) {
-if (parent != null) {
-parent.addTrace(this);
-}
+AbandonedTrace.add(parent, this);
 }
 
 /**
diff --git a/java/org/apache/tomcat/dbcp/dbcp2/DelegatingConnection.java 
b/java/org/apache/tomcat/dbcp/dbcp2/DelegatingConnection.java
index 6394875366..a9e620d3b7 100644
--- a/java/org/apache/tomcat/dbcp/dbcp2/DelegatingConnection.java
+++ b/java/org/apache/tomcat/dbcp/dbcp2/DelegatingConnection.java
@@ -41,6 +41,8 @@ import java.util.Map;
 import java.util.Properties;
 import java.util.concurrent.Executor;
 
+import org.apache.tomcat.dbcp.dbcp2.managed.ManagedConnection;
+
 /**
  * A base delegating implementation of {@link Connection}.
  * 
@@ -78,7 +80,7 @@ public class DelegatingConnection 
extends AbandonedTrace i
 /**
  * Creates a wrapper for the Connection which traces this Connection in 
the AbandonedObjectPool.
  *
- * @param connection the {@link Connection} to delegate all calls to.
+ * @param connection the {@link Connection} to delegate all calls to, may 
be null (see {@link ManagedConnection}).
  */
 public DelegatingConnection(final C connection) {
 this.connection = connection;
@@ -104,11 +106,12 @@ public class DelegatingConnection 
extends AbandonedTrace i
 protected void checkOpen() throws SQLException {
 if (closed) {
 if (null != connection) {
-String label = "";
+String label;
 try {
 label = connection.toString();
-} catch (final Exception ignored) {
-// ignore, leave label empty
+} catch (final Exception e) {
+// leave label empty
+label = "";
 }
 throw new SQLException("Connection " + label + " is closed.");
 }
@@ -889,7 +892,7 @@ public class DelegatingConnection 
extends AbandonedTrace i
  * Sets my delegate.
  *
  * @param connection
- *my delegate.
+ *my delegate, may be null.
  */
 public void setDelegate(final C connection) {
 this.connection = connection;
diff --git a/java/org/apache/tomcat/dbcp/dbcp2/DelegatingDatabaseMetaData.java 
b/java/org/apache/tomcat/dbcp/dbcp2/DelegatingDatabaseMetaData.java
index 00f816cbd2..fa9bfaa34e 10064

Re: PersistentManager, PersistentValve, and DataSource/JDBCStore can cause PK violations

[Christopher Schultz]

Romain,

On 3/8/23 04:10, Romain Manni-Bucau wrote:

Seems doing it only there will get back to the issue the synchronization
was introduced (there are other synchronized(session) for "local" instance).


Don't forget that there are multiple reasons to synchronize on a 
session, and they solve different problems. Some examples:


1. Don't corrupt the internal state of the Session

synchronized(session) {
  Integer count = (Integer)session.getAttribute("count");
  count = count +1;
  session.setAttribute("count", count);
}

This ensures that you don't "count twice" or fail to count even once 
when there are multiple threads in play. There are application business 
reasons to do this kind of thing, and this doesn't solve them all (e.g. 
across a cluster, or for different threads working on the "same" session 
but with different JVM objects representing them).


If there are multiple JVM objects in play, this is still appropriate 
because it solves the immediate problem.


2. Don't do something with the session simultaneously that's Important

That's where this proposal falls: the PresistentManager knows it only 
wants to do something very specific, here, and no other code needs to 
know about it or cares about it.


So I think it's okay not to worry about /other/ instances of 
synchronized(session) in the code and having to update those. Those are 
being done for a different reason.



However you hit a real point, the instance does not have to be stable, only
its equals/hashCode could be considered stable so guess the idea would be
to add to the session *manager* a getLock() method (don't think a RWLock is
needed there due to current usage) and use it instead of synchronized.


The synchronization mechanism itself isn't super important, it's just 
that we want to:


1. Perform operations for this one session (regardless of how many JVM 
objects are present in memory)


2. Allow other sessions to continue without interfering with them

It would be easy to just synchronzied(this) and never have a problem, 
but that kills performance.



The lock eviction should get some kind of delay for its own eviction and
not be evicted directly with the session to work.
A trivial impl could be to use a Map<$sessionId, $lock> and add a default
delay of 0 when purely in memory and a few seconds when a remote manager is
used (this logic belonging to the manager).


Are you talking about a distributed Manager that can handle 
cross-cluster synchronization?


A Map of sessionId -> Lock would work locally, and may be the better 
solution but it does require more memory, more management, 
synchronization (or ConcurrentMap, etc.), etc.



That said it stays a workaround and a better protocol handling that in a
cluster can be a more robust (but more complex) solution so not sure which
compromise is the best.


I'd like to solve the single-JVM case first, here, but I think you have 
some interesting ideas for the future.


If I were implementing a clustered application, I would not choose 
HttpSession as the way to share data across the cluster, for exactly 
these reasons.


-chris


Le mer. 8 mars 2023 à 04:43, Han Li  a écrit :





On Mar 8, 2023, at 07:29, Christopher Schultz <

ch...@christopherschultz.net> wrote:


All,

Please see https://bz.apache.org/bugzilla/show_bug.cgi?id=66513 for

reference.


It appears that the synchronization used by the PersistentManager can

cause problems when used with the PersistentValve and DataSource/JDBCStore.


The problem is that PersistentManager assumes that the Session object

can be used as a synchronization monitor to load/update the session in the
Store. The DataSource/JDBCStore implementation uses an INSERT to create a
new session, and a DELETE-then-INSERT to re-write the session data in the
db.


When two requests arrive simultaneously, thread scheduling can cause

DELETE-then-DELETE-then-INSERT-then-INSERT which causes a duplicate PK
violation.


If the PersistentValve were not in use, the in-memory Session would be

stable. PersistentValve re-loads the Session from the Store on ever
request, rendering the PersistentManager's synchronized(session) attempt to
protect things useless.


I think a simple way to fix this might be to change:

// PersistentManager.java:478~
if(session != null) {
synchronized(session){
session = super.findSession(session.getIdInternal());
if(session != null){
   // To keep any external calling code from messing up

the

   // concurrency.
   session.access();
   session.endAccess();
}
}
}

to this:

if(session != null) {
sessionId = String.intern(sessionId);
synchronized(sessionId){
session = super.findSession(session.getIdInternal());
if(session != null){
   // To keep any external calling code from messing up

the

Re: PersistentManager, PersistentValve, and DataSource/JDBCStore can cause PK violations

[Romain Manni-Bucau]

Le mer. 8 mars 2023 à 16:23, Christopher Schultz <
ch...@christopherschultz.net> a écrit :

> Romain,
>
> On 3/8/23 04:10, Romain Manni-Bucau wrote:
> > Seems doing it only there will get back to the issue the synchronization
> > was introduced (there are other synchronized(session) for "local"
> instance).
>
> Don't forget that there are multiple reasons to synchronize on a
> session, and they solve different problems. Some examples:
>
> 1. Don't corrupt the internal state of the Session
>
> synchronized(session) {
>Integer count = (Integer)session.getAttribute("count");
>count = count +1;
>session.setAttribute("count", count);
> }
>
> This ensures that you don't "count twice" or fail to count even once
> when there are multiple threads in play. There are application business
> reasons to do this kind of thing, and this doesn't solve them all (e.g.
> across a cluster, or for different threads working on the "same" session
> but with different JVM objects representing them).
>
> If there are multiple JVM objects in play, this is still appropriate
> because it solves the immediate problem.
>

Hmm, i didn't check recently but this was not guaranteed by servlet spec,
is it - if so the issue you describe wouldn't be probably too?


>
> 2. Don't do something with the session simultaneously that's Important
>
> That's where this proposal falls: the PresistentManager knows it only
> wants to do something very specific, here, and no other code needs to
> know about it or cares about it.
>
> So I think it's okay not to worry about /other/ instances of
> synchronized(session) in the code and having to update those. Those are
> being done for a different reason.
>
> > However you hit a real point, the instance does not have to be stable,
> only
> > its equals/hashCode could be considered stable so guess the idea would be
> > to add to the session *manager* a getLock() method (don't think a RWLock
> is
> > needed there due to current usage) and use it instead of synchronized.
>
> The synchronization mechanism itself isn't super important, it's just
> that we want to:
>
> 1. Perform operations for this one session (regardless of how many JVM
> objects are present in memory)
>
> 2. Allow other sessions to continue without interfering with them
>
> It would be easy to just synchronzied(this) and never have a problem,
> but that kills performance.
>
> > The lock eviction should get some kind of delay for its own eviction and
> > not be evicted directly with the session to work.
> > A trivial impl could be to use a Map<$sessionId, $lock> and add a default
> > delay of 0 when purely in memory and a few seconds when a remote manager
> is
> > used (this logic belonging to the manager).
>
> Are you talking about a distributed Manager that can handle
> cross-cluster synchronization?
>

No, the (concurrent)map solution was really for local instances just adding
to support a small latency in session destruction/recreation.


>
> A Map of sessionId -> Lock would work locally, and may be the better
> solution but it does require more memory, more management,
> synchronization (or ConcurrentMap, etc.), etc.
>

For me it is not a big deal when you use session because it is exactly the
issue you hit with session themselves (in a worse manner since sessions are
not "a few session" latent objects) so sounds/sounded like a compromise
short term.


>
> > That said it stays a workaround and a better protocol handling that in a
> > cluster can be a more robust (but more complex) solution so not sure
> which
> > compromise is the best.
>
> I'd like to solve the single-JVM case first, here, but I think you have
> some interesting ideas for the future.
>
> If I were implementing a clustered application, I would not choose
> HttpSession as the way to share data across the cluster, for exactly
> these reasons.
>

+1


>
> -chris
>
> > Le mer. 8 mars 2023 à 04:43, Han Li  a écrit :
> >
> >>
> >>
> >>> On Mar 8, 2023, at 07:29, Christopher Schultz <
> >> ch...@christopherschultz.net> wrote:
> >>>
> >>> All,
> >>>
> >>> Please see https://bz.apache.org/bugzilla/show_bug.cgi?id=66513 for
> >> reference.
> >>>
> >>> It appears that the synchronization used by the PersistentManager can
> >> cause problems when used with the PersistentValve and
> DataSource/JDBCStore.
> >>>
> >>> The problem is that PersistentManager assumes that the Session object
> >> can be used as a synchronization monitor to load/update the session in
> the
> >> Store. The DataSource/JDBCStore implementation uses an INSERT to create
> a
> >> new session, and a DELETE-then-INSERT to re-write the session data in
> the
> >> db.
> >>>
> >>> When two requests arrive simultaneously, thread scheduling can cause
> >> DELETE-then-DELETE-then-INSERT-then-INSERT which causes a duplicate PK
> >> violation.
> >>>
> >>> If the PersistentValve were not in use, the in-memory Session would be
> >> stable. PersistentValve re-loads the Session from the Store on ever
> >> request, renderi

[GitHub] [tomcat] ChristopherSchultz opened a new pull request, #596: Use SELECT...FOR UPDATE to update session data in the database instead of DELETE, INSERT.

[via GitHub]

ChristopherSchultz opened a new pull request, #596:
URL: https://github.com/apache/tomcat/pull/596

   I have only compile-tested this; I wanted to get feedback on the approach, 
how to handle errors, etc.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


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

[GitHub] [tomcat] ChristopherSchultz opened a new pull request, #597: Use a deep copy of query stats whose values won't change during sorting.

[via GitHub]

ChristopherSchultz opened a new pull request, #597:
URL: https://github.com/apache/tomcat/pull/597

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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


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

[Bug 58489] QueryStatsComparator throws IllegalArgumentException: Comparison method violates its general contract!

[bugzilla]

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

--- Comment #14 from Christopher Schultz  ---
https://github.com/apache/tomcat/pull/597

-- 
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 66508] Tomcat after a GC pause causes the HTTP threads to be blocked to acquire a semaphore to process WebSockets connection closure.

[bugzilla]

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

--- Comment #3 from Mark Thomas  ---
Thanks for the steps to reproduce.

I'll note for the record that you also need to be using the NIO2 connector.

I can see what the root cause is. The write in "Event 3" fails and isn't
cleaned up properly which then blocks the close message. I am currently
exploring options for fixing this.

-- 
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 10.1.x updated: Fix BZ-66508 - avoid delay on close after write error with NIO2

[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 46590145a9 Fix BZ-66508 - avoid delay on close after write error with 
NIO2
46590145a9 is described below

commit 46590145a996627390eadb8f81afd587ccb0
Author: Mark Thomas 
AuthorDate: Wed Mar 8 18:23:05 2023 +

Fix BZ-66508 - avoid delay on close after write error with NIO2

https://bz.apache.org/bugzilla/show_bug.cgi?id=66508
---
 .../websocket/server/WsHttpUpgradeHandler.java | 22 +-
 .../server/WsRemoteEndpointImplServer.java |  2 +-
 webapps/docs/changelog.xml |  9 +
 3 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/java/org/apache/tomcat/websocket/server/WsHttpUpgradeHandler.java 
b/java/org/apache/tomcat/websocket/server/WsHttpUpgradeHandler.java
index ef39f74136..22e9c93ff3 100644
--- a/java/org/apache/tomcat/websocket/server/WsHttpUpgradeHandler.java
+++ b/java/org/apache/tomcat/websocket/server/WsHttpUpgradeHandler.java
@@ -16,6 +16,7 @@
  */
 package org.apache.tomcat.websocket.server;
 
+import java.io.EOFException;
 import java.io.IOException;
 import java.util.List;
 import java.util.Map;
@@ -241,11 +242,22 @@ public class WsHttpUpgradeHandler implements 
InternalHttpUpgradeHandler {
 private void close(CloseReason cr) {
 /*
  * Any call to this method is a result of a problem reading from the
- * client. At this point that state of the connection is unknown.
- * Attempt to send a close frame to the client and then close the 
socket
- * immediately. There is no point in waiting for a close frame from the
- * client because there is no guarantee that we can recover from
- * whatever messed up state the client put the connection into.
+ * client. At this point that state of the connection is unknown. First
+ * attempt to clear the handler for any in-flight message write (that
+ * probably failed). If using NIO2 is is possible that the original
+ * error occurred on a write but this method was called during a read.
+ * The in-progress write will block the sending of the close frame
+ * unless the handler is cleared (effectively signalling the write
+ * failed).
+ */
+wsRemoteEndpointServer.clearHandler(new EOFException(), true);
+
+/* Then:
+ *  - send a close frame to the client
+ * - close the socket immediately.
+ * There is no point in waiting for a close frame from the client
+ * because there is no guarantee that we can recover from whatever
+ * messed up state the client put the connection into.
  */
 wsSession.onClose(cr);
 }
diff --git 
a/java/org/apache/tomcat/websocket/server/WsRemoteEndpointImplServer.java 
b/java/org/apache/tomcat/websocket/server/WsRemoteEndpointImplServer.java
index 735eb40104..524f8e972d 100644
--- a/java/org/apache/tomcat/websocket/server/WsRemoteEndpointImplServer.java
+++ b/java/org/apache/tomcat/websocket/server/WsRemoteEndpointImplServer.java
@@ -268,7 +268,7 @@ public class WsRemoteEndpointImplServer extends 
WsRemoteEndpointImplBase {
  *  requirements of
  *  {@link jakarta.websocket.RemoteEndpoint.Async}
  */
-private void clearHandler(Throwable t, boolean useDispatch) {
+void clearHandler(Throwable t, boolean useDispatch) {
 // Setting the result marks this (partial) message as
 // complete which means the next one may be sent which
 // could update the value of the handler. Therefore, keep a
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 6f2d84842c..a2bec27624 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -136,6 +136,15 @@
   
 
   
+  
+
+  
+66508: When using WebSocket with NIO2, avoid waiting for
+a timeout before sending the close frame if an I/O error occurs during 
a
+write. (markt)
+  
+
+  
   
 
   


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

[tomcat] branch main updated: Fix BZ-66508 - avoid delay on close after write error with NIO2

[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 a999580acb Fix BZ-66508 - avoid delay on close after write error with 
NIO2
a999580acb is described below

commit a999580acb29a47fb1910cbba448ee7fadf1599d
Author: Mark Thomas 
AuthorDate: Wed Mar 8 18:23:05 2023 +

Fix BZ-66508 - avoid delay on close after write error with NIO2

https://bz.apache.org/bugzilla/show_bug.cgi?id=66508
---
 .../websocket/server/WsHttpUpgradeHandler.java | 22 +-
 .../server/WsRemoteEndpointImplServer.java |  2 +-
 webapps/docs/changelog.xml |  9 +
 3 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/java/org/apache/tomcat/websocket/server/WsHttpUpgradeHandler.java 
b/java/org/apache/tomcat/websocket/server/WsHttpUpgradeHandler.java
index ef39f74136..22e9c93ff3 100644
--- a/java/org/apache/tomcat/websocket/server/WsHttpUpgradeHandler.java
+++ b/java/org/apache/tomcat/websocket/server/WsHttpUpgradeHandler.java
@@ -16,6 +16,7 @@
  */
 package org.apache.tomcat.websocket.server;
 
+import java.io.EOFException;
 import java.io.IOException;
 import java.util.List;
 import java.util.Map;
@@ -241,11 +242,22 @@ public class WsHttpUpgradeHandler implements 
InternalHttpUpgradeHandler {
 private void close(CloseReason cr) {
 /*
  * Any call to this method is a result of a problem reading from the
- * client. At this point that state of the connection is unknown.
- * Attempt to send a close frame to the client and then close the 
socket
- * immediately. There is no point in waiting for a close frame from the
- * client because there is no guarantee that we can recover from
- * whatever messed up state the client put the connection into.
+ * client. At this point that state of the connection is unknown. First
+ * attempt to clear the handler for any in-flight message write (that
+ * probably failed). If using NIO2 is is possible that the original
+ * error occurred on a write but this method was called during a read.
+ * The in-progress write will block the sending of the close frame
+ * unless the handler is cleared (effectively signalling the write
+ * failed).
+ */
+wsRemoteEndpointServer.clearHandler(new EOFException(), true);
+
+/* Then:
+ *  - send a close frame to the client
+ * - close the socket immediately.
+ * There is no point in waiting for a close frame from the client
+ * because there is no guarantee that we can recover from whatever
+ * messed up state the client put the connection into.
  */
 wsSession.onClose(cr);
 }
diff --git 
a/java/org/apache/tomcat/websocket/server/WsRemoteEndpointImplServer.java 
b/java/org/apache/tomcat/websocket/server/WsRemoteEndpointImplServer.java
index 735eb40104..524f8e972d 100644
--- a/java/org/apache/tomcat/websocket/server/WsRemoteEndpointImplServer.java
+++ b/java/org/apache/tomcat/websocket/server/WsRemoteEndpointImplServer.java
@@ -268,7 +268,7 @@ public class WsRemoteEndpointImplServer extends 
WsRemoteEndpointImplBase {
  *  requirements of
  *  {@link jakarta.websocket.RemoteEndpoint.Async}
  */
-private void clearHandler(Throwable t, boolean useDispatch) {
+void clearHandler(Throwable t, boolean useDispatch) {
 // Setting the result marks this (partial) message as
 // complete which means the next one may be sent which
 // could update the value of the handler. Therefore, keep a
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 48fd16e923..abaa23a342 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -146,6 +146,15 @@
   
 
   
+  
+
+  
+66508: When using WebSocket with NIO2, avoid waiting for
+a timeout before sending the close frame if an I/O error occurs during 
a
+write. (markt)
+  
+
+  
   
 
   


-
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: Fix BZ-66508 - avoid delay on close after write error with NIO2

[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 3d5cd321b7 Fix BZ-66508 - avoid delay on close after write error with 
NIO2
3d5cd321b7 is described below

commit 3d5cd321b7a41ca4a3da8c20a80737021779d742
Author: Mark Thomas 
AuthorDate: Wed Mar 8 18:23:05 2023 +

Fix BZ-66508 - avoid delay on close after write error with NIO2

https://bz.apache.org/bugzilla/show_bug.cgi?id=66508
---
 .../websocket/server/WsHttpUpgradeHandler.java | 22 +-
 .../server/WsRemoteEndpointImplServer.java |  2 +-
 webapps/docs/changelog.xml |  9 +
 3 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/java/org/apache/tomcat/websocket/server/WsHttpUpgradeHandler.java 
b/java/org/apache/tomcat/websocket/server/WsHttpUpgradeHandler.java
index b49858ccdc..7660c48cdf 100644
--- a/java/org/apache/tomcat/websocket/server/WsHttpUpgradeHandler.java
+++ b/java/org/apache/tomcat/websocket/server/WsHttpUpgradeHandler.java
@@ -16,6 +16,7 @@
  */
 package org.apache.tomcat.websocket.server;
 
+import java.io.EOFException;
 import java.io.IOException;
 import java.util.List;
 import java.util.Map;
@@ -241,11 +242,22 @@ public class WsHttpUpgradeHandler implements 
InternalHttpUpgradeHandler {
 private void close(CloseReason cr) {
 /*
  * Any call to this method is a result of a problem reading from the
- * client. At this point that state of the connection is unknown.
- * Attempt to send a close frame to the client and then close the 
socket
- * immediately. There is no point in waiting for a close frame from the
- * client because there is no guarantee that we can recover from
- * whatever messed up state the client put the connection into.
+ * client. At this point that state of the connection is unknown. First
+ * attempt to clear the handler for any in-flight message write (that
+ * probably failed). If using NIO2 is is possible that the original
+ * error occurred on a write but this method was called during a read.
+ * The in-progress write will block the sending of the close frame
+ * unless the handler is cleared (effectively signalling the write
+ * failed).
+ */
+wsRemoteEndpointServer.clearHandler(new EOFException(), true);
+
+/* Then:
+ *  - send a close frame to the client
+ * - close the socket immediately.
+ * There is no point in waiting for a close frame from the client
+ * because there is no guarantee that we can recover from whatever
+ * messed up state the client put the connection into.
  */
 wsSession.onClose(cr);
 }
diff --git 
a/java/org/apache/tomcat/websocket/server/WsRemoteEndpointImplServer.java 
b/java/org/apache/tomcat/websocket/server/WsRemoteEndpointImplServer.java
index 1b5ac740f1..28d591f65d 100644
--- a/java/org/apache/tomcat/websocket/server/WsRemoteEndpointImplServer.java
+++ b/java/org/apache/tomcat/websocket/server/WsRemoteEndpointImplServer.java
@@ -268,7 +268,7 @@ public class WsRemoteEndpointImplServer extends 
WsRemoteEndpointImplBase {
  *  requirements of
  *  {@link javax.websocket.RemoteEndpoint.Async}
  */
-private void clearHandler(Throwable t, boolean useDispatch) {
+void clearHandler(Throwable t, boolean useDispatch) {
 // Setting the result marks this (partial) message as
 // complete which means the next one may be sent which
 // could update the value of the handler. Therefore, keep a
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 9c775e9f3d..42e5dec4db 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -136,6 +136,15 @@
   
 
   
+  
+
+  
+66508: When using WebSocket with NIO2, avoid waiting for
+a timeout before sending the close frame if an I/O error occurs during 
a
+write. (markt)
+  
+
+  
   
 
   


-
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 BZ-66508 - avoid delay on close after write error with NIO2

[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 683b74f75e Fix BZ-66508 - avoid delay on close after write error with 
NIO2
683b74f75e is described below

commit 683b74f75ebf89945f0f1c5af42e52f66ace9de8
Author: Mark Thomas 
AuthorDate: Wed Mar 8 18:23:05 2023 +

Fix BZ-66508 - avoid delay on close after write error with NIO2

https://bz.apache.org/bugzilla/show_bug.cgi?id=66508
---
 .../websocket/server/WsHttpUpgradeHandler.java | 22 +-
 .../server/WsRemoteEndpointImplServer.java |  2 +-
 webapps/docs/changelog.xml |  9 +
 3 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/java/org/apache/tomcat/websocket/server/WsHttpUpgradeHandler.java 
b/java/org/apache/tomcat/websocket/server/WsHttpUpgradeHandler.java
index b49858ccdc..7660c48cdf 100644
--- a/java/org/apache/tomcat/websocket/server/WsHttpUpgradeHandler.java
+++ b/java/org/apache/tomcat/websocket/server/WsHttpUpgradeHandler.java
@@ -16,6 +16,7 @@
  */
 package org.apache.tomcat.websocket.server;
 
+import java.io.EOFException;
 import java.io.IOException;
 import java.util.List;
 import java.util.Map;
@@ -241,11 +242,22 @@ public class WsHttpUpgradeHandler implements 
InternalHttpUpgradeHandler {
 private void close(CloseReason cr) {
 /*
  * Any call to this method is a result of a problem reading from the
- * client. At this point that state of the connection is unknown.
- * Attempt to send a close frame to the client and then close the 
socket
- * immediately. There is no point in waiting for a close frame from the
- * client because there is no guarantee that we can recover from
- * whatever messed up state the client put the connection into.
+ * client. At this point that state of the connection is unknown. First
+ * attempt to clear the handler for any in-flight message write (that
+ * probably failed). If using NIO2 is is possible that the original
+ * error occurred on a write but this method was called during a read.
+ * The in-progress write will block the sending of the close frame
+ * unless the handler is cleared (effectively signalling the write
+ * failed).
+ */
+wsRemoteEndpointServer.clearHandler(new EOFException(), true);
+
+/* Then:
+ *  - send a close frame to the client
+ * - close the socket immediately.
+ * There is no point in waiting for a close frame from the client
+ * because there is no guarantee that we can recover from whatever
+ * messed up state the client put the connection into.
  */
 wsSession.onClose(cr);
 }
diff --git 
a/java/org/apache/tomcat/websocket/server/WsRemoteEndpointImplServer.java 
b/java/org/apache/tomcat/websocket/server/WsRemoteEndpointImplServer.java
index e5987a2703..f94c871b85 100644
--- a/java/org/apache/tomcat/websocket/server/WsRemoteEndpointImplServer.java
+++ b/java/org/apache/tomcat/websocket/server/WsRemoteEndpointImplServer.java
@@ -268,7 +268,7 @@ public class WsRemoteEndpointImplServer extends 
WsRemoteEndpointImplBase {
  *  requirements of
  *  {@link javax.websocket.RemoteEndpoint.Async}
  */
-private void clearHandler(Throwable t, boolean useDispatch) {
+void clearHandler(Throwable t, boolean useDispatch) {
 // Setting the result marks this (partial) message as
 // complete which means the next one may be sent which
 // could update the value of the handler. Therefore, keep a
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index affc066fa7..7fa4629893 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -136,6 +136,15 @@
   
 
   
+  
+
+  
+66508: When using WebSocket with NIO2, avoid waiting for
+a timeout before sending the close frame if an I/O error occurs during 
a
+write. (markt)
+  
+
+  
   
 
   


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

[Bug 66508] Tomcat after a GC pause causes the HTTP threads to be blocked to acquire a semaphore to process WebSockets connection closure.

[bugzilla]

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

Mark Thomas  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |FIXED

--- Comment #4 from Mark Thomas  ---
Fixed in:
- 11.0.x for 11.0.0-M5 onwards
- 10.1.x for 10.1.8 onwards
-  9.0.x for  9.0.74 onwards
-  8.5.x for  8.5.88 onwards

If you'd like to test a dev build before the next release, let me know.

-- 
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: Code clean-up. Formatting. No functional change.

[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 33c6a2edc1 Code clean-up. Formatting. No functional change.
33c6a2edc1 is described below

commit 33c6a2edc1f7f23206c2cd20af1ce90a677dc835
Author: Mark Thomas 
AuthorDate: Wed Mar 8 18:26:52 2023 +

Code clean-up. Formatting. No functional change.
---
 .../apache/tomcat/websocket/pojo/Constants.java|   3 +-
 .../tomcat/websocket/pojo/PojoEndpointBase.java|  49 ++--
 .../tomcat/websocket/pojo/PojoEndpointClient.java  |  10 +-
 .../tomcat/websocket/pojo/PojoEndpointServer.java  |  12 +-
 .../websocket/pojo/PojoMessageHandlerBase.java |  15 +-
 .../pojo/PojoMessageHandlerPartialBase.java|  22 +-
 .../pojo/PojoMessageHandlerPartialBinary.java  |  11 +-
 .../pojo/PojoMessageHandlerPartialText.java|  11 +-
 .../pojo/PojoMessageHandlerWholeBase.java  |  21 +-
 .../pojo/PojoMessageHandlerWholeBinary.java|  23 +-
 .../pojo/PojoMessageHandlerWholePong.java  |  11 +-
 .../pojo/PojoMessageHandlerWholeText.java  |  23 +-
 .../tomcat/websocket/pojo/PojoMethodMapping.java   | 318 -
 .../tomcat/websocket/pojo/PojoPathParam.java   |  12 +-
 .../apache/tomcat/websocket/pojo/package-info.java |   4 +-
 15 files changed, 216 insertions(+), 329 deletions(-)

diff --git a/java/org/apache/tomcat/websocket/pojo/Constants.java 
b/java/org/apache/tomcat/websocket/pojo/Constants.java
index 762ac9fe3f..1eba53131a 100644
--- a/java/org/apache/tomcat/websocket/pojo/Constants.java
+++ b/java/org/apache/tomcat/websocket/pojo/Constants.java
@@ -21,8 +21,7 @@ package org.apache.tomcat.websocket.pojo;
  */
 public class Constants {
 
-public static final String POJO_METHOD_MAPPING_KEY =
-"org.apache.tomcat.websocket.pojo.PojoEndpoint.methodMapping";
+public static final String POJO_METHOD_MAPPING_KEY = 
"org.apache.tomcat.websocket.pojo.PojoEndpoint.methodMapping";
 
 private Constants() {
 // Hide default constructor
diff --git a/java/org/apache/tomcat/websocket/pojo/PojoEndpointBase.java 
b/java/org/apache/tomcat/websocket/pojo/PojoEndpointBase.java
index 98e45cc0e1..8067b72325 100644
--- a/java/org/apache/tomcat/websocket/pojo/PojoEndpointBase.java
+++ b/java/org/apache/tomcat/websocket/pojo/PojoEndpointBase.java
@@ -33,9 +33,8 @@ import org.apache.tomcat.util.ExceptionUtils;
 import org.apache.tomcat.util.res.StringManager;
 
 /**
- * Base implementation (client and server have different concrete
- * implementations) of the wrapper that converts a POJO instance into a
- * WebSocket endpoint instance.
+ * Base implementation (client and server have different concrete 
implementations) of the wrapper that converts a POJO
+ * instance into a WebSocket endpoint instance.
  */
 public abstract class PojoEndpointBase extends Endpoint {
 
@@ -43,11 +42,11 @@ public abstract class PojoEndpointBase extends Endpoint {
 private static final StringManager sm = 
StringManager.getManager(PojoEndpointBase.class);
 
 private Object pojo;
-private final Map pathParameters;
+private final Map pathParameters;
 private PojoMethodMapping methodMapping;
 
 
-protected PojoEndpointBase(Map pathParameters) {
+protected PojoEndpointBase(Map pathParameters) {
 this.pathParameters = pathParameters;
 }
 
@@ -59,22 +58,17 @@ public abstract class PojoEndpointBase extends Endpoint {
 // Add message handlers before calling onOpen since that may trigger a
 // message which in turn could trigger a response and/or close the
 // session
-for (MessageHandler mh : methodMapping.getMessageHandlers(pojo,
-pathParameters, session, config)) {
+for (MessageHandler mh : methodMapping.getMessageHandlers(pojo, 
pathParameters, session, config)) {
 session.addMessageHandler(mh);
 }
 
 if (methodMapping.getOnOpen() != null) {
 try {
-methodMapping.getOnOpen().invoke(pojo,
-methodMapping.getOnOpenArgs(
-pathParameters, session, config));
+methodMapping.getOnOpen().invoke(pojo, 
methodMapping.getOnOpenArgs(pathParameters, session, config));
 
 } catch (IllegalAccessException e) {
 // Reflection related problems
-log.error(sm.getString(
-"pojoEndpointBase.onOpenFail",
-pojo.getClass().getName()), e);
+log.error(sm.getString("pojoEndpointBase.onOpenFail", 
pojo.getClass().getName()), e);
 handleOnOpenOrCloseError(session, e);
 } catch (InvocationTargetException e) {
 Throwable cause = e.getCause();
@@ -107,8 +101,7 @@ public abstract class PojoEndpoin

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

[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 597dcea653 Code clean-up. Formatting. No functional change.
597dcea653 is described below

commit 597dcea65324b44c5bcd9cbb0971fbc640b47805
Author: Mark Thomas 
AuthorDate: Wed Mar 8 18:27:25 2023 +

Code clean-up. Formatting. No functional change.
---
 .../tomcat/websocket/TestConnectionLimit.java  |   9 +-
 .../tomcat/websocket/TestPerMessageDeflate.java|  18 +--
 test/org/apache/tomcat/websocket/TestUtil.java |  82 +
 .../tomcat/websocket/TestWebSocketFrameClient.java |  41 +++
 .../websocket/TestWebSocketFrameClientSSL.java |  54 -
 test/org/apache/tomcat/websocket/TestWsFrame.java  |   6 +-
 .../tomcat/websocket/TestWsPingPongMessages.java   |  19 ++-
 .../tomcat/websocket/TestWsRemoteEndpoint.java |  18 +--
 .../websocket/TestWsSessionSuspendResume.java  |   6 +-
 .../tomcat/websocket/TestWsSubprotocols.java   |  28 ++---
 .../tomcat/websocket/TestWsWebSocketContainer.java | 134 -
 .../TestWsWebSocketContainerGetOpenSessions.java   |  23 ++--
 .../websocket/TestWsWebSocketContainerSSL.java |  24 ++--
 ...WsWebSocketContainerSessionExpiryContainer.java |  19 ++-
 ...stWsWebSocketContainerSessionExpirySession.java |  16 +--
 .../TestWsWebSocketContainerTimeoutClient.java |  16 +--
 .../TestWsWebSocketContainerTimeoutServer.java |  38 +++---
 .../TestWsWebSocketContainerWithProxy.java |   2 +-
 .../apache/tomcat/websocket/TesterAsyncTiming.java |   4 +-
 .../tomcat/websocket/TesterBlockWebSocketSCI.java  |  13 +-
 .../apache/tomcat/websocket/TesterEchoServer.java  |   8 +-
 .../tomcat/websocket/TesterFirehoseServer.java |   7 +-
 .../tomcat/websocket/TesterMessageCountClient.java |  13 +-
 .../websocket/TesterWebSocketClientProxy.java  |   7 +-
 .../tomcat/websocket/TesterWsClientAutobahn.java   |  24 ++--
 .../apache/tomcat/websocket/WebSocketBaseTest.java |   4 +-
 .../websocket/WsWebSocketContainerBaseTest.java|   7 +-
 27 files changed, 244 insertions(+), 396 deletions(-)

diff --git a/test/org/apache/tomcat/websocket/TestConnectionLimit.java 
b/test/org/apache/tomcat/websocket/TestConnectionLimit.java
index 80b4045bda..2408f1a3e5 100644
--- a/test/org/apache/tomcat/websocket/TestConnectionLimit.java
+++ b/test/org/apache/tomcat/websocket/TestConnectionLimit.java
@@ -38,8 +38,7 @@ import 
org.apache.tomcat.websocket.TesterMessageCountClient.TesterProgrammaticEn
 public class TestConnectionLimit extends TomcatBaseTest {
 
 /*
- * Simple test to see how many outgoing connections can be created on a
- * single machine.
+ * Simple test to see how many outgoing connections can be created on a 
single machine.
  */
 @Test
 public void testSingleMachine() throws Exception {
@@ -54,8 +53,7 @@ public class TestConnectionLimit extends TomcatBaseTest {
 
 tomcat.start();
 
-URI uri = new URI("ws://localhost:" + getPort() +
-TesterEchoServer.Config.PATH_ASYNC);
+URI uri = new URI("ws://localhost:" + getPort() + 
TesterEchoServer.Config.PATH_ASYNC);
 AtomicInteger counter = new AtomicInteger(0);
 
 int threadCount = 50;
@@ -87,8 +85,7 @@ public class TestConnectionLimit extends TomcatBaseTest {
 
 @Override
 public void run() {
-WebSocketContainer wsContainer =
-ContainerProvider.getWebSocketContainer();
+WebSocketContainer wsContainer = 
ContainerProvider.getWebSocketContainer();
 
 int count = 0;
 
diff --git a/test/org/apache/tomcat/websocket/TestPerMessageDeflate.java 
b/test/org/apache/tomcat/websocket/TestPerMessageDeflate.java
index 0c6a17ed20..fd62f976a6 100644
--- a/test/org/apache/tomcat/websocket/TestPerMessageDeflate.java
+++ b/test/org/apache/tomcat/websocket/TestPerMessageDeflate.java
@@ -93,17 +93,16 @@ public class TestPerMessageDeflate {
 
 ByteBuffer received = ByteBuffer.allocate(8192);
 
-TransformationResult tr = perMessageDeflateRx.getMoreData(
-compressedPart.getOpCode(), compressedPart.isFin(), 
compressedPart.getRsv(), received);
+TransformationResult tr = 
perMessageDeflateRx.getMoreData(compressedPart.getOpCode(), 
compressedPart.isFin(),
+compressedPart.getRsv(), received);
 
 Assert.assertEquals(8192, received.position());
-Assert.assertEquals(TransformationResult.END_OF_FRAME , tr);
+Assert.assertEquals(TransformationResult.END_OF_FRAME, tr);
 }
 
 
 /*
- * Minimal implementation to enable other transformations to be tested. It
- * is NOT robust.
+ * Minimal implementation to enable other transformations to be tested. It 
is NOT robust.
  */
 private static class TesterTran

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

[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 95487f9312 Code clean-up. Formatting. No functional change.
95487f9312 is described below

commit 95487f93128ebc68953416e1a23c47fa61be0212
Author: Mark Thomas 
AuthorDate: Wed Mar 8 18:27:43 2023 +

Code clean-up. Formatting. No functional change.
---
 .../tomcat/websocket/server/TestAsyncMessages.java | 21 +++---
 .../tomcat/websocket/server/TestClassLoader.java   |  7 +-
 .../apache/tomcat/websocket/server/TestClose.java  |  2 +-
 .../tomcat/websocket/server/TestCloseBug58624.java | 13 ++--
 .../tomcat/websocket/server/TestShutdown.java  | 14 ++--
 .../tomcat/websocket/server/TestSlowClient.java|  3 +-
 .../tomcat/websocket/server/TestUriTemplate.java   | 54 
 .../server/TestWsRemoteEndpointImplServer.java | 28 +++-
 .../websocket/server/TestWsServerContainer.java| 75 --
 .../websocket/server/TesterEndpointConfig.java |  4 +-
 .../tomcat/websocket/server/TesterWsClient.java| 17 ++---
 11 files changed, 98 insertions(+), 140 deletions(-)

diff --git a/test/org/apache/tomcat/websocket/server/TestAsyncMessages.java 
b/test/org/apache/tomcat/websocket/server/TestAsyncMessages.java
index e9a5893781..ed47f05af3 100644
--- a/test/org/apache/tomcat/websocket/server/TestAsyncMessages.java
+++ b/test/org/apache/tomcat/websocket/server/TestAsyncMessages.java
@@ -54,9 +54,7 @@ public class TestAsyncMessages extends TomcatBaseTest {
 
 WebSocketContainer wsContainer = 
ContainerProvider.getWebSocketContainer();
 ClientEndpointConfig clientEndpointConfig = 
ClientEndpointConfig.Builder.create().build();
-Session wsSession = wsContainer.connectToServer(
-TesterProgrammaticEndpoint.class,
-clientEndpointConfig,
+Session wsSession = 
wsContainer.connectToServer(TesterProgrammaticEndpoint.class, 
clientEndpointConfig,
 new URI("ws://localhost:" + getPort() + 
TesterAsyncTiming.Config.PATH));
 
 AsyncTimingClientHandler handler = new AsyncTimingClientHandler();
@@ -80,7 +78,7 @@ public class TestAsyncMessages extends TomcatBaseTest {
 public void onMessage(ByteBuffer message, boolean last) {
 if (lastMessage == 0) {
 // First message. Don't check
-sequence ++;
+sequence++;
 lastMessage = System.nanoTime();
 } else {
 long newTime = System.nanoTime();
@@ -88,9 +86,10 @@ public class TestAsyncMessages extends TomcatBaseTest {
 lastMessage = newTime;
 
 if (sequence == 0) {
-sequence ++;
+sequence++;
 if (message.capacity() != 8192) {
-System.out.println("Expected size 8192 but was [" + 
message.capacity() + "], count [" + count + "]");
+System.out.println(
+"Expected size 8192 but was [" + 
message.capacity() + "], count [" + count + "]");
 fail = true;
 }
 if (diff < 4000) {
@@ -98,9 +97,10 @@ public class TestAsyncMessages extends TomcatBaseTest {
 fail = true;
 }
 } else if (sequence == 1) {
-sequence ++;
+sequence++;
 if (message.capacity() != 8192) {
-System.out.println("Expected size 8192 but was [" + 
message.capacity() + "], count [" + count + "]");
+System.out.println(
+"Expected size 8192 but was [" + 
message.capacity() + "], count [" + count + "]");
 fail = true;
 }
 if (diff > 50) {
@@ -110,7 +110,8 @@ public class TestAsyncMessages extends TomcatBaseTest {
 } else if (sequence == 2) {
 sequence = 0;
 if (message.capacity() != 4096) {
-System.out.println("Expected size 4096 but was [" + 
message.capacity() + "], count [" + count + "]");
+System.out.println(
+"Expected size 4096 but was [" + 
message.capacity() + "], count [" + count + "]");
 fail = true;
 }
 if (diff > 50) {
@@ -120,7 +121,7 @@ public class TestAsyncMessages extends TomcatBaseTest {
 }
 }
 
-count ++;
+count++;
 if (count >= TesterAsyncTiming.Config.ITERATIONS * 3) {
 latch.countDown();
 }
diff --git a/test/org/apache/tomcat/websocket/server/TestCla

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

[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 74ffc1f399 Code clean-up. Formatting. No functional change.
74ffc1f399 is described below

commit 74ffc1f3990d44b374ea0ba740f193b38a961332
Author: Mark Thomas 
AuthorDate: Wed Mar 8 18:28:23 2023 +

Code clean-up. Formatting. No functional change.
---
 .../apache/tomcat/websocket/pojo/Constants.java|   3 +-
 .../tomcat/websocket/pojo/PojoEndpointBase.java|  49 ++--
 .../tomcat/websocket/pojo/PojoEndpointClient.java  |  10 +-
 .../tomcat/websocket/pojo/PojoEndpointServer.java  |  12 +-
 .../websocket/pojo/PojoMessageHandlerBase.java |  15 +-
 .../pojo/PojoMessageHandlerPartialBase.java|  22 +-
 .../pojo/PojoMessageHandlerPartialBinary.java  |  11 +-
 .../pojo/PojoMessageHandlerPartialText.java|  11 +-
 .../pojo/PojoMessageHandlerWholeBase.java  |  21 +-
 .../pojo/PojoMessageHandlerWholeBinary.java|  23 +-
 .../pojo/PojoMessageHandlerWholePong.java  |  11 +-
 .../pojo/PojoMessageHandlerWholeText.java  |  23 +-
 .../tomcat/websocket/pojo/PojoMethodMapping.java   | 318 -
 .../tomcat/websocket/pojo/PojoPathParam.java   |  12 +-
 .../apache/tomcat/websocket/pojo/package-info.java |   4 +-
 15 files changed, 216 insertions(+), 329 deletions(-)

diff --git a/java/org/apache/tomcat/websocket/pojo/Constants.java 
b/java/org/apache/tomcat/websocket/pojo/Constants.java
index 762ac9fe3f..1eba53131a 100644
--- a/java/org/apache/tomcat/websocket/pojo/Constants.java
+++ b/java/org/apache/tomcat/websocket/pojo/Constants.java
@@ -21,8 +21,7 @@ package org.apache.tomcat.websocket.pojo;
  */
 public class Constants {
 
-public static final String POJO_METHOD_MAPPING_KEY =
-"org.apache.tomcat.websocket.pojo.PojoEndpoint.methodMapping";
+public static final String POJO_METHOD_MAPPING_KEY = 
"org.apache.tomcat.websocket.pojo.PojoEndpoint.methodMapping";
 
 private Constants() {
 // Hide default constructor
diff --git a/java/org/apache/tomcat/websocket/pojo/PojoEndpointBase.java 
b/java/org/apache/tomcat/websocket/pojo/PojoEndpointBase.java
index 98e45cc0e1..8067b72325 100644
--- a/java/org/apache/tomcat/websocket/pojo/PojoEndpointBase.java
+++ b/java/org/apache/tomcat/websocket/pojo/PojoEndpointBase.java
@@ -33,9 +33,8 @@ import org.apache.tomcat.util.ExceptionUtils;
 import org.apache.tomcat.util.res.StringManager;
 
 /**
- * Base implementation (client and server have different concrete
- * implementations) of the wrapper that converts a POJO instance into a
- * WebSocket endpoint instance.
+ * Base implementation (client and server have different concrete 
implementations) of the wrapper that converts a POJO
+ * instance into a WebSocket endpoint instance.
  */
 public abstract class PojoEndpointBase extends Endpoint {
 
@@ -43,11 +42,11 @@ public abstract class PojoEndpointBase extends Endpoint {
 private static final StringManager sm = 
StringManager.getManager(PojoEndpointBase.class);
 
 private Object pojo;
-private final Map pathParameters;
+private final Map pathParameters;
 private PojoMethodMapping methodMapping;
 
 
-protected PojoEndpointBase(Map pathParameters) {
+protected PojoEndpointBase(Map pathParameters) {
 this.pathParameters = pathParameters;
 }
 
@@ -59,22 +58,17 @@ public abstract class PojoEndpointBase extends Endpoint {
 // Add message handlers before calling onOpen since that may trigger a
 // message which in turn could trigger a response and/or close the
 // session
-for (MessageHandler mh : methodMapping.getMessageHandlers(pojo,
-pathParameters, session, config)) {
+for (MessageHandler mh : methodMapping.getMessageHandlers(pojo, 
pathParameters, session, config)) {
 session.addMessageHandler(mh);
 }
 
 if (methodMapping.getOnOpen() != null) {
 try {
-methodMapping.getOnOpen().invoke(pojo,
-methodMapping.getOnOpenArgs(
-pathParameters, session, config));
+methodMapping.getOnOpen().invoke(pojo, 
methodMapping.getOnOpenArgs(pathParameters, session, config));
 
 } catch (IllegalAccessException e) {
 // Reflection related problems
-log.error(sm.getString(
-"pojoEndpointBase.onOpenFail",
-pojo.getClass().getName()), e);
+log.error(sm.getString("pojoEndpointBase.onOpenFail", 
pojo.getClass().getName()), e);
 handleOnOpenOrCloseError(session, e);
 } catch (InvocationTargetException e) {
 Throwable cause = e.getCause();
@@ -107,8 +101,7 @@ public abstract class PojoEnd

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

[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 5fd6d601a9 Code clean-up. Formatting. No functional change.
5fd6d601a9 is described below

commit 5fd6d601a9e73c4fa01d6cfd8316f30a3d6ba331
Author: Mark Thomas 
AuthorDate: Wed Mar 8 18:28:42 2023 +

Code clean-up. Formatting. No functional change.
---
 .../apache/tomcat/websocket/server/Constants.java  |   9 +-
 .../server/DefaultServerEndpointConfigurator.java  |  17 +--
 .../tomcat/websocket/server/UpgradeUtil.java   | 130 +++--
 .../tomcat/websocket/server/UriTemplate.java   |  26 ++--
 .../tomcat/websocket/server/WsContextListener.java |  10 +-
 .../apache/tomcat/websocket/server/WsFilter.java   |  13 +-
 .../tomcat/websocket/server/WsFrameServer.java |  63 -
 .../websocket/server/WsHandshakeRequest.java   |  49 +++
 .../websocket/server/WsHttpUpgradeHandler.java |  58 +++-
 .../tomcat/websocket/server/WsMappingResult.java   |   7 +-
 .../server/WsPerSessionServerEndpointConfig.java   |  12 +-
 .../server/WsRemoteEndpointImplServer.java |  33 ++---
 java/org/apache/tomcat/websocket/server/WsSci.java |  36 ++---
 .../tomcat/websocket/server/WsServerContainer.java | 156 -
 .../tomcat/websocket/server/WsSessionListener.java |   2 +-
 .../tomcat/websocket/server/WsWriteTimeout.java|  18 +--
 .../tomcat/websocket/server/package-info.java  |   4 +-
 17 files changed, 250 insertions(+), 393 deletions(-)

diff --git a/java/org/apache/tomcat/websocket/server/Constants.java 
b/java/org/apache/tomcat/websocket/server/Constants.java
index d30c880af0..5aa99bbb5e 100644
--- a/java/org/apache/tomcat/websocket/server/Constants.java
+++ b/java/org/apache/tomcat/websocket/server/Constants.java
@@ -21,13 +21,10 @@ package org.apache.tomcat.websocket.server;
  */
 public class Constants {
 
-public static final String BINARY_BUFFER_SIZE_SERVLET_CONTEXT_INIT_PARAM =
-"org.apache.tomcat.websocket.binaryBufferSize";
-public static final String TEXT_BUFFER_SIZE_SERVLET_CONTEXT_INIT_PARAM =
-"org.apache.tomcat.websocket.textBufferSize";
+public static final String BINARY_BUFFER_SIZE_SERVLET_CONTEXT_INIT_PARAM = 
"org.apache.tomcat.websocket.binaryBufferSize";
+public static final String TEXT_BUFFER_SIZE_SERVLET_CONTEXT_INIT_PARAM = 
"org.apache.tomcat.websocket.textBufferSize";
 
-public static final String SERVER_CONTAINER_SERVLET_CONTEXT_ATTRIBUTE =
-"jakarta.websocket.server.ServerContainer";
+public static final String SERVER_CONTAINER_SERVLET_CONTEXT_ATTRIBUTE = 
"jakarta.websocket.server.ServerContainer";
 
 
 private Constants() {
diff --git 
a/java/org/apache/tomcat/websocket/server/DefaultServerEndpointConfigurator.java
 
b/java/org/apache/tomcat/websocket/server/DefaultServerEndpointConfigurator.java
index 50252f61c1..24ed9e5c0f 100644
--- 
a/java/org/apache/tomcat/websocket/server/DefaultServerEndpointConfigurator.java
+++ 
b/java/org/apache/tomcat/websocket/server/DefaultServerEndpointConfigurator.java
@@ -26,13 +26,11 @@ import jakarta.websocket.HandshakeResponse;
 import jakarta.websocket.server.HandshakeRequest;
 import jakarta.websocket.server.ServerEndpointConfig;
 
-@aQute.bnd.annotation.spi.ServiceProvider(value=ServerEndpointConfig.Configurator.class)
-public class DefaultServerEndpointConfigurator
-extends ServerEndpointConfig.Configurator {
+@aQute.bnd.annotation.spi.ServiceProvider(value = 
ServerEndpointConfig.Configurator.class)
+public class DefaultServerEndpointConfigurator extends 
ServerEndpointConfig.Configurator {
 
 @Override
-public  T getEndpointInstance(Class clazz)
-throws InstantiationException {
+public  T getEndpointInstance(Class clazz) throws 
InstantiationException {
 try {
 return clazz.getConstructor().newInstance();
 } catch (InstantiationException e) {
@@ -46,8 +44,7 @@ public class DefaultServerEndpointConfigurator
 
 
 @Override
-public String getNegotiatedSubprotocol(List supported,
-List requested) {
+public String getNegotiatedSubprotocol(List supported, 
List requested) {
 
 for (String request : requested) {
 if (supported.contains(request)) {
@@ -59,8 +56,7 @@ public class DefaultServerEndpointConfigurator
 
 
 @Override
-public List getNegotiatedExtensions(List installed,
-List requested) {
+public List getNegotiatedExtensions(List installed, 
List requested) {
 Set installedNames = new HashSet<>();
 for (Extension e : installed) {
 installedNames.add(e.getName());
@@ -81,8 +77,7 @@ public class DefaultServerEndpointConfigurator
 }
 
 @Override
-public void modifyHandshake(ServerEndpointConfig sec,
-  

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

[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 a7d4c7b336 Code clean-up. Formatting. No functional change.
a7d4c7b336 is described below

commit a7d4c7b3360bebbdd39ebbe152eddeeb62d2a92c
Author: Mark Thomas 
AuthorDate: Wed Mar 8 18:27:07 2023 +

Code clean-up. Formatting. No functional change.
---
 .../apache/tomcat/websocket/server/Constants.java  |   9 +-
 .../server/DefaultServerEndpointConfigurator.java  |  17 +--
 .../tomcat/websocket/server/UpgradeUtil.java   | 130 +++--
 .../tomcat/websocket/server/UriTemplate.java   |  26 ++--
 .../tomcat/websocket/server/WsContextListener.java |  10 +-
 .../apache/tomcat/websocket/server/WsFilter.java   |  13 +-
 .../tomcat/websocket/server/WsFrameServer.java |  63 -
 .../websocket/server/WsHandshakeRequest.java   |  49 +++
 .../websocket/server/WsHttpUpgradeHandler.java |  58 +++-
 .../tomcat/websocket/server/WsMappingResult.java   |   7 +-
 .../server/WsPerSessionServerEndpointConfig.java   |  12 +-
 .../server/WsRemoteEndpointImplServer.java |  33 ++---
 java/org/apache/tomcat/websocket/server/WsSci.java |  36 ++---
 .../tomcat/websocket/server/WsServerContainer.java | 156 -
 .../tomcat/websocket/server/WsSessionListener.java |   2 +-
 .../tomcat/websocket/server/WsWriteTimeout.java|  18 +--
 .../tomcat/websocket/server/package-info.java  |   4 +-
 17 files changed, 250 insertions(+), 393 deletions(-)

diff --git a/java/org/apache/tomcat/websocket/server/Constants.java 
b/java/org/apache/tomcat/websocket/server/Constants.java
index d30c880af0..5aa99bbb5e 100644
--- a/java/org/apache/tomcat/websocket/server/Constants.java
+++ b/java/org/apache/tomcat/websocket/server/Constants.java
@@ -21,13 +21,10 @@ package org.apache.tomcat.websocket.server;
  */
 public class Constants {
 
-public static final String BINARY_BUFFER_SIZE_SERVLET_CONTEXT_INIT_PARAM =
-"org.apache.tomcat.websocket.binaryBufferSize";
-public static final String TEXT_BUFFER_SIZE_SERVLET_CONTEXT_INIT_PARAM =
-"org.apache.tomcat.websocket.textBufferSize";
+public static final String BINARY_BUFFER_SIZE_SERVLET_CONTEXT_INIT_PARAM = 
"org.apache.tomcat.websocket.binaryBufferSize";
+public static final String TEXT_BUFFER_SIZE_SERVLET_CONTEXT_INIT_PARAM = 
"org.apache.tomcat.websocket.textBufferSize";
 
-public static final String SERVER_CONTAINER_SERVLET_CONTEXT_ATTRIBUTE =
-"jakarta.websocket.server.ServerContainer";
+public static final String SERVER_CONTAINER_SERVLET_CONTEXT_ATTRIBUTE = 
"jakarta.websocket.server.ServerContainer";
 
 
 private Constants() {
diff --git 
a/java/org/apache/tomcat/websocket/server/DefaultServerEndpointConfigurator.java
 
b/java/org/apache/tomcat/websocket/server/DefaultServerEndpointConfigurator.java
index 50252f61c1..24ed9e5c0f 100644
--- 
a/java/org/apache/tomcat/websocket/server/DefaultServerEndpointConfigurator.java
+++ 
b/java/org/apache/tomcat/websocket/server/DefaultServerEndpointConfigurator.java
@@ -26,13 +26,11 @@ import jakarta.websocket.HandshakeResponse;
 import jakarta.websocket.server.HandshakeRequest;
 import jakarta.websocket.server.ServerEndpointConfig;
 
-@aQute.bnd.annotation.spi.ServiceProvider(value=ServerEndpointConfig.Configurator.class)
-public class DefaultServerEndpointConfigurator
-extends ServerEndpointConfig.Configurator {
+@aQute.bnd.annotation.spi.ServiceProvider(value = 
ServerEndpointConfig.Configurator.class)
+public class DefaultServerEndpointConfigurator extends 
ServerEndpointConfig.Configurator {
 
 @Override
-public  T getEndpointInstance(Class clazz)
-throws InstantiationException {
+public  T getEndpointInstance(Class clazz) throws 
InstantiationException {
 try {
 return clazz.getConstructor().newInstance();
 } catch (InstantiationException e) {
@@ -46,8 +44,7 @@ public class DefaultServerEndpointConfigurator
 
 
 @Override
-public String getNegotiatedSubprotocol(List supported,
-List requested) {
+public String getNegotiatedSubprotocol(List supported, 
List requested) {
 
 for (String request : requested) {
 if (supported.contains(request)) {
@@ -59,8 +56,7 @@ public class DefaultServerEndpointConfigurator
 
 
 @Override
-public List getNegotiatedExtensions(List installed,
-List requested) {
+public List getNegotiatedExtensions(List installed, 
List requested) {
 Set installedNames = new HashSet<>();
 for (Extension e : installed) {
 installedNames.add(e.getName());
@@ -81,8 +77,7 @@ public class DefaultServerEndpointConfigurator
 }
 
 @Override
-public void modifyHandshake(ServerEndpointConfig sec,
-Ha

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

[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 6364efcb63 Code clean-up. Formatting. No functional change.
6364efcb63 is described below

commit 6364efcb635f46279c1fb40fcdf03f5a0011aed6
Author: Mark Thomas 
AuthorDate: Wed Mar 8 18:27:34 2023 +

Code clean-up. Formatting. No functional change.
---
 .../websocket/pojo/TestEncodingDecoding.java   | 128 ++---
 .../websocket/pojo/TestPojoEndpointBase.java   |  11 +-
 .../websocket/pojo/TestPojoMethodMapping.java  |  23 ++--
 .../apache/tomcat/websocket/pojo/TesterUtil.java   |   3 +-
 4 files changed, 77 insertions(+), 88 deletions(-)

diff --git a/test/org/apache/tomcat/websocket/pojo/TestEncodingDecoding.java 
b/test/org/apache/tomcat/websocket/pojo/TestEncodingDecoding.java
index b3166be31a..597b240e34 100644
--- a/test/org/apache/tomcat/websocket/pojo/TestEncodingDecoding.java
+++ b/test/org/apache/tomcat/websocket/pojo/TestEncodingDecoding.java
@@ -74,7 +74,7 @@ public class TestEncodingDecoding extends TomcatBaseTest {
 
 
 @Test
-public void testProgrammaticEndPoints() throws Exception{
+public void testProgrammaticEndPoints() throws Exception {
 Tomcat tomcat = getTomcatInstance();
 // No file system docBase required
 Context ctx = tomcat.addContext("", null);
@@ -96,8 +96,7 @@ public class TestEncodingDecoding extends TomcatBaseTest {
 // Should not take very long
 int i = 0;
 while (i < WAIT_LOOPS) {
-if (MsgStringMessageHandler.received.size() > 0 &&
-client.received.size() > 0) {
+if (MsgStringMessageHandler.received.size() > 0 && 
client.received.size() > 0) {
 break;
 }
 i++;
@@ -109,10 +108,8 @@ public class TestEncodingDecoding extends TomcatBaseTest {
 Assert.assertEquals(1, client.received.size());
 
 // Check correct messages were received
-Assert.assertEquals(MESSAGE_ONE,
-((MsgString) 
MsgStringMessageHandler.received.peek()).getData());
-Assert.assertEquals(MESSAGE_ONE,
-new String(((MsgByte) client.received.peek()).getData()));
+Assert.assertEquals(MESSAGE_ONE, ((MsgString) 
MsgStringMessageHandler.received.peek()).getData());
+Assert.assertEquals(MESSAGE_ONE, new String(((MsgByte) 
client.received.peek()).getData()));
 session.close();
 }
 
@@ -131,8 +128,7 @@ public class TestEncodingDecoding extends TomcatBaseTest {
 Tomcat.addServlet(ctx, "default", new DefaultServlet());
 ctx.addServletMappingDecoded("/", "default");
 
-WebSocketContainer wsContainer =
-ContainerProvider.getWebSocketContainer();
+WebSocketContainer wsContainer = 
ContainerProvider.getWebSocketContainer();
 
 tomcat.start();
 
@@ -159,21 +155,19 @@ public class TestEncodingDecoding extends TomcatBaseTest {
 Assert.assertEquals(1, client.received.size());
 
 // Check correct messages were received
-Assert.assertEquals(MESSAGE_ONE,
-((MsgString) server.received.peek()).getData());
-Assert.assertEquals(MESSAGE_ONE,
-((MsgString) client.received.peek()).getData());
+Assert.assertEquals(MESSAGE_ONE, ((MsgString) 
server.received.peek()).getData());
+Assert.assertEquals(MESSAGE_ONE, ((MsgString) 
client.received.peek()).getData());
 session.close();
 
 // Should not take very long but some failures have been seen
-i = testEvent(MsgStringEncoder.class.getName()+":init", 0);
-i = testEvent(MsgStringDecoder.class.getName()+":init", i);
-i = testEvent(MsgByteEncoder.class.getName()+":init", i);
-i = testEvent(MsgByteDecoder.class.getName()+":init", i);
-i = testEvent(MsgStringEncoder.class.getName()+":destroy", i);
-i = testEvent(MsgStringDecoder.class.getName()+":destroy", i);
-i = testEvent(MsgByteEncoder.class.getName()+":destroy", i);
-i = testEvent(MsgByteDecoder.class.getName()+":destroy", i);
+i = testEvent(MsgStringEncoder.class.getName() + ":init", 0);
+i = testEvent(MsgStringDecoder.class.getName() + ":init", i);
+i = testEvent(MsgByteEncoder.class.getName() + ":init", i);
+i = testEvent(MsgByteDecoder.class.getName() + ":init", i);
+i = testEvent(MsgStringEncoder.class.getName() + ":destroy", i);
+i = testEvent(MsgStringDecoder.class.getName() + ":destroy", i);
+i = testEvent(MsgByteEncoder.class.getName() + ":destroy", i);
+i = testEvent(MsgByteDecoder.class.getName() + ":destroy", i);
 }
 
 
@@ -191,8 +185,7 @@ public class TestEncodingDecoding extends TomcatBaseTest {
 Tomcat.addServlet(ctx, "default", new DefaultS

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

[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 cd4e9c7124 Code clean-up. Formatting. No functional change.
cd4e9c7124 is described below

commit cd4e9c7124684ce5284cbab2dcfce22539c07ccc
Author: Mark Thomas 
AuthorDate: Wed Mar 8 18:30:02 2023 +

Code clean-up. Formatting. No functional change.
---
 .../apache/tomcat/websocket/server/Constants.java  |  18 +-
 .../server/DefaultServerEndpointConfigurator.java  |  17 +-
 .../tomcat/websocket/server/UpgradeUtil.java   | 130 +---
 .../tomcat/websocket/server/UriTemplate.java   |  26 +--
 .../tomcat/websocket/server/WsContextListener.java |  10 +-
 .../apache/tomcat/websocket/server/WsFilter.java   |  13 +-
 .../tomcat/websocket/server/WsFrameServer.java |  63 +++---
 .../websocket/server/WsHandshakeRequest.java   |  49 ++---
 .../websocket/server/WsHttpUpgradeHandler.java |  58 ++
 .../tomcat/websocket/server/WsMappingResult.java   |   7 +-
 .../server/WsPerSessionServerEndpointConfig.java   |  12 +-
 .../server/WsRemoteEndpointImplServer.java |  33 ++-
 java/org/apache/tomcat/websocket/server/WsSci.java |  36 ++--
 .../tomcat/websocket/server/WsServerContainer.java | 226 -
 .../tomcat/websocket/server/WsSessionListener.java |   2 +-
 .../tomcat/websocket/server/WsWriteTimeout.java|  18 +-
 .../tomcat/websocket/server/package-info.java  |   4 +-
 17 files changed, 284 insertions(+), 438 deletions(-)

diff --git a/java/org/apache/tomcat/websocket/server/Constants.java 
b/java/org/apache/tomcat/websocket/server/Constants.java
index de5041a527..94b52849ca 100644
--- a/java/org/apache/tomcat/websocket/server/Constants.java
+++ b/java/org/apache/tomcat/websocket/server/Constants.java
@@ -21,24 +21,18 @@ package org.apache.tomcat.websocket.server;
  */
 public class Constants {
 
-public static final String BINARY_BUFFER_SIZE_SERVLET_CONTEXT_INIT_PARAM =
-"org.apache.tomcat.websocket.binaryBufferSize";
-public static final String TEXT_BUFFER_SIZE_SERVLET_CONTEXT_INIT_PARAM =
-"org.apache.tomcat.websocket.textBufferSize";
+public static final String BINARY_BUFFER_SIZE_SERVLET_CONTEXT_INIT_PARAM = 
"org.apache.tomcat.websocket.binaryBufferSize";
+public static final String TEXT_BUFFER_SIZE_SERVLET_CONTEXT_INIT_PARAM = 
"org.apache.tomcat.websocket.textBufferSize";
 
 /**
- * Allows the deployment restriction defined in section 6.4 of the Jakarta
- * WebSocket specification to be ignored.
+ * Allows the deployment restriction defined in section 6.4 of the Jakarta 
WebSocket specification to be ignored.
  *
- * @deprecated  This is no longer required in Jakarta EE 10 onwards and 
will
- *  be removed in Tomcat 10.1.
+ * @deprecated This is no longer required in Jakarta EE 10 onwards and 
will be removed in Tomcat 10.1.
  */
 @Deprecated
-public static final String 
ENFORCE_NO_ADD_AFTER_HANDSHAKE_CONTEXT_INIT_PARAM =
-"org.apache.tomcat.websocket.noAddAfterHandshake";
+public static final String 
ENFORCE_NO_ADD_AFTER_HANDSHAKE_CONTEXT_INIT_PARAM = 
"org.apache.tomcat.websocket.noAddAfterHandshake";
 
-public static final String SERVER_CONTAINER_SERVLET_CONTEXT_ATTRIBUTE =
-"javax.websocket.server.ServerContainer";
+public static final String SERVER_CONTAINER_SERVLET_CONTEXT_ATTRIBUTE = 
"javax.websocket.server.ServerContainer";
 
 
 private Constants() {
diff --git 
a/java/org/apache/tomcat/websocket/server/DefaultServerEndpointConfigurator.java
 
b/java/org/apache/tomcat/websocket/server/DefaultServerEndpointConfigurator.java
index 5c385ed7ad..00f492ec4a 100644
--- 
a/java/org/apache/tomcat/websocket/server/DefaultServerEndpointConfigurator.java
+++ 
b/java/org/apache/tomcat/websocket/server/DefaultServerEndpointConfigurator.java
@@ -26,13 +26,11 @@ import javax.websocket.HandshakeResponse;
 import javax.websocket.server.HandshakeRequest;
 import javax.websocket.server.ServerEndpointConfig;
 
-@aQute.bnd.annotation.spi.ServiceProvider(value=ServerEndpointConfig.Configurator.class)
-public class DefaultServerEndpointConfigurator
-extends ServerEndpointConfig.Configurator {
+@aQute.bnd.annotation.spi.ServiceProvider(value = 
ServerEndpointConfig.Configurator.class)
+public class DefaultServerEndpointConfigurator extends 
ServerEndpointConfig.Configurator {
 
 @Override
-public  T getEndpointInstance(Class clazz)
-throws InstantiationException {
+public  T getEndpointInstance(Class clazz) throws 
InstantiationException {
 try {
 return clazz.getConstructor().newInstance();
 } catch (InstantiationException e) {
@@ -46,8 +44,7 @@ public class DefaultServerEndpointConfigurator
 
 
 @Override
-public Strin

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

[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 0c7815ddb2 Code clean-up. Formatting. No functional change.
0c7815ddb2 is described below

commit 0c7815ddb244eb3899b30e88cc75402d1eb63143
Author: Mark Thomas 
AuthorDate: Wed Mar 8 18:30:29 2023 +

Code clean-up. Formatting. No functional change.
---
 .../tomcat/websocket/TestConnectionLimit.java  |   9 +-
 .../tomcat/websocket/TestPerMessageDeflate.java|  18 ++-
 test/org/apache/tomcat/websocket/TestUtil.java |  82 ---
 .../tomcat/websocket/TestWebSocketFrameClient.java |  41 +++---
 .../websocket/TestWebSocketFrameClientSSL.java |  47 +++
 test/org/apache/tomcat/websocket/TestWsFrame.java  |   6 +-
 .../tomcat/websocket/TestWsPingPongMessages.java   |  19 +--
 .../tomcat/websocket/TestWsRemoteEndpoint.java |  18 +--
 .../websocket/TestWsSessionSuspendResume.java  |   6 +-
 .../tomcat/websocket/TestWsSubprotocols.java   |  28 ++--
 .../tomcat/websocket/TestWsWebSocketContainer.java | 150 -
 .../TestWsWebSocketContainerGetOpenSessions.java   |  23 ++--
 ...WsWebSocketContainerSessionExpiryContainer.java |  19 +--
 ...stWsWebSocketContainerSessionExpirySession.java |  16 +--
 .../TestWsWebSocketContainerTimeoutClient.java |  16 +--
 .../TestWsWebSocketContainerTimeoutServer.java |  38 ++
 .../TestWsWebSocketContainerWithProxy.java |   2 +-
 .../apache/tomcat/websocket/TesterAsyncTiming.java |   4 +-
 .../tomcat/websocket/TesterBlockWebSocketSCI.java  |  13 +-
 .../apache/tomcat/websocket/TesterEchoServer.java  |   8 +-
 .../tomcat/websocket/TesterFirehoseServer.java |   7 +-
 .../tomcat/websocket/TesterMessageCountClient.java |  13 +-
 .../websocket/TesterWebSocketClientProxy.java  |   7 +-
 .../tomcat/websocket/TesterWsClientAutobahn.java   |  24 ++--
 .../apache/tomcat/websocket/WebSocketBaseTest.java |   4 +-
 .../websocket/WsWebSocketContainerBaseTest.java|   7 +-
 .../websocket/pojo/TestEncodingDecoding.java   | 128 +-
 .../websocket/pojo/TestPojoEndpointBase.java   |  11 +-
 .../websocket/pojo/TestPojoMethodMapping.java  |  23 ++--
 .../apache/tomcat/websocket/pojo/TesterUtil.java   |   3 +-
 .../tomcat/websocket/server/TestAsyncMessages.java |  21 +--
 .../tomcat/websocket/server/TestClassLoader.java   |   7 +-
 .../apache/tomcat/websocket/server/TestClose.java  |   2 +-
 .../tomcat/websocket/server/TestCloseBug58624.java |  13 +-
 .../tomcat/websocket/server/TestShutdown.java  |  14 +-
 .../tomcat/websocket/server/TestSlowClient.java|   3 +-
 .../tomcat/websocket/server/TestUriTemplate.java   |  54 
 .../server/TestWsRemoteEndpointImplServer.java |  28 ++--
 .../websocket/server/TestWsServerContainer.java|  75 ---
 .../websocket/server/TesterEndpointConfig.java |   4 +-
 .../tomcat/websocket/server/TesterWsClient.java|  17 +--
 41 files changed, 407 insertions(+), 621 deletions(-)

diff --git a/test/org/apache/tomcat/websocket/TestConnectionLimit.java 
b/test/org/apache/tomcat/websocket/TestConnectionLimit.java
index e4eb14d57e..f1ec95dcdb 100644
--- a/test/org/apache/tomcat/websocket/TestConnectionLimit.java
+++ b/test/org/apache/tomcat/websocket/TestConnectionLimit.java
@@ -38,8 +38,7 @@ import 
org.apache.tomcat.websocket.TesterMessageCountClient.TesterProgrammaticEn
 public class TestConnectionLimit extends TomcatBaseTest {
 
 /*
- * Simple test to see how many outgoing connections can be created on a
- * single machine.
+ * Simple test to see how many outgoing connections can be created on a 
single machine.
  */
 @Test
 public void testSingleMachine() throws Exception {
@@ -54,8 +53,7 @@ public class TestConnectionLimit extends TomcatBaseTest {
 
 tomcat.start();
 
-URI uri = new URI("ws://localhost:" + getPort() +
-TesterEchoServer.Config.PATH_ASYNC);
+URI uri = new URI("ws://localhost:" + getPort() + 
TesterEchoServer.Config.PATH_ASYNC);
 AtomicInteger counter = new AtomicInteger(0);
 
 int threadCount = 50;
@@ -87,8 +85,7 @@ public class TestConnectionLimit extends TomcatBaseTest {
 
 @Override
 public void run() {
-WebSocketContainer wsContainer =
-ContainerProvider.getWebSocketContainer();
+WebSocketContainer wsContainer = 
ContainerProvider.getWebSocketContainer();
 
 int count = 0;
 
diff --git a/test/org/apache/tomcat/websocket/TestPerMessageDeflate.java 
b/test/org/apache/tomcat/websocket/TestPerMessageDeflate.java
index 4538eba976..d1676b6c96 100644
--- a/test/org/apache/tomcat/websocket/TestPerMessageDeflate.java
+++ b/test/org/apache/tomcat/websocket/TestPerMessageDeflate.java
@@ -93,17 +93,16 @@ public class TestPerMes

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

[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 996d77a753 Code clean-up. Formatting. No functional change.
996d77a753 is described below

commit 996d77a753024a7572c83b82555fe12a01b637b4
Author: Mark Thomas 
AuthorDate: Wed Mar 8 18:31:00 2023 +

Code clean-up. Formatting. No functional change.
---
 .../apache/tomcat/websocket/pojo/Constants.java|   3 +-
 .../tomcat/websocket/pojo/PojoEndpointBase.java|  49 ++-
 .../tomcat/websocket/pojo/PojoEndpointClient.java  |  13 +-
 .../tomcat/websocket/pojo/PojoEndpointServer.java  |  12 +-
 .../websocket/pojo/PojoMessageHandlerBase.java |  15 +-
 .../pojo/PojoMessageHandlerPartialBase.java|  22 +-
 .../pojo/PojoMessageHandlerPartialBinary.java  |  11 +-
 .../pojo/PojoMessageHandlerPartialText.java|  11 +-
 .../pojo/PojoMessageHandlerWholeBase.java  |  21 +-
 .../pojo/PojoMessageHandlerWholeBinary.java|  23 +-
 .../pojo/PojoMessageHandlerWholePong.java  |  11 +-
 .../pojo/PojoMessageHandlerWholeText.java  |  23 +-
 .../tomcat/websocket/pojo/PojoMethodMapping.java   | 328 -
 .../tomcat/websocket/pojo/PojoPathParam.java   |  12 +-
 .../apache/tomcat/websocket/pojo/package-info.java |   4 +-
 15 files changed, 222 insertions(+), 336 deletions(-)

diff --git a/java/org/apache/tomcat/websocket/pojo/Constants.java 
b/java/org/apache/tomcat/websocket/pojo/Constants.java
index 762ac9fe3f..1eba53131a 100644
--- a/java/org/apache/tomcat/websocket/pojo/Constants.java
+++ b/java/org/apache/tomcat/websocket/pojo/Constants.java
@@ -21,8 +21,7 @@ package org.apache.tomcat.websocket.pojo;
  */
 public class Constants {
 
-public static final String POJO_METHOD_MAPPING_KEY =
-"org.apache.tomcat.websocket.pojo.PojoEndpoint.methodMapping";
+public static final String POJO_METHOD_MAPPING_KEY = 
"org.apache.tomcat.websocket.pojo.PojoEndpoint.methodMapping";
 
 private Constants() {
 // Hide default constructor
diff --git a/java/org/apache/tomcat/websocket/pojo/PojoEndpointBase.java 
b/java/org/apache/tomcat/websocket/pojo/PojoEndpointBase.java
index fa71489acd..bebfc6b67a 100644
--- a/java/org/apache/tomcat/websocket/pojo/PojoEndpointBase.java
+++ b/java/org/apache/tomcat/websocket/pojo/PojoEndpointBase.java
@@ -33,9 +33,8 @@ import org.apache.tomcat.util.ExceptionUtils;
 import org.apache.tomcat.util.res.StringManager;
 
 /**
- * Base implementation (client and server have different concrete
- * implementations) of the wrapper that converts a POJO instance into a
- * WebSocket endpoint instance.
+ * Base implementation (client and server have different concrete 
implementations) of the wrapper that converts a POJO
+ * instance into a WebSocket endpoint instance.
  */
 public abstract class PojoEndpointBase extends Endpoint {
 
@@ -43,11 +42,11 @@ public abstract class PojoEndpointBase extends Endpoint {
 private static final StringManager sm = 
StringManager.getManager(PojoEndpointBase.class);
 
 private Object pojo;
-private final Map pathParameters;
+private final Map pathParameters;
 private PojoMethodMapping methodMapping;
 
 
-protected PojoEndpointBase(Map pathParameters) {
+protected PojoEndpointBase(Map pathParameters) {
 this.pathParameters = pathParameters;
 }
 
@@ -59,22 +58,17 @@ public abstract class PojoEndpointBase extends Endpoint {
 // Add message handlers before calling onOpen since that may trigger a
 // message which in turn could trigger a response and/or close the
 // session
-for (MessageHandler mh : methodMapping.getMessageHandlers(pojo,
-pathParameters, session, config)) {
+for (MessageHandler mh : methodMapping.getMessageHandlers(pojo, 
pathParameters, session, config)) {
 session.addMessageHandler(mh);
 }
 
 if (methodMapping.getOnOpen() != null) {
 try {
-methodMapping.getOnOpen().invoke(pojo,
-methodMapping.getOnOpenArgs(
-pathParameters, session, config));
+methodMapping.getOnOpen().invoke(pojo, 
methodMapping.getOnOpenArgs(pathParameters, session, config));
 
 } catch (IllegalAccessException e) {
 // Reflection related problems
-log.error(sm.getString(
-"pojoEndpointBase.onOpenFail",
-pojo.getClass().getName()), e);
+log.error(sm.getString("pojoEndpointBase.onOpenFail", 
pojo.getClass().getName()), e);
 handleOnOpenOrCloseError(session, e);
 } catch (InvocationTargetException e) {
 Throwable cause = e.getCause();
@@ -107,8 +101,7 @@ public abstract class PojoEndpoi

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

[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 787b688203 Code clean-up. Formatting. No functional change.
787b688203 is described below

commit 787b688203c0e5d86a87a2f7818943a8eabf918f
Author: Mark Thomas 
AuthorDate: Wed Mar 8 18:29:53 2023 +

Code clean-up. Formatting. No functional change.
---
 .../apache/tomcat/websocket/pojo/Constants.java|   3 +-
 .../tomcat/websocket/pojo/PojoEndpointBase.java|  49 ++-
 .../tomcat/websocket/pojo/PojoEndpointClient.java  |  13 +-
 .../tomcat/websocket/pojo/PojoEndpointServer.java  |  12 +-
 .../websocket/pojo/PojoMessageHandlerBase.java |  15 +-
 .../pojo/PojoMessageHandlerPartialBase.java|  22 +-
 .../pojo/PojoMessageHandlerPartialBinary.java  |  11 +-
 .../pojo/PojoMessageHandlerPartialText.java|  11 +-
 .../pojo/PojoMessageHandlerWholeBase.java  |  21 +-
 .../pojo/PojoMessageHandlerWholeBinary.java|  23 +-
 .../pojo/PojoMessageHandlerWholePong.java  |  11 +-
 .../pojo/PojoMessageHandlerWholeText.java  |  23 +-
 .../tomcat/websocket/pojo/PojoMethodMapping.java   | 328 -
 .../tomcat/websocket/pojo/PojoPathParam.java   |  12 +-
 .../apache/tomcat/websocket/pojo/package-info.java |   4 +-
 15 files changed, 222 insertions(+), 336 deletions(-)

diff --git a/java/org/apache/tomcat/websocket/pojo/Constants.java 
b/java/org/apache/tomcat/websocket/pojo/Constants.java
index 762ac9fe3f..1eba53131a 100644
--- a/java/org/apache/tomcat/websocket/pojo/Constants.java
+++ b/java/org/apache/tomcat/websocket/pojo/Constants.java
@@ -21,8 +21,7 @@ package org.apache.tomcat.websocket.pojo;
  */
 public class Constants {
 
-public static final String POJO_METHOD_MAPPING_KEY =
-"org.apache.tomcat.websocket.pojo.PojoEndpoint.methodMapping";
+public static final String POJO_METHOD_MAPPING_KEY = 
"org.apache.tomcat.websocket.pojo.PojoEndpoint.methodMapping";
 
 private Constants() {
 // Hide default constructor
diff --git a/java/org/apache/tomcat/websocket/pojo/PojoEndpointBase.java 
b/java/org/apache/tomcat/websocket/pojo/PojoEndpointBase.java
index fa71489acd..bebfc6b67a 100644
--- a/java/org/apache/tomcat/websocket/pojo/PojoEndpointBase.java
+++ b/java/org/apache/tomcat/websocket/pojo/PojoEndpointBase.java
@@ -33,9 +33,8 @@ import org.apache.tomcat.util.ExceptionUtils;
 import org.apache.tomcat.util.res.StringManager;
 
 /**
- * Base implementation (client and server have different concrete
- * implementations) of the wrapper that converts a POJO instance into a
- * WebSocket endpoint instance.
+ * Base implementation (client and server have different concrete 
implementations) of the wrapper that converts a POJO
+ * instance into a WebSocket endpoint instance.
  */
 public abstract class PojoEndpointBase extends Endpoint {
 
@@ -43,11 +42,11 @@ public abstract class PojoEndpointBase extends Endpoint {
 private static final StringManager sm = 
StringManager.getManager(PojoEndpointBase.class);
 
 private Object pojo;
-private final Map pathParameters;
+private final Map pathParameters;
 private PojoMethodMapping methodMapping;
 
 
-protected PojoEndpointBase(Map pathParameters) {
+protected PojoEndpointBase(Map pathParameters) {
 this.pathParameters = pathParameters;
 }
 
@@ -59,22 +58,17 @@ public abstract class PojoEndpointBase extends Endpoint {
 // Add message handlers before calling onOpen since that may trigger a
 // message which in turn could trigger a response and/or close the
 // session
-for (MessageHandler mh : methodMapping.getMessageHandlers(pojo,
-pathParameters, session, config)) {
+for (MessageHandler mh : methodMapping.getMessageHandlers(pojo, 
pathParameters, session, config)) {
 session.addMessageHandler(mh);
 }
 
 if (methodMapping.getOnOpen() != null) {
 try {
-methodMapping.getOnOpen().invoke(pojo,
-methodMapping.getOnOpenArgs(
-pathParameters, session, config));
+methodMapping.getOnOpen().invoke(pojo, 
methodMapping.getOnOpenArgs(pathParameters, session, config));
 
 } catch (IllegalAccessException e) {
 // Reflection related problems
-log.error(sm.getString(
-"pojoEndpointBase.onOpenFail",
-pojo.getClass().getName()), e);
+log.error(sm.getString("pojoEndpointBase.onOpenFail", 
pojo.getClass().getName()), e);
 handleOnOpenOrCloseError(session, e);
 } catch (InvocationTargetException e) {
 Throwable cause = e.getCause();
@@ -107,8 +101,7 @@ public abstract class PojoEndpoi

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

[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 eb5cca161b Code clean-up. Formatting. No functional change.
eb5cca161b is described below

commit eb5cca161b8c471460ceb5c1b7b12b9001070c90
Author: Mark Thomas 
AuthorDate: Wed Mar 8 18:31:12 2023 +

Code clean-up. Formatting. No functional change.
---
 .../apache/tomcat/websocket/server/Constants.java  |  12 +-
 .../server/DefaultServerEndpointConfigurator.java  |  15 +-
 .../tomcat/websocket/server/UpgradeUtil.java   | 131 +---
 .../tomcat/websocket/server/UriTemplate.java   |  26 +--
 .../tomcat/websocket/server/WsContextListener.java |  10 +-
 .../apache/tomcat/websocket/server/WsFilter.java   |  14 +-
 .../tomcat/websocket/server/WsFrameServer.java |  63 +++---
 .../websocket/server/WsHandshakeRequest.java   |  49 ++---
 .../websocket/server/WsHttpUpgradeHandler.java |  58 ++---
 .../tomcat/websocket/server/WsMappingResult.java   |   7 +-
 .../server/WsPerSessionServerEndpointConfig.java   |  12 +-
 .../server/WsRemoteEndpointImplServer.java |  33 ++-
 java/org/apache/tomcat/websocket/server/WsSci.java |  36 ++--
 .../tomcat/websocket/server/WsServerContainer.java | 235 -
 .../tomcat/websocket/server/WsSessionListener.java |   2 +-
 .../tomcat/websocket/server/WsWriteTimeout.java|  23 +-
 .../tomcat/websocket/server/package-info.java  |   4 +-
 17 files changed, 286 insertions(+), 444 deletions(-)

diff --git a/java/org/apache/tomcat/websocket/server/Constants.java 
b/java/org/apache/tomcat/websocket/server/Constants.java
index 876f1b8074..20e263fd18 100644
--- a/java/org/apache/tomcat/websocket/server/Constants.java
+++ b/java/org/apache/tomcat/websocket/server/Constants.java
@@ -21,15 +21,11 @@ package org.apache.tomcat.websocket.server;
  */
 public class Constants {
 
-public static final String BINARY_BUFFER_SIZE_SERVLET_CONTEXT_INIT_PARAM =
-"org.apache.tomcat.websocket.binaryBufferSize";
-public static final String TEXT_BUFFER_SIZE_SERVLET_CONTEXT_INIT_PARAM =
-"org.apache.tomcat.websocket.textBufferSize";
-public static final String 
ENFORCE_NO_ADD_AFTER_HANDSHAKE_CONTEXT_INIT_PARAM =
-"org.apache.tomcat.websocket.noAddAfterHandshake";
+public static final String BINARY_BUFFER_SIZE_SERVLET_CONTEXT_INIT_PARAM = 
"org.apache.tomcat.websocket.binaryBufferSize";
+public static final String TEXT_BUFFER_SIZE_SERVLET_CONTEXT_INIT_PARAM = 
"org.apache.tomcat.websocket.textBufferSize";
+public static final String 
ENFORCE_NO_ADD_AFTER_HANDSHAKE_CONTEXT_INIT_PARAM = 
"org.apache.tomcat.websocket.noAddAfterHandshake";
 
-public static final String SERVER_CONTAINER_SERVLET_CONTEXT_ATTRIBUTE =
-"javax.websocket.server.ServerContainer";
+public static final String SERVER_CONTAINER_SERVLET_CONTEXT_ATTRIBUTE = 
"javax.websocket.server.ServerContainer";
 
 
 private Constants() {
diff --git 
a/java/org/apache/tomcat/websocket/server/DefaultServerEndpointConfigurator.java
 
b/java/org/apache/tomcat/websocket/server/DefaultServerEndpointConfigurator.java
index 75b8d7059e..5e1b253709 100644
--- 
a/java/org/apache/tomcat/websocket/server/DefaultServerEndpointConfigurator.java
+++ 
b/java/org/apache/tomcat/websocket/server/DefaultServerEndpointConfigurator.java
@@ -26,12 +26,10 @@ import javax.websocket.HandshakeResponse;
 import javax.websocket.server.HandshakeRequest;
 import javax.websocket.server.ServerEndpointConfig;
 
-public class DefaultServerEndpointConfigurator
-extends ServerEndpointConfig.Configurator {
+public class DefaultServerEndpointConfigurator extends 
ServerEndpointConfig.Configurator {
 
 @Override
-public  T getEndpointInstance(Class clazz)
-throws InstantiationException {
+public  T getEndpointInstance(Class clazz) throws 
InstantiationException {
 try {
 return clazz.getConstructor().newInstance();
 } catch (InstantiationException e) {
@@ -45,8 +43,7 @@ public class DefaultServerEndpointConfigurator
 
 
 @Override
-public String getNegotiatedSubprotocol(List supported,
-List requested) {
+public String getNegotiatedSubprotocol(List supported, 
List requested) {
 
 for (String request : requested) {
 if (supported.contains(request)) {
@@ -58,8 +55,7 @@ public class DefaultServerEndpointConfigurator
 
 
 @Override
-public List getNegotiatedExtensions(List installed,
-List requested) {
+public List getNegotiatedExtensions(List installed, 
List requested) {
 Set installedNames = new HashSet<>();
 for (Extension e : installed) {
 installedNames.add(e.getName());
@@ -80,8 +76,7 @@ public class DefaultServerEndpointConfigurator
 }
 
 @Overrid

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

[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 f46a2a61ea Code clean-up. Formatting. No functional change.
f46a2a61ea is described below

commit f46a2a61ea8aa0de9e4c88b38e671767b5e5b290
Author: Mark Thomas 
AuthorDate: Wed Mar 8 18:31:36 2023 +

Code clean-up. Formatting. No functional change.
---
 .../tomcat/websocket/TestConnectionLimit.java  |   9 +-
 .../tomcat/websocket/TestPerMessageDeflate.java|  18 ++-
 test/org/apache/tomcat/websocket/TestUtil.java |  82 ---
 .../tomcat/websocket/TestWebSocketFrameClient.java |  37 ++---
 .../websocket/TestWebSocketFrameClientSSL.java |  52 +++
 test/org/apache/tomcat/websocket/TestWsFrame.java  |   6 +-
 .../tomcat/websocket/TestWsPingPongMessages.java   |  22 ++-
 .../tomcat/websocket/TestWsRemoteEndpoint.java |  18 +--
 .../websocket/TestWsSessionSuspendResume.java  |   6 +-
 .../tomcat/websocket/TestWsSubprotocols.java   |  28 ++--
 .../tomcat/websocket/TestWsWebSocketContainer.java | 153 -
 .../TestWsWebSocketContainerGetOpenSessions.java   |  23 ++--
 ...WsWebSocketContainerSessionExpiryContainer.java |  19 +--
 ...stWsWebSocketContainerSessionExpirySession.java |  16 +--
 .../TestWsWebSocketContainerTimeoutClient.java |  16 +--
 .../TestWsWebSocketContainerTimeoutServer.java |  38 ++---
 .../TestWsWebSocketContainerWithProxy.java |   2 +-
 .../tomcat/websocket/TesterBlockWebSocketSCI.java  |  13 +-
 .../apache/tomcat/websocket/TesterEchoServer.java  |   8 +-
 .../tomcat/websocket/TesterFirehoseServer.java |   7 +-
 .../tomcat/websocket/TesterMessageCountClient.java |  13 +-
 .../websocket/TesterWebSocketClientProxy.java  |   7 +-
 .../tomcat/websocket/TesterWsClientAutobahn.java   |  24 ++--
 .../apache/tomcat/websocket/WebSocketBaseTest.java |   4 +-
 .../websocket/WsWebSocketContainerBaseTest.java|   7 +-
 .../websocket/pojo/TestEncodingDecoding.java   | 128 -
 .../websocket/pojo/TestPojoEndpointBase.java   |  11 +-
 .../websocket/pojo/TestPojoMethodMapping.java  |  23 ++--
 .../apache/tomcat/websocket/pojo/TesterUtil.java   |   3 +-
 .../tomcat/websocket/server/TestClassLoader.java   |   7 +-
 .../apache/tomcat/websocket/server/TestClose.java  |   2 +-
 .../tomcat/websocket/server/TestCloseBug58624.java |  13 +-
 .../tomcat/websocket/server/TestShutdown.java  |  14 +-
 .../tomcat/websocket/server/TestSlowClient.java|   3 +-
 .../tomcat/websocket/server/TestUriTemplate.java   |  54 
 .../server/TestWsRemoteEndpointImplServer.java |  28 ++--
 .../websocket/server/TestWsServerContainer.java|  75 --
 .../websocket/server/TesterEndpointConfig.java |   4 +-
 .../tomcat/websocket/server/TesterWsClient.java|  17 +--
 39 files changed, 397 insertions(+), 613 deletions(-)

diff --git a/test/org/apache/tomcat/websocket/TestConnectionLimit.java 
b/test/org/apache/tomcat/websocket/TestConnectionLimit.java
index e4eb14d57e..f1ec95dcdb 100644
--- a/test/org/apache/tomcat/websocket/TestConnectionLimit.java
+++ b/test/org/apache/tomcat/websocket/TestConnectionLimit.java
@@ -38,8 +38,7 @@ import 
org.apache.tomcat.websocket.TesterMessageCountClient.TesterProgrammaticEn
 public class TestConnectionLimit extends TomcatBaseTest {
 
 /*
- * Simple test to see how many outgoing connections can be created on a
- * single machine.
+ * Simple test to see how many outgoing connections can be created on a 
single machine.
  */
 @Test
 public void testSingleMachine() throws Exception {
@@ -54,8 +53,7 @@ public class TestConnectionLimit extends TomcatBaseTest {
 
 tomcat.start();
 
-URI uri = new URI("ws://localhost:" + getPort() +
-TesterEchoServer.Config.PATH_ASYNC);
+URI uri = new URI("ws://localhost:" + getPort() + 
TesterEchoServer.Config.PATH_ASYNC);
 AtomicInteger counter = new AtomicInteger(0);
 
 int threadCount = 50;
@@ -87,8 +85,7 @@ public class TestConnectionLimit extends TomcatBaseTest {
 
 @Override
 public void run() {
-WebSocketContainer wsContainer =
-ContainerProvider.getWebSocketContainer();
+WebSocketContainer wsContainer = 
ContainerProvider.getWebSocketContainer();
 
 int count = 0;
 
diff --git a/test/org/apache/tomcat/websocket/TestPerMessageDeflate.java 
b/test/org/apache/tomcat/websocket/TestPerMessageDeflate.java
index 4538eba976..d1676b6c96 100644
--- a/test/org/apache/tomcat/websocket/TestPerMessageDeflate.java
+++ b/test/org/apache/tomcat/websocket/TestPerMessageDeflate.java
@@ -93,17 +93,16 @@ public class TestPerMessageDeflate {
 
 ByteBuffer received = ByteBuffer.allocate(8192);
 
-TransformationResult tr = perMessageDeflat

[tomcat] branch 9.0.x updated: Align with 10.1.x

[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 89192df457 Align with 10.1.x
89192df457 is described below

commit 89192df4573ce1ce1f09a29f0155dffdc6a2d54c
Author: Mark Thomas 
AuthorDate: Wed Mar 8 19:04:40 2023 +

Align with 10.1.x
---
 .../apache/tomcat/websocket/PerMessageDeflate.java |  6 +-
 .../tomcat/websocket/TestWsWebSocketContainer.java | 36 --
 .../websocket/TestWsWebSocketContainerSSL.java | 76 ++
 3 files changed, 79 insertions(+), 39 deletions(-)

diff --git a/java/org/apache/tomcat/websocket/PerMessageDeflate.java 
b/java/org/apache/tomcat/websocket/PerMessageDeflate.java
index eaaf4b6c67..4bc97b8199 100644
--- a/java/org/apache/tomcat/websocket/PerMessageDeflate.java
+++ b/java/org/apache/tomcat/websocket/PerMessageDeflate.java
@@ -94,7 +94,7 @@ public class PerMessageDeflate implements Transformation {
 throw new 
IllegalArgumentException(sm.getString("perMessageDeflate.invalidWindowSize",
 SERVER_MAX_WINDOW_BITS, 
Integer.valueOf(serverMaxWindowBits)));
 }
-// Java SE API (as of Java 8) does not expose the API 
to
+// Java SE API (as of Java 11) does not expose the API 
to
 // control the Window size. It is effectively 
hard-coded
 // to 15
 if (isServer && serverMaxWindowBits != 15) {
@@ -114,7 +114,7 @@ public class PerMessageDeflate implements Transformation {
 if (clientMaxWindowBits == -1) {
 if (param.getValue() == null) {
 // Hint to server that the client supports this
-// option. Java SE API (as of Java 8) does not
+// option. Java SE API (as of Java 11) does not
 // expose the API to control the Window size. It is
 // effectively hard-coded to 15
 clientMaxWindowBits = 15;
@@ -125,7 +125,7 @@ public class PerMessageDeflate implements Transformation {
 CLIENT_MAX_WINDOW_BITS, 
Integer.valueOf(clientMaxWindowBits)));
 }
 }
-// Java SE API (as of Java 8) does not expose the API 
to
+// Java SE API (as of Java 11) does not expose the API 
to
 // control the Window size. It is effectively 
hard-coded
 // to 15
 if (!isServer && clientMaxWindowBits != 15) {
diff --git a/test/org/apache/tomcat/websocket/TestWsWebSocketContainer.java 
b/test/org/apache/tomcat/websocket/TestWsWebSocketContainer.java
index 5087396b5a..1fab006848 100644
--- a/test/org/apache/tomcat/websocket/TestWsWebSocketContainer.java
+++ b/test/org/apache/tomcat/websocket/TestWsWebSocketContainer.java
@@ -46,7 +46,6 @@ import org.junit.Test;
 import org.apache.catalina.Context;
 import org.apache.catalina.servlets.DefaultServlet;
 import org.apache.catalina.startup.Tomcat;
-import org.apache.tomcat.util.net.TesterSupport;
 import org.apache.tomcat.websocket.TesterMessageCountClient.BasicBinary;
 import org.apache.tomcat.websocket.TesterMessageCountClient.BasicHandler;
 import org.apache.tomcat.websocket.TesterMessageCountClient.BasicText;
@@ -508,41 +507,6 @@ public class TestWsWebSocketContainer extends 
WsWebSocketContainerBaseTest {
 }
 
 
-@Test
-public void testConnectToServerEndpointSSL() throws Exception {
-
-Tomcat tomcat = getTomcatInstance();
-// No file system docBase required
-Context ctx = tomcat.addContext("", null);
-ctx.addApplicationListener(TesterEchoServer.Config.class.getName());
-Tomcat.addServlet(ctx, "default", new DefaultServlet());
-ctx.addServletMappingDecoded("/", "default");
-
-TesterSupport.initSsl(tomcat);
-
-tomcat.start();
-
-WebSocketContainer wsContainer = 
ContainerProvider.getWebSocketContainer();
-ClientEndpointConfig clientEndpointConfig = 
ClientEndpointConfig.Builder.create().build();
-
clientEndpointConfig.getUserProperties().put(org.apache.tomcat.websocket.Constants.SSL_TRUSTSTORE_PROPERTY,
-TesterSupport.CA_JKS);
-Session wsSession = 
wsContainer.connectToServer(TesterProgrammaticEndpoint.class, 
clientEndpointConfig,
-new URI("wss://" + getHostName() + ":" + getPort() + 
TesterEchoServer.Config.PATH_ASYNC));
-CountDownLatch latch = new CountDownLatch(1);
-BasicText handler = new BasicText(latch);
-wsSession.addMessageHandler(handler);
-wsSession.getBa

[tomcat] branch 9.0.x updated: Align with 10.1.x

[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 d34cc7f087 Align with 10.1.x
d34cc7f087 is described below

commit d34cc7f0877f959f3526563a862676019400837c
Author: Mark Thomas 
AuthorDate: Wed Mar 8 19:05:01 2023 +

Align with 10.1.x
---
 java/org/apache/tomcat/websocket/AsyncChannelWrapperSecure.java | 1 +
 1 file changed, 1 insertion(+)

diff --git a/java/org/apache/tomcat/websocket/AsyncChannelWrapperSecure.java 
b/java/org/apache/tomcat/websocket/AsyncChannelWrapperSecure.java
index 085e05cbe0..6e6888494f 100644
--- a/java/org/apache/tomcat/websocket/AsyncChannelWrapperSecure.java
+++ b/java/org/apache/tomcat/websocket/AsyncChannelWrapperSecure.java
@@ -388,6 +388,7 @@ public class AsyncChannelWrapperSecure implements 
AsyncChannelWrapper {
 break;
 }
 case NOT_HANDSHAKING: {
+// Don't expect to see this during a handshake
 throw new 
SSLException(sm.getString("asyncChannelWrapperSecure.notHandshaking"));
 }
 }


-
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: Align with 9.0.x

[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 f81d0719c4 Align with 9.0.x
f81d0719c4 is described below

commit f81d0719c479e89df05346af5170dda30c4d07d9
Author: Mark Thomas 
AuthorDate: Wed Mar 8 19:15:28 2023 +

Align with 9.0.x
---
 .../websocket/AsyncChannelWrapperSecure.java   |  1 +
 .../apache/tomcat/websocket/PerMessageDeflate.java |  6 +-
 .../tomcat/websocket/TestWsWebSocketContainer.java | 60 -
 .../websocket/TestWsWebSocketContainerSSL.java | 76 ++
 .../tomcat/websocket/server/TestCloseBug58624.java |  2 +-
 .../websocket/server/TestWsServerContainer.java|  2 +-
 6 files changed, 82 insertions(+), 65 deletions(-)

diff --git a/java/org/apache/tomcat/websocket/AsyncChannelWrapperSecure.java 
b/java/org/apache/tomcat/websocket/AsyncChannelWrapperSecure.java
index 085e05cbe0..6e6888494f 100644
--- a/java/org/apache/tomcat/websocket/AsyncChannelWrapperSecure.java
+++ b/java/org/apache/tomcat/websocket/AsyncChannelWrapperSecure.java
@@ -388,6 +388,7 @@ public class AsyncChannelWrapperSecure implements 
AsyncChannelWrapper {
 break;
 }
 case NOT_HANDSHAKING: {
+// Don't expect to see this during a handshake
 throw new 
SSLException(sm.getString("asyncChannelWrapperSecure.notHandshaking"));
 }
 }
diff --git a/java/org/apache/tomcat/websocket/PerMessageDeflate.java 
b/java/org/apache/tomcat/websocket/PerMessageDeflate.java
index eaaf4b6c67..4bc97b8199 100644
--- a/java/org/apache/tomcat/websocket/PerMessageDeflate.java
+++ b/java/org/apache/tomcat/websocket/PerMessageDeflate.java
@@ -94,7 +94,7 @@ public class PerMessageDeflate implements Transformation {
 throw new 
IllegalArgumentException(sm.getString("perMessageDeflate.invalidWindowSize",
 SERVER_MAX_WINDOW_BITS, 
Integer.valueOf(serverMaxWindowBits)));
 }
-// Java SE API (as of Java 8) does not expose the API 
to
+// Java SE API (as of Java 11) does not expose the API 
to
 // control the Window size. It is effectively 
hard-coded
 // to 15
 if (isServer && serverMaxWindowBits != 15) {
@@ -114,7 +114,7 @@ public class PerMessageDeflate implements Transformation {
 if (clientMaxWindowBits == -1) {
 if (param.getValue() == null) {
 // Hint to server that the client supports this
-// option. Java SE API (as of Java 8) does not
+// option. Java SE API (as of Java 11) does not
 // expose the API to control the Window size. It is
 // effectively hard-coded to 15
 clientMaxWindowBits = 15;
@@ -125,7 +125,7 @@ public class PerMessageDeflate implements Transformation {
 CLIENT_MAX_WINDOW_BITS, 
Integer.valueOf(clientMaxWindowBits)));
 }
 }
-// Java SE API (as of Java 8) does not expose the API 
to
+// Java SE API (as of Java 11) does not expose the API 
to
 // control the Window size. It is effectively 
hard-coded
 // to 15
 if (!isServer && clientMaxWindowBits != 15) {
diff --git a/test/org/apache/tomcat/websocket/TestWsWebSocketContainer.java 
b/test/org/apache/tomcat/websocket/TestWsWebSocketContainer.java
index f14809fab4..1fab006848 100644
--- a/test/org/apache/tomcat/websocket/TestWsWebSocketContainer.java
+++ b/test/org/apache/tomcat/websocket/TestWsWebSocketContainer.java
@@ -16,13 +16,9 @@
  */
 package org.apache.tomcat.websocket;
 
-import java.io.File;
-import java.io.FileInputStream;
 import java.io.IOException;
-import java.io.InputStream;
 import java.net.URI;
 import java.nio.ByteBuffer;
-import java.security.KeyStore;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Queue;
@@ -30,8 +26,6 @@ import java.util.Set;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 
-import javax.net.ssl.SSLContext;
-import javax.net.ssl.TrustManagerFactory;
 import javax.servlet.ServletContextEvent;
 import javax.websocket.ClientEndpointConfig;
 import javax.websocket.ContainerProvider;
@@ -52,7 +46,6 @@ import org.junit.Test;
 import org.apache.catalina.Context;
 import org.apache.catalina.servlets.DefaultServlet;
 import org.apache.catalina.startup.Tomcat;
-

Rate Limiter Filter or Valve

[Igal Sapir]

All,

I would like to add a Rate Limiter Filter or Valve which will help mitigate
DoS and Brute Force attacks, and want to get feedback from the community
and the PMC.  The checks will run before the request reaches the servlet
and will be dropped if too many requests arrive from the same IP address
within a certain time window.

It has been suggested that a Valve might be the better choice because it
can be set up on a Host or Engine level, but in my opinion a Filter is a
good choice for the following reasons:

1) While in the past it was common to reuse the same server for different
applications due to costs and challenges in setting up servers, nowadays it
is more common to set up a single application per server, many times in a
containerized environment, so setting up a Rate Limiter on a Host or Engine
does not offer much benefit over setting it up on the Context level.

2) Different applications have different requirements  In fact, different
URIs of the same application could have different requirements: a Login /
Authentication script expects far less requests from a single IP address
compared to a Dashboard page, for example.  Filter mapping allows us to map
different URIs to different configurations.

3) Filters are part of the Servlet spec, and therefore more users are
familiar with them and know how to configure them.

Either way it is implemented, I propose the following requirements for the
Rate Limiter itself (with the possibility of adding some of the features
later):

A) Low overhead - The checks will take place with every request so the
implementation must be efficient and make good utilization of resources.

B) Close approximation is good enough - If a URI is configured to allow 300
requests per minute and instead it allows 300 requests per 1:05 minute
before dropping the requests then that should be good enough, if that
allows the implementation to be more efficient with computation time and
memory consumption.  The approximation can offer leniency but not
strictness, meaning that it's ok if it allows more requests than the
configured value, but not less.

C) Drop excessive requests - Requests from an IP that exceeds the allowed
limit will be dropped and "429 Too Many Requests" will be returned to the
client.

D) Tag only mode - If configured as such, then rather than dropping the
request with a 429 error code, a Request Attribute will be set and that
would allow the Servlet to determine what to do next, e.g. it might allow
authenticated clients more requests than it would to unauthenticated
clients.

E) Allow list of URI patterns - Static resources have very little overhead,
so requests for "*.jpg" or "*.png" should not be counted by the Rate
Limiter.

F) Allow list of IP addresses - Known IP addresses that are used by your
organization, or 3rd party partners, should not be blocked.

G) Block list of IP addresses - Repeat offenders can be added automatically
to the block list for 4 hours, for example, preventing them from hitting
the server each minute and contributing to a DDoS attack.

H) Logging

Please offer your thoughts and ideas.

Thank you,

Igal

[Bug 66508] Tomcat after a GC pause causes the HTTP threads to be blocked to acquire a semaphore to process WebSockets connection closure.

[bugzilla]

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

--- Comment #5 from Venkata Manda  ---
(In reply to Mark Thomas from comment #3)
> Thanks for the steps to reproduce.
> 
> I'll note for the record that you also need to be using the NIO2 connector.
> 
> I can see what the root cause is. The write in "Event 3" fails and isn't
> cleaned up properly which then blocks the close message. I am currently
> exploring options for fixing this.

Thank you for working on the issue.
I am using the NI02 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 66508] Tomcat after a GC pause causes the HTTP threads to be blocked to acquire a semaphore to process WebSockets connection closure.

[bugzilla]

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

--- Comment #6 from Venkata Manda  ---
(In reply to Mark Thomas from comment #4)
> Fixed in:
> - 11.0.x for 11.0.0-M5 onwards
> - 10.1.x for 10.1.8 onwards
> -  9.0.x for  9.0.74 onwards
> -  8.5.x for  8.5.88 onwards
> 
> If you'd like to test a dev build before the next release, let me know.

Yes. Let me work on setting this up in my environment and will post you my
results here.

Thanks a lot for fixing the issue.

-- 
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] isapir commented on pull request #596: Use SELECT...FOR UPDATE to update session data in the database instead of DELETE, INSERT.

[via GitHub]

isapir commented on PR #596:
URL: https://github.com/apache/tomcat/pull/596#issuecomment-1460841013

   @ChristopherSchultz Is there a list of supported database systems with which 
the DataSourceStore is compatible?  Are you sure that they all support "SELECT 
FOR UPDATE"?  I tried to look that up and the "best" thing I found was 
https://www.sql-workbench.eu/dbms_comparison.html


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


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

[GitHub] [tomcat] isapir commented on a diff in pull request #596: Use SELECT...FOR UPDATE to update session data in the database instead of DELETE, INSERT.

[via GitHub]

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


##
java/org/apache/catalina/session/DataSourceStore.java:
##
@@ -626,15 +626,77 @@ public void save(Session session) throws IOException {
 byte[] obs = bos.toByteArray();
 int size = obs.length;
 try (ByteArrayInputStream bis = new 
ByteArrayInputStream(obs, 0, size);
-InputStream in = new BufferedInputStream(bis, 
size);
-PreparedStatement preparedSaveSql = 
_conn.prepareStatement(saveSql)) {
-preparedSaveSql.setString(1, session.getIdInternal());
-preparedSaveSql.setString(2, getName());
-preparedSaveSql.setBinaryStream(3, in, size);
-preparedSaveSql.setString(4, session.isValid() ? "1" : 
"0");
-preparedSaveSql.setInt(5, 
session.getMaxInactiveInterval());
-preparedSaveSql.setLong(6, 
session.getLastAccessedTime());
-preparedSaveSql.execute();
+ InputStream in = new BufferedInputStream(bis, size);
+ PreparedStatement preparedSaveSql = 
_conn.prepareStatement(saveSql, ResultSet.TYPE_FORWARD_ONLY, 
ResultSet.CONCUR_UPDATABLE)) {
+
+// Store auto-commit state
+boolean autoCommit = _conn.getAutoCommit();
+
+try {
+if(autoCommit) {
+_conn.setAutoCommit(false); // BEGIN 
TRANSACTION
+}
+
+preparedSaveSql.setString(1, getName());
+preparedSaveSql.setString(2, 
session.getIdInternal());
+
+ResultSet rs = preparedSaveSql.executeQuery();
+
+if(rs.next()) {
+// Session already exists in the db; update 
the various fields
+rs.updateBinaryStream(sessionDataCol, in, 
size);
+rs.updateString(sessionValidCol, 
session.isValid() ? "1" : "0");
+rs.updateInt(sessionMaxInactiveCol, 
session.getMaxInactiveInterval());
+rs.updateLong(sessionLastAccessedCol, 
session.getLastAccessedTime());
+
+rs.updateRow();
+} else {
+// Session does not exist. Insert.
+rs.moveToInsertRow();
+
+rs.updateString(sessionAppCol, getName());
+rs.updateString(sessionIdCol, 
session.getIdInternal());
+rs.updateBinaryStream(sessionIdCol, in, size);
+rs.updateString(sessionValidCol, 
session.isValid() ? "1" : "0");
+rs.updateInt(sessionMaxInactiveCol, 
session.getMaxInactiveInterval());
+rs.updateLong(sessionLastAccessedCol, 
session.getLastAccessedTime());
+
+rs.updateRow();
+}
+
+_conn.commit();
+} catch (SQLException sqle) {

Review Comment:
   Is it possible to put all the handlers in the same block [1]?  e.g.
   
   ```
   } catch (SQLException | RuntimeException | Error e) {
   // looks like mostly the same block to me
   }
   ```
   
   We don't need to support Java 6
   
   [1] 
https://docs.oracle.com/javase/7/docs/technotes/guides/language/catch-multiple.html



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


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

Re: Rate Limiter Filter or Valve

[Christopher Schultz]

Igal,

On 3/8/23 14:36, Igal Sapir wrote:

I would like to add a Rate Limiter Filter or Valve which will help mitigate
DoS and Brute Force attacks, and want to get feedback from the community
and the PMC.  The checks will run before the request reaches the servlet
and will be dropped if too many requests arrive from the same IP address
within a certain time window.

It has been suggested that a Valve might be the better choice because it
can be set up on a Host or Engine level, but in my opinion a Filter is a
good choice for the following reasons:

1) While in the past it was common to reuse the same server for different
applications due to costs and challenges in setting up servers, nowadays it
is more common to set up a single application per server, many times in a
containerized environment, so setting up a Rate Limiter on a Host or Engine
does not offer much benefit over setting it up on the Context level.


Counter-point: an attacker can make requests to non-existent contexts 
and avoid your Filter. You could put the Filter on the ROOT application 
as well, but (a) nobody will remember to do that and (b) it won't share 
data with the primary web application.


Honestly, it should pretty easy to write a general rate-limiter and then 
wrap a Filter and/or Valve around it.



2) Different applications have different requirements  In fact, different
URIs of the same application could have different requirements: a Login /
Authentication script expects far less requests from a single IP address
compared to a Dashboard page, for example.  Filter mapping allows us to map
different URIs to different configurations.


+1


3) Filters are part of the Servlet spec, and therefore more users are
familiar with them and know how to configure them.


+1

A Filter is also portable to other containers. Speaking of which... has 
this wheel already been invented? I would be surprised if Spring 
Security doesn't already have this kind of thing.



Either way it is implemented, I propose the following requirements for the
Rate Limiter itself (with the possibility of adding some of the features
later):

A) Low overhead - The checks will take place with every request so the
implementation must be efficient and make good utilization of resources.

B) Close approximation is good enough - If a URI is configured to allow 300
requests per minute and instead it allows 300 requests per 1:05 minute
before dropping the requests then that should be good enough, if that
allows the implementation to be more efficient with computation time and
memory consumption.  The approximation can offer leniency but not
strictness, meaning that it's ok if it allows more requests than the
configured value, but not less.

C) Drop excessive requests - Requests from an IP that exceeds the allowed
limit will be dropped and "429 Too Many Requests" will be returned to the
client.

D) Tag only mode - If configured as such, then rather than dropping the
request with a 429 error code, a Request Attribute will be set and that
would allow the Servlet to determine what to do next, e.g. it might allow
authenticated clients more requests than it would to unauthenticated
clients.


+1


E) Allow list of URI patterns - Static resources have very little overhead,
so requests for "*.jpg" or "*.png" should not be counted by the Rate
Limiter.

F) Allow list of IP addresses - Known IP addresses that are used by your
organization, or 3rd party partners, should not be blocked.

G) Block list of IP addresses - Repeat offenders can be added automatically
to the block list for 4 hours, for example, preventing them from hitting
the server each minute and contributing to a DDoS attack.

H) Logging

Please offer your thoughts and ideas.


Avoid regular expressions. For IP range matching, search the archives 
for references to CIDR. I think there was a discussion some years ago 
about that. In fact, I there should be a CIDR evaluator/matcher 
somewhere in the Tomcat source code already.


-chris

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

[GitHub] [tomcat] aooohan commented on a diff in pull request #596: Use SELECT...FOR UPDATE to update session data in the database instead of DELETE, INSERT.

[via GitHub]

aooohan commented on code in PR #596:
URL: https://github.com/apache/tomcat/pull/596#discussion_r1130309107


##
java/org/apache/catalina/session/DataSourceStore.java:
##
@@ -626,15 +626,77 @@ public void save(Session session) throws IOException {
 byte[] obs = bos.toByteArray();
 int size = obs.length;
 try (ByteArrayInputStream bis = new 
ByteArrayInputStream(obs, 0, size);
-InputStream in = new BufferedInputStream(bis, 
size);
-PreparedStatement preparedSaveSql = 
_conn.prepareStatement(saveSql)) {
-preparedSaveSql.setString(1, session.getIdInternal());
-preparedSaveSql.setString(2, getName());
-preparedSaveSql.setBinaryStream(3, in, size);
-preparedSaveSql.setString(4, session.isValid() ? "1" : 
"0");
-preparedSaveSql.setInt(5, 
session.getMaxInactiveInterval());
-preparedSaveSql.setLong(6, 
session.getLastAccessedTime());
-preparedSaveSql.execute();
+ InputStream in = new BufferedInputStream(bis, size);
+ PreparedStatement preparedSaveSql = 
_conn.prepareStatement(saveSql, ResultSet.TYPE_FORWARD_ONLY, 
ResultSet.CONCUR_UPDATABLE)) {
+
+// Store auto-commit state
+boolean autoCommit = _conn.getAutoCommit();
+
+try {
+if(autoCommit) {
+_conn.setAutoCommit(false); // BEGIN 
TRANSACTION
+}
+
+preparedSaveSql.setString(1, getName());
+preparedSaveSql.setString(2, 
session.getIdInternal());
+
+ResultSet rs = preparedSaveSql.executeQuery();
+
+if(rs.next()) {
+// Session already exists in the db; update 
the various fields
+rs.updateBinaryStream(sessionDataCol, in, 
size);
+rs.updateString(sessionValidCol, 
session.isValid() ? "1" : "0");
+rs.updateInt(sessionMaxInactiveCol, 
session.getMaxInactiveInterval());
+rs.updateLong(sessionLastAccessedCol, 
session.getLastAccessedTime());
+
+rs.updateRow();
+} else {
+// Session does not exist. Insert.
+rs.moveToInsertRow();
+
+rs.updateString(sessionAppCol, getName());
+rs.updateString(sessionIdCol, 
session.getIdInternal());
+rs.updateBinaryStream(sessionIdCol, in, size);
+rs.updateString(sessionValidCol, 
session.isValid() ? "1" : "0");
+rs.updateInt(sessionMaxInactiveCol, 
session.getMaxInactiveInterval());
+rs.updateLong(sessionLastAccessedCol, 
session.getLastAccessedTime());
+
+rs.updateRow();

Review Comment:
   ```suggestion
   rs.insertRow();
   ```
   Here is not correct.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


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

Re: Rate Limiter Filter or Valve

[Igal Sapir]

Chris,

On Wed, Mar 8, 2023 at 4:18 PM Christopher Schultz <
ch...@christopherschultz.net> wrote:

> Igal,
>
> On 3/8/23 14:36, Igal Sapir wrote:
> > I would like to add a Rate Limiter Filter or Valve which will help
> mitigate
> > DoS and Brute Force attacks, and want to get feedback from the community
> > and the PMC.  The checks will run before the request reaches the servlet
> > and will be dropped if too many requests arrive from the same IP address
> > within a certain time window.
> >
> > It has been suggested that a Valve might be the better choice because it
> > can be set up on a Host or Engine level, but in my opinion a Filter is a
> > good choice for the following reasons:
> >
> > 1) While in the past it was common to reuse the same server for different
> > applications due to costs and challenges in setting up servers, nowadays
> it
> > is more common to set up a single application per server, many times in a
> > containerized environment, so setting up a Rate Limiter on a Host or
> Engine
> > does not offer much benefit over setting it up on the Context level.
>
> Counter-point: an attacker can make requests to non-existent contexts
> and avoid your Filter. You could put the Filter on the ROOT application
> as well, but (a) nobody will remember to do that and (b) it won't share
> data with the primary web application.
>
> Honestly, it should pretty easy to write a general rate-limiter and then
> wrap a Filter and/or Valve around it.
>

Fair enough.  There is no reason to not implement both.


>
> > 2) Different applications have different requirements  In fact, different
> > URIs of the same application could have different requirements: a Login /
> > Authentication script expects far less requests from a single IP address
> > compared to a Dashboard page, for example.  Filter mapping allows us to
> map
> > different URIs to different configurations.
>
> +1
>
> > 3) Filters are part of the Servlet spec, and therefore more users are
> > familiar with them and know how to configure them.
>
> +1
>
> A Filter is also portable to other containers. Speaking of which... has
> this wheel already been invented? I would be surprised if Spring
> Security doesn't already have this kind of thing.
>

I don't know about Spring Security, but Jetty has a DoSFilter.  I haven't
looked into it though,
as I don't think that Tomcat users should have to go and get a filter from
another engine.


>
> > Either way it is implemented, I propose the following requirements for
> the
> > Rate Limiter itself (with the possibility of adding some of the features
> > later):
> >
> > A) Low overhead - The checks will take place with every request so the
> > implementation must be efficient and make good utilization of resources.
> >
> > B) Close approximation is good enough - If a URI is configured to allow
> 300
> > requests per minute and instead it allows 300 requests per 1:05 minute
> > before dropping the requests then that should be good enough, if that
> > allows the implementation to be more efficient with computation time and
> > memory consumption.  The approximation can offer leniency but not
> > strictness, meaning that it's ok if it allows more requests than the
> > configured value, but not less.
> >
> > C) Drop excessive requests - Requests from an IP that exceeds the allowed
> > limit will be dropped and "429 Too Many Requests" will be returned to the
> > client.
> >
> > D) Tag only mode - If configured as such, then rather than dropping the
> > request with a 429 error code, a Request Attribute will be set and that
> > would allow the Servlet to determine what to do next, e.g. it might allow
> > authenticated clients more requests than it would to unauthenticated
> > clients.
>
> +1
>
> > E) Allow list of URI patterns - Static resources have very little
> overhead,
> > so requests for "*.jpg" or "*.png" should not be counted by the Rate
> > Limiter.
> >
> > F) Allow list of IP addresses - Known IP addresses that are used by your
> > organization, or 3rd party partners, should not be blocked.
> >
> > G) Block list of IP addresses - Repeat offenders can be added
> automatically
> > to the block list for 4 hours, for example, preventing them from hitting
> > the server each minute and contributing to a DDoS attack.
> >
> > H) Logging
> >
> > Please offer your thoughts and ideas.
>
> Avoid regular expressions. For IP range matching, search the archives
> for references to CIDR. I think there was a discussion some years ago
> about that. In fact, I there should be a CIDR evaluator/matcher
> somewhere in the Tomcat source code already.
>

Agreed.  I am not a fan of Regex in general.  I will definitely look at the
Tomcat code for that first, thanks!

Igal


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