[GitHub] [tomcat] michael-o commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
michael-o commented on a change in pull request #428:
URL: https://github.com/apache/tomcat/pull/428#discussion_r655173166
##
File path: java/org/apache/catalina/realm/DataSourceRealm.java
##
@@ -539,6 +612,162 @@ private boolean isRoleStoreDefined() {
}
+/**
+ * Return the specified user's requested user attributes as a map.
+ *
+ * @param dbConnection The database connection to be used
+ * @param username User name for which to return user attributes
+ *
+ * @return a map containing the specified user's requested user attributes
+ */
+protected Map getUserAttributesMap(Connection
dbConnection, String username) {
+
+String preparedAttributes = getUserAttributesStatement(dbConnection);
+if (preparedAttributes == null || preparedAttributes ==
USER_ATTRIBUTES_NONE_REQUESTED) {
Review comment:
Why not return `null`? Is is necessary for string concat?
--
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.
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] michael-o commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
michael-o commented on a change in pull request #428: URL: https://github.com/apache/tomcat/pull/428#discussion_r655173373 ## File path: java/org/apache/catalina/realm/JNDIRealm.java ## @@ -469,6 +474,19 @@ */ protected boolean useContextClassLoader = true; +/** + * The comma separated names of user attributes to additionally query from the + * user's directory entry. These will be provided to the user through the + * created Principal's attributes map. + */ +protected String userAttributes; Review comment: Agreed. -- 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. 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] michael-o commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
michael-o commented on a change in pull request #428:
URL: https://github.com/apache/tomcat/pull/428#discussion_r655174381
##
File path: java/org/apache/catalina/realm/DataSourceRealm.java
##
@@ -539,6 +612,162 @@ private boolean isRoleStoreDefined() {
}
+/**
+ * Return the specified user's requested user attributes as a map.
+ *
+ * @param dbConnection The database connection to be used
+ * @param username User name for which to return user attributes
+ *
+ * @return a map containing the specified user's requested user attributes
+ */
+protected Map getUserAttributesMap(Connection
dbConnection, String username) {
+
+String preparedAttributes = getUserAttributesStatement(dbConnection);
+if (preparedAttributes == null || preparedAttributes ==
USER_ATTRIBUTES_NONE_REQUESTED) {
+// The above reference comparison is intentional.
USER_ATTRIBUTES_NONE_REQUESTED
+// is a tag object (empty String) to distinguish between null (not
yet
+// initialized) and empty (no attributes requested).
+// TODO Could as well be changed to `preparedAttributes.lenghth()
= 0`
+
+// Return null if no user attributes are requested (or if the
statement was not
+// yet built successfully)
+return null;
+}
+
+try (PreparedStatement stmt =
dbConnection.prepareStatement(preparedAttributes)) {
+stmt.setString(1, username);
+
+try (ResultSet rs = stmt.executeQuery()) {
+
+if (rs.next()) {
+Map attrs = new LinkedHashMap<>();
Review comment:
I have no strong opinion about it. I cannot also judge about the
overhead when there are high load. Maybe Mark and others can share their
opinion.
--
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.
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] michael-o commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
michael-o commented on a change in pull request #428:
URL: https://github.com/apache/tomcat/pull/428#discussion_r655175314
##
File path: java/org/apache/catalina/realm/DataSourceRealm.java
##
@@ -572,6 +801,21 @@ protected void startInternal() throws LifecycleException {
temp.append(" = ?");
preparedCredentials = temp.toString();
+// Create the user attributes PreparedStatement string (only its tail
w/o SELECT
+// clause)
+temp = new StringBuilder(" FROM ");
+temp.append(userTable);
+temp.append(" WHERE ");
+temp.append(userNameCol);
+temp.append(" = ?");
+preparedAttributesTail = temp.toString();
+
+// Create the available user attributes PreparedStatement string
+temp = new StringBuilder("SELECT * FROM ");
+temp.append(userTable);
+temp.append(" WHERE FALSE");
Review comment:
BTW, such a "static" string builder gives you zero benefit compared over
`+=`. You can also use `+=` and the compliler will transform to string builder
automatically. String builders make sense when you have dynamic data like loops.
--
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.
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] michael-o commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
michael-o commented on a change in pull request #428:
URL: https://github.com/apache/tomcat/pull/428#discussion_r655176832
##
File path: java/org/apache/catalina/realm/DataSourceRealm.java
##
@@ -539,6 +612,162 @@ private boolean isRoleStoreDefined() {
}
+/**
+ * Return the specified user's requested user attributes as a map.
+ *
+ * @param dbConnection The database connection to be used
+ * @param username User name for which to return user attributes
+ *
+ * @return a map containing the specified user's requested user attributes
+ */
+protected Map getUserAttributesMap(Connection
dbConnection, String username) {
+
+String preparedAttributes = getUserAttributesStatement(dbConnection);
+if (preparedAttributes == null || preparedAttributes ==
USER_ATTRIBUTES_NONE_REQUESTED) {
+// The above reference comparison is intentional.
USER_ATTRIBUTES_NONE_REQUESTED
+// is a tag object (empty String) to distinguish between null (not
yet
+// initialized) and empty (no attributes requested).
+// TODO Could as well be changed to `preparedAttributes.lenghth()
= 0`
+
+// Return null if no user attributes are requested (or if the
statement was not
+// yet built successfully)
+return null;
+}
+
+try (PreparedStatement stmt =
dbConnection.prepareStatement(preparedAttributes)) {
+stmt.setString(1, username);
+
+try (ResultSet rs = stmt.executeQuery()) {
+
+if (rs.next()) {
+Map attrs = new LinkedHashMap<>();
+ResultSetMetaData md = rs.getMetaData();
+int ncols = md.getColumnCount();
+for (int columnIndex = 1; columnIndex <= ncols;
columnIndex++) {
+String columnName = md.getColumnName(columnIndex);
+// Ignore case, database may have case-insensitive
field names
+if (columnName.equalsIgnoreCase(userCredCol)) {
+// Always skip userCredCol (must be there if all
columns
+// have been requested)
+continue;
+}
+attrs.put(columnName, rs.getObject(columnIndex));
+}
+return attrs.size() > 0 ? attrs : null;
Review comment:
Not array-typed columns. A query might return more than one line per
user attribute which would actually override all previous ones with the last
one. I don't expect support for this here, but just like to mention it. If you
don't want to support that is perfectly fine, but it should simply be
documented that's it.
--
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.
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] michael-o commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
michael-o commented on a change in pull request #428:
URL: https://github.com/apache/tomcat/pull/428#discussion_r655177819
##
File path: java/org/apache/catalina/realm/DataSourceRealm.java
##
@@ -539,6 +612,162 @@ private boolean isRoleStoreDefined() {
}
+/**
+ * Return the specified user's requested user attributes as a map.
+ *
+ * @param dbConnection The database connection to be used
+ * @param username User name for which to return user attributes
+ *
+ * @return a map containing the specified user's requested user attributes
+ */
+protected Map getUserAttributesMap(Connection
dbConnection, String username) {
+
+String preparedAttributes = getUserAttributesStatement(dbConnection);
+if (preparedAttributes == null || preparedAttributes ==
USER_ATTRIBUTES_NONE_REQUESTED) {
+// The above reference comparison is intentional.
USER_ATTRIBUTES_NONE_REQUESTED
+// is a tag object (empty String) to distinguish between null (not
yet
+// initialized) and empty (no attributes requested).
+// TODO Could as well be changed to `preparedAttributes.lenghth()
= 0`
+
+// Return null if no user attributes are requested (or if the
statement was not
+// yet built successfully)
+return null;
+}
+
+try (PreparedStatement stmt =
dbConnection.prepareStatement(preparedAttributes)) {
+stmt.setString(1, username);
+
+try (ResultSet rs = stmt.executeQuery()) {
+
+if (rs.next()) {
+Map attrs = new LinkedHashMap<>();
+ResultSetMetaData md = rs.getMetaData();
+int ncols = md.getColumnCount();
+for (int columnIndex = 1; columnIndex <= ncols;
columnIndex++) {
+String columnName = md.getColumnName(columnIndex);
+// Ignore case, database may have case-insensitive
field names
+if (columnName.equalsIgnoreCase(userCredCol)) {
+// Always skip userCredCol (must be there if all
columns
+// have been requested)
+continue;
+}
+attrs.put(columnName, rs.getObject(columnIndex));
+}
+return attrs.size() > 0 ? attrs : null;
Review comment:
> What other types of multi-valued attributes are you thinking of?
I haven't use SQL databases for any user store for the last 15 years, but
only Active Directory so I am used to multi-valued values.
--
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.
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] michael-o commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
michael-o commented on a change in pull request #428: URL: https://github.com/apache/tomcat/pull/428#discussion_r655178151 ## File path: java/org/apache/catalina/TomcatPrincipal.java ## @@ -47,4 +48,58 @@ * exception to LoginContext */ void logout() throws Exception; + +/** + * Returns the value of the named attribute as an Object, or + * null if no attribute of the given name exists. May also + * return null, if the named attribute exists but cannot be + * returned in a way that ensures that changes made to the returned object + * are not reflected by objects returned by subsequent calls to this method. + * + * Only the servlet container may set attributes to make available custom + * information about a Principal or the user it represents. For example, + * some of the Realm implementations can be configured to additionally query + * user attributes from the user database, which then are provided + * through the Principal's attributes map. + * + * In order to keep the attributes map immutable, the objects + * returned by this method should always be defensive copies of the + * objects contained in the attributes map. Any changes applied to these + * objects must not be reflected by objects returned by subsequent calls to + * this method. If that cannot be guaranteed (e. g. there is no way to copy + * the object), the object's string representation (or even + * null) shall be returned. + * + * Attribute names and naming conventions are maintained by the Tomcat + * components that contribute to this map, like some of the Realm + * implementations. + * + * @param name a String specifying the name of the attribute Review comment: That's fine. -- 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. 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] michael-o commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
michael-o commented on a change in pull request #428:
URL: https://github.com/apache/tomcat/pull/428#discussion_r655182143
##
File path: java/org/apache/catalina/realm/DataSourceRealm.java
##
@@ -539,6 +612,162 @@ private boolean isRoleStoreDefined() {
}
+/**
+ * Return the specified user's requested user attributes as a map.
+ *
+ * @param dbConnection The database connection to be used
+ * @param username User name for which to return user attributes
+ *
+ * @return a map containing the specified user's requested user attributes
+ */
+protected Map getUserAttributesMap(Connection
dbConnection, String username) {
+
+String preparedAttributes = getUserAttributesStatement(dbConnection);
+if (preparedAttributes == null || preparedAttributes ==
USER_ATTRIBUTES_NONE_REQUESTED) {
+// The above reference comparison is intentional.
USER_ATTRIBUTES_NONE_REQUESTED
+// is a tag object (empty String) to distinguish between null (not
yet
+// initialized) and empty (no attributes requested).
+// TODO Could as well be changed to `preparedAttributes.lenghth()
= 0`
+
+// Return null if no user attributes are requested (or if the
statement was not
+// yet built successfully)
+return null;
+}
+
+try (PreparedStatement stmt =
dbConnection.prepareStatement(preparedAttributes)) {
+stmt.setString(1, username);
+
+try (ResultSet rs = stmt.executeQuery()) {
+
+if (rs.next()) {
+Map attrs = new LinkedHashMap<>();
+ResultSetMetaData md = rs.getMetaData();
+int ncols = md.getColumnCount();
+for (int columnIndex = 1; columnIndex <= ncols;
columnIndex++) {
+String columnName = md.getColumnName(columnIndex);
+// Ignore case, database may have case-insensitive
field names
+if (columnName.equalsIgnoreCase(userCredCol)) {
+// Always skip userCredCol (must be there if all
columns
+// have been requested)
+continue;
+}
+attrs.put(columnName, rs.getObject(columnIndex));
+}
+return attrs.size() > 0 ? attrs : null;
+}
+}
+} catch (SQLException e) {
+containerLog.error(
+
sm.getString("dataSourceRealm.getUserAttributes.exception", username), e);
+}
+
+return null;
+}
+
+
+/**
+ * Return the SQL statement for querying additional user attributes. The
+ * statement is lazily initialized (lazily initialized singleton
with
+ * double-checked locking, DCL) since building it may require an
extra
+ * database query under some conditions.
+ *
+ * @param dbConnection connection for accessing the database
+ */
+private String getUserAttributesStatement(Connection dbConnection) {
+// DCL so userAttributesStatement MUST be volatile
+if (userAttributesStatement == null) {
+synchronized (userAttributesStatementLock) {
+if (userAttributesStatement == null) {
+List requestedAttributes =
parseUserAttributes(userAttributes);
+if (requestedAttributes == null) {
+return USER_ATTRIBUTES_NONE_REQUESTED;
+}
+if (requestedAttributes.size() > 0
+&&
requestedAttributes.get(0).equals(USER_ATTRIBUTES_WILDCARD)) {
+userAttributesStatement = "SELECT *" +
preparedAttributesTail;
Review comment:
I see it now. Looked like a bug from first glance.
--
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.
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] michael-o commented on pull request #428: Enhancement: Additional user attributes queried by (some) realms
michael-o commented on pull request #428: URL: https://github.com/apache/tomcat/pull/428#issuecomment-864846238 > > > > Can you please explain the purpose of the `deniedAttributes`? Why is it necessary, what is the usecase for? > > _Denied Attributes_ is the internal term of attributes, for which access is denied to. Those attributes could never be exposed as _user attributes_ in the Principal's attributes map. Basically, this applies to attributes/fields that contain the user's password. Requesting such an attribute causes a `userAttributeAccessDenied` message to be logged. > > Both MemoryRealm and UserDatabaseRealm provide all possible (and actually used, MemoryRealm does not use the group attribute) XML attributes of the `` entry in tomcat-users.xml. Obviously, the password attribute is sensitive and should not be exposed so, these Realms make the `password` attribute a _denied_ attribute. > > DataSourceRealm makes the column name configured in `userCredCol` a _denied attribute_ and JNDIRealm does this for the attribute name configured in `userPassword`. So you basically want to protest the admin/developer to shoot in the foot? -- 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. 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] michael-o edited a comment on pull request #428: Enhancement: Additional user attributes queried by (some) realms
michael-o edited a comment on pull request #428: URL: https://github.com/apache/tomcat/pull/428#issuecomment-864846238 > > > > Can you please explain the purpose of the `deniedAttributes`? Why is it necessary, what is the usecase for? > > _Denied Attributes_ is the internal term of attributes, for which access is denied to. Those attributes could never be exposed as _user attributes_ in the Principal's attributes map. Basically, this applies to attributes/fields that contain the user's password. Requesting such an attribute causes a `userAttributeAccessDenied` message to be logged. > > Both MemoryRealm and UserDatabaseRealm provide all possible (and actually used, MemoryRealm does not use the group attribute) XML attributes of the `` entry in tomcat-users.xml. Obviously, the password attribute is sensitive and should not be exposed so, these Realms make the `password` attribute a _denied_ attribute. > > DataSourceRealm makes the column name configured in `userCredCol` a _denied attribute_ and JNDIRealm does this for the attribute name configured in `userPassword`. So you basically want to protect the admin/developer to shoot in the foot? -- 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. 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] cklein05 commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
cklein05 commented on a change in pull request #428:
URL: https://github.com/apache/tomcat/pull/428#discussion_r655188601
##
File path: java/org/apache/catalina/realm/DataSourceRealm.java
##
@@ -539,6 +612,162 @@ private boolean isRoleStoreDefined() {
}
+/**
+ * Return the specified user's requested user attributes as a map.
+ *
+ * @param dbConnection The database connection to be used
+ * @param username User name for which to return user attributes
+ *
+ * @return a map containing the specified user's requested user attributes
+ */
+protected Map getUserAttributesMap(Connection
dbConnection, String username) {
+
+String preparedAttributes = getUserAttributesStatement(dbConnection);
+if (preparedAttributes == null || preparedAttributes ==
USER_ATTRIBUTES_NONE_REQUESTED) {
Review comment:
With DataSourceRealm, parsing and validating is done lazily, so `null`
means _still uninitialized_. When not using a value different from `null` for
_empty/none required_, I end with constantly parsing and validating in method
`getUserAttributesStatement`. I could have uses another boolean
`userAttributesNoneRequested`, however I like that _3-state value approach_
some more.
--
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.
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] cklein05 commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
cklein05 commented on a change in pull request #428:
URL: https://github.com/apache/tomcat/pull/428#discussion_r655190479
##
File path: java/org/apache/catalina/realm/DataSourceRealm.java
##
@@ -539,6 +612,162 @@ private boolean isRoleStoreDefined() {
}
+/**
+ * Return the specified user's requested user attributes as a map.
+ *
+ * @param dbConnection The database connection to be used
+ * @param username User name for which to return user attributes
+ *
+ * @return a map containing the specified user's requested user attributes
+ */
+protected Map getUserAttributesMap(Connection
dbConnection, String username) {
+
+String preparedAttributes = getUserAttributesStatement(dbConnection);
+if (preparedAttributes == null || preparedAttributes ==
USER_ATTRIBUTES_NONE_REQUESTED) {
+// The above reference comparison is intentional.
USER_ATTRIBUTES_NONE_REQUESTED
+// is a tag object (empty String) to distinguish between null (not
yet
+// initialized) and empty (no attributes requested).
+// TODO Could as well be changed to `preparedAttributes.lenghth()
= 0`
+
+// Return null if no user attributes are requested (or if the
statement was not
+// yet built successfully)
+return null;
+}
+
+try (PreparedStatement stmt =
dbConnection.prepareStatement(preparedAttributes)) {
+stmt.setString(1, username);
+
+try (ResultSet rs = stmt.executeQuery()) {
+
+if (rs.next()) {
+Map attrs = new LinkedHashMap<>();
+ResultSetMetaData md = rs.getMetaData();
+int ncols = md.getColumnCount();
+for (int columnIndex = 1; columnIndex <= ncols;
columnIndex++) {
+String columnName = md.getColumnName(columnIndex);
+// Ignore case, database may have case-insensitive
field names
+if (columnName.equalsIgnoreCase(userCredCol)) {
+// Always skip userCredCol (must be there if all
columns
+// have been requested)
+continue;
+}
+attrs.put(columnName, rs.getObject(columnIndex));
+}
+return attrs.size() > 0 ? attrs : null;
Review comment:
I'm about to implement support for (at least) arrays, BLOBS and CLOBS.
Likely it is sufficient to support one-dimensional arrays only. Everything
_not_ supported will be documented (both in Javadoc comments as well as in
Tomcat's configuration reference).
--
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.
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] cklein05 commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
cklein05 commented on a change in pull request #428:
URL: https://github.com/apache/tomcat/pull/428#discussion_r655192559
##
File path: java/org/apache/catalina/realm/DataSourceRealm.java
##
@@ -572,6 +801,21 @@ protected void startInternal() throws LifecycleException {
temp.append(" = ?");
preparedCredentials = temp.toString();
+// Create the user attributes PreparedStatement string (only its tail
w/o SELECT
+// clause)
+temp = new StringBuilder(" FROM ");
+temp.append(userTable);
+temp.append(" WHERE ");
+temp.append(userNameCol);
+temp.append(" = ?");
+preparedAttributesTail = temp.toString();
+
+// Create the available user attributes PreparedStatement string
+temp = new StringBuilder("SELECT * FROM ");
+temp.append(userTable);
+temp.append(" WHERE FALSE");
Review comment:
I know. However, the contributing guideline recommends to _copy_ the
coding style of the modified source file. That's what I did: the _roles_ and
_credentials_ statements have already been there, using the `StringBuilder`.
--
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.
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] cklein05 commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
cklein05 commented on a change in pull request #428:
URL: https://github.com/apache/tomcat/pull/428#discussion_r655202844
##
File path: java/org/apache/catalina/realm/GenericPrincipal.java
##
@@ -304,10 +333,165 @@ public void logout() throws Exception {
}
+@Override
+public Object getAttribute(String name) {
+if (attributes == null) {
+return null;
+}
+Object value = attributes.get(name);
+if (value == null) {
+return null;
+}
+Object copy = copyObject(value);
+return copy != null ? copy : value.toString();
+}
+
+
+@Override
+public Enumeration getAttributeNames() {
+if (attributes == null) {
+return Collections.emptyEnumeration();
+}
+return Collections.enumeration(attributes.keySet());
+}
+
+
+@Override
+public boolean isAttributesCaseIgnored() {
+return (attributes instanceof CaseInsensitiveKeyMap);
+}
+
+
+/**
+ * Creates and returns a deep copy of the specified object. Deep-copying
+ * works only for objects of a couple of hard coded types or, if the
+ * object implements java.io.Serializable. In all other cases,
+ * this method returns null.
+ *
+ * @param obj the object to copy
+ *
+ * @return a deep copied clone of the specified object, or
null
+ * if deep-copying was not possible
+ */
+private Object copyObject(Object obj) {
+
+// first, try some commonly used object types
+if (obj instanceof String) {
+return new String((String) obj);
+
+} else if (obj instanceof Integer) {
+return Integer.valueOf((int) obj);
+
+} else if (obj instanceof Long) {
+return Long.valueOf((long) obj);
+
+} else if (obj instanceof Boolean) {
+return Boolean.valueOf((boolean) obj);
+
+} else if (obj instanceof Double) {
+return Double.valueOf((double) obj);
+
+} else if (obj instanceof Float) {
+return Float.valueOf((float) obj);
+
+} else if (obj instanceof Character) {
+return Character.valueOf((char) obj);
+
+} else if (obj instanceof Byte) {
+return Byte.valueOf((byte) obj);
+
+} else if (obj instanceof Short) {
+return Short.valueOf((short) obj);
+
+} else if (obj instanceof BigDecimal) {
+return new BigDecimal(((BigDecimal) obj).toString());
+
+} else if (obj instanceof BigInteger) {
+return new BigInteger(((BigInteger) obj).toString());
+
+}
+
+// Date and JDBC date and time types
+else if (obj instanceof java.sql.Date) {
+return ((java.sql.Date) obj).clone();
+
+} else if (obj instanceof java.sql.Time) {
+return ((java.sql.Time) obj).clone();
+
+} else if (obj instanceof java.sql.Timestamp) {
+return ((java.sql.Timestamp) obj).clone();
+
+} else if (obj instanceof Date) {
+return ((Date) obj).clone();
+
+}
+
+// these types may come up as well
+else if (obj instanceof URI) {
+try {
+return new URI(((URI) obj).toString());
+} catch (URISyntaxException e) {
+// not expected
+}
+} else if (obj instanceof URL) {
+try {
+return new URI(((URL) obj).toString());
+} catch (URISyntaxException e) {
+// not expected
+}
+} else if (obj instanceof UUID) {
+return new UUID(((UUID) obj).getMostSignificantBits(),
+((UUID) obj).getLeastSignificantBits());
+
+}
Review comment:
I also mentioned that in this PR's initial comment under _Defensive
copies of attribute values_. So, consider my solution as a starting point (e.
g. for a in-deep discussion about that). There was not yet any agreement for a
strategy so, I started off with a working solution that should be safe. Now we
see, that it might even by over-engineered or paranoid.
When modifying the final 'value' field in an `Integer`, I'm quite sure that
Java uses that new value subsequently. So, It's not only the in-memory
representation.
(Das Sprichwort kannte ich bisher nicht. Sehr treffend :)
--
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.
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] michael-o commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
michael-o commented on a change in pull request #428:
URL: https://github.com/apache/tomcat/pull/428#discussion_r655204617
##
File path: java/org/apache/catalina/realm/DataSourceRealm.java
##
@@ -539,6 +612,162 @@ private boolean isRoleStoreDefined() {
}
+/**
+ * Return the specified user's requested user attributes as a map.
+ *
+ * @param dbConnection The database connection to be used
+ * @param username User name for which to return user attributes
+ *
+ * @return a map containing the specified user's requested user attributes
+ */
+protected Map getUserAttributesMap(Connection
dbConnection, String username) {
+
+String preparedAttributes = getUserAttributesStatement(dbConnection);
+if (preparedAttributes == null || preparedAttributes ==
USER_ATTRIBUTES_NONE_REQUESTED) {
Review comment:
Yes, that's a problem. I agree. Since I cannot provide anything better I
accept this.
--
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.
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] cklein05 commented on pull request #428: Enhancement: Additional user attributes queried by (some) realms
cklein05 commented on pull request #428: URL: https://github.com/apache/tomcat/pull/428#issuecomment-864868657 > So you basically want to protect the admin/developer to shoot in the foot? Yes, as it may hurt badly. Also, only these _denied attributes_ makes using the (otherwise quite comfortable) wildcard character * possible/safe. -- 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. 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] cklein05 commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
cklein05 commented on a change in pull request #428:
URL: https://github.com/apache/tomcat/pull/428#discussion_r655222693
##
File path: java/org/apache/catalina/realm/DataSourceRealm.java
##
@@ -539,6 +612,162 @@ private boolean isRoleStoreDefined() {
}
+/**
+ * Return the specified user's requested user attributes as a map.
+ *
+ * @param dbConnection The database connection to be used
+ * @param username User name for which to return user attributes
+ *
+ * @return a map containing the specified user's requested user attributes
+ */
+protected Map getUserAttributesMap(Connection
dbConnection, String username) {
+
+String preparedAttributes = getUserAttributesStatement(dbConnection);
+if (preparedAttributes == null || preparedAttributes ==
USER_ATTRIBUTES_NONE_REQUESTED) {
+// The above reference comparison is intentional.
USER_ATTRIBUTES_NONE_REQUESTED
+// is a tag object (empty String) to distinguish between null (not
yet
+// initialized) and empty (no attributes requested).
+// TODO Could as well be changed to `preparedAttributes.lenghth()
= 0`
+
+// Return null if no user attributes are requested (or if the
statement was not
+// yet built successfully)
+return null;
+}
+
+try (PreparedStatement stmt =
dbConnection.prepareStatement(preparedAttributes)) {
+stmt.setString(1, username);
+
+try (ResultSet rs = stmt.executeQuery()) {
+
+if (rs.next()) {
+Map attrs = new LinkedHashMap<>();
+ResultSetMetaData md = rs.getMetaData();
+int ncols = md.getColumnCount();
+for (int columnIndex = 1; columnIndex <= ncols;
columnIndex++) {
+String columnName = md.getColumnName(columnIndex);
+// Ignore case, database may have case-insensitive
field names
+if (columnName.equalsIgnoreCase(userCredCol)) {
+// Always skip userCredCol (must be there if all
columns
+// have been requested)
+continue;
+}
+attrs.put(columnName, rs.getObject(columnIndex));
+}
+return attrs.size() > 0 ? attrs : null;
Review comment:
Ah, OK, multiple records for a user... Since by definition, the
attributes are extra fields of the _user table_ (that has logon name and
password), there is generally no support for user tables that contain the user
more than once. That's just not allowed. In method `getPassword` credentials
are read from the first record returned (there's no `while (rs.hasNext())`
loop).
So generally, having a user more than once in the user table may cause
problems with attributes: in `getUserAttributesMap` the multiple rows for that
username could come in a different order. Than, the Principal will get the
wrong attributes.
We could generally deny authenticating in a user, for which more than one
row is present in the user table (as done in JNDIRealm, AFAIK). However, that
has nothing to do with the enhancement and should probably got to a new PR.
As an alternative, we could add an ORDER BY to both the
_credential_ and the _attributes_ statements in order to force the same order
of rows.
--
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.
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] cklein05 commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
cklein05 commented on a change in pull request #428:
URL: https://github.com/apache/tomcat/pull/428#discussion_r655222693
##
File path: java/org/apache/catalina/realm/DataSourceRealm.java
##
@@ -539,6 +612,162 @@ private boolean isRoleStoreDefined() {
}
+/**
+ * Return the specified user's requested user attributes as a map.
+ *
+ * @param dbConnection The database connection to be used
+ * @param username User name for which to return user attributes
+ *
+ * @return a map containing the specified user's requested user attributes
+ */
+protected Map getUserAttributesMap(Connection
dbConnection, String username) {
+
+String preparedAttributes = getUserAttributesStatement(dbConnection);
+if (preparedAttributes == null || preparedAttributes ==
USER_ATTRIBUTES_NONE_REQUESTED) {
+// The above reference comparison is intentional.
USER_ATTRIBUTES_NONE_REQUESTED
+// is a tag object (empty String) to distinguish between null (not
yet
+// initialized) and empty (no attributes requested).
+// TODO Could as well be changed to `preparedAttributes.lenghth()
= 0`
+
+// Return null if no user attributes are requested (or if the
statement was not
+// yet built successfully)
+return null;
+}
+
+try (PreparedStatement stmt =
dbConnection.prepareStatement(preparedAttributes)) {
+stmt.setString(1, username);
+
+try (ResultSet rs = stmt.executeQuery()) {
+
+if (rs.next()) {
+Map attrs = new LinkedHashMap<>();
+ResultSetMetaData md = rs.getMetaData();
+int ncols = md.getColumnCount();
+for (int columnIndex = 1; columnIndex <= ncols;
columnIndex++) {
+String columnName = md.getColumnName(columnIndex);
+// Ignore case, database may have case-insensitive
field names
+if (columnName.equalsIgnoreCase(userCredCol)) {
+// Always skip userCredCol (must be there if all
columns
+// have been requested)
+continue;
+}
+attrs.put(columnName, rs.getObject(columnIndex));
+}
+return attrs.size() > 0 ? attrs : null;
Review comment:
Ah, OK, multiple records for a user... Since by definition, the
attributes are extra fields of the _user table_ (that has logon name and
password), there is generally no support for user tables that contain the user
more than once. That's just not allowed. In method `getPassword` credentials
are read from the first record returned (there's no `while (rs.hasNext())`
loop).
So generally, having a user more than once in the user table may cause
problems with attributes: in `getUserAttributesMap` the multiple rows for that
username could come in a different order. Than, the Principal will get the
wrong attributes.
We could generally deny authenticating in a user, for which more than one
row is present in the user table (as done in JNDIRealm, AFAIK). However, that
has nothing to do with the enhancement and should probably go into a new PR.
As an alternative, we could add an ORDER BY to both the
_credential_ and the _attributes_ statements in order to force the same order
of rows.
So, since the _multiple rows for one user_ case should actually not occur,
it is no proper way to add multi-valued attributes by having multiple records.
--
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.
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] michael-o commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
michael-o commented on a change in pull request #428:
URL: https://github.com/apache/tomcat/pull/428#discussion_r655234294
##
File path: java/org/apache/catalina/realm/DataSourceRealm.java
##
@@ -539,6 +612,162 @@ private boolean isRoleStoreDefined() {
}
+/**
+ * Return the specified user's requested user attributes as a map.
+ *
+ * @param dbConnection The database connection to be used
+ * @param username User name for which to return user attributes
+ *
+ * @return a map containing the specified user's requested user attributes
+ */
+protected Map getUserAttributesMap(Connection
dbConnection, String username) {
+
+String preparedAttributes = getUserAttributesStatement(dbConnection);
+if (preparedAttributes == null || preparedAttributes ==
USER_ATTRIBUTES_NONE_REQUESTED) {
+// The above reference comparison is intentional.
USER_ATTRIBUTES_NONE_REQUESTED
+// is a tag object (empty String) to distinguish between null (not
yet
+// initialized) and empty (no attributes requested).
+// TODO Could as well be changed to `preparedAttributes.lenghth()
= 0`
+
+// Return null if no user attributes are requested (or if the
statement was not
+// yet built successfully)
+return null;
+}
+
+try (PreparedStatement stmt =
dbConnection.prepareStatement(preparedAttributes)) {
+stmt.setString(1, username);
+
+try (ResultSet rs = stmt.executeQuery()) {
+
+if (rs.next()) {
+Map attrs = new LinkedHashMap<>();
+ResultSetMetaData md = rs.getMetaData();
+int ncols = md.getColumnCount();
+for (int columnIndex = 1; columnIndex <= ncols;
columnIndex++) {
+String columnName = md.getColumnName(columnIndex);
+// Ignore case, database may have case-insensitive
field names
+if (columnName.equalsIgnoreCase(userCredCol)) {
+// Always skip userCredCol (must be there if all
columns
+// have been requested)
+continue;
+}
+attrs.put(columnName, rs.getObject(columnIndex));
+}
+return attrs.size() > 0 ? attrs : null;
Review comment:
Yes, I completely forgot the row orientation on the database. No need to
add support since this would require a schema change. Don't bother, you already
did a lot.
--
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.
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] cklein05 commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
cklein05 commented on a change in pull request #428:
URL: https://github.com/apache/tomcat/pull/428#discussion_r655239478
##
File path: java/org/apache/catalina/realm/DataSourceRealm.java
##
@@ -539,6 +612,162 @@ private boolean isRoleStoreDefined() {
}
+/**
+ * Return the specified user's requested user attributes as a map.
+ *
+ * @param dbConnection The database connection to be used
+ * @param username User name for which to return user attributes
+ *
+ * @return a map containing the specified user's requested user attributes
+ */
+protected Map getUserAttributesMap(Connection
dbConnection, String username) {
+
+String preparedAttributes = getUserAttributesStatement(dbConnection);
+if (preparedAttributes == null || preparedAttributes ==
USER_ATTRIBUTES_NONE_REQUESTED) {
+// The above reference comparison is intentional.
USER_ATTRIBUTES_NONE_REQUESTED
+// is a tag object (empty String) to distinguish between null (not
yet
+// initialized) and empty (no attributes requested).
+// TODO Could as well be changed to `preparedAttributes.lenghth()
= 0`
+
+// Return null if no user attributes are requested (or if the
statement was not
+// yet built successfully)
+return null;
+}
+
+try (PreparedStatement stmt =
dbConnection.prepareStatement(preparedAttributes)) {
+stmt.setString(1, username);
+
+try (ResultSet rs = stmt.executeQuery()) {
+
+if (rs.next()) {
+Map attrs = new LinkedHashMap<>();
+ResultSetMetaData md = rs.getMetaData();
+int ncols = md.getColumnCount();
+for (int columnIndex = 1; columnIndex <= ncols;
columnIndex++) {
+String columnName = md.getColumnName(columnIndex);
+// Ignore case, database may have case-insensitive
field names
+if (columnName.equalsIgnoreCase(userCredCol)) {
+// Always skip userCredCol (must be there if all
columns
+// have been requested)
+continue;
+}
+attrs.put(columnName, rs.getObject(columnIndex));
+}
+return attrs.size() > 0 ? attrs : null;
Review comment:
I'm almost done with support for SQL-Arrays, BLOBs and CLOBS. I thought
that these come with `getObject()` for free. But, a couple of lines of code are
required to get these types as well.
Arrays are a quite cool feature and provide multi-valued attribute support.
BLOBS may be quite useful to provide binary data, like images and similar stuff.
--
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.
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 65387] JSP compile error after upgrade 10.0.6 -> 10.0.7
https://bz.apache.org/bugzilla/show_bug.cgi?id=65387 --- Comment #18 from Mark Thomas --- Just following up in this as we'd really like to catch regressions like this in the ~3 working day window between the release candidate being available and the release vote concluding. It is great that you are using the latest Tomcat release as soon as it as available but what would we, the Tomcat team, need to do for you to be able to test the release candidate as soon as it as available rather than you waiting for the release? -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 65390] Problem in the code generated by jasper
https://bz.apache.org/bugzilla/show_bug.cgi?id=65390 --- Comment #6 from Mark Thomas --- Just following up in this as we'd really like to catch regressions like this in the ~3 working day window between the release candidate being available and the release vote concluding. It is great that you are using the latest Tomcat release as soon as it as available but what would we, the Tomcat team, need to do for you to be able to test the release candidate as soon as it as available rather than you waiting for the release? -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
JSP regressions - do we need a new set of releases
All, Despite the extra tests I added to improve code coverage to almost 100% for JSP code generation, the changes I made to remove unnecessary code have triggered a couple of regressions. There is no easy way to avoid these regressions. Given the above do we want to pull forward the July release round and start the next set of releases soon / today / this week? There is a balance to strike between getting fixed releases out that address these regressions and allowing enough time for any further regressions to be identified. Thoughts? Mark - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 65390] Problem in the code generated by jasper
https://bz.apache.org/bugzilla/show_bug.cgi?id=65390 --- Comment #7 from tk...@ai-ag.de --- Hallo Mark, first I’d like to thank you for your very fast response on our bug report :) We use tomcat-embedded and pull it from a maven repository. If you made your release candidates available via a public maven repository which always hosted the current release or upcoming patch, e.g. "org.apache.tomcat:tomcat-catalina:9.0.next_or_rc" our nightly could pick that up. Of course you’d need to do that for all artifacts. Tank you! -- 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] rmaucher commented on pull request #428: Enhancement: Additional user attributes queried by (some) realms
rmaucher commented on pull request #428: URL: https://github.com/apache/tomcat/pull/428#issuecomment-865007741 This looks good enough to put it in. I can probably think about enhancements late. The only thing that I'm really wondering about is why the UserDatabaseRealm did not get arbitrary attributes. -- 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. 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] cklein05 commented on pull request #428: Enhancement: Additional user attributes queried by (some) realms
cklein05 commented on pull request #428: URL: https://github.com/apache/tomcat/pull/428#issuecomment-865024538 > The only thing that I'm really wondering about is why the UserDatabaseRealm did not get arbitrary attributes. My first focus was on UserDatabaseRealm and JDNIRealm. Arbitrary user attributes for UserDatabaseRealm sounds interesting. However, I did not yet think about it in detail. AFAIK, there is a schema file for tomcat-users.xml. Some new Digester rules must be defined. Actually, the attributes come from the `User` instance. Likely this class should get an attributes map as well (which we then can provide through `UserDatabasePrincipal`). I'm almost finished with all the changes caused by that code review. Also, I've added support for SQL types ARRAY, BLOB, and CLOB. I will push my latest commits soon. Still open/pending: - Using *linked* hash maps (yes/no) - Defensive copies (how paranoid shall we go?) - Documentation (likely my job) The latter might be quite challenging. Needs additions in `config/realm.html` and likely in `realm-howto.html`. Since, at current, only `TomcatPrincipal` supports the attribute stuff, maybe a code example in one of the HTML sites will be required. Has anyone thought about putting these methods to a more _official_ interface `jakarta.servlet.Principal`? Since it is now _jakarta_, is Apache or Tomcat now responsible for the Servlet Standard? -- 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. 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: JSP regressions - do we need a new set of releases
On Mon, Jun 21, 2021 at 12:17 PM Mark Thomas wrote: > > All, > > Despite the extra tests I added to improve code coverage to almost 100% > for JSP code generation, the changes I made to remove unnecessary code > have triggered a couple of regressions. There is no easy way to avoid > these regressions. > > Given the above do we want to pull forward the July release round and > start the next set of releases soon / today / this week? > > There is a balance to strike between getting fixed releases out that > address these regressions and allowing enough time for any further > regressions to be identified. > > Thoughts? The rate at which they are reported is not too alarming so far. Maybe we can keep these as July releases but move them forward (the previous tags was on 06/08, with further delays for retags and cleanups, so maybe we could plan a tag on 07/01). Rémy > > Mark > > - > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] rmaucher commented on pull request #428: Enhancement: Additional user attributes queried by (some) realms
rmaucher commented on pull request #428: URL: https://github.com/apache/tomcat/pull/428#issuecomment-865049209 No, the Tomcat project is not responsible for Jakarta and has no control over it, so we cannot make that change. I'm perfectly fine as it is, you're not supposed to do everything by yourself and I'm interested in improving things a bit too if I can. -- 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. 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] cklein05 commented on pull request #428: Enhancement: Additional user attributes queried by (some) realms
cklein05 commented on pull request #428: URL: https://github.com/apache/tomcat/pull/428#issuecomment-865053410 Please, wait at least until I've pushed my latest changes. Also, JDBC stuff in DataSourceRealm needs some adjustments in order not to put arbitrary `java.sql` objects into the map. I'd really love to make ends meet for things that I've started... -- 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. 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: JSP regressions - do we need a new set of releases
On 21/06/2021 14:16, Rémy Maucherat wrote: On Mon, Jun 21, 2021 at 12:17 PM Mark Thomas wrote: All, Despite the extra tests I added to improve code coverage to almost 100% for JSP code generation, the changes I made to remove unnecessary code have triggered a couple of regressions. There is no easy way to avoid these regressions. Given the above do we want to pull forward the July release round and start the next set of releases soon / today / this week? There is a balance to strike between getting fixed releases out that address these regressions and allowing enough time for any further regressions to be identified. Thoughts? The rate at which they are reported is not too alarming so far. Maybe we can keep these as July releases but move them forward (the previous tags was on 06/08, with further delays for retags and cleanups, so maybe we could plan a tag on 07/01). Sounds reasonable. The tag only ever slips as things come in at the last minute so I'll start the process shortly of updating the translations, checking the dependencies etc and we can see where we are once everything is ready to tag. Mark - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
svn commit: r48450 - /release/tomcat/tomcat-10/v10.0.6/
Author: markt Date: Mon Jun 21 16:14:14 2021 New Revision: 48450 Log: Drop Apache Tomcat 10.0.6 from mirror system Removed: release/tomcat/tomcat-10/v10.0.6/ - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
svn commit: r48451 - /release/tomcat/tomcat-9/v9.0.46/
Author: markt Date: Mon Jun 21 16:14:49 2021 New Revision: 48451 Log: Drop Apache Tomcat 9.0.46 from mirror system Removed: release/tomcat/tomcat-9/v9.0.46/ - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch main updated (98d61c1 -> 2b92581)
This is an automated email from the ASF dual-hosted git repository. markt pushed a change to branch main in repository https://gitbox.apache.org/repos/asf/tomcat.git. from 98d61c1 Fix typos new fa24edd Improve French translations (remm) new 2b92581 Improvements to Korean translations. (woonsan) The 2 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference. Summary of changes: java/jakarta/servlet/http/LocalStrings_fr.properties | 1 + java/jakarta/servlet/http/LocalStrings_ko.properties | 1 + webapps/docs/changelog.xml | 6 ++ 3 files changed, 8 insertions(+) - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] 01/02: Improve French translations (remm)
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
commit fa24eddfc49cc2e19b4cd4606a4eb3b7d55fed98
Author: Mark Thomas
AuthorDate: Mon Jun 21 17:34:12 2021 +0100
Improve French translations (remm)
---
java/jakarta/servlet/http/LocalStrings_fr.properties | 1 +
webapps/docs/changelog.xml | 3 +++
2 files changed, 4 insertions(+)
diff --git a/java/jakarta/servlet/http/LocalStrings_fr.properties
b/java/jakarta/servlet/http/LocalStrings_fr.properties
index 5352f35..bdf7898 100644
--- a/java/jakarta/servlet/http/LocalStrings_fr.properties
+++ b/java/jakarta/servlet/http/LocalStrings_fr.properties
@@ -21,6 +21,7 @@ err.cookie_name_is_token=Le nom de cookie [{0}] est un
"token" réservé
err.io.indexOutOfBounds=L''offset [{0}] et/ou la longueur [{1}] spécifiés pour
la taille du tableau [{2}] sont invalides
err.io.nullArray=Null a été passée comme tableau d'octets à la méthode
d'écriture
err.io.short_read=Lecture partielle
+err.state.commit=Interdit une fois que la réponse a été commitée
http.method_delete_not_supported=La méthode HTTP DELETE n'est pas supportée
par cette URL
http.method_get_not_supported=La méthode HTTP GET n'est pas supportée par
cette URL
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 24ab417..f02d11d 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -217,6 +217,9 @@
16 onwards to the service.bat script to align it with the
other start-up scripts. PR provided by MCMicS. (markt)
+
+Improvements to French translations. (remm)
+
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] 02/02: Improvements to Korean translations. (woonsan)
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
commit 2b925814e6af699d4ea6f740d408310389fee03a
Author: Mark Thomas
AuthorDate: Mon Jun 21 17:37:50 2021 +0100
Improvements to Korean translations. (woonsan)
---
java/jakarta/servlet/http/LocalStrings_ko.properties | 1 +
webapps/docs/changelog.xml | 3 +++
2 files changed, 4 insertions(+)
diff --git a/java/jakarta/servlet/http/LocalStrings_ko.properties
b/java/jakarta/servlet/http/LocalStrings_ko.properties
index 13c501d..30cdc43 100644
--- a/java/jakarta/servlet/http/LocalStrings_ko.properties
+++ b/java/jakarta/servlet/http/LocalStrings_ko.properties
@@ -21,6 +21,7 @@ err.cookie_name_is_token=쿠키 이름 [{0}]은(는) 예약된 토큰입니다.
err.io.indexOutOfBounds=크기 [{2}]인 배열에 대하여, 유효하지 않은 offset [{0}] 그리고/또는 길이
[{1}].
err.io.nullArray=write 메소드에 널인 바이트 배열이 전달되었습니다.
err.io.short_read=Short Read
+err.state.commit=응답이 한번 커밋된 이후에는 허용되지 않습니다.
http.method_delete_not_supported=HTTP 메소드 DELETE는 이 URL에 의해 지원되지 않습니다.
http.method_get_not_supported=HTTP 메소드 GET은 이 URL에 의해 지원되지 않습니다.
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index f02d11d..d09a26b 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -220,6 +220,9 @@
Improvements to French translations. (remm)
+
+Improvements to Korean translations. (woonsan)
+
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch main updated (2b92581 -> e40d778)
This is an automated email from the ASF dual-hosted git repository. markt pushed a change to branch main in repository https://gitbox.apache.org/repos/asf/tomcat.git. from 2b92581 Improvements to Korean translations. (woonsan) add e40d778 Make fields accessible to sub-classes of JNDIRealm No new revisions were added by this update. Summary of changes: java/org/apache/catalina/realm/JNDIRealm.java | 10 +- webapps/docs/changelog.xml| 4 2 files changed, 9 insertions(+), 5 deletions(-) - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch 10.0.x updated: Make fields accessible to sub-classes of JNDIRealm
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 10.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/10.0.x by this push:
new b651945 Make fields accessible to sub-classes of JNDIRealm
b651945 is described below
commit b651945857d9a47523e336a83cb4024a65f6e518
Author: Mark Thomas
AuthorDate: Mon Jun 21 18:12:49 2021 +0100
Make fields accessible to sub-classes of JNDIRealm
Access problem for sub-classes reported via users@tomcat.a.o
https://tomcat.markmail.org/thread/l6exgocuy46v75ch
---
java/org/apache/catalina/realm/JNDIRealm.java | 10 +-
webapps/docs/changelog.xml| 4
2 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/java/org/apache/catalina/realm/JNDIRealm.java
b/java/org/apache/catalina/realm/JNDIRealm.java
index b6318b4..89564ea 100644
--- a/java/org/apache/catalina/realm/JNDIRealm.java
+++ b/java/org/apache/catalina/realm/JNDIRealm.java
@@ -3083,29 +3083,29 @@ public class JNDIRealm extends RealmBase {
* The MessageFormat object associated with the current
* userSearch.
*/
-protected MessageFormat userSearchFormat = null;
+public MessageFormat userSearchFormat = null;
/**
* An array of MessageFormat objects associated with the current
* userPatternArray.
*/
-protected MessageFormat[] userPatternFormatArray = null;
+public MessageFormat[] userPatternFormatArray = null;
/**
* The MessageFormat object associated with the current
* roleBase.
*/
-protected MessageFormat roleBaseFormat = null;
+public MessageFormat roleBaseFormat = null;
/**
* The MessageFormat object associated with the current
* roleSearch.
*/
-protected MessageFormat roleFormat = null;
+public MessageFormat roleFormat = null;
/**
* The directory context linking us to our directory server.
*/
-protected DirContext context = null;
+public DirContext context = null;
}
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index b3b1a15..02973d5 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -111,6 +111,10 @@
Refactor the RemoteIpValve to use the common utility
method
for list to comma separated string conversion. (markt)
+
+Refactor JNDIRealm$JNDIConnection so its fields are
+accessible to sub-classes of JNDIRealm. (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: Make fields accessible to sub-classes of JNDIRealm
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 86b9958 Make fields accessible to sub-classes of JNDIRealm
86b9958 is described below
commit 86b9958b191a7cb924a54c86205a5a247174f1be
Author: Mark Thomas
AuthorDate: Mon Jun 21 18:12:49 2021 +0100
Make fields accessible to sub-classes of JNDIRealm
Access problem for sub-classes reported via users@tomcat.a.o
https://tomcat.markmail.org/thread/l6exgocuy46v75ch
---
java/org/apache/catalina/realm/JNDIRealm.java | 10 +-
webapps/docs/changelog.xml| 4
2 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/java/org/apache/catalina/realm/JNDIRealm.java
b/java/org/apache/catalina/realm/JNDIRealm.java
index 315d9e6..7f72ae3 100644
--- a/java/org/apache/catalina/realm/JNDIRealm.java
+++ b/java/org/apache/catalina/realm/JNDIRealm.java
@@ -3084,29 +3084,29 @@ public class JNDIRealm extends RealmBase {
* The MessageFormat object associated with the current
* userSearch.
*/
-protected MessageFormat userSearchFormat = null;
+public MessageFormat userSearchFormat = null;
/**
* An array of MessageFormat objects associated with the current
* userPatternArray.
*/
-protected MessageFormat[] userPatternFormatArray = null;
+public MessageFormat[] userPatternFormatArray = null;
/**
* The MessageFormat object associated with the current
* roleBase.
*/
-protected MessageFormat roleBaseFormat = null;
+public MessageFormat roleBaseFormat = null;
/**
* The MessageFormat object associated with the current
* roleSearch.
*/
-protected MessageFormat roleFormat = null;
+public MessageFormat roleFormat = null;
/**
* The directory context linking us to our directory server.
*/
-protected DirContext context = null;
+public DirContext context = null;
}
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 598969e..72db4ee 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -111,6 +111,10 @@
Refactor the RemoteIpValve to use the common utility
method
for list to comma separated string conversion. (markt)
+
+Refactor JNDIRealm$JNDIConnection so its fields are
+accessible to sub-classes of JNDIRealm. (markt)
+
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
