This is an automated email from the ASF dual-hosted git repository.
rmaucher 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 9c62ac12f2 Code review items for DS user database
9c62ac12f2 is described below
commit 9c62ac12f28fdad037fbbbbb08e3cbc2f1f08a5a
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 c2f7c1c0c1..1003d4bcb1 100644
--- a/java/org/apache/catalina/users/GenericGroup.java
+++ b/java/org/apache/catalina/users/GenericGroup.java
@@ -143,7 +143,8 @@ public class GenericGroup<UD extends UserDatabase> extends
AbstractGroup {
public boolean equals(Object obj) {
if (obj instanceof GenericGroup) {
GenericGroup<?> group = (GenericGroup<?>) obj;
- 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 bbc5842466..27efe04fe1 100644
--- a/java/org/apache/catalina/users/GenericRole.java
+++ b/java/org/apache/catalina/users/GenericRole.java
@@ -86,7 +86,8 @@ public class GenericRole<UD extends UserDatabase> extends
AbstractRole {
public boolean equals(Object obj) {
if (obj instanceof GenericRole) {
GenericRole<?> role = (GenericRole<?>) obj;
- 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 2a4eb6b319..7dca4521cc 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,16 +188,16 @@ public class GenericUser<UD extends UserDatabase> extends
AbstractUser {
@Override
public void setUsername(String username) {
- database.modifiedUser(this);
- // Note: changing the user name 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) {
GenericUser<?> user = (GenericUser<?>) obj;
- 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 2bd73e6769..9802e4e428 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -190,6 +190,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]
