This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 11.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/11.0.x by this push:
new 19005d0a4a Code review items for DS user database
19005d0a4a is described below
commit 19005d0a4ab3ab5e994c8f76e168c5a31370f3af
Author: remm <[email protected]>
AuthorDate: Tue May 26 15:12:06 2026 +0200
Code review items for DS user database
The most significant seems to be the missed modifiedUsers.clear() which
would cause an update loop.
---
.../catalina/users/DataSourceUserDatabase.java | 35 ++++++++++++----------
java/org/apache/catalina/users/GenericGroup.java | 3 +-
java/org/apache/catalina/users/GenericRole.java | 3 +-
java/org/apache/catalina/users/GenericUser.java | 10 +++----
webapps/docs/changelog.xml | 4 +++
5 files changed, 32 insertions(+), 23 deletions(-)
diff --git a/java/org/apache/catalina/users/DataSourceUserDatabase.java
b/java/org/apache/catalina/users/DataSourceUserDatabase.java
index 7842ca25ff..61b78fed3f 100644
--- a/java/org/apache/catalina/users/DataSourceUserDatabase.java
+++ b/java/org/apache/catalina/users/DataSourceUserDatabase.java
@@ -622,27 +622,26 @@ public class DataSourceUserDatabase extends
SparseUserDatabase {
users.putAll(createdUsers);
users.putAll(modifiedUsers);
- Connection dbConnection = openConnection();
- if (dbConnection != null) {
- try (PreparedStatement stmt =
dbConnection.prepareStatement(preparedAllUsers)) {
- try (ResultSet rs = stmt.executeQuery()) {
- while (rs.next()) {
- String userName = rs.getString(1);
- if (userName != null) {
- if (!users.containsKey(userName) &&
!removedUsers.containsKey(userName)) {
- User user =
findUserInternal(dbConnection, userName);
- if (user != null) {
- users.put(userName, user);
+ try (Connection dbConnection = openConnection()) {
+ if (dbConnection != null) {
+ try (PreparedStatement stmt =
dbConnection.prepareStatement(preparedAllUsers)) {
+ try (ResultSet rs = stmt.executeQuery()) {
+ while (rs.next()) {
+ String userName = rs.getString(1);
+ if (userName != null) {
+ if (!users.containsKey(userName) &&
!removedUsers.containsKey(userName)) {
+ User user =
findUserInternal(dbConnection, userName);
+ if (user != null) {
+ users.put(userName, user);
+ }
}
}
}
}
}
- } catch (SQLException e) {
-
log.error(sm.getString("dataSourceUserDatabase.exception"), e);
- } finally {
- closeConnection(dbConnection);
}
+ } catch (SQLException e) {
+
log.error(sm.getString("dataSourceUserDatabase.exception"), e);
}
return users.values().iterator();
} finally {
@@ -785,6 +784,7 @@ public class DataSourceUserDatabase extends
SparseUserDatabase {
}
} catch (SQLException e) {
log.error(sm.getString("dataSourceUserDatabase.exception"), e);
+ return null;
}
}
group = new GenericGroup<>(this, groupName,
description, groupRoles);
@@ -928,6 +928,7 @@ public class DataSourceUserDatabase extends
SparseUserDatabase {
}
} catch (SQLException e) {
log.error(sm.getString("dataSourceUserDatabase.exception"), e);
+ return null;
}
// Lookup groups
@@ -948,6 +949,7 @@ public class DataSourceUserDatabase extends
SparseUserDatabase {
}
} catch (SQLException e) {
log.error(sm.getString("dataSourceUserDatabase.exception"), e);
+ return null;
}
}
@@ -968,6 +970,7 @@ public class DataSourceUserDatabase extends
SparseUserDatabase {
}
} catch (SQLException e) {
log.error(sm.getString("dataSourceUserDatabase.exception"), e);
+ return null;
}
}
@@ -1660,7 +1663,7 @@ public class DataSourceUserDatabase extends
SparseUserDatabase {
}
}
}
- modifiedGroups.clear();
+ modifiedUsers.clear();
}
}
diff --git a/java/org/apache/catalina/users/GenericGroup.java
b/java/org/apache/catalina/users/GenericGroup.java
index cb86b287ee..912ad3f405 100644
--- a/java/org/apache/catalina/users/GenericGroup.java
+++ b/java/org/apache/catalina/users/GenericGroup.java
@@ -142,7 +142,8 @@ public class GenericGroup<UD extends UserDatabase> extends
AbstractGroup {
@Override
public boolean equals(Object obj) {
if (obj instanceof GenericGroup<?> group) {
- return group.database == database &&
groupname.equals(group.getGroupname());
+ return group.database == database &&
+ ((groupname == null && group.getGroupname() == null) ||
groupname.equals(group.getGroupname()));
}
return super.equals(obj);
}
diff --git a/java/org/apache/catalina/users/GenericRole.java
b/java/org/apache/catalina/users/GenericRole.java
index 9f8e21dd8c..fdcf380d55 100644
--- a/java/org/apache/catalina/users/GenericRole.java
+++ b/java/org/apache/catalina/users/GenericRole.java
@@ -85,7 +85,8 @@ public class GenericRole<UD extends UserDatabase> extends
AbstractRole {
@Override
public boolean equals(Object obj) {
if (obj instanceof GenericRole<?> role) {
- return role.database == database &&
rolename.equals(role.getRolename());
+ return role.database == database &&
+ ((rolename == null && role.getRolename() == null) ||
rolename.equals(role.getRolename()));
}
return super.equals(obj);
}
diff --git a/java/org/apache/catalina/users/GenericUser.java
b/java/org/apache/catalina/users/GenericUser.java
index 644a59ae10..eb8b3f148a 100644
--- a/java/org/apache/catalina/users/GenericUser.java
+++ b/java/org/apache/catalina/users/GenericUser.java
@@ -166,9 +166,9 @@ public class GenericUser<UD extends UserDatabase> extends
AbstractUser {
@Override
public void removeRoles() {
if (!roles.isEmpty()) {
+ roles.clear();
database.modifiedUser(this);
}
- roles.clear();
}
@@ -188,15 +188,15 @@ public class GenericUser<UD extends UserDatabase> extends
AbstractUser {
@Override
public void setUsername(String username) {
- database.modifiedUser(this);
- // Note: changing the username is a problem ...
- super.setUsername(username);
+ // Note: changing the username (which is the key) in a database will
not work
+ // and the user should be removed and added instead
}
@Override
public boolean equals(Object obj) {
if (obj instanceof GenericUser<?> user) {
- return user.database == database &&
username.equals(user.getUsername());
+ return user.database == database &&
+ ((username == null && user.getUsername() == null) ||
username.equals(user.getUsername()));
}
return super.equals(obj);
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index beb56403f2..d179c8b9d0 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -194,6 +194,10 @@
Mixup of WrapperListener and WrapperLifecycle elements in storeconfig.
(remm)
</fix>
+ <fix>
+ Incorrect processing of modified users in
+ <code>DataSourceUSerDatabase</code>. (remm)
+ </fix>
</changelog>
</subsection>
<subsection name="Coyote">
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
