Re: Add support for additional user attributes in TomcatPrincipal
On Fri, Feb 11, 2022 at 8:10 AM Carsten Klein wrote: > > Hello Rémy, Mark and Michael, > > I'm new to the dev list and did not get your recent mails and didn't > figure out how to get them in order to answer. So, I decided to start a > new thread (sorry for that). > > I guess, having those attributes with the Principal (aka Principal > metadata) does not make the Principal a data storage. For my mind, even > the Principal's getName() returns kind of metadata, since an application > typically does not really need the logged on user's name in order to > function properly. Much more important are the user's roles, for example. > > So, the new read-only(!) attributes map is just a way of dynamically > adding more of such metadata fields. Another way would be to add a > getUserDisplayName() and a new Realm config attribute > 'userDisplayNameCol' (e.g. for the DataSourceRealm), and maybe a > getUserMailAddress() method later. However, that's not flexible at all > and many requests from people to extend this scheme may be the result. If we get there, it could include mail addresses, ssn, payment info, user profile pictures (binary), etc etc. Also one thing I don't get now is how this became Object getAttribute(String name) instead of String getAttribute(String name) ? The legitimate examples you gave are text, not binary objects. Ultimately, all of this is still application data ... Moving it in the Tomcat realm creates a proprietary behavior, the application is no longer portable while at the same time the benefit is minimal (saving a query ?). When logging in, the app should pull all the metadata it needs, store it in the session. > So, the dynamic attributes map is the better choice, right? As long as > it remains read-only, also no one can abuse the Principal as data > storage. Also, there is really no need to ever request that, since an > application already has a fully featured data storage around: the > Session's attributes list. It is primarily intended for exactly that: > storing the application's data. So, you could always deny any upcoming > PRs adding write support to the Principal's attributes by referring to > the Session's attributes map. > > Providing such metadata through the Principal is new and was not done > before. However, since, more or less, it's just an extension to the > already available getName() method, I guess, it's a quite good idea. > > In the Javadoc, of course, we could emphasize more, that this attributes > map is intentionally read-only and will never be writable. > > Michael-o and I agreed on having individual PRs for the three parts of > the initial enhancement (TomcatPrincipal/GenericPrincipal, > DataSourceRealm, JNDIRealm). So, I will provide a third PR for the > JNDIRealm after the DataSourceRealm has been merged. > > @Rémy: That was the deal/agreement. We do not touch UserDatabaseRealm > and you do not vote against DataSourceRealm and JNDIRealm. Yes, well, unfortunately, due to more background thinking ... The purpose of the UserDatabase is to be able to write, so given the API it is an object database at this point. > @Remy: Maybe you would feel better about this, if we use a completely > different approach? > > We could use the Realm for technically querying those attributes, but > provide them to the application through the Session's attributes map? > > We could either put each single attribute directly into the Session's > attributes using a prefixed name like "userAttribute.user_display_name", > or we could add a UserAttributesStore instance (would be a new Tomcat > specific class) under a "userAttributes" name, which has > getAttribute(String name) and getAttributeNames() methods. > > However, I guess, implementing this approach is a bit more complicated > than the current one. Ok, but ... Your actual use case is the DataSourceRealm, which uses a DataSource. That DataSource is a JNDI resource which is also available to the application. Getting a connection from the pool is not expensive at all, and running an extra query from a prepared statement is just that. If more state is needed (I believe that will always be the case), then the difference becomes minimal. Also, the whole data layout is in the hands of the developer, who then chooses to abuse the realm backend. So overall in that case, all that you mention is still best done in the application, replacing the API with something different (like storing in the session) does not change that and this is simply about moving a small piece of code from the application to the container. Although I heavily changed my mind on the rest, JNDI/LDAP always looked to me like the legitimate use case. There's indeed metadata in there. It could be more difficult to get to it from the app. Maybe it's less scalable than a DB, and there's no shared connection pool with the app. So it's always going to be significantly less efficient to get them from the application. Ok that there's an agreement on javadoc clarifications (wh
Re: Re: [tomcat] branch main updated: Add support for additional user, attributes in TomcatPrincipal
Rémy, really, I don't understand your work attitude. Carsten contacted us on the mailing list before providing the PR. Then he provided the PR. It was open for EIGHT months. We discussed this in and out, with you and Mark. Everyone expressed the conditions he is willing to accept the PR. I have even asked Carsten to break up the PR to please you. The purpose of RTC/PR is to express your opinion BEFORE it gets merged to avoid post-mortem discussions like this. You expressed your conditions and now you object? This is absolutely not fair and unprofessional. I cannot share your further explanations as well. The Realm *is* already an object store. Principal name and roles are attributes on that object, so is password. Without this possibility you have the following problems: * You cannot abstract from the persistence layer in your application code, at worst you need to double code for retrieval * You double access time since you need to query the principal store again. A performance penalty in a stateless application, e.g. APIs * You maybe even not have access to the realm. Consider the identity provider gives you some form of token containing the attributes which you CANNOT retreive yourself. How to solve? You can't. No one is aiming here for an ORM layer. Mark, you have even assisted on how to backport to previous versions and object somewhat as well? I don't understand that. From my PoV Carsten did everything we requested over a very long period of time. M
Re: Add support for additional user attributes in TomcatPrincipal
On Fri, Feb 11, 2022 at 8:10 AM Carsten Klein wrote: Hello R=C3=A9my, Mark and Michael, I'm new to the dev list and did not get your recent mails and didn't figure out how to get them in order to answer. So, I decided to start a new thread (sorry for that). I guess, having those attributes with the Principal (aka Principal metadata) does not make the Principal a data storage. For my mind, even the Principal's getName() returns kind of metadata, since an application typically does not really need the logged on user's name in order to function properly. Much more important are the user's roles, for example. So, the new read-only(!) attributes map is just a way of dynamically adding more of such metadata fields. Another way would be to add a getUserDisplayName() and a new Realm config attribute 'userDisplayNameCol' (e.g. for the DataSourceRealm), and maybe a getUserMailAddress() method later. However, that's not flexible at all and many requests from people to extend this scheme may be the result. If we get there, it could include mail addresses, ssn, payment info, user profile pictures (binary), etc etc. Also one thing I don't get now is how this became Object getAttribute(String name) instead of String getAttribute(String name) ? The legitimate examples you gave are text, not binary objects. It must be Object and nothing else. You cannot assume it just to be string. Binary attributes, e.g., certificates for encryption. Multivalued attributes, e.g. List. Ultimately, all of this is still application data ... Moving it in the Tomcat realm creates a proprietary behavior, the application is no longer portable while at the same time the benefit is minimal (saving a query ?). When logging in, the app should pull all the metadata it needs, store it in the session. No, this is user data, the source is opaque to the application and that is good, you can exchange for testing purposes. Never assume that you have sessions. Minimal? How do you know that the retrieval time is minimal? Why are you making these assumptions? REST APIs are stateless, you will pay for every additional query. Yes, well, unfortunately, due to more background thinking ... The purpose of the UserDatabase is to be able to write, so given the API it is an object database at this point. This is a UserDatabase problem, not a Realm one. You can easily modify it to be read only at config time if the code allows it. Maybe that should even be the default. Least priviledge principal. @Remy: Maybe you would feel better about this, if we use a completely different approach? We could use the Realm for technically querying those attributes, but provide them to the application through the Session's attributes map? We could either put each single attribute directly into the Session's attributes using a prefixed name like "userAttribute.user_display_name", or we could add a UserAttributesStore instance (would be a new Tomcat specific class) under a "userAttributes" name, which has getAttribute(String name) and getAttributeNames() methods. However, I guess, implementing this approach is a bit more complicated than the current one. Ok, but ... Your actual use case is the DataSourceRealm, which uses a DataSource. That DataSource is a JNDI resource which is also available to the application. Getting a connection from the pool is not expensive at all, and running an extra query from a prepared statement is just that. If more state is needed (I believe that will always be the case), then the difference becomes minimal. Also, the whole data layout is in the hands of the developer, who then chooses to abuse the realm backend. So overall in that case, all that you mention is still best done in the application, replacing the API with something different (like storing in the session) does not change that and this is simply about moving a small piece of code from the application to the container. Again, you are making wrong assumptions from your personal usecases. Although I heavily changed my mind on the rest, JNDI/LDAP always looked to me like the legitimate use case. There's indeed metadata in there. It could be more difficult to get to it from the app. Maybe it's less scalable than a DB, and there's no shared connection pool with the app. So it's always going to be significantly less efficient to get them from the application. ...and here you finally understand it is not just a pool to get a connection. We don't something like a PoolingContextSource like Spring has. Even if, queries still require time. Believe me, I work with it in the largest AD setup on the planet, likely. M - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Re: [tomcat] branch main updated: Add support for additional user, attributes in TomcatPrincipal
On Fri, Feb 11, 2022 at 10:47 AM Michael Osipov wrote: > > Rémy, > > really, I don't understand your work attitude. Carsten contacted us on > the mailing list before providing the PR. Then he provided the PR. It > was open for EIGHT months. We discussed this in and out, with you and > Mark. Everyone expressed the conditions he is willing to accept the PR. > I have even asked Carsten to break up the PR to please you. The purpose > of RTC/PR is to express your opinion BEFORE it gets merged to avoid > post-mortem discussions like this. You expressed your conditions and now > you object? This is absolutely not fair and unprofessional. I apologized for this already. I am really sorry about this but background thinking led to different conclusions over time. > I cannot share your further explanations as well. The Realm *is* already > an object store. Principal name and roles are attributes on that object, > so is password. Without this possibility you have the following problems: That statement is just weird. The realm stores credentials associated with a principal name, as well as a list of roles (other names) associated with that name. That's it. An object store can store objects (like if there is a get/setAttribute that deals with Object - and that's my problem now, as it turns out ...). > * You cannot abstract from the persistence layer in your application > code, at worst you need to double code for retrieval > * You double access time since you need to query the principal store > again. A performance penalty in a stateless application, e.g. APIs > * You maybe even not have access to the realm. Consider the identity > provider gives you some form of token containing the attributes which > you CANNOT retreive yourself. How to solve? You can't. I don't believe the penalty exists for a DataSource backend. Most likely for LDAP. Either way, the application is now non portable. Given that the benefit for DataSource does not seem large, this simply sounds bad in that case. For LDAP, it sounds like a tradeoff. > No one is aiming here for an ORM layer. One of my points is that this is really far from obvious in the API, so it needs a round of javadoc tightening up. Rémy > > > Mark, > > you have even assisted on how to backport to previous versions and > object somewhat as well? I don't understand that. > > > From my PoV Carsten did everything we requested over a very long period > of time. > > M - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Add support for additional user attributes in TomcatPrincipal
On Fri, Feb 11, 2022 at 10:10 AM Rémy Maucherat wrote If we get there, it could include mail addresses, ssn, payment info, user profile pictures (binary), etc etc. Also one thing I don't get now is how this became Object getAttribute(String name) instead of String getAttribute(String name) ? The legitimate examples you gave are text, not binary objects. Ultimately, all of this is still application data ... Moving it in the Tomcat realm creates a proprietary behavior, the application is no longer portable while at the same time the benefit is minimal (saving a query ?). When logging in, the app should pull all the metadata it needs, store it in the session. Yes, it could be that much metadata, so what? And yes, it's all application data. But that must not be something negative. You may say, this is up to the application but, isn't it convenient if the realm does it for you? Tomcat's Request (e.g. HTTPServletRequest) returns all data POSTed from a client; all that data may be application specific data as well. Nobody cares... Again, it's not about saving one query while logging in a user. It's primarily about 1. not needing to configure an extra connection to the user database 2. not needing much custom code an knowledge/skills of the user database (SQL, LDAP etc.) 3. getting extra attributes from Tomcat "for free" with 'any' Realm simply by configuring a list of fields uniformly As you are correctly saying, point 1. is not that complicated for DataSourceRealm, as there is a JDBC pool around. Nevertheless, the application still needs to know the name of the connection (e. g. "jdbc/userdatabase") and the name of the user table, the column containing the logon name (what Principal.getName() returns) and the extra columns to query. With my solution, all that configuration is "hidden" in the Realm's configuration. The only new thing is the "userAttributes" config attribute. No duplicated configuration is required. What do you mean with 'proprietary behavior'? Why shouldn't the application be no longer portable? The Realm's configuration always contains any access data (JNDI resource names, URLs and passwords in case of JNDIRealm), so it wasn't ever portable at all. If you think about moving the application between different servers on the same site (targeting the same user database), the new "userAttributes" should work from any Tomcat server you use. Moving between different user databases is also much simpler, since the whole configuration is centralized at the Realm. Why do I store Object and not String attributes? Because I can... When querying a JDBC database or a LDAP server dynamically, you must expect various different types being returned. We could add a rule, that all must be Strings or we call the toString() method on each value. But why? Yes, well, unfortunately, due to more background thinking ... The purpose of the UserDatabase is to be able to write, so given the API it is an object database at this point. In my recent implementation, the AbstractUser got a /** * Additional attributes of this user. */ protected Map attributes = new HashMap<>(); so, the UserDatabase does not store Object typed attributes, but only Strings (since XML attributes are strings only). So, I don't understand why you consider it an object database. Ok, but ... Your actual use case is the DataSourceRealm, which uses a DataSource. That DataSource is a JNDI resource which is also available to the application. Getting a connection from the pool is not expensive at all, and running an extra query from a prepared statement is just that. If more state is needed (I believe that will always be the case), then the difference becomes minimal. Also, the whole data layout is in the hands of the developer, who then chooses to abuse the realm backend. So overall in that case, all that you mention is still best done in the application, replacing the API with something different (like storing in the session) does not change that and this is simply about moving a small piece of code from the application to the container. Yes, with JDBC and a DataSource it's quite simple to do that in the application. However, this ends in much custom code required to run after login. Actually this makes the application not portable (or at least hard to port). Believe me, I know what I'm saying. I'm running a software company with < 20 customers, each having different user databases and needs. Having tailored code for each of them is something you could call `JAR-hell` (you know Windows DLL-hell?). Although I heavily changed my mind on the rest, JNDI/LDAP always looked to me like the legitimate use case. There's indeed metadata in there. It could be more difficult to get to it from the app. Maybe it's less scalable than a DB, and there's no shared connection pool with the app. So it's always going to be significantly less efficient to get them from the application. Yes, LDAP i
[tomcat] 01/02: Revert "Cherry-pick DBCP2 Javadoc fix"
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 c1c4ee5d67980b2f8de28bb42932faf8d4ebb1c5
Author: Mark Thomas
AuthorDate: Fri Feb 11 10:10:27 2022 +
Revert "Cherry-pick DBCP2 Javadoc fix"
This reverts commit 0a9afdfa6a481b29a954a67cf6ea352043201ed0.
---
.../apache/tomcat/dbcp/dbcp2/BasicDataSource.java | 7 --
.../apache/tomcat/dbcp/dbcp2/DataSourceMXBean.java | 77 --
webapps/docs/changelog.xml | 5 ++
3 files changed, 5 insertions(+), 84 deletions(-)
diff --git a/java/org/apache/tomcat/dbcp/dbcp2/BasicDataSource.java
b/java/org/apache/tomcat/dbcp/dbcp2/BasicDataSource.java
index b0562e8..f260a53 100644
--- a/java/org/apache/tomcat/dbcp/dbcp2/BasicDataSource.java
+++ b/java/org/apache/tomcat/dbcp/dbcp2/BasicDataSource.java
@@ -1063,7 +1063,6 @@ public class BasicDataSource implements DataSource,
BasicDataSourceMXBean, MBean
* @return the maximum permitted duration of a connection.
* @since 2.10.0
*/
-@Override
public Duration getMaxConnDuration() {
return maxConnDuration;
}
@@ -1123,7 +1122,6 @@ public class BasicDataSource implements DataSource,
BasicDataSourceMXBean, MBean
* @return the maxWaitDuration property value.
* @since 2.10.0
*/
-@Override
public synchronized Duration getMaxWaitDuration() {
return this.maxWaitDuration;
}
@@ -1148,7 +1146,6 @@ public class BasicDataSource implements DataSource,
BasicDataSourceMXBean, MBean
* @see #setMinEvictableIdle(Duration)
* @since 2.10.0
*/
-@Override
public synchronized Duration getMinEvictableIdleDuration() {
return this.minEvictableIdleDuration;
}
@@ -1326,7 +1323,6 @@ public class BasicDataSource implements DataSource,
BasicDataSourceMXBean, MBean
* @return Timeout before an abandoned connection can be removed.
* @since 2.10.0
*/
-@Override
public Duration getRemoveAbandonedTimeoutDuration() {
return abandonedConfig == null ? Duration.ofSeconds(300) :
abandonedConfig.getRemoveAbandonedTimeoutDuration();
}
@@ -1356,7 +1352,6 @@ public class BasicDataSource implements DataSource,
BasicDataSourceMXBean, MBean
* there are minIdle idle connections in the pool
* @since 2.10.0
*/
-@Override
public synchronized Duration getSoftMinEvictableIdleDuration() {
return softMinEvictableIdleDuration;
}
@@ -1433,7 +1428,6 @@ public class BasicDataSource implements DataSource,
BasicDataSourceMXBean, MBean
* @see #setDurationBetweenEvictionRuns(Duration)
* @since 2.10.0
*/
-@Override
public synchronized Duration getDurationBetweenEvictionRuns() {
return this.durationBetweenEvictionRuns;
}
@@ -1487,7 +1481,6 @@ public class BasicDataSource implements DataSource,
BasicDataSourceMXBean, MBean
*
* @return the timeout in seconds before connection validation queries
fail.
*/
-@Override
public Duration getValidationQueryTimeoutDuration() {
return validationQueryTimeoutDuration;
}
diff --git a/java/org/apache/tomcat/dbcp/dbcp2/DataSourceMXBean.java
b/java/org/apache/tomcat/dbcp/dbcp2/DataSourceMXBean.java
index a6d522b..7315c58 100644
--- a/java/org/apache/tomcat/dbcp/dbcp2/DataSourceMXBean.java
+++ b/java/org/apache/tomcat/dbcp/dbcp2/DataSourceMXBean.java
@@ -17,7 +17,6 @@
package org.apache.tomcat.dbcp.dbcp2;
import java.sql.SQLException;
-import java.time.Duration;
/**
* Defines the methods that will be made available via
@@ -139,21 +138,10 @@ public interface DataSourceMXBean {
boolean getLogExpiredConnections();
/**
- * See {@link BasicDataSource#getMaxConnDuration()}.
- *
- * @return {@link BasicDataSource#getMaxConnDuration()}.
- * @since 2.10.0
- */
-Duration getMaxConnDuration();
-
-/**
* See {@link BasicDataSource#getMaxConnLifetimeMillis()}.
*
* @return {@link BasicDataSource#getMaxConnLifetimeMillis()}.
- *
- * @deprecated Use {@link #getMaxConnDuration()}.
*/
-@Deprecated
long getMaxConnLifetimeMillis();
/**
@@ -178,39 +166,17 @@ public interface DataSourceMXBean {
int getMaxTotal();
/**
- * See {@link BasicDataSource#getMaxWaitDuration()}.
- *
- * @return {@link BasicDataSource#getMaxWaitDuration()}.
- * @since 2.10.0
- */
-Duration getMaxWaitDuration();
-
-/**
* See {@link BasicDataSource#getMaxWaitMillis()}.
*
* @return {@link BasicDataSource#getMaxWaitMillis()}.
- *
- * @deprecated Use {@link #getMaxWaitDuration()}.
*/
-@Deprecated
long getMaxWaitMillis();
/**
- * See {@link BasicDataSource#getMinEvictableIdleDuration()}.
- *
- * @return {@link BasicDataSource#getMinEvictableIdleDu
[tomcat] branch main updated (164da63 -> 71acd04)
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 164da63 Add workaround to allow running testsuite on Java 8 new c1c4ee5 Revert "Cherry-pick DBCP2 Javadoc fix" new 71acd04 Suppress Javadoc warnings 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: .../apache/tomcat/dbcp/dbcp2/BasicDataSource.java | 7 -- .../apache/tomcat/dbcp/dbcp2/DataSourceMXBean.java | 84 ++ webapps/docs/changelog.xml | 5 ++ 3 files changed, 12 insertions(+), 84 deletions(-) - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] 02/02: Suppress Javadoc warnings
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 71acd043f33f4f12fd5fd40d66beb9659bc9b682
Author: Mark Thomas
AuthorDate: Fri Feb 11 10:15:31 2022 +
Suppress Javadoc warnings
The MXBean has to use the deprecated methods as the replacement methods
use Duration which isn't a valid type for use in an MXBean
---
java/org/apache/tomcat/dbcp/dbcp2/DataSourceMXBean.java | 7 +++
1 file changed, 7 insertions(+)
diff --git a/java/org/apache/tomcat/dbcp/dbcp2/DataSourceMXBean.java
b/java/org/apache/tomcat/dbcp/dbcp2/DataSourceMXBean.java
index 7315c58..f6dbef1 100644
--- a/java/org/apache/tomcat/dbcp/dbcp2/DataSourceMXBean.java
+++ b/java/org/apache/tomcat/dbcp/dbcp2/DataSourceMXBean.java
@@ -142,6 +142,7 @@ public interface DataSourceMXBean {
*
* @return {@link BasicDataSource#getMaxConnLifetimeMillis()}.
*/
+@SuppressWarnings("javadoc")
long getMaxConnLifetimeMillis();
/**
@@ -170,6 +171,7 @@ public interface DataSourceMXBean {
*
* @return {@link BasicDataSource#getMaxWaitMillis()}.
*/
+@SuppressWarnings("javadoc")
long getMaxWaitMillis();
/**
@@ -177,6 +179,7 @@ public interface DataSourceMXBean {
*
* @return {@link BasicDataSource#getMinEvictableIdleTimeMillis()}.
*/
+@SuppressWarnings("javadoc")
long getMinEvictableIdleTimeMillis();
/**
@@ -226,6 +229,7 @@ public interface DataSourceMXBean {
*
* @return {@link BasicDataSource#getRemoveAbandonedTimeout()}.
*/
+@SuppressWarnings("javadoc")
int getRemoveAbandonedTimeout();
/**
@@ -233,6 +237,7 @@ public interface DataSourceMXBean {
*
* @return {@link BasicDataSource#getSoftMinEvictableIdleTimeMillis()}.
*/
+@SuppressWarnings("javadoc")
long getSoftMinEvictableIdleTimeMillis();
/**
@@ -261,6 +266,7 @@ public interface DataSourceMXBean {
*
* @return {@link BasicDataSource#getTimeBetweenEvictionRunsMillis()}.
*/
+@SuppressWarnings("javadoc")
long getTimeBetweenEvictionRunsMillis();
/**
@@ -289,6 +295,7 @@ public interface DataSourceMXBean {
*
* @return {@link BasicDataSource#getValidationQueryTimeout()}.
*/
+@SuppressWarnings("javadoc")
int getValidationQueryTimeout();
/**
-
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 (c7b3502 -> 7ca1b25)
This is an automated email from the ASF dual-hosted git repository. markt pushed a change to branch 10.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git. from c7b3502 Remove accidental add new 559a3b0 Revert "Cherry-pick DBCP2 Javadoc fix" new 7ca1b25 Suppress Javadoc warnings 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: .../apache/tomcat/dbcp/dbcp2/BasicDataSource.java | 7 -- .../apache/tomcat/dbcp/dbcp2/DataSourceMXBean.java | 84 ++ webapps/docs/changelog.xml | 5 ++ 3 files changed, 12 insertions(+), 84 deletions(-) - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] 01/02: Revert "Cherry-pick DBCP2 Javadoc fix"
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
commit 559a3b0d7e4d7de741691359fab546e03c710cd0
Author: Mark Thomas
AuthorDate: Fri Feb 11 10:10:27 2022 +
Revert "Cherry-pick DBCP2 Javadoc fix"
This reverts commit 0a9afdfa6a481b29a954a67cf6ea352043201ed0.
---
.../apache/tomcat/dbcp/dbcp2/BasicDataSource.java | 7 --
.../apache/tomcat/dbcp/dbcp2/DataSourceMXBean.java | 77 --
webapps/docs/changelog.xml | 5 ++
3 files changed, 5 insertions(+), 84 deletions(-)
diff --git a/java/org/apache/tomcat/dbcp/dbcp2/BasicDataSource.java
b/java/org/apache/tomcat/dbcp/dbcp2/BasicDataSource.java
index b0562e8..f260a53 100644
--- a/java/org/apache/tomcat/dbcp/dbcp2/BasicDataSource.java
+++ b/java/org/apache/tomcat/dbcp/dbcp2/BasicDataSource.java
@@ -1063,7 +1063,6 @@ public class BasicDataSource implements DataSource,
BasicDataSourceMXBean, MBean
* @return the maximum permitted duration of a connection.
* @since 2.10.0
*/
-@Override
public Duration getMaxConnDuration() {
return maxConnDuration;
}
@@ -1123,7 +1122,6 @@ public class BasicDataSource implements DataSource,
BasicDataSourceMXBean, MBean
* @return the maxWaitDuration property value.
* @since 2.10.0
*/
-@Override
public synchronized Duration getMaxWaitDuration() {
return this.maxWaitDuration;
}
@@ -1148,7 +1146,6 @@ public class BasicDataSource implements DataSource,
BasicDataSourceMXBean, MBean
* @see #setMinEvictableIdle(Duration)
* @since 2.10.0
*/
-@Override
public synchronized Duration getMinEvictableIdleDuration() {
return this.minEvictableIdleDuration;
}
@@ -1326,7 +1323,6 @@ public class BasicDataSource implements DataSource,
BasicDataSourceMXBean, MBean
* @return Timeout before an abandoned connection can be removed.
* @since 2.10.0
*/
-@Override
public Duration getRemoveAbandonedTimeoutDuration() {
return abandonedConfig == null ? Duration.ofSeconds(300) :
abandonedConfig.getRemoveAbandonedTimeoutDuration();
}
@@ -1356,7 +1352,6 @@ public class BasicDataSource implements DataSource,
BasicDataSourceMXBean, MBean
* there are minIdle idle connections in the pool
* @since 2.10.0
*/
-@Override
public synchronized Duration getSoftMinEvictableIdleDuration() {
return softMinEvictableIdleDuration;
}
@@ -1433,7 +1428,6 @@ public class BasicDataSource implements DataSource,
BasicDataSourceMXBean, MBean
* @see #setDurationBetweenEvictionRuns(Duration)
* @since 2.10.0
*/
-@Override
public synchronized Duration getDurationBetweenEvictionRuns() {
return this.durationBetweenEvictionRuns;
}
@@ -1487,7 +1481,6 @@ public class BasicDataSource implements DataSource,
BasicDataSourceMXBean, MBean
*
* @return the timeout in seconds before connection validation queries
fail.
*/
-@Override
public Duration getValidationQueryTimeoutDuration() {
return validationQueryTimeoutDuration;
}
diff --git a/java/org/apache/tomcat/dbcp/dbcp2/DataSourceMXBean.java
b/java/org/apache/tomcat/dbcp/dbcp2/DataSourceMXBean.java
index a6d522b..7315c58 100644
--- a/java/org/apache/tomcat/dbcp/dbcp2/DataSourceMXBean.java
+++ b/java/org/apache/tomcat/dbcp/dbcp2/DataSourceMXBean.java
@@ -17,7 +17,6 @@
package org.apache.tomcat.dbcp.dbcp2;
import java.sql.SQLException;
-import java.time.Duration;
/**
* Defines the methods that will be made available via
@@ -139,21 +138,10 @@ public interface DataSourceMXBean {
boolean getLogExpiredConnections();
/**
- * See {@link BasicDataSource#getMaxConnDuration()}.
- *
- * @return {@link BasicDataSource#getMaxConnDuration()}.
- * @since 2.10.0
- */
-Duration getMaxConnDuration();
-
-/**
* See {@link BasicDataSource#getMaxConnLifetimeMillis()}.
*
* @return {@link BasicDataSource#getMaxConnLifetimeMillis()}.
- *
- * @deprecated Use {@link #getMaxConnDuration()}.
*/
-@Deprecated
long getMaxConnLifetimeMillis();
/**
@@ -178,39 +166,17 @@ public interface DataSourceMXBean {
int getMaxTotal();
/**
- * See {@link BasicDataSource#getMaxWaitDuration()}.
- *
- * @return {@link BasicDataSource#getMaxWaitDuration()}.
- * @since 2.10.0
- */
-Duration getMaxWaitDuration();
-
-/**
* See {@link BasicDataSource#getMaxWaitMillis()}.
*
* @return {@link BasicDataSource#getMaxWaitMillis()}.
- *
- * @deprecated Use {@link #getMaxWaitDuration()}.
*/
-@Deprecated
long getMaxWaitMillis();
/**
- * See {@link BasicDataSource#getMinEvictableIdleDuration()}.
- *
- * @return {@link BasicDataSource#getMinEvictableIdle
[tomcat] 02/02: Suppress Javadoc warnings
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
commit 7ca1b250136d0fdae15965f09538fde96859568b
Author: Mark Thomas
AuthorDate: Fri Feb 11 10:15:31 2022 +
Suppress Javadoc warnings
The MXBean has to use the deprecated methods as the replacement methods
use Duration which isn't a valid type for use in an MXBean
---
java/org/apache/tomcat/dbcp/dbcp2/DataSourceMXBean.java | 7 +++
1 file changed, 7 insertions(+)
diff --git a/java/org/apache/tomcat/dbcp/dbcp2/DataSourceMXBean.java
b/java/org/apache/tomcat/dbcp/dbcp2/DataSourceMXBean.java
index 7315c58..f6dbef1 100644
--- a/java/org/apache/tomcat/dbcp/dbcp2/DataSourceMXBean.java
+++ b/java/org/apache/tomcat/dbcp/dbcp2/DataSourceMXBean.java
@@ -142,6 +142,7 @@ public interface DataSourceMXBean {
*
* @return {@link BasicDataSource#getMaxConnLifetimeMillis()}.
*/
+@SuppressWarnings("javadoc")
long getMaxConnLifetimeMillis();
/**
@@ -170,6 +171,7 @@ public interface DataSourceMXBean {
*
* @return {@link BasicDataSource#getMaxWaitMillis()}.
*/
+@SuppressWarnings("javadoc")
long getMaxWaitMillis();
/**
@@ -177,6 +179,7 @@ public interface DataSourceMXBean {
*
* @return {@link BasicDataSource#getMinEvictableIdleTimeMillis()}.
*/
+@SuppressWarnings("javadoc")
long getMinEvictableIdleTimeMillis();
/**
@@ -226,6 +229,7 @@ public interface DataSourceMXBean {
*
* @return {@link BasicDataSource#getRemoveAbandonedTimeout()}.
*/
+@SuppressWarnings("javadoc")
int getRemoveAbandonedTimeout();
/**
@@ -233,6 +237,7 @@ public interface DataSourceMXBean {
*
* @return {@link BasicDataSource#getSoftMinEvictableIdleTimeMillis()}.
*/
+@SuppressWarnings("javadoc")
long getSoftMinEvictableIdleTimeMillis();
/**
@@ -261,6 +266,7 @@ public interface DataSourceMXBean {
*
* @return {@link BasicDataSource#getTimeBetweenEvictionRunsMillis()}.
*/
+@SuppressWarnings("javadoc")
long getTimeBetweenEvictionRunsMillis();
/**
@@ -289,6 +295,7 @@ public interface DataSourceMXBean {
*
* @return {@link BasicDataSource#getValidationQueryTimeout()}.
*/
+@SuppressWarnings("javadoc")
int getValidationQueryTimeout();
/**
-
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 (dcd881e -> a40a65e)
This is an automated email from the ASF dual-hosted git repository. markt pushed a change to branch 9.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git. from dcd881e Remove accidental add new 8420e18 Revert "Cherry-pick DBCP2 Javadoc fix" new a40a65e Suppress Javadoc warnings 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: .../apache/tomcat/dbcp/dbcp2/BasicDataSource.java | 7 -- .../apache/tomcat/dbcp/dbcp2/DataSourceMXBean.java | 84 ++ webapps/docs/changelog.xml | 5 ++ 3 files changed, 12 insertions(+), 84 deletions(-) - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] 02/02: Suppress Javadoc warnings
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
commit a40a65e02cc76a9dd2e30a7e30cd3f313d6b8481
Author: Mark Thomas
AuthorDate: Fri Feb 11 10:15:31 2022 +
Suppress Javadoc warnings
The MXBean has to use the deprecated methods as the replacement methods
use Duration which isn't a valid type for use in an MXBean
---
java/org/apache/tomcat/dbcp/dbcp2/DataSourceMXBean.java | 7 +++
1 file changed, 7 insertions(+)
diff --git a/java/org/apache/tomcat/dbcp/dbcp2/DataSourceMXBean.java
b/java/org/apache/tomcat/dbcp/dbcp2/DataSourceMXBean.java
index 7315c58..f6dbef1 100644
--- a/java/org/apache/tomcat/dbcp/dbcp2/DataSourceMXBean.java
+++ b/java/org/apache/tomcat/dbcp/dbcp2/DataSourceMXBean.java
@@ -142,6 +142,7 @@ public interface DataSourceMXBean {
*
* @return {@link BasicDataSource#getMaxConnLifetimeMillis()}.
*/
+@SuppressWarnings("javadoc")
long getMaxConnLifetimeMillis();
/**
@@ -170,6 +171,7 @@ public interface DataSourceMXBean {
*
* @return {@link BasicDataSource#getMaxWaitMillis()}.
*/
+@SuppressWarnings("javadoc")
long getMaxWaitMillis();
/**
@@ -177,6 +179,7 @@ public interface DataSourceMXBean {
*
* @return {@link BasicDataSource#getMinEvictableIdleTimeMillis()}.
*/
+@SuppressWarnings("javadoc")
long getMinEvictableIdleTimeMillis();
/**
@@ -226,6 +229,7 @@ public interface DataSourceMXBean {
*
* @return {@link BasicDataSource#getRemoveAbandonedTimeout()}.
*/
+@SuppressWarnings("javadoc")
int getRemoveAbandonedTimeout();
/**
@@ -233,6 +237,7 @@ public interface DataSourceMXBean {
*
* @return {@link BasicDataSource#getSoftMinEvictableIdleTimeMillis()}.
*/
+@SuppressWarnings("javadoc")
long getSoftMinEvictableIdleTimeMillis();
/**
@@ -261,6 +266,7 @@ public interface DataSourceMXBean {
*
* @return {@link BasicDataSource#getTimeBetweenEvictionRunsMillis()}.
*/
+@SuppressWarnings("javadoc")
long getTimeBetweenEvictionRunsMillis();
/**
@@ -289,6 +295,7 @@ public interface DataSourceMXBean {
*
* @return {@link BasicDataSource#getValidationQueryTimeout()}.
*/
+@SuppressWarnings("javadoc")
int getValidationQueryTimeout();
/**
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] 01/02: Revert "Cherry-pick DBCP2 Javadoc fix"
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
commit 8420e18e991615dd2f5fb864b1091ded523b13ab
Author: Mark Thomas
AuthorDate: Fri Feb 11 10:10:27 2022 +
Revert "Cherry-pick DBCP2 Javadoc fix"
This reverts commit 0a9afdfa6a481b29a954a67cf6ea352043201ed0.
---
.../apache/tomcat/dbcp/dbcp2/BasicDataSource.java | 7 --
.../apache/tomcat/dbcp/dbcp2/DataSourceMXBean.java | 77 --
webapps/docs/changelog.xml | 5 ++
3 files changed, 5 insertions(+), 84 deletions(-)
diff --git a/java/org/apache/tomcat/dbcp/dbcp2/BasicDataSource.java
b/java/org/apache/tomcat/dbcp/dbcp2/BasicDataSource.java
index b0562e8..f260a53 100644
--- a/java/org/apache/tomcat/dbcp/dbcp2/BasicDataSource.java
+++ b/java/org/apache/tomcat/dbcp/dbcp2/BasicDataSource.java
@@ -1063,7 +1063,6 @@ public class BasicDataSource implements DataSource,
BasicDataSourceMXBean, MBean
* @return the maximum permitted duration of a connection.
* @since 2.10.0
*/
-@Override
public Duration getMaxConnDuration() {
return maxConnDuration;
}
@@ -1123,7 +1122,6 @@ public class BasicDataSource implements DataSource,
BasicDataSourceMXBean, MBean
* @return the maxWaitDuration property value.
* @since 2.10.0
*/
-@Override
public synchronized Duration getMaxWaitDuration() {
return this.maxWaitDuration;
}
@@ -1148,7 +1146,6 @@ public class BasicDataSource implements DataSource,
BasicDataSourceMXBean, MBean
* @see #setMinEvictableIdle(Duration)
* @since 2.10.0
*/
-@Override
public synchronized Duration getMinEvictableIdleDuration() {
return this.minEvictableIdleDuration;
}
@@ -1326,7 +1323,6 @@ public class BasicDataSource implements DataSource,
BasicDataSourceMXBean, MBean
* @return Timeout before an abandoned connection can be removed.
* @since 2.10.0
*/
-@Override
public Duration getRemoveAbandonedTimeoutDuration() {
return abandonedConfig == null ? Duration.ofSeconds(300) :
abandonedConfig.getRemoveAbandonedTimeoutDuration();
}
@@ -1356,7 +1352,6 @@ public class BasicDataSource implements DataSource,
BasicDataSourceMXBean, MBean
* there are minIdle idle connections in the pool
* @since 2.10.0
*/
-@Override
public synchronized Duration getSoftMinEvictableIdleDuration() {
return softMinEvictableIdleDuration;
}
@@ -1433,7 +1428,6 @@ public class BasicDataSource implements DataSource,
BasicDataSourceMXBean, MBean
* @see #setDurationBetweenEvictionRuns(Duration)
* @since 2.10.0
*/
-@Override
public synchronized Duration getDurationBetweenEvictionRuns() {
return this.durationBetweenEvictionRuns;
}
@@ -1487,7 +1481,6 @@ public class BasicDataSource implements DataSource,
BasicDataSourceMXBean, MBean
*
* @return the timeout in seconds before connection validation queries
fail.
*/
-@Override
public Duration getValidationQueryTimeoutDuration() {
return validationQueryTimeoutDuration;
}
diff --git a/java/org/apache/tomcat/dbcp/dbcp2/DataSourceMXBean.java
b/java/org/apache/tomcat/dbcp/dbcp2/DataSourceMXBean.java
index a6d522b..7315c58 100644
--- a/java/org/apache/tomcat/dbcp/dbcp2/DataSourceMXBean.java
+++ b/java/org/apache/tomcat/dbcp/dbcp2/DataSourceMXBean.java
@@ -17,7 +17,6 @@
package org.apache.tomcat.dbcp.dbcp2;
import java.sql.SQLException;
-import java.time.Duration;
/**
* Defines the methods that will be made available via
@@ -139,21 +138,10 @@ public interface DataSourceMXBean {
boolean getLogExpiredConnections();
/**
- * See {@link BasicDataSource#getMaxConnDuration()}.
- *
- * @return {@link BasicDataSource#getMaxConnDuration()}.
- * @since 2.10.0
- */
-Duration getMaxConnDuration();
-
-/**
* See {@link BasicDataSource#getMaxConnLifetimeMillis()}.
*
* @return {@link BasicDataSource#getMaxConnLifetimeMillis()}.
- *
- * @deprecated Use {@link #getMaxConnDuration()}.
*/
-@Deprecated
long getMaxConnLifetimeMillis();
/**
@@ -178,39 +166,17 @@ public interface DataSourceMXBean {
int getMaxTotal();
/**
- * See {@link BasicDataSource#getMaxWaitDuration()}.
- *
- * @return {@link BasicDataSource#getMaxWaitDuration()}.
- * @since 2.10.0
- */
-Duration getMaxWaitDuration();
-
-/**
* See {@link BasicDataSource#getMaxWaitMillis()}.
*
* @return {@link BasicDataSource#getMaxWaitMillis()}.
- *
- * @deprecated Use {@link #getMaxWaitDuration()}.
*/
-@Deprecated
long getMaxWaitMillis();
/**
- * See {@link BasicDataSource#getMinEvictableIdleDuration()}.
- *
- * @return {@link BasicDataSource#getMinEvictableIdleD
Re: [tomcat] branch main updated: Add support for additional user, attributes in TomcatPrincipal
Am 2022-02-11 um 11:16 schrieb Rémy Maucherat: On Fri, Feb 11, 2022 at 10:47 AM Michael Osipov wrote: I cannot share your further explanations as well. The Realm *is* already an object store. Principal name and roles are attributes on that object, so is password. Without this possibility you have the following problems: That statement is just weird. The realm stores credentials associated with a principal name, as well as a list of roles (other names) associated with that name. That's it. An object store can store objects (like if there is a get/setAttribute that deals with Object - and that's my problem now, as it turns out ...). No, the realm stores the principal and can additionally store crdentials. You have at least two mechanism which do not verify credentials: SPNEGO and TLS mutual auth. I use both, authentication is completely decoupled from the Realm. Does this onw disqualify? The realm is always a read-only view on a store. Tomcat's intention is not to provide any read/write interface to that store. Strictly reading only. * You cannot abstract from the persistence layer in your application code, at worst you need to double code for retrieval * You double access time since you need to query the principal store again. A performance penalty in a stateless application, e.g. APIs * You maybe even not have access to the realm. Consider the identity provider gives you some form of token containing the attributes which you CANNOT retreive yourself. How to solve? You can't. I don't believe the penalty exists for a DataSource backend. Most likely for LDAP. Either way, the application is now non portable. Given that the benefit for DataSource does not seem large, this simply sounds bad in that case. For LDAP, it sounds like a tradeoff. I disagree here also. Two options: * The Realm database is decoupled form the app database, the app shall not have access to it. What now? * Consider the database is not in the same rack as the app server is. I have cases where it is 500 km away, you instantly feel that the roundtrip matters. Sometimes you simply don't have control over the environment. Yet another example: I have written code to locate the closest domain controller, if I connect to that one I have response times < 10 ms, if I connect to a DC in Munich, I suddenly have > 100 ms. Scale here. No one is aiming here for an ORM layer. One of my points is that this is really far from obvious in the API, so it needs a round of javadoc tightening up. Please do so. M
[tomcat] branch main updated: Remove trailing space
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 6a0dde0 Remove trailing space 6a0dde0 is described below commit 6a0dde03effe2ff5795cc8a9b3fb915b0dfbce0d Author: Mark Thomas AuthorDate: Fri Feb 11 11:05:58 2022 + Remove trailing space --- webapps/docs/changelog.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 9829b59..2a16a97 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -216,7 +216,7 @@ Revert the cherry-pick of JavaDoc fix from DBCP applied in 10.1.0.M9 that broke the DataSourceMXBean by using a type that isn't -supported by MXBeans. (markt) +supported by MXBeans. (markt) - 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: Remove trailing space
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 9c7bfb0 Remove trailing space 9c7bfb0 is described below commit 9c7bfb08915b23bf0af6ced4226cce806156abc0 Author: Mark Thomas AuthorDate: Fri Feb 11 11:05:58 2022 + Remove trailing space --- webapps/docs/changelog.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 0393544..e870286 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -216,7 +216,7 @@ Revert the cherry-pick of JavaDoc fix from DBCP applied in 10.0.15 that broke the DataSourceMXBean by using a type that isn't -supported by MXBeans. (markt) +supported by MXBeans. (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: Remove trailing space
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 d06a867 Remove trailing space d06a867 is described below commit d06a867d1af9b86390271f4757473154e39577c8 Author: Mark Thomas AuthorDate: Fri Feb 11 11:05:58 2022 + Remove trailing space --- webapps/docs/changelog.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 4ca7211..c92ed56 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -222,7 +222,7 @@ Revert the cherry-pick of JavaDoc fix from DBCP applied in 9.0.57 that broke the DataSourceMXBean by using a type that isn't -supported by MXBeans. (markt) +supported by MXBeans. (markt) - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch main updated (6a0dde0 -> 5569e41)
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 6a0dde0 Remove trailing space new c521dc5 Fix a potential exception when generating a WebDAV multi-status response new 5569e41 Improve human readability of WebDAV XML responses. 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: .../apache/catalina/servlets/WebdavServlet.java| 19 - java/org/apache/catalina/util/XMLWriter.java | 32 -- webapps/docs/changelog.xml | 11 3 files changed, 40 insertions(+), 22 deletions(-) - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] 02/02: Improve human readability of WebDAV XML responses.
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 5569e41c1412d0495f0685d2989055582347b414
Author: Mark Thomas
AuthorDate: Fri Feb 11 15:35:59 2022 +
Improve human readability of WebDAV XML responses.
---
java/org/apache/catalina/util/XMLWriter.java | 32 +---
webapps/docs/changelog.xml | 5 +
2 files changed, 29 insertions(+), 8 deletions(-)
diff --git a/java/org/apache/catalina/util/XMLWriter.java
b/java/org/apache/catalina/util/XMLWriter.java
index 937fd07..019c4b9 100644
--- a/java/org/apache/catalina/util/XMLWriter.java
+++ b/java/org/apache/catalina/util/XMLWriter.java
@@ -24,10 +24,8 @@ import java.io.Writer;
*/
public class XMLWriter {
-
// -- Constants
-
/**
* Opening tag.
*/
@@ -48,7 +46,6 @@ public class XMLWriter {
// - Instance Variables
-
/**
* Buffer.
*/
@@ -61,8 +58,10 @@ public class XMLWriter {
protected final Writer writer;
-// --- Constructors
+protected boolean lastWriteWasOpen;
+
+// --- Constructors
/**
* New XML writer utility that will store its data in an internal buffer.
@@ -87,7 +86,6 @@ public class XMLWriter {
// - Public Methods
-
/**
* Retrieve generated XML.
*
@@ -138,6 +136,9 @@ public class XMLWriter {
if ((namespace != null) && (namespace.length() > 0)) {
switch (type) {
case OPENING:
+if (lastWriteWasOpen) {
+buffer.append('\n');
+}
if (namespaceInfo != null) {
buffer.append("<" + namespace + ":" + name + " xmlns:"
+ namespace + "=\""
@@ -145,32 +146,47 @@ public class XMLWriter {
} else {
buffer.append("<" + namespace + ":" + name + ">");
}
+lastWriteWasOpen = true;
break;
case CLOSING:
buffer.append("\n");
+lastWriteWasOpen = false;
break;
case NO_CONTENT:
default:
+if (lastWriteWasOpen) {
+buffer.append('\n');
+}
if (namespaceInfo != null) {
buffer.append("<" + namespace + ":" + name + " xmlns:"
+ namespace + "=\""
- + namespaceInfo + "\"/>");
+ + namespaceInfo + "\"/>\n");
} else {
-buffer.append("<" + namespace + ":" + name + "/>");
+buffer.append("<" + namespace + ":" + name + "/>\n");
}
+lastWriteWasOpen = false;
break;
}
} else {
switch (type) {
case OPENING:
+if (lastWriteWasOpen) {
+buffer.append('\n');
+}
buffer.append("<" + name + ">");
+lastWriteWasOpen = true;
break;
case CLOSING:
buffer.append("\n");
+lastWriteWasOpen = false;
break;
case NO_CONTENT:
default:
-buffer.append("<" + name + "/>");
+if (lastWriteWasOpen) {
+buffer.append('\n');
+}
+buffer.append("<" + name + "/>\n");
+lastWriteWasOpen = false;
break;
}
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 62d492d..5dfed80 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -145,6 +145,11 @@
copy or delete. Report the paths relative to the server root for any
resources with an error. (markt)
+
+Improve the format of WebDAV XML responses to make them easier for
+humans to read. The change ensures that there is always a line break
+before starting a new element. (markt)
+
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] 01/02: Fix a potential exception when generating a WebDAV multi-status response
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 c521dc50c85c7ee8b458282a9c90c150ebab8e84
Author: Mark Thomas
AuthorDate: Fri Feb 11 15:33:44 2022 +
Fix a potential exception when generating a WebDAV multi-status response
Fix a potential StringIndexOutOfBoundsException exception when
generating a WebDAV multi-status response after an error during a copy
or delete.
Report the paths relative to the server root for any resources with an
error.
---
java/org/apache/catalina/servlets/WebdavServlet.java | 19 +--
webapps/docs/changelog.xml | 6 ++
2 files changed, 11 insertions(+), 14 deletions(-)
diff --git a/java/org/apache/catalina/servlets/WebdavServlet.java
b/java/org/apache/catalina/servlets/WebdavServlet.java
index a9701b0..c6a0963 100644
--- a/java/org/apache/catalina/servlets/WebdavServlet.java
+++ b/java/org/apache/catalina/servlets/WebdavServlet.java
@@ -1922,22 +1922,18 @@ public class WebdavServlet extends DefaultServlet {
* @param req Servlet request
* @param resp Servlet response
* @param errorList List of error to be displayed
+ *
* @throws IOException If an IO error occurs
*/
private void sendReport(HttpServletRequest req, HttpServletResponse resp,
-Hashtable errorList)
-throws IOException {
+Hashtable errorList) throws IOException {
resp.setStatus(WebdavStatus.SC_MULTI_STATUS);
-String absoluteUri = req.getRequestURI();
-String relativePath = getRelativePath(req);
-
XMLWriter generatedXML = new XMLWriter();
generatedXML.writeXMLHeader();
-generatedXML.writeElement("D", DEFAULT_NAMESPACE, "multistatus",
-XMLWriter.OPENING);
+generatedXML.writeElement("D", DEFAULT_NAMESPACE, "multistatus",
XMLWriter.OPENING);
Enumeration pathList = errorList.keys();
while (pathList.hasMoreElements()) {
@@ -1948,18 +1944,14 @@ public class WebdavServlet extends DefaultServlet {
generatedXML.writeElement("D", "response", XMLWriter.OPENING);
generatedXML.writeElement("D", "href", XMLWriter.OPENING);
-String toAppend = errorPath.substring(relativePath.length());
-if (!toAppend.startsWith("/")) {
-toAppend = "/" + toAppend;
-}
-generatedXML.writeText(absoluteUri + toAppend);
+generatedXML.writeText(getServletContext().getContextPath() +
errorPath);
generatedXML.writeElement("D", "href", XMLWriter.CLOSING);
+
generatedXML.writeElement("D", "status", XMLWriter.OPENING);
generatedXML.writeText("HTTP/1.1 " + errorCode + " ");
generatedXML.writeElement("D", "status", XMLWriter.CLOSING);
generatedXML.writeElement("D", "response", XMLWriter.CLOSING);
-
}
generatedXML.writeElement("D", "multistatus", XMLWriter.CLOSING);
@@ -1967,7 +1959,6 @@ public class WebdavServlet extends DefaultServlet {
Writer writer = resp.getWriter();
writer.write(generatedXML.toString());
writer.close();
-
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 2a16a97..62d492d 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -139,6 +139,12 @@
Tomcat will not load from web applications. Pull request provided by
ppkarwasz. (markt)
+
+Fix a potential StringIndexOutOfBoundsException exception
+when generating a WebDAV multi-status response after an error during a
+copy or delete. Report the paths relative to the server root for any
+resources with an error. (markt)
+
-
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 (9c7bfb0 -> 63dc7fb)
This is an automated email from the ASF dual-hosted git repository. markt pushed a change to branch 10.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git. from 9c7bfb0 Remove trailing space new 43e9c8c Fix a potential exception when generating a WebDAV multi-status response new 63dc7fb Improve human readability of WebDAV XML responses. 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: .../apache/catalina/servlets/WebdavServlet.java| 19 - java/org/apache/catalina/util/XMLWriter.java | 32 -- webapps/docs/changelog.xml | 11 3 files changed, 40 insertions(+), 22 deletions(-) - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] 02/02: Improve human readability of WebDAV XML responses.
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
commit 63dc7fb7e94cbc0b44d0c5edb0648141b3dd51d3
Author: Mark Thomas
AuthorDate: Fri Feb 11 15:35:59 2022 +
Improve human readability of WebDAV XML responses.
---
java/org/apache/catalina/util/XMLWriter.java | 32 +---
webapps/docs/changelog.xml | 5 +
2 files changed, 29 insertions(+), 8 deletions(-)
diff --git a/java/org/apache/catalina/util/XMLWriter.java
b/java/org/apache/catalina/util/XMLWriter.java
index 937fd07..019c4b9 100644
--- a/java/org/apache/catalina/util/XMLWriter.java
+++ b/java/org/apache/catalina/util/XMLWriter.java
@@ -24,10 +24,8 @@ import java.io.Writer;
*/
public class XMLWriter {
-
// -- Constants
-
/**
* Opening tag.
*/
@@ -48,7 +46,6 @@ public class XMLWriter {
// - Instance Variables
-
/**
* Buffer.
*/
@@ -61,8 +58,10 @@ public class XMLWriter {
protected final Writer writer;
-// --- Constructors
+protected boolean lastWriteWasOpen;
+
+// --- Constructors
/**
* New XML writer utility that will store its data in an internal buffer.
@@ -87,7 +86,6 @@ public class XMLWriter {
// - Public Methods
-
/**
* Retrieve generated XML.
*
@@ -138,6 +136,9 @@ public class XMLWriter {
if ((namespace != null) && (namespace.length() > 0)) {
switch (type) {
case OPENING:
+if (lastWriteWasOpen) {
+buffer.append('\n');
+}
if (namespaceInfo != null) {
buffer.append("<" + namespace + ":" + name + " xmlns:"
+ namespace + "=\""
@@ -145,32 +146,47 @@ public class XMLWriter {
} else {
buffer.append("<" + namespace + ":" + name + ">");
}
+lastWriteWasOpen = true;
break;
case CLOSING:
buffer.append("\n");
+lastWriteWasOpen = false;
break;
case NO_CONTENT:
default:
+if (lastWriteWasOpen) {
+buffer.append('\n');
+}
if (namespaceInfo != null) {
buffer.append("<" + namespace + ":" + name + " xmlns:"
+ namespace + "=\""
- + namespaceInfo + "\"/>");
+ + namespaceInfo + "\"/>\n");
} else {
-buffer.append("<" + namespace + ":" + name + "/>");
+buffer.append("<" + namespace + ":" + name + "/>\n");
}
+lastWriteWasOpen = false;
break;
}
} else {
switch (type) {
case OPENING:
+if (lastWriteWasOpen) {
+buffer.append('\n');
+}
buffer.append("<" + name + ">");
+lastWriteWasOpen = true;
break;
case CLOSING:
buffer.append("\n");
+lastWriteWasOpen = false;
break;
case NO_CONTENT:
default:
-buffer.append("<" + name + "/>");
+if (lastWriteWasOpen) {
+buffer.append('\n');
+}
+buffer.append("<" + name + "/>\n");
+lastWriteWasOpen = false;
break;
}
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 8920403..7aa98fc 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -145,6 +145,11 @@
copy or delete. Report the paths relative to the server root for any
resources with an error. (markt)
+
+Improve the format of WebDAV XML responses to make them easier for
+humans to read. The change ensures that there is always a line break
+before starting a new element. (markt)
+
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] 01/02: Fix a potential exception when generating a WebDAV multi-status response
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
commit 43e9c8c1195b33e52b06af9c348d21f51d643355
Author: Mark Thomas
AuthorDate: Fri Feb 11 15:33:44 2022 +
Fix a potential exception when generating a WebDAV multi-status response
Fix a potential StringIndexOutOfBoundsException exception when
generating a WebDAV multi-status response after an error during a copy
or delete.
Report the paths relative to the server root for any resources with an
error.
---
java/org/apache/catalina/servlets/WebdavServlet.java | 19 +--
webapps/docs/changelog.xml | 6 ++
2 files changed, 11 insertions(+), 14 deletions(-)
diff --git a/java/org/apache/catalina/servlets/WebdavServlet.java
b/java/org/apache/catalina/servlets/WebdavServlet.java
index a9701b0..c6a0963 100644
--- a/java/org/apache/catalina/servlets/WebdavServlet.java
+++ b/java/org/apache/catalina/servlets/WebdavServlet.java
@@ -1922,22 +1922,18 @@ public class WebdavServlet extends DefaultServlet {
* @param req Servlet request
* @param resp Servlet response
* @param errorList List of error to be displayed
+ *
* @throws IOException If an IO error occurs
*/
private void sendReport(HttpServletRequest req, HttpServletResponse resp,
-Hashtable errorList)
-throws IOException {
+Hashtable errorList) throws IOException {
resp.setStatus(WebdavStatus.SC_MULTI_STATUS);
-String absoluteUri = req.getRequestURI();
-String relativePath = getRelativePath(req);
-
XMLWriter generatedXML = new XMLWriter();
generatedXML.writeXMLHeader();
-generatedXML.writeElement("D", DEFAULT_NAMESPACE, "multistatus",
-XMLWriter.OPENING);
+generatedXML.writeElement("D", DEFAULT_NAMESPACE, "multistatus",
XMLWriter.OPENING);
Enumeration pathList = errorList.keys();
while (pathList.hasMoreElements()) {
@@ -1948,18 +1944,14 @@ public class WebdavServlet extends DefaultServlet {
generatedXML.writeElement("D", "response", XMLWriter.OPENING);
generatedXML.writeElement("D", "href", XMLWriter.OPENING);
-String toAppend = errorPath.substring(relativePath.length());
-if (!toAppend.startsWith("/")) {
-toAppend = "/" + toAppend;
-}
-generatedXML.writeText(absoluteUri + toAppend);
+generatedXML.writeText(getServletContext().getContextPath() +
errorPath);
generatedXML.writeElement("D", "href", XMLWriter.CLOSING);
+
generatedXML.writeElement("D", "status", XMLWriter.OPENING);
generatedXML.writeText("HTTP/1.1 " + errorCode + " ");
generatedXML.writeElement("D", "status", XMLWriter.CLOSING);
generatedXML.writeElement("D", "response", XMLWriter.CLOSING);
-
}
generatedXML.writeElement("D", "multistatus", XMLWriter.CLOSING);
@@ -1967,7 +1959,6 @@ public class WebdavServlet extends DefaultServlet {
Writer writer = resp.getWriter();
writer.write(generatedXML.toString());
writer.close();
-
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index e870286..8920403 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -139,6 +139,12 @@
TomcatPrincipal and GenericPrincipal.
Patch provided by Carsten Klein. (michaelo)
+
+Fix a potential StringIndexOutOfBoundsException exception
+when generating a WebDAV multi-status response after an error during a
+copy or delete. Report the paths relative to the server root for any
+resources with an error. (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 (d06a867 -> c2faa05)
This is an automated email from the ASF dual-hosted git repository. markt pushed a change to branch 9.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git. from d06a867 Remove trailing space new 16b3580 Fix a potential exception when generating a WebDAV multi-status response new c2faa05 Improve human readability of WebDAV XML responses. 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: .../apache/catalina/servlets/WebdavServlet.java| 19 - java/org/apache/catalina/util/XMLWriter.java | 32 -- webapps/docs/changelog.xml | 11 3 files changed, 40 insertions(+), 22 deletions(-) - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] 01/02: Fix a potential exception when generating a WebDAV multi-status response
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
commit 16b3580af462a7ee932835a527d961ee9b1be648
Author: Mark Thomas
AuthorDate: Fri Feb 11 15:33:44 2022 +
Fix a potential exception when generating a WebDAV multi-status response
Fix a potential StringIndexOutOfBoundsException exception when
generating a WebDAV multi-status response after an error during a copy
or delete.
Report the paths relative to the server root for any resources with an
error.
---
java/org/apache/catalina/servlets/WebdavServlet.java | 19 +--
webapps/docs/changelog.xml | 6 ++
2 files changed, 11 insertions(+), 14 deletions(-)
diff --git a/java/org/apache/catalina/servlets/WebdavServlet.java
b/java/org/apache/catalina/servlets/WebdavServlet.java
index 73c0c5f..b5b08f8 100644
--- a/java/org/apache/catalina/servlets/WebdavServlet.java
+++ b/java/org/apache/catalina/servlets/WebdavServlet.java
@@ -1921,22 +1921,18 @@ public class WebdavServlet extends DefaultServlet {
* @param req Servlet request
* @param resp Servlet response
* @param errorList List of error to be displayed
+ *
* @throws IOException If an IO error occurs
*/
private void sendReport(HttpServletRequest req, HttpServletResponse resp,
-Hashtable errorList)
-throws IOException {
+Hashtable errorList) throws IOException {
resp.setStatus(WebdavStatus.SC_MULTI_STATUS);
-String absoluteUri = req.getRequestURI();
-String relativePath = getRelativePath(req);
-
XMLWriter generatedXML = new XMLWriter();
generatedXML.writeXMLHeader();
-generatedXML.writeElement("D", DEFAULT_NAMESPACE, "multistatus",
-XMLWriter.OPENING);
+generatedXML.writeElement("D", DEFAULT_NAMESPACE, "multistatus",
XMLWriter.OPENING);
Enumeration pathList = errorList.keys();
while (pathList.hasMoreElements()) {
@@ -1947,18 +1943,14 @@ public class WebdavServlet extends DefaultServlet {
generatedXML.writeElement("D", "response", XMLWriter.OPENING);
generatedXML.writeElement("D", "href", XMLWriter.OPENING);
-String toAppend = errorPath.substring(relativePath.length());
-if (!toAppend.startsWith("/")) {
-toAppend = "/" + toAppend;
-}
-generatedXML.writeText(absoluteUri + toAppend);
+generatedXML.writeText(getServletContext().getContextPath() +
errorPath);
generatedXML.writeElement("D", "href", XMLWriter.CLOSING);
+
generatedXML.writeElement("D", "status", XMLWriter.OPENING);
generatedXML.writeText("HTTP/1.1 " + errorCode + " ");
generatedXML.writeElement("D", "status", XMLWriter.CLOSING);
generatedXML.writeElement("D", "response", XMLWriter.CLOSING);
-
}
generatedXML.writeElement("D", "multistatus", XMLWriter.CLOSING);
@@ -1966,7 +1958,6 @@ public class WebdavServlet extends DefaultServlet {
Writer writer = resp.getWriter();
writer.write(generatedXML.toString());
writer.close();
-
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index c92ed56..1c80769 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -139,6 +139,12 @@
TomcatPrincipal and GenericPrincipal.
Patch provided by Carsten Klein. (michaelo)
+
+Fix a potential StringIndexOutOfBoundsException exception
+when generating a WebDAV multi-status response after an error during a
+copy or delete. Report the paths relative to the server root for any
+resources with an error. (markt)
+
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] 02/02: Improve human readability of WebDAV XML responses.
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
commit c2faa058fcebefb4a61a5669b00018308f4aaa91
Author: Mark Thomas
AuthorDate: Fri Feb 11 15:35:59 2022 +
Improve human readability of WebDAV XML responses.
---
java/org/apache/catalina/util/XMLWriter.java | 32 +---
webapps/docs/changelog.xml | 5 +
2 files changed, 29 insertions(+), 8 deletions(-)
diff --git a/java/org/apache/catalina/util/XMLWriter.java
b/java/org/apache/catalina/util/XMLWriter.java
index 937fd07..019c4b9 100644
--- a/java/org/apache/catalina/util/XMLWriter.java
+++ b/java/org/apache/catalina/util/XMLWriter.java
@@ -24,10 +24,8 @@ import java.io.Writer;
*/
public class XMLWriter {
-
// -- Constants
-
/**
* Opening tag.
*/
@@ -48,7 +46,6 @@ public class XMLWriter {
// - Instance Variables
-
/**
* Buffer.
*/
@@ -61,8 +58,10 @@ public class XMLWriter {
protected final Writer writer;
-// --- Constructors
+protected boolean lastWriteWasOpen;
+
+// --- Constructors
/**
* New XML writer utility that will store its data in an internal buffer.
@@ -87,7 +86,6 @@ public class XMLWriter {
// - Public Methods
-
/**
* Retrieve generated XML.
*
@@ -138,6 +136,9 @@ public class XMLWriter {
if ((namespace != null) && (namespace.length() > 0)) {
switch (type) {
case OPENING:
+if (lastWriteWasOpen) {
+buffer.append('\n');
+}
if (namespaceInfo != null) {
buffer.append("<" + namespace + ":" + name + " xmlns:"
+ namespace + "=\""
@@ -145,32 +146,47 @@ public class XMLWriter {
} else {
buffer.append("<" + namespace + ":" + name + ">");
}
+lastWriteWasOpen = true;
break;
case CLOSING:
buffer.append("\n");
+lastWriteWasOpen = false;
break;
case NO_CONTENT:
default:
+if (lastWriteWasOpen) {
+buffer.append('\n');
+}
if (namespaceInfo != null) {
buffer.append("<" + namespace + ":" + name + " xmlns:"
+ namespace + "=\""
- + namespaceInfo + "\"/>");
+ + namespaceInfo + "\"/>\n");
} else {
-buffer.append("<" + namespace + ":" + name + "/>");
+buffer.append("<" + namespace + ":" + name + "/>\n");
}
+lastWriteWasOpen = false;
break;
}
} else {
switch (type) {
case OPENING:
+if (lastWriteWasOpen) {
+buffer.append('\n');
+}
buffer.append("<" + name + ">");
+lastWriteWasOpen = true;
break;
case CLOSING:
buffer.append("\n");
+lastWriteWasOpen = false;
break;
case NO_CONTENT:
default:
-buffer.append("<" + name + "/>");
+if (lastWriteWasOpen) {
+buffer.append('\n');
+}
+buffer.append("<" + name + "/>\n");
+lastWriteWasOpen = false;
break;
}
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 1c80769..4a2a6c0 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -145,6 +145,11 @@
copy or delete. Report the paths relative to the server root for any
resources with an error. (markt)
+
+Improve the format of WebDAV XML responses to make them easier for
+humans to read. The change ensures that there is always a line break
+before starting a new element. (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 (1912714 -> cbe7018)
This is an automated email from the ASF dual-hosted git repository. markt pushed a change to branch 8.5.x in repository https://gitbox.apache.org/repos/asf/tomcat.git. from 1912714 Add workaround to allow running testsuite on Java 8 new 8ed541f Fix a potential exception when generating a WebDAV multi-status response new cbe7018 Improve human readability of WebDAV XML responses. 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: .../apache/catalina/servlets/WebdavServlet.java| 19 - java/org/apache/catalina/util/XMLWriter.java | 32 -- webapps/docs/changelog.xml | 11 3 files changed, 40 insertions(+), 22 deletions(-) - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] 02/02: Improve human readability of WebDAV XML responses.
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
commit cbe70188fc0c7c42db16a7bd9dd0478475062540
Author: Mark Thomas
AuthorDate: Fri Feb 11 15:35:59 2022 +
Improve human readability of WebDAV XML responses.
---
java/org/apache/catalina/util/XMLWriter.java | 32 +---
webapps/docs/changelog.xml | 5 +
2 files changed, 29 insertions(+), 8 deletions(-)
diff --git a/java/org/apache/catalina/util/XMLWriter.java
b/java/org/apache/catalina/util/XMLWriter.java
index 937fd07..019c4b9 100644
--- a/java/org/apache/catalina/util/XMLWriter.java
+++ b/java/org/apache/catalina/util/XMLWriter.java
@@ -24,10 +24,8 @@ import java.io.Writer;
*/
public class XMLWriter {
-
// -- Constants
-
/**
* Opening tag.
*/
@@ -48,7 +46,6 @@ public class XMLWriter {
// - Instance Variables
-
/**
* Buffer.
*/
@@ -61,8 +58,10 @@ public class XMLWriter {
protected final Writer writer;
-// --- Constructors
+protected boolean lastWriteWasOpen;
+
+// --- Constructors
/**
* New XML writer utility that will store its data in an internal buffer.
@@ -87,7 +86,6 @@ public class XMLWriter {
// - Public Methods
-
/**
* Retrieve generated XML.
*
@@ -138,6 +136,9 @@ public class XMLWriter {
if ((namespace != null) && (namespace.length() > 0)) {
switch (type) {
case OPENING:
+if (lastWriteWasOpen) {
+buffer.append('\n');
+}
if (namespaceInfo != null) {
buffer.append("<" + namespace + ":" + name + " xmlns:"
+ namespace + "=\""
@@ -145,32 +146,47 @@ public class XMLWriter {
} else {
buffer.append("<" + namespace + ":" + name + ">");
}
+lastWriteWasOpen = true;
break;
case CLOSING:
buffer.append("\n");
+lastWriteWasOpen = false;
break;
case NO_CONTENT:
default:
+if (lastWriteWasOpen) {
+buffer.append('\n');
+}
if (namespaceInfo != null) {
buffer.append("<" + namespace + ":" + name + " xmlns:"
+ namespace + "=\""
- + namespaceInfo + "\"/>");
+ + namespaceInfo + "\"/>\n");
} else {
-buffer.append("<" + namespace + ":" + name + "/>");
+buffer.append("<" + namespace + ":" + name + "/>\n");
}
+lastWriteWasOpen = false;
break;
}
} else {
switch (type) {
case OPENING:
+if (lastWriteWasOpen) {
+buffer.append('\n');
+}
buffer.append("<" + name + ">");
+lastWriteWasOpen = true;
break;
case CLOSING:
buffer.append("\n");
+lastWriteWasOpen = false;
break;
case NO_CONTENT:
default:
-buffer.append("<" + name + "/>");
+if (lastWriteWasOpen) {
+buffer.append('\n');
+}
+buffer.append("<" + name + "/>\n");
+lastWriteWasOpen = false;
break;
}
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index d69f9d0..157f436 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -140,6 +140,11 @@
copy or delete. Report the paths relative to the server root for any
resources with an error. (markt)
+
+Improve the format of WebDAV XML responses to make them easier for
+humans to read. The change ensures that there is always a line break
+before starting a new element. (markt)
+
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] 01/02: Fix a potential exception when generating a WebDAV multi-status response
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
commit 8ed541f67662f9dbd98a73ca0701de6b84e823e3
Author: Mark Thomas
AuthorDate: Fri Feb 11 15:33:44 2022 +
Fix a potential exception when generating a WebDAV multi-status response
Fix a potential StringIndexOutOfBoundsException exception when
generating a WebDAV multi-status response after an error during a copy
or delete.
Report the paths relative to the server root for any resources with an
error.
---
java/org/apache/catalina/servlets/WebdavServlet.java | 19 +--
webapps/docs/changelog.xml | 6 ++
2 files changed, 11 insertions(+), 14 deletions(-)
diff --git a/java/org/apache/catalina/servlets/WebdavServlet.java
b/java/org/apache/catalina/servlets/WebdavServlet.java
index e32cbe8..e82e26c 100644
--- a/java/org/apache/catalina/servlets/WebdavServlet.java
+++ b/java/org/apache/catalina/servlets/WebdavServlet.java
@@ -1924,22 +1924,18 @@ public class WebdavServlet extends DefaultServlet {
* @param req Servlet request
* @param resp Servlet response
* @param errorList List of error to be displayed
+ *
* @throws IOException If an IO error occurs
*/
private void sendReport(HttpServletRequest req, HttpServletResponse resp,
-Hashtable errorList)
-throws IOException {
+Hashtable errorList) throws IOException {
resp.setStatus(WebdavStatus.SC_MULTI_STATUS);
-String absoluteUri = req.getRequestURI();
-String relativePath = getRelativePath(req);
-
XMLWriter generatedXML = new XMLWriter();
generatedXML.writeXMLHeader();
-generatedXML.writeElement("D", DEFAULT_NAMESPACE, "multistatus",
-XMLWriter.OPENING);
+generatedXML.writeElement("D", DEFAULT_NAMESPACE, "multistatus",
XMLWriter.OPENING);
Enumeration pathList = errorList.keys();
while (pathList.hasMoreElements()) {
@@ -1950,19 +1946,15 @@ public class WebdavServlet extends DefaultServlet {
generatedXML.writeElement("D", "response", XMLWriter.OPENING);
generatedXML.writeElement("D", "href", XMLWriter.OPENING);
-String toAppend = errorPath.substring(relativePath.length());
-if (!toAppend.startsWith("/")) {
-toAppend = "/" + toAppend;
-}
-generatedXML.writeText(absoluteUri + toAppend);
+generatedXML.writeText(getServletContext().getContextPath() +
errorPath);
generatedXML.writeElement("D", "href", XMLWriter.CLOSING);
+
generatedXML.writeElement("D", "status", XMLWriter.OPENING);
generatedXML.writeText("HTTP/1.1 " + errorCode + " "
+ WebdavStatus.getStatusText(errorCode));
generatedXML.writeElement("D", "status", XMLWriter.CLOSING);
generatedXML.writeElement("D", "response", XMLWriter.CLOSING);
-
}
generatedXML.writeElement("D", "multistatus", XMLWriter.CLOSING);
@@ -1970,7 +1962,6 @@ public class WebdavServlet extends DefaultServlet {
Writer writer = resp.getWriter();
writer.write(generatedXML.toString());
writer.close();
-
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index e2414fd..d69f9d0 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -134,6 +134,12 @@
Tomcat will not load from web applications. Pull request provided by
ppkarwasz. (markt)
+
+Fix a potential StringIndexOutOfBoundsException exception
+when generating a WebDAV multi-status response after an error during a
+copy or delete. Report the paths relative to the server root for any
+resources with an error. (markt)
+
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Add support for additional user attributes in TomcatPrincipal
On Fri, Feb 11, 2022 at 11:17 AM Carsten Klein wrote: > > > > > On Fri, Feb 11, 2022 at 10:10 AM Rémy Maucherat wrote > > > > If we get there, it could include mail addresses, ssn, payment info, > > user profile pictures (binary), etc etc. > > Also one thing I don't get now is how this became Object > > getAttribute(String name) instead of String getAttribute(String name) > > ? The legitimate examples you gave are text, not binary objects. > > > > Ultimately, all of this is still application data ... Moving it in the > > Tomcat realm creates a proprietary behavior, the application is no > > longer portable while at the same time the benefit is minimal (saving > > a query ?). When logging in, the app should pull all the metadata it > > needs, store it in the session. > > > Yes, it could be that much metadata, so what? And yes, it's all > application data. But that must not be something negative. You may say, > this is up to the application but, isn't it convenient if the realm does > it for you? Tomcat's Request (e.g. HTTPServletRequest) returns all data > POSTed from a client; all that data may be application specific data as > well. Nobody cares... > > Again, it's not about saving one query while logging in a user. It's > primarily about > > 1. not needing to configure an extra connection to the user database > 2. not needing much custom code an knowledge/skills of the user database > (SQL, LDAP etc.) > 3. getting extra attributes from Tomcat "for free" with 'any' Realm > simply by configuring a list of fields uniformly Maybe you should consider some bespoke technology like for example using JPA instead ? Or any other object persistence framework or object database obviously. > As you are correctly saying, point 1. is not that complicated for > DataSourceRealm, as there is a JDBC pool around. Nevertheless, the > application still needs to know the name of the connection (e. g. > "jdbc/userdatabase") and the name of the user table, the column > containing the logon name (what Principal.getName() returns) and the > extra columns to query. > > With my solution, all that configuration is "hidden" in the Realm's > configuration. The only new thing is the "userAttributes" config > attribute. No duplicated configuration is required. > > What do you mean with 'proprietary behavior'? Why shouldn't the > application be no longer portable? The Realm's configuration always > contains any access data (JNDI resource names, URLs and passwords in > case of JNDIRealm), so it wasn't ever portable at all. If you think > about moving the application between different servers on the same site > (targeting the same user database), the new "userAttributes" should work > from any Tomcat server you use. Tomcat is a Servlet container implementing the EE specifications. You are supposed to be able to take your webapp and be able to run it unchanged on, for example, Wildfly (with just configuration for the DataSource resource in the JNDI environment). This, however, is a proprietary feature that makes the webapp non portable as other containers don't have TomcatPrincipal with attributes pulled from some location. So how would the same app retrieve the data on Wildfly ? > Moving between different user databases is also much simpler, since the > whole configuration is centralized at the Realm. > > Why do I store Object and not String attributes? Because I can... When > querying a JDBC database or a LDAP server dynamically, you must expect > various different types being returned. We could add a rule, that all > must be Strings or we call the toString() method on each value. But why? Because not having that limitation opens the possibility of having to implement automatic type mapping. Coincidentally, Michael stated he wanted List to be handled. A String is better from the Tomcat perspective because the implementation provided can be simple (then, let's say it's really a binary, it has to be encoded maybe with base64 - extended custom realm here - and then handled by the application). > > > > > Yes, well, unfortunately, due to more background thinking ... > > The purpose of the UserDatabase is to be able to write, so given the > > API it is an object database at this point. > > > > In my recent implementation, the AbstractUser got a > > /** > * Additional attributes of this user. > */ > protected Map attributes = new HashMap<>(); > > so, the UserDatabase does not store Object typed attributes, but only > Strings (since XML attributes are strings only). So, I don't understand > why you consider it an object database. So it's fine to replace Object with String then. More seriously, this means the data is best stored somewhere else instead and that it's not a good fit for this realm. > > > > > Ok, but ... Your actual use case is the DataSourceRealm, which uses a > > DataSource. That DataSource is a JNDI resource which is also available > > to the application. Getting a connection from the pool is not > > expensive at all, and
