Re: [tomcat] branch main updated: Add support for additional user attributes in TomcatPrincipal
On 10/02/2022 12:01, Rémy Maucherat wrote: On Tue, Feb 8, 2022 at 2:13 PM wrote: This is an automated email from the ASF dual-hosted git repository. michaelo 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 c3edf43 Add support for additional user attributes in TomcatPrincipal c3edf43 is described below commit c3edf437da20af0f11edc0ad6d893399b01e6287 Author: Carsten Klein AuthorDate: Wed Jan 12 15:06:42 2022 +0100 Add support for additional user attributes in TomcatPrincipal This closes #463 -1 (veto) Unfortunately, the more I think about this, the more it seems wrong to me. ACK. I won't tag until this has been reverted or you retract the veto. I will note that if I get to the point where I am ready to tag and this is the only issue remaining to resolve I will revert the change prior to the tag - although I'd prefer that michaelo did that before then. This makes the realm an object store, which is not a good idea at all, as it will: - Encourage all sorts of bad hacks and practices for users - Force us to gradually add more and more features to this generic object store feature - Have massive discrepancies between realms, with the users trying to figure out pros and cons between realms - Is a proprietary feature which ties up users to Tomcat Just trying to understand the concern at this point. You think the risk is that developers will push data in the Realm and then access from the authenticated principal? More importantly: application data storage should remain the prerogative of the application, Tomcat should not attempt to do that. The only thing Tomcat should provide is a principal name, which is then the primary key for the third party object store. There is zero reason to integrate this within the realm or principal API. It also feels like it can never be complete or appropriate. Looking at the proposed change for the DataSourceRealm I can see it has clearly defined limits. I can see how we might get requests to push back those limits in the future. I have a general concern / unease that doing this in an application specific manner would be relatively easy but doing it generically could get complex. I apologize for having to do this, after initially helping out with this PR, but this is one of those situations where the more I look about it, the worse I feel about it. I do recognise the use case of wanting to expose data that is already in the Realm to the application. I wonder whether this is a case where we can just provide some hooks and, if the developer chooses to (ab)use them, that is up to them. I'm thinking something like a getUserAttributes() method on Realm that returns null by default. If an application wants to populate additional attributes, then it needs to provide a custom realm that overrides that method. The issue with this approach is it requires a custom realm. Given your concerns, backing this out while we discuss it some more seems like the right approach to me. Mark - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch main updated (fd0775c -> ca8e2c2)
This is an automated email from the ASF dual-hosted git repository. remm pushed a change to branch main in repository https://gitbox.apache.org/repos/asf/tomcat.git. from fd0775c Remove unused import. add ca8e2c2 Update OWB to 2.0.26 No new revisions were added by this update. Summary of changes: modules/openssl-java17/pom.xml | 2 +- modules/owb/pom.xml| 4 ++-- webapps/docs/changelog.xml | 3 +++ 3 files changed, 6 insertions(+), 3 deletions(-) - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Tagging 10.1.x and 10.0.x tomorrow
On Wed, Feb 9, 2022 at 1:18 PM Mark Thomas wrote: > > On 07/02/2022 21:06, Mark Thomas wrote: > > Hi all, > > > > Just a heads up that, barring surprises, I plan to be in a position to > > tag 10.1.x and 10.0.x for the February release round towards the end of > > tomorrow. > > There were surprises. Looking more like the end of the week at the moment. The situation with c3edf437da20af0f11edc0ad6d893399b01e6287 will have to be resolved before tagging since it is an API change. As usual in this case, if nobody seconds my objections, then this can move along. 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
Re: Running unit tests with JVMs < 11
On 09/02/2022 14:04, Rainer Jung wrote: Hi all, I wonder how important it is to be able to run the unit tests for TC 8.5-10.0 with JVM versions 8 or even 7 (TC 8.5). The unconditional addition of unit test jvmargs "--add-opens=..." in 2b6e19e971a980e38bcd30f05c554d9b798666c0 (TC 10, backported) means, we can no longer run the tests with Java 8 or even 7. If I remove those jvmargs from build.xml, I run into a second problem when testing TC 8.5 with Java 7. In 014f4746176c7f27cda7c04b2c6036a539c385ff EasyMock was updated from 3.x to 4.x, which needs Java 8 to run. Several tests fail with UnsupportedClassVersionError in a few EasyMock classes. I can work around all of these myself for running the tests here, but I wonder, what kind of setup for testing we want to support out-of-the box. Note that by reintroducing the previous compatibility trick for jvmargs (TC 8.5, 9 and 10) and using the old EasyMock branch for TC 8.5 we could still build and test with Java 11, but would also allow testing on the other target Java versions we support. WDYT? I've been thinking about this since I read this yesterday. I like that the move to Java 11 allowed us to reduce the differences between the build scripts. So the decision is: Is the benefit of being able to test with older Java versions worth the differences this would re-introduce? We don't test the JreCompat code so there aren't any Java version specific tests. So what do we gain by testing on Java 7 / Java 8? I can think of a few things: - that we really have coded to the correct Java version - that the code runs as expected on that Java version Both of those things are more testing Java than testing Tomcat but I can see that there is some benefit - especially if one is concerned about the possibility of the code behaving differently on Java 7/8 compared to Java 11. I'm not concerned about that but neither am I too concerned about the re-introduction of a few differences. I guess I am -0 overall provided that any change does not impact the existing CI systems. Mark - 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: Update OWB to 2.0.26
This is an automated email from the ASF dual-hosted git repository. remm 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 4e98b00 Update OWB to 2.0.26 4e98b00 is described below commit 4e98b00f7ae6fabca88afb81ab07ad5214e9450c Author: remm AuthorDate: Thu Feb 10 13:47:35 2022 +0100 Update OWB to 2.0.26 --- modules/owb/pom.xml| 4 ++-- webapps/docs/changelog.xml | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/modules/owb/pom.xml b/modules/owb/pom.xml index 8979ce3..b222058 100644 --- a/modules/owb/pom.xml +++ b/modules/owb/pom.xml @@ -29,14 +29,14 @@ Apache Tomcat CDI 2 support Apache Tomcat CDI 2 support using Apache OpenWebBeans -2.0.25 +2.0.26 jar 1.2 1.2 1.3 -9.0.56 +9.0.58 diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 785c4ad..b5182aa 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -200,6 +200,9 @@ Fix dependencies for individual test targets in Ant build file. Based on 468 provided by Totoo chenyonghui. (markt) + +Update the OWB module to Apache OpenWebBeans 2.0.26. (remm) + - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] cklein05 opened a new pull request #473: Add support for additional user attributes to DataSourceRealm
cklein05 opened a new pull request #473: URL: https://github.com/apache/tomcat/pull/473 @michael-o Here's the patch to add support for additional user attributes to DataSourceRealm. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [tomcat] branch main updated: Add support for additional user attributes in TomcatPrincipal
On Tue, Feb 8, 2022 at 2:13 PM wrote:
>
> This is an automated email from the ASF dual-hosted git repository.
>
> michaelo 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 c3edf43 Add support for additional user attributes in
> TomcatPrincipal
> c3edf43 is described below
>
> commit c3edf437da20af0f11edc0ad6d893399b01e6287
> Author: Carsten Klein
> AuthorDate: Wed Jan 12 15:06:42 2022 +0100
>
> Add support for additional user attributes in TomcatPrincipal
>
> This closes #463
-1 (veto)
Unfortunately, the more I think about this, the more it seems wrong to me.
This makes the realm an object store, which is not a good idea at all,
as it will:
- Encourage all sorts of bad hacks and practices for users
- Force us to gradually add more and more features to this generic
object store feature
- Have massive discrepancies between realms, with the users trying to
figure out pros and cons between realms
- Is a proprietary feature which ties up users to Tomcat
More importantly: application data storage should remain the
prerogative of the application, Tomcat should not attempt to do that.
The only thing Tomcat should provide is a principal name, which is
then the primary key for the third party object store. There is zero
reason to integrate this within the realm or principal API. It also
feels like it can never be complete or appropriate.
I apologize for having to do this, after initially helping out with
this PR, but this is one of those situations where the more I look
about it, the worse I feel about it.
Rémy
> ---
> java/org/apache/catalina/TomcatPrincipal.java | 28 +
> .../apache/catalina/realm/GenericPrincipal.java| 46
> ++
> java/org/apache/catalina/realm/JNDIRealm.java | 2 +-
> webapps/docs/changelog.xml | 5 +++
> webapps/examples/jsp/security/protected/index.jsp | 44 +
> 5 files changed, 117 insertions(+), 8 deletions(-)
>
> diff --git a/java/org/apache/catalina/TomcatPrincipal.java
> b/java/org/apache/catalina/TomcatPrincipal.java
> index 83f9035..1e3d9f6 100644
> --- a/java/org/apache/catalina/TomcatPrincipal.java
> +++ b/java/org/apache/catalina/TomcatPrincipal.java
> @@ -17,6 +17,8 @@
> package org.apache.catalina;
>
> import java.security.Principal;
> +import java.util.Collections;
> +import java.util.Enumeration;
>
> import org.ietf.jgss.GSSCredential;
>
> @@ -47,4 +49,30 @@ public interface TomcatPrincipal extends Principal {
> * 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, or if
> + * null has been specified as the attribute's name.
> + *
> + * Only the servlet container may set attributes to make available custom
> + * information about a Principal or the user it represents.
> + *
> + * @param name a String specifying the name of the attribute
> + * @return an Object containing the value of the attribute,
> or
> + * null if the attribute does not exist, or if
> + * null has been specified as the attribute's name
> + */
> +Object getAttribute(String name);
> +
> +/**
> + * Returns an Enumeration containing the names of the
> + * attributes available to this Principal. This method returns an empty
> + * Enumeration if the Principal has no attributes available
> to
> + * it.
> + *
> + * @return an Enumeration of strings containing the names of
> + * the Principal's attributes
> + */
> +Enumeration getAttributeNames();
> }
> diff --git a/java/org/apache/catalina/realm/GenericPrincipal.java
> b/java/org/apache/catalina/realm/GenericPrincipal.java
> index 7260da4..584c104 100644
> --- a/java/org/apache/catalina/realm/GenericPrincipal.java
> +++ b/java/org/apache/catalina/realm/GenericPrincipal.java
> @@ -19,7 +19,10 @@ package org.apache.catalina.realm;
> import java.io.Serializable;
> import java.security.Principal;
> import java.util.Arrays;
> +import java.util.Collections;
> +import java.util.Enumeration;
> import java.util.List;
> +import java.util.Map;
>
> import javax.security.auth.login.LoginContext;
>
> @@ -120,7 +123,7 @@ public class GenericPrincipal implements TomcatPrincipal,
> Serializable {
> */
> public GenericPrincipal(String name, List roles,
> Principal userPrincipal, LoginContext loginContext) {
> -this(name, roles, userPrincipal, loginContext, null);
> +this(name, roles, userPrincipal, loginContext, null, null);
> }
>
> /**
> @@ -140,7 +143,7 @@ public class GenericPrincipal implements TomcatPrincipal,
> Serializable {
> @Deprecated
> public GenericPrincipal(String
[tomcat] branch 10.0.x updated: Update OWB to 2.0.26
This is an automated email from the ASF dual-hosted git repository. remm 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 5799a68 Update OWB to 2.0.26 5799a68 is described below commit 5799a68adec122b8da46e0ab7548844b81b96757 Author: remm AuthorDate: Thu Feb 10 13:47:35 2022 +0100 Update OWB to 2.0.26 --- modules/owb/pom.xml| 4 ++-- webapps/docs/changelog.xml | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/modules/owb/pom.xml b/modules/owb/pom.xml index 6c5c9bc..0799bd5 100644 --- a/modules/owb/pom.xml +++ b/modules/owb/pom.xml @@ -29,14 +29,14 @@ Apache Tomcat CDI 2 support Apache Tomcat CDI 2 support using Apache OpenWebBeans -2.0.25 +2.0.26 jar 1.2 1.2 1.3 -10.0.14 +10.0.16 diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 8fe5f70..3ad0ad2 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -194,6 +194,9 @@ Fix dependencies for individual test targets in Ant build file. Based on 468 provided by Totoo chenyonghui. (markt) + +Update the OWB module to Apache OpenWebBeans 2.0.26. (remm) + - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Running unit tests with JVMs < 11
On Thu, Feb 10, 2022 at 2:11 PM Mark Thomas wrote: > > On 09/02/2022 14:04, Rainer Jung wrote: > > Hi all, > > > > I wonder how important it is to be able to run the unit tests for TC > > 8.5-10.0 with JVM versions 8 or even 7 (TC 8.5). > > > > The unconditional addition of unit test jvmargs "--add-opens=..." in > > 2b6e19e971a980e38bcd30f05c554d9b798666c0 (TC 10, backported) means, we > > can no longer run the tests with Java 8 or even 7. > > > > If I remove those jvmargs from build.xml, I run into a second problem > > when testing TC 8.5 with Java 7. In > > 014f4746176c7f27cda7c04b2c6036a539c385ff EasyMock was updated from 3.x > > to 4.x, which needs Java 8 to run. Several tests fail with > > UnsupportedClassVersionError in a few EasyMock classes. > > > > I can work around all of these myself for running the tests here, but I > > wonder, what kind of setup for testing we want to support out-of-the box. > > > > Note that by reintroducing the previous compatibility trick for jvmargs > > (TC 8.5, 9 and 10) and using the old EasyMock branch for TC 8.5 we could > > still build and test with Java 11, but would also allow testing on the > > other target Java versions we support. > > > > WDYT? > > I've been thinking about this since I read this yesterday. > > I like that the move to Java 11 allowed us to reduce the differences > between the build scripts. +1 > So the decision is: Is the benefit of being able to test with older Java > versions worth the differences this would re-introduce? > > We don't test the JreCompat code so there aren't any Java version > specific tests. So what do we gain by testing on Java 7 / Java 8? I can > think of a few things: > - that we really have coded to the correct Java version > - that the code runs as expected on that Java version > > Both of those things are more testing Java than testing Tomcat but I can > see that there is some benefit - especially if one is concerned about > the possibility of the code behaving differently on Java 7/8 compared to > Java 11. It is a good point that this is about testing Java (the bytecode produced should target Java 8 exclusively - talking about Tomcat 9, for example -, and the behavior of the API should be the same; if either becomes false, then this is actually a Java bug). To be honest, I only partially trust that "release" feature right now. Although it seems to work, it might simply need time to get used to the concept ! I did point out the testsuite problem when this was initially discussed. And I understand the lack of trust if Tomcat is simply not tested on Java 8 (or 7, cough; let's pretend it is really tested ;) ) before release. Instead of completely reverting to the previous properties hack and reintroducing differences between branches, I would simply advocate moving the offending values to hardcoded property values in build.xml (which can then be replaced by Java 8 compatible placeholder values in your own build.properties - same idea than for the dependencies which can be replaced by Java 8 compatible ones). > I'm not concerned about that but neither am I too concerned about the > re-introduction of a few differences. I guess I am -0 overall provided > that any change does not impact the existing CI systems. Rémy - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch main updated: Follow up to 113b8ef. Correct check for *not* Windows.
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 d768a91 Follow up to 113b8ef. Correct check for *not* Windows.
d768a91 is described below
commit d768a915f5c2ba456d0776c04db4e54ea982a7d4
Author: Mark Thomas
AuthorDate: Thu Feb 10 16:44:48 2022 +
Follow up to 113b8ef. Correct check for *not* Windows.
---
java/org/apache/tomcat/util/net/Nio2Endpoint.java | 2 +-
java/org/apache/tomcat/util/net/NioEndpoint.java | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/java/org/apache/tomcat/util/net/Nio2Endpoint.java
b/java/org/apache/tomcat/util/net/Nio2Endpoint.java
index 11bd099..01f3c46 100644
--- a/java/org/apache/tomcat/util/net/Nio2Endpoint.java
+++ b/java/org/apache/tomcat/util/net/Nio2Endpoint.java
@@ -365,7 +365,7 @@ public class Nio2Endpoint extends
AbstractJsseEndpoint
SocketChannel result = serverSock.accept();
// Bug does not affect Windows. Skip the check on that platform.
-if (JrePlatform.IS_WINDOWS) {
+if (!JrePlatform.IS_WINDOWS) {
SocketAddress currentRemoteAddress = result.getRemoteAddress();
long currentNanoTime = System.nanoTime();
if
(currentRemoteAddress.equals(previousAcceptedSocketRemoteAddress) &&
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch main updated: Allow http response date field set to older date
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 e2c0fe3 Allow http response date field set to older date
e2c0fe3 is described below
commit e2c0fe327a96258aec6391be3b69b75a7ad8e45d
Author: zhenguoli
AuthorDate: Wed Jan 19 22:18:42 2022 +0800
Allow http response date field set to older date
---
java/org/apache/tomcat/util/http/FastHttpDateFormat.java | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/java/org/apache/tomcat/util/http/FastHttpDateFormat.java
b/java/org/apache/tomcat/util/http/FastHttpDateFormat.java
index f052474..76cffcd 100644
--- a/java/org/apache/tomcat/util/http/FastHttpDateFormat.java
+++ b/java/org/apache/tomcat/util/http/FastHttpDateFormat.java
@@ -94,7 +94,7 @@ public final class FastHttpDateFormat {
*/
public static final String getCurrentDate() {
long now = System.currentTimeMillis();
-if ((now - currentDateGenerated) > 1000) {
+if (Math.abs(now - currentDateGenerated) > 1000) {
currentDate = FORMAT_RFC5322.format(new Date(now));
currentDateGenerated = now;
}
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch main updated: Fix BZ 65776. Reduce false positives detecting duplicate accept bug
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 113be8f Fix BZ 65776. Reduce false positives detecting duplicate
accept bug
113be8f is described below
commit 113be8f7e133af77a81d381769bacc0584e29eb8
Author: Mark Thomas
AuthorDate: Thu Feb 10 16:02:05 2022 +
Fix BZ 65776. Reduce false positives detecting duplicate accept bug
https://bz.apache.org/bugzilla/show_bug.cgi?id=65776
---
java/org/apache/tomcat/util/net/Nio2Endpoint.java | 18 +-
java/org/apache/tomcat/util/net/NioEndpoint.java | 18 +-
webapps/docs/changelog.xml| 4
3 files changed, 30 insertions(+), 10 deletions(-)
diff --git a/java/org/apache/tomcat/util/net/Nio2Endpoint.java
b/java/org/apache/tomcat/util/net/Nio2Endpoint.java
index 687665b..11bd099 100644
--- a/java/org/apache/tomcat/util/net/Nio2Endpoint.java
+++ b/java/org/apache/tomcat/util/net/Nio2Endpoint.java
@@ -44,6 +44,7 @@ import org.apache.juli.logging.Log;
import org.apache.juli.logging.LogFactory;
import org.apache.tomcat.util.ExceptionUtils;
import org.apache.tomcat.util.collections.SynchronizedStack;
+import org.apache.tomcat.util.compat.JrePlatform;
import org.apache.tomcat.util.net.AbstractEndpoint.Handler.SocketState;
import org.apache.tomcat.util.net.Acceptor.AcceptorState;
import org.apache.tomcat.util.net.jsse.JSSESupport;
@@ -85,9 +86,10 @@ public class Nio2Endpoint extends
AbstractJsseEndpoint nioChannels;
private SocketAddress previousAcceptedSocketRemoteAddress = null;
+private long previouspreviousAcceptedSocketNanoTime = 0;
-// - Public Methods
+// - Public Methods
/**
* Number of keep-alive sockets.
@@ -362,11 +364,17 @@ public class Nio2Endpoint extends
AbstractJsseEndpoint
private SynchronizedStack nioChannels;
private SocketAddress previousAcceptedSocketRemoteAddress = null;
+private long previouspreviousAcceptedSocketNanoTime = 0;
// - Properties
-
/**
* Use System.inheritableChannel to obtain channel from stdin/stdout.
*/
@@ -516,11 +517,18 @@ public class NioEndpoint extends
AbstractJsseEndpoint
@Override
protected SocketChannel serverSocketAccept() throws Exception {
SocketChannel result = serverSock.accept();
-SocketAddress currentRemoteAddress = result.getRemoteAddress();
-if (currentRemoteAddress.equals(previousAcceptedSocketRemoteAddress)) {
-throw new
IOException(sm.getString("endpoint.err.duplicateAccept"));
+
+// Bug does not affect Windows. Skip the check on that platform.
+if (JrePlatform.IS_WINDOWS) {
+SocketAddress currentRemoteAddress = result.getRemoteAddress();
+long currentNanoTime = System.nanoTime();
+if
(currentRemoteAddress.equals(previousAcceptedSocketRemoteAddress) &&
+currentNanoTime - previouspreviousAcceptedSocketNanoTime <
1000) {
+throw new
IOException(sm.getString("endpoint.err.duplicateAccept"));
+}
+previousAcceptedSocketRemoteAddress = currentRemoteAddress;
+previouspreviousAcceptedSocketNanoTime = currentNanoTime;
}
-previousAcceptedSocketRemoteAddress = currentRemoteAddress;
return result;
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 00b3af8..280f03b 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -143,6 +143,10 @@
ignored when the Connector used an internal executor. (markt)
+65776: Improve the detection of the Linux duplicate accept
+bug and reduce (hopefully avoid) instances of false positives. (markt)
+
+
65848: Revert the change that attempted to align the
behaviour of client certificate authentication with NIO or NIO2 with
OpenSSL for TLS between MacOS and Linux/Windows as the root cause was
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch 8.5.x updated: Fix copy/paste error. Fix backport.
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/8.5.x by this push:
new defad3a Fix copy/paste error. Fix backport.
defad3a is described below
commit defad3a508c730c283bd550d644aac04ba03df20
Author: Mark Thomas
AuthorDate: Thu Feb 10 17:16:30 2022 +
Fix copy/paste error. Fix backport.
---
java/org/apache/tomcat/util/net/AprEndpoint.java | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/java/org/apache/tomcat/util/net/AprEndpoint.java
b/java/org/apache/tomcat/util/net/AprEndpoint.java
index 0c47367..8653707 100644
--- a/java/org/apache/tomcat/util/net/AprEndpoint.java
+++ b/java/org/apache/tomcat/util/net/AprEndpoint.java
@@ -110,7 +110,7 @@ public class AprEndpoint extends AbstractEndpoint
implements SNICallBack {
private int previousAcceptedPort = -1;
private String previousAcceptedAddress = null;
-private long previouspreviousAcceptedSocketNanoTime = 0;
+private long previousAcceptedSocketNanoTime = 0;
@@ -780,13 +780,14 @@ public class AprEndpoint extends AbstractEndpoint
implements SNICallBack {
long currentNanoTime = System.nanoTime();
if (wrapper.getRemotePort() == previousAcceptedPort) {
if
(wrapper.getRemoteAddr().equals(previousAcceptedAddress)) {
-if (currentNanoTime -
previouspreviousAcceptedSocketNanoTime < 1000) {
+if (currentNanoTime -
previousAcceptedSocketNanoTime < 1000) {
throw new
IOException(sm.getString("endpoint.err.duplicateAccept"));
}
}
}
previousAcceptedPort = wrapper.getRemotePort();
previousAcceptedAddress = wrapper.getRemoteAddr();
+previousAcceptedSocketNanoTime = currentNanoTime;
}
wrapper.setKeepAliveLeft(getMaxKeepAliveRequests());
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 65776] "Duplicate accept detected" error from a subsequent request with the same local port
https://bz.apache.org/bugzilla/show_bug.cgi?id=65776 Mark Thomas changed: What|Removed |Added Resolution|--- |FIXED Status|NEW |RESOLVED --- Comment #16 from Mark Thomas --- Fixed in: - 10.1.x for 10.1.0-M11 onwards - 10.0.x for 10.0.17 onwards - 9.0.x for 9.0.59 onwards - 8.5.x for 8.5.76 onwards -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch main updated: Update changelog for #467
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 a754fc0 Update changelog for #467
a754fc0 is described below
commit a754fc05208215bde7016c0e274b0f845dc457e1
Author: Mark Thomas
AuthorDate: Thu Feb 10 17:24:49 2022 +
Update changelog for #467
---
java/org/apache/tomcat/util/http/FastHttpDateFormat.java | 1 +
webapps/docs/changelog.xml | 6 ++
2 files changed, 7 insertions(+)
diff --git a/java/org/apache/tomcat/util/http/FastHttpDateFormat.java
b/java/org/apache/tomcat/util/http/FastHttpDateFormat.java
index 76cffcd..932208b 100644
--- a/java/org/apache/tomcat/util/http/FastHttpDateFormat.java
+++ b/java/org/apache/tomcat/util/http/FastHttpDateFormat.java
@@ -94,6 +94,7 @@ public final class FastHttpDateFormat {
*/
public static final String getCurrentDate() {
long now = System.currentTimeMillis();
+// Handle case where time moves backwards (e.g. system time corrected)
if (Math.abs(now - currentDateGenerated) > 1000) {
currentDate = FORMAT_RFC5322.format(new Date(now));
currentDateGenerated = now;
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 280f03b..0bf6ed7 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -152,6 +152,12 @@
OpenSSL for TLS between MacOS and Linux/Windows as the root cause was
traced to configuration differences. (markt)
+
+467: When system time moves backwards (e.g. after clock
+correction), ensure that the cached formatted current date used for
+HTTP headers tracks this change. Pull request provided by zhenguoli.
+(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: Fix BZ 65776. Reduce false positives detecting duplicate accept bug
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 2b9d155 Fix BZ 65776. Reduce false positives detecting duplicate
accept bug
2b9d155 is described below
commit 2b9d1552ca23fbd197cec5803561ecb7f10649e0
Author: Mark Thomas
AuthorDate: Thu Feb 10 16:02:05 2022 +
Fix BZ 65776. Reduce false positives detecting duplicate accept bug
https://bz.apache.org/bugzilla/show_bug.cgi?id=65776
---
java/org/apache/tomcat/util/net/AprEndpoint.java | 22 --
java/org/apache/tomcat/util/net/Nio2Endpoint.java | 18 +-
java/org/apache/tomcat/util/net/NioEndpoint.java | 18 +-
webapps/docs/changelog.xml| 4
4 files changed, 46 insertions(+), 16 deletions(-)
diff --git a/java/org/apache/tomcat/util/net/AprEndpoint.java
b/java/org/apache/tomcat/util/net/AprEndpoint.java
index ef6dcdf..ea025f4 100644
--- a/java/org/apache/tomcat/util/net/AprEndpoint.java
+++ b/java/org/apache/tomcat/util/net/AprEndpoint.java
@@ -62,6 +62,7 @@ import org.apache.tomcat.jni.Status;
import org.apache.tomcat.util.ExceptionUtils;
import org.apache.tomcat.util.buf.ByteBufferUtils;
import org.apache.tomcat.util.collections.SynchronizedStack;
+import org.apache.tomcat.util.compat.JrePlatform;
import org.apache.tomcat.util.net.AbstractEndpoint.Handler.SocketState;
import org.apache.tomcat.util.net.Acceptor.AcceptorState;
import org.apache.tomcat.util.net.openssl.OpenSSLContext;
@@ -118,6 +119,8 @@ public class AprEndpoint extends
AbstractEndpoint implements SNICallB
private int previousAcceptedPort = -1;
private String previousAcceptedAddress = null;
+private long previouspreviousAcceptedSocketNanoTime = 0;
+
// Constructor
@@ -129,8 +132,8 @@ public class AprEndpoint extends
AbstractEndpoint implements SNICallB
setUseAsyncIO(false);
}
-// - Properties
+// - Properties
/**
* Defer accept.
@@ -810,13 +813,20 @@ public class AprEndpoint extends
AbstractEndpoint implements SNICallB
// Do the duplicate accept check here rather than in
serverSocketaccept()
// so we can cache the results in the SocketWrapper
AprSocketWrapper wrapper = new AprSocketWrapper(socket, this);
-if (wrapper.getRemotePort() == previousAcceptedPort) {
-if (wrapper.getRemoteAddr().equals(previousAcceptedAddress)) {
-throw new
IOException(sm.getString("endpoint.err.duplicateAccept"));
+// Bug does not affect Windows. Skip the check on that platform.
+if (!JrePlatform.IS_WINDOWS) {
+long currentNanoTime = System.nanoTime();
+if (wrapper.getRemotePort() == previousAcceptedPort) {
+if
(wrapper.getRemoteAddr().equals(previousAcceptedAddress)) {
+if (currentNanoTime -
previouspreviousAcceptedSocketNanoTime < 1000) {
+throw new
IOException(sm.getString("endpoint.err.duplicateAccept"));
+}
+}
}
+previousAcceptedPort = wrapper.getRemotePort();
+previousAcceptedAddress = wrapper.getRemoteAddr();
+previouspreviousAcceptedSocketNanoTime = currentNanoTime;
}
-previousAcceptedPort = wrapper.getRemotePort();
-previousAcceptedAddress = wrapper.getRemoteAddr();
connections.put(socket, wrapper);
wrapper.setKeepAliveLeft(getMaxKeepAliveRequests());
diff --git a/java/org/apache/tomcat/util/net/Nio2Endpoint.java
b/java/org/apache/tomcat/util/net/Nio2Endpoint.java
index 00719a6..23b9434 100644
--- a/java/org/apache/tomcat/util/net/Nio2Endpoint.java
+++ b/java/org/apache/tomcat/util/net/Nio2Endpoint.java
@@ -44,6 +44,7 @@ import org.apache.juli.logging.Log;
import org.apache.juli.logging.LogFactory;
import org.apache.tomcat.util.ExceptionUtils;
import org.apache.tomcat.util.collections.SynchronizedStack;
+import org.apache.tomcat.util.compat.JrePlatform;
import org.apache.tomcat.util.net.AbstractEndpoint.Handler.SocketState;
import org.apache.tomcat.util.net.Acceptor.AcceptorState;
import org.apache.tomcat.util.net.jsse.JSSESupport;
@@ -85,9 +86,10 @@ public class Nio2Endpoint extends
AbstractJsseEndpoint nioChannels;
private SocketAddress previousAcceptedSocketRemoteAddress = null;
+private long previouspreviousAcceptedSocketNanoTime = 0;
-// - Public Methods
+//
[tomcat] branch 9.0.x updated: Fix BZ 65776. Reduce false positives detecting duplicate accept bug
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 5a3c6f4 Fix BZ 65776. Reduce false positives detecting duplicate
accept bug
5a3c6f4 is described below
commit 5a3c6f46568086e1e2503bb7fb0d48b9356e4217
Author: Mark Thomas
AuthorDate: Thu Feb 10 16:02:05 2022 +
Fix BZ 65776. Reduce false positives detecting duplicate accept bug
https://bz.apache.org/bugzilla/show_bug.cgi?id=65776
---
java/org/apache/tomcat/util/net/AprEndpoint.java | 22 --
java/org/apache/tomcat/util/net/Nio2Endpoint.java | 16
java/org/apache/tomcat/util/net/NioEndpoint.java | 18 +-
webapps/docs/changelog.xml| 4
4 files changed, 45 insertions(+), 15 deletions(-)
diff --git a/java/org/apache/tomcat/util/net/AprEndpoint.java
b/java/org/apache/tomcat/util/net/AprEndpoint.java
index 01c2b77..6d0f10c 100644
--- a/java/org/apache/tomcat/util/net/AprEndpoint.java
+++ b/java/org/apache/tomcat/util/net/AprEndpoint.java
@@ -62,6 +62,7 @@ import org.apache.tomcat.jni.Status;
import org.apache.tomcat.util.ExceptionUtils;
import org.apache.tomcat.util.buf.ByteBufferUtils;
import org.apache.tomcat.util.collections.SynchronizedStack;
+import org.apache.tomcat.util.compat.JrePlatform;
import org.apache.tomcat.util.net.AbstractEndpoint.Handler.SocketState;
import org.apache.tomcat.util.net.Acceptor.AcceptorState;
import org.apache.tomcat.util.net.openssl.OpenSSLContext;
@@ -114,6 +115,8 @@ public class AprEndpoint extends
AbstractEndpoint implements SNICallB
private int previousAcceptedPort = -1;
private String previousAcceptedAddress = null;
+private long previouspreviousAcceptedSocketNanoTime = 0;
+
// Constructor
@@ -125,8 +128,8 @@ public class AprEndpoint extends
AbstractEndpoint implements SNICallB
setUseAsyncIO(false);
}
-// - Properties
+// - Properties
/**
* Defer accept.
@@ -806,13 +809,20 @@ public class AprEndpoint extends
AbstractEndpoint implements SNICallB
// Do the duplicate accept check here rather than in
serverSocketaccept()
// so we can cache the results in the SocketWrapper
AprSocketWrapper wrapper = new AprSocketWrapper(socket, this);
-if (wrapper.getRemotePort() == previousAcceptedPort) {
-if (wrapper.getRemoteAddr().equals(previousAcceptedAddress)) {
-throw new
IOException(sm.getString("endpoint.err.duplicateAccept"));
+// Bug does not affect Windows. Skip the check on that platform.
+if (!JrePlatform.IS_WINDOWS) {
+long currentNanoTime = System.nanoTime();
+if (wrapper.getRemotePort() == previousAcceptedPort) {
+if
(wrapper.getRemoteAddr().equals(previousAcceptedAddress)) {
+if (currentNanoTime -
previouspreviousAcceptedSocketNanoTime < 1000) {
+throw new
IOException(sm.getString("endpoint.err.duplicateAccept"));
+}
+}
}
+previousAcceptedPort = wrapper.getRemotePort();
+previousAcceptedAddress = wrapper.getRemoteAddr();
+previouspreviousAcceptedSocketNanoTime = currentNanoTime;
}
-previousAcceptedPort = wrapper.getRemotePort();
-previousAcceptedAddress = wrapper.getRemoteAddr();
connections.put(socket, wrapper);
wrapper.setKeepAliveLeft(getMaxKeepAliveRequests());
diff --git a/java/org/apache/tomcat/util/net/Nio2Endpoint.java
b/java/org/apache/tomcat/util/net/Nio2Endpoint.java
index 0f6d8a0..f82f738 100644
--- a/java/org/apache/tomcat/util/net/Nio2Endpoint.java
+++ b/java/org/apache/tomcat/util/net/Nio2Endpoint.java
@@ -44,6 +44,7 @@ import org.apache.juli.logging.Log;
import org.apache.juli.logging.LogFactory;
import org.apache.tomcat.util.ExceptionUtils;
import org.apache.tomcat.util.collections.SynchronizedStack;
+import org.apache.tomcat.util.compat.JrePlatform;
import org.apache.tomcat.util.net.AbstractEndpoint.Handler.SocketState;
import org.apache.tomcat.util.net.Acceptor.AcceptorState;
import org.apache.tomcat.util.net.jsse.JSSESupport;
@@ -85,6 +86,7 @@ public class Nio2Endpoint extends
AbstractJsseEndpoint nioChannels;
private SocketAddress previousAcceptedSocketRemoteAddress = null;
+private long previouspreviousAcceptedSocketNanoTime = 0;
// - Properties
@@ -373,11 +375,17 @@ public class
Re: Running unit tests with JVMs < 11
Am 10.02.2022 um 14:41 schrieb Rémy Maucherat: On Thu, Feb 10, 2022 at 2:11 PM Mark Thomas wrote: On 09/02/2022 14:04, Rainer Jung wrote: Hi all, I wonder how important it is to be able to run the unit tests for TC 8.5-10.0 with JVM versions 8 or even 7 (TC 8.5). The unconditional addition of unit test jvmargs "--add-opens=..." in 2b6e19e971a980e38bcd30f05c554d9b798666c0 (TC 10, backported) means, we can no longer run the tests with Java 8 or even 7. If I remove those jvmargs from build.xml, I run into a second problem when testing TC 8.5 with Java 7. In 014f4746176c7f27cda7c04b2c6036a539c385ff EasyMock was updated from 3.x to 4.x, which needs Java 8 to run. Several tests fail with UnsupportedClassVersionError in a few EasyMock classes. I can work around all of these myself for running the tests here, but I wonder, what kind of setup for testing we want to support out-of-the box. Note that by reintroducing the previous compatibility trick for jvmargs (TC 8.5, 9 and 10) and using the old EasyMock branch for TC 8.5 we could still build and test with Java 11, but would also allow testing on the other target Java versions we support. WDYT? I've been thinking about this since I read this yesterday. I like that the move to Java 11 allowed us to reduce the differences between the build scripts. +1 So the decision is: Is the benefit of being able to test with older Java versions worth the differences this would re-introduce? We don't test the JreCompat code so there aren't any Java version specific tests. So what do we gain by testing on Java 7 / Java 8? I can think of a few things: - that we really have coded to the correct Java version - that the code runs as expected on that Java version For example TLS behavior. There are functionalities that have the same API in different Java versions, but implementations provide different behavior. Both of those things are more testing Java than testing Tomcat but I can see that there is some benefit - especially if one is concerned about the possibility of the code behaving differently on Java 7/8 compared to Java 11. It is a good point that this is about testing Java (the bytecode produced should target Java 8 exclusively - talking about Tomcat 9, for example -, and the behavior of the API should be the same; if either becomes false, then this is actually a Java bug). Concerning the same API behavior, wouldn't changes in TLS support qualify as such a change which is not a bug? Of course, most of the time Tomcat would be agnostic in itself to such changes, but I wouldn't be so sure that we would not every now and then find a relevant difference via the unit tests. To be honest, I only partially trust that "release" feature right now. Although it seems to work, it might simply need time to get used to the concept ! I did point out the testsuite problem when this was initially discussed. And I understand the lack of trust if Tomcat is simply not tested on Java 8 (or 7, cough; let's pretend it is really tested ;) ) before release. Instead of completely reverting to the previous properties hack and reintroducing differences between branches, I would simply advocate moving the offending values to hardcoded property values in build.xml (which can then be replaced by Java 8 compatible placeholder values in your own build.properties - same idea than for the dependencies which can be replaced by Java 8 compatible ones). Sounds like a reasonable compromise. If we would find over time more reasons to actually run the tests, would it be easy to define JVM version specific build.properties lines on CI nodes? No idea, whether we already overwrite build properties on CI. I'm not concerned about that but neither am I too concerned about the re-introduction of a few differences. I guess I am -0 overall provided that any change does not impact the existing CI systems. Rémy Thanks and regards! Rainer - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch 9.0.x updated: Fix copy/paste error
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 e4b4054 Fix copy/paste error
e4b4054 is described below
commit e4b4054948c9770ff8263c5023514546e16d6335
Author: Mark Thomas
AuthorDate: Thu Feb 10 17:12:58 2022 +
Fix copy/paste error
---
java/org/apache/tomcat/util/net/AprEndpoint.java | 6 +++---
java/org/apache/tomcat/util/net/Nio2Endpoint.java | 6 +++---
java/org/apache/tomcat/util/net/NioEndpoint.java | 6 +++---
3 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/java/org/apache/tomcat/util/net/AprEndpoint.java
b/java/org/apache/tomcat/util/net/AprEndpoint.java
index 6d0f10c..1b5f170 100644
--- a/java/org/apache/tomcat/util/net/AprEndpoint.java
+++ b/java/org/apache/tomcat/util/net/AprEndpoint.java
@@ -115,7 +115,7 @@ public class AprEndpoint extends
AbstractEndpoint implements SNICallB
private int previousAcceptedPort = -1;
private String previousAcceptedAddress = null;
-private long previouspreviousAcceptedSocketNanoTime = 0;
+private long previousAcceptedSocketNanoTime = 0;
// Constructor
@@ -814,14 +814,14 @@ public class AprEndpoint extends
AbstractEndpoint implements SNICallB
long currentNanoTime = System.nanoTime();
if (wrapper.getRemotePort() == previousAcceptedPort) {
if
(wrapper.getRemoteAddr().equals(previousAcceptedAddress)) {
-if (currentNanoTime -
previouspreviousAcceptedSocketNanoTime < 1000) {
+if (currentNanoTime - previousAcceptedSocketNanoTime <
1000) {
throw new
IOException(sm.getString("endpoint.err.duplicateAccept"));
}
}
}
previousAcceptedPort = wrapper.getRemotePort();
previousAcceptedAddress = wrapper.getRemoteAddr();
-previouspreviousAcceptedSocketNanoTime = currentNanoTime;
+previousAcceptedSocketNanoTime = currentNanoTime;
}
connections.put(socket, wrapper);
diff --git a/java/org/apache/tomcat/util/net/Nio2Endpoint.java
b/java/org/apache/tomcat/util/net/Nio2Endpoint.java
index f82f738..49ee411 100644
--- a/java/org/apache/tomcat/util/net/Nio2Endpoint.java
+++ b/java/org/apache/tomcat/util/net/Nio2Endpoint.java
@@ -86,7 +86,7 @@ public class Nio2Endpoint extends
AbstractJsseEndpoint nioChannels;
private SocketAddress previousAcceptedSocketRemoteAddress = null;
-private long previouspreviousAcceptedSocketNanoTime = 0;
+private long previousAcceptedSocketNanoTime = 0;
// - Properties
@@ -380,11 +380,11 @@ public class Nio2Endpoint extends
AbstractJsseEndpoint
private SynchronizedStack nioChannels;
private SocketAddress previousAcceptedSocketRemoteAddress = null;
-private long previouspreviousAcceptedSocketNanoTime = 0;
+private long previousAcceptedSocketNanoTime = 0;
// - Properties
@@ -550,11 +550,11 @@ public class NioEndpoint extends
AbstractJsseEndpoint
SocketAddress currentRemoteAddress = result.getRemoteAddress();
long currentNanoTime = System.nanoTime();
if
(currentRemoteAddress.equals(previousAcceptedSocketRemoteAddress) &&
-currentNanoTime - previouspreviousAcceptedSocketNanoTime <
1000) {
+currentNanoTime - previousAcceptedSocketNanoTime < 1000) {
throw new
IOException(sm.getString("endpoint.err.duplicateAccept"));
}
previousAcceptedSocketRemoteAddress = currentRemoteAddress;
-previouspreviousAcceptedSocketNanoTime = currentNanoTime;
+previousAcceptedSocketNanoTime = currentNanoTime;
}
return result;
-
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: Fix copy/paste error
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 5561fd6 Fix copy/paste error
5561fd6 is described below
commit 5561fd6a1d8762eef7c3aedf2cd1f37856453fdc
Author: Mark Thomas
AuthorDate: Thu Feb 10 17:12:32 2022 +
Fix copy/paste error
---
java/org/apache/tomcat/util/net/AprEndpoint.java | 6 +++---
java/org/apache/tomcat/util/net/Nio2Endpoint.java | 6 +++---
java/org/apache/tomcat/util/net/NioEndpoint.java | 6 +++---
3 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/java/org/apache/tomcat/util/net/AprEndpoint.java
b/java/org/apache/tomcat/util/net/AprEndpoint.java
index ea025f4..7e9f994 100644
--- a/java/org/apache/tomcat/util/net/AprEndpoint.java
+++ b/java/org/apache/tomcat/util/net/AprEndpoint.java
@@ -119,7 +119,7 @@ public class AprEndpoint extends
AbstractEndpoint implements SNICallB
private int previousAcceptedPort = -1;
private String previousAcceptedAddress = null;
-private long previouspreviousAcceptedSocketNanoTime = 0;
+private long previousAcceptedSocketNanoTime = 0;
// Constructor
@@ -818,14 +818,14 @@ public class AprEndpoint extends
AbstractEndpoint implements SNICallB
long currentNanoTime = System.nanoTime();
if (wrapper.getRemotePort() == previousAcceptedPort) {
if
(wrapper.getRemoteAddr().equals(previousAcceptedAddress)) {
-if (currentNanoTime -
previouspreviousAcceptedSocketNanoTime < 1000) {
+if (currentNanoTime - previousAcceptedSocketNanoTime <
1000) {
throw new
IOException(sm.getString("endpoint.err.duplicateAccept"));
}
}
}
previousAcceptedPort = wrapper.getRemotePort();
previousAcceptedAddress = wrapper.getRemoteAddr();
-previouspreviousAcceptedSocketNanoTime = currentNanoTime;
+previousAcceptedSocketNanoTime = currentNanoTime;
}
connections.put(socket, wrapper);
diff --git a/java/org/apache/tomcat/util/net/Nio2Endpoint.java
b/java/org/apache/tomcat/util/net/Nio2Endpoint.java
index 23b9434..8ee9049 100644
--- a/java/org/apache/tomcat/util/net/Nio2Endpoint.java
+++ b/java/org/apache/tomcat/util/net/Nio2Endpoint.java
@@ -86,7 +86,7 @@ public class Nio2Endpoint extends
AbstractJsseEndpoint nioChannels;
private SocketAddress previousAcceptedSocketRemoteAddress = null;
-private long previouspreviousAcceptedSocketNanoTime = 0;
+private long previousAcceptedSocketNanoTime = 0;
// - Public Methods
@@ -369,11 +369,11 @@ public class Nio2Endpoint extends
AbstractJsseEndpoint
private SynchronizedStack nioChannels;
private SocketAddress previousAcceptedSocketRemoteAddress = null;
-private long previouspreviousAcceptedSocketNanoTime = 0;
+private long previousAcceptedSocketNanoTime = 0;
// - Properties
@@ -523,11 +523,11 @@ public class NioEndpoint extends
AbstractJsseEndpoint
SocketAddress currentRemoteAddress = result.getRemoteAddress();
long currentNanoTime = System.nanoTime();
if
(currentRemoteAddress.equals(previousAcceptedSocketRemoteAddress) &&
-currentNanoTime - previouspreviousAcceptedSocketNanoTime <
1000) {
+currentNanoTime - previousAcceptedSocketNanoTime < 1000) {
throw new
IOException(sm.getString("endpoint.err.duplicateAccept"));
}
previousAcceptedSocketRemoteAddress = currentRemoteAddress;
-previouspreviousAcceptedSocketNanoTime = currentNanoTime;
+previousAcceptedSocketNanoTime = currentNanoTime;
}
return result;
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] markt-asf merged pull request #467: Allow http response date field set to older date
markt-asf merged pull request #467: URL: https://github.com/apache/tomcat/pull/467 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch 9.0.x updated: Allow http response date field set to older date
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 11b19fd Allow http response date field set to older date
11b19fd is described below
commit 11b19fd861231dfe256c024fdcc3cdb6dc7e17b3
Author: zhenguoli
AuthorDate: Wed Jan 19 22:18:42 2022 +0800
Allow http response date field set to older date
---
java/org/apache/tomcat/util/http/FastHttpDateFormat.java | 3 ++-
webapps/docs/changelog.xml | 6 ++
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/java/org/apache/tomcat/util/http/FastHttpDateFormat.java
b/java/org/apache/tomcat/util/http/FastHttpDateFormat.java
index a546ee6..73c578e 100644
--- a/java/org/apache/tomcat/util/http/FastHttpDateFormat.java
+++ b/java/org/apache/tomcat/util/http/FastHttpDateFormat.java
@@ -103,7 +103,8 @@ public final class FastHttpDateFormat {
*/
public static final String getCurrentDate() {
long now = System.currentTimeMillis();
-if ((now - currentDateGenerated) > 1000) {
+// Handle case where time moves backwards (e.g. system time corrected)
+if (Math.abs(now - currentDateGenerated) > 1000) {
currentDate = FORMAT_RFC5322.format(new Date(now));
currentDateGenerated = now;
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index b7bfc09..5b1e395 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -152,6 +152,12 @@
OpenSSL for TLS between MacOS and Linux/Windows as the root cause was
traced to configuration differences. (markt)
+
+467: When system time moves backwards (e.g. after clock
+correction), ensure that the cached formatted current date used for
+HTTP headers tracks this change. Pull request provided by zhenguoli.
+(markt)
+
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch 8.5.x updated: Fix BZ 65776. Reduce false positives detecting duplicate accept bug
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/8.5.x by this push:
new bebb779 Fix BZ 65776. Reduce false positives detecting duplicate
accept bug
bebb779 is described below
commit bebb779cd8a3c7402b07f5ff12f2a6c02bd2698b
Author: Mark Thomas
AuthorDate: Thu Feb 10 16:02:05 2022 +
Fix BZ 65776. Reduce false positives detecting duplicate accept bug
https://bz.apache.org/bugzilla/show_bug.cgi?id=65776
---
java/org/apache/tomcat/util/net/AprEndpoint.java | 21 +++--
java/org/apache/tomcat/util/net/Nio2Endpoint.java | 17 -
java/org/apache/tomcat/util/net/NioEndpoint.java | 17 -
webapps/docs/changelog.xml| 4
4 files changed, 43 insertions(+), 16 deletions(-)
diff --git a/java/org/apache/tomcat/util/net/AprEndpoint.java
b/java/org/apache/tomcat/util/net/AprEndpoint.java
index 001f8f6..0c47367 100644
--- a/java/org/apache/tomcat/util/net/AprEndpoint.java
+++ b/java/org/apache/tomcat/util/net/AprEndpoint.java
@@ -57,6 +57,7 @@ import org.apache.tomcat.jni.Status;
import org.apache.tomcat.util.ExceptionUtils;
import org.apache.tomcat.util.buf.ByteBufferUtils;
import org.apache.tomcat.util.collections.SynchronizedStack;
+import org.apache.tomcat.util.compat.JrePlatform;
import org.apache.tomcat.util.net.AbstractEndpoint.Acceptor.AcceptorState;
import org.apache.tomcat.util.net.AbstractEndpoint.Handler.SocketState;
import org.apache.tomcat.util.net.openssl.OpenSSLContext;
@@ -109,6 +110,8 @@ public class AprEndpoint extends AbstractEndpoint
implements SNICallBack {
private int previousAcceptedPort = -1;
private String previousAcceptedAddress = null;
+private long previouspreviousAcceptedSocketNanoTime = 0;
+
private final Map connections = new
ConcurrentHashMap<>();
@@ -127,8 +130,8 @@ public class AprEndpoint extends AbstractEndpoint
implements SNICallBack {
setMaxConnections(8 * 1024);
}
-// - Properties
+// - Properties
/**
* Defer accept.
@@ -772,13 +775,19 @@ public class AprEndpoint extends AbstractEndpoint
implements SNICallBack {
// Do the duplicate accept check here rather than in
Acceptor.run()
// so we can cache the results in the SocketWrapper
AprSocketWrapper wrapper = new
AprSocketWrapper(Long.valueOf(socket), this);
-if (wrapper.getRemotePort() == previousAcceptedPort) {
-if
(wrapper.getRemoteAddr().equals(previousAcceptedAddress)) {
-throw new
IOException(sm.getString("endpoint.err.duplicateAccept"));
+// Bug does not affect Windows. Skip the check on that
platform.
+if (!JrePlatform.IS_WINDOWS) {
+long currentNanoTime = System.nanoTime();
+if (wrapper.getRemotePort() == previousAcceptedPort) {
+if
(wrapper.getRemoteAddr().equals(previousAcceptedAddress)) {
+if (currentNanoTime -
previouspreviousAcceptedSocketNanoTime < 1000) {
+throw new
IOException(sm.getString("endpoint.err.duplicateAccept"));
+}
+}
}
+previousAcceptedPort = wrapper.getRemotePort();
+previousAcceptedAddress = wrapper.getRemoteAddr();
}
-previousAcceptedPort = wrapper.getRemotePort();
-previousAcceptedAddress = wrapper.getRemoteAddr();
wrapper.setKeepAliveLeft(getMaxKeepAliveRequests());
wrapper.setReadTimeout(getConnectionTimeout());
diff --git a/java/org/apache/tomcat/util/net/Nio2Endpoint.java
b/java/org/apache/tomcat/util/net/Nio2Endpoint.java
index 6eddfd7..30c054d 100644
--- a/java/org/apache/tomcat/util/net/Nio2Endpoint.java
+++ b/java/org/apache/tomcat/util/net/Nio2Endpoint.java
@@ -44,6 +44,7 @@ import org.apache.juli.logging.Log;
import org.apache.juli.logging.LogFactory;
import org.apache.tomcat.util.ExceptionUtils;
import org.apache.tomcat.util.collections.SynchronizedStack;
+import org.apache.tomcat.util.compat.JrePlatform;
import org.apache.tomcat.util.net.AbstractEndpoint.Handler.SocketState;
import org.apache.tomcat.util.net.jsse.JSSESupport;
@@ -84,6 +85,7 @@ public class Nio2Endpoint extends
AbstractJsseEndpoint {
private SynchronizedStack nioChannels;
private SocketAddress previousAcceptedSocketRemoteAddress = null;
+private long previousAcceptedSocketNanoTime = 0;
public Nio2Endpoint() {
@@ -414,12 +416,17 @@ public class Nio2Endpoint extends
[tomcat] branch 8.5.x updated: Allow http response date field set to older date
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/8.5.x by this push:
new 9a9a10d Allow http response date field set to older date
9a9a10d is described below
commit 9a9a10d874bfeb844eee1a987b47bf5701b766f1
Author: zhenguoli
AuthorDate: Wed Jan 19 22:18:42 2022 +0800
Allow http response date field set to older date
---
java/org/apache/tomcat/util/http/FastHttpDateFormat.java | 3 ++-
webapps/docs/changelog.xml | 6 ++
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/java/org/apache/tomcat/util/http/FastHttpDateFormat.java
b/java/org/apache/tomcat/util/http/FastHttpDateFormat.java
index a546ee6..73c578e 100644
--- a/java/org/apache/tomcat/util/http/FastHttpDateFormat.java
+++ b/java/org/apache/tomcat/util/http/FastHttpDateFormat.java
@@ -103,7 +103,8 @@ public final class FastHttpDateFormat {
*/
public static final String getCurrentDate() {
long now = System.currentTimeMillis();
-if ((now - currentDateGenerated) > 1000) {
+// Handle case where time moves backwards (e.g. system time corrected)
+if (Math.abs(now - currentDateGenerated) > 1000) {
currentDate = FORMAT_RFC5322.format(new Date(now));
currentDateGenerated = now;
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 093ea78..e1a2526 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -147,6 +147,12 @@
OpenSSL for TLS between MacOS and Linux/Windows as the root cause was
traced to configuration differences. (markt)
+
+467: When system time moves backwards (e.g. after clock
+correction), ensure that the cached formatted current date used for
+HTTP headers tracks this change. Pull request provided by zhenguoli.
+(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: Allow http response date field set to older date
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 2286276 Allow http response date field set to older date
2286276 is described below
commit 22862766981076d0800aa01cac2e6eda175a6eba
Author: zhenguoli
AuthorDate: Wed Jan 19 22:18:42 2022 +0800
Allow http response date field set to older date
---
java/org/apache/tomcat/util/http/FastHttpDateFormat.java | 3 ++-
webapps/docs/changelog.xml | 6 ++
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/java/org/apache/tomcat/util/http/FastHttpDateFormat.java
b/java/org/apache/tomcat/util/http/FastHttpDateFormat.java
index f052474..932208b 100644
--- a/java/org/apache/tomcat/util/http/FastHttpDateFormat.java
+++ b/java/org/apache/tomcat/util/http/FastHttpDateFormat.java
@@ -94,7 +94,8 @@ public final class FastHttpDateFormat {
*/
public static final String getCurrentDate() {
long now = System.currentTimeMillis();
-if ((now - currentDateGenerated) > 1000) {
+// Handle case where time moves backwards (e.g. system time corrected)
+if (Math.abs(now - currentDateGenerated) > 1000) {
currentDate = FORMAT_RFC5322.format(new Date(now));
currentDateGenerated = now;
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index cf4b3c8..1b7aac1 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -152,6 +152,12 @@
OpenSSL for TLS between MacOS and Linux/Windows as the root cause was
traced to configuration differences. (markt)
+
+467: When system time moves backwards (e.g. after clock
+correction), ensure that the cached formatted current date used for
+HTTP headers tracks this change. Pull request provided by zhenguoli.
+(markt)
+
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] markt-asf commented on pull request #464: avoid null classloader when thread context CL is null
markt-asf commented on pull request #464: URL: https://github.com/apache/tomcat/pull/464#issuecomment-1035330398 For Tomcat `getClass().getClassLoader()` will be the same as `ClassLoader.getSystemClassLoader()` but I agree that generally, `getClass().getClassLoader()` is the better choice. I'm working on a slightly wider patch that makes this more consistent throughout JULI. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch 9.0.x updated: Based on #464. Avoid using null for class loader during lookups
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 7046c20 Based on #464. Avoid using null for class loader during
lookups
7046c20 is described below
commit 7046c204bd663ea104c7f54710e6ef410c792c73
Author: Mark Thomas
AuthorDate: Thu Feb 10 19:53:15 2022 +
Based on #464. Avoid using null for class loader during lookups
Fall back to the class loader used to load JULI when the thread context
class loader is not set. In a normal Tomcat configuration, this will be
the system class loader.
Based on a pull request by jackshirazi.
---
java/org/apache/juli/ClassLoaderLogManager.java | 49 +
java/org/apache/juli/FileHandler.java | 2 +-
webapps/docs/changelog.xml | 6 +++
3 files changed, 40 insertions(+), 17 deletions(-)
diff --git a/java/org/apache/juli/ClassLoaderLogManager.java
b/java/org/apache/juli/ClassLoaderLogManager.java
index 227b4e1..b290972 100644
--- a/java/org/apache/juli/ClassLoaderLogManager.java
+++ b/java/org/apache/juli/ClassLoaderLogManager.java
@@ -145,8 +145,7 @@ public class ClassLoaderLogManager extends LogManager {
final String loggerName = logger.getName();
-ClassLoader classLoader =
-Thread.currentThread().getContextClassLoader();
+ClassLoader classLoader = getClassLoader();
ClassLoaderLogInfo info = getClassLoaderInfo(classLoader);
if (info.loggers.containsKey(loggerName)) {
return false;
@@ -236,8 +235,7 @@ public class ClassLoaderLogManager extends LogManager {
*/
@Override
public synchronized Logger getLogger(final String name) {
-ClassLoader classLoader = Thread.currentThread()
-.getContextClassLoader();
+ClassLoader classLoader = getClassLoader();
return getClassLoaderInfo(classLoader).loggers.get(name);
}
@@ -248,8 +246,7 @@ public class ClassLoaderLogManager extends LogManager {
*/
@Override
public synchronized Enumeration getLoggerNames() {
-ClassLoader classLoader = Thread.currentThread()
-.getContextClassLoader();
+ClassLoader classLoader = getClassLoader();
return
Collections.enumeration(getClassLoaderInfo(classLoader).loggers.keySet());
}
@@ -292,7 +289,7 @@ public class ClassLoaderLogManager extends LogManager {
private synchronized String findProperty(String name) {
-ClassLoader classLoader =
Thread.currentThread().getContextClassLoader();
+ClassLoader classLoader = getClassLoader();
ClassLoaderLogInfo info = getClassLoaderInfo(classLoader);
String result = info.props.getProperty(name);
// If the property was not found, and the current classloader had no
@@ -325,7 +322,7 @@ public class ClassLoaderLogManager extends LogManager {
checkAccess();
-readConfiguration(Thread.currentThread().getContextClassLoader());
+readConfiguration(getClassLoader());
}
@@ -336,7 +333,7 @@ public class ClassLoaderLogManager extends LogManager {
checkAccess();
reset();
-readConfiguration(is, Thread.currentThread().getContextClassLoader());
+readConfiguration(is, getClassLoader());
}
@@ -349,7 +346,7 @@ public class ClassLoaderLogManager extends LogManager {
// because we have our own shutdown hook
return;
}
-ClassLoader classLoader = thread.getContextClassLoader();
+ClassLoader classLoader = getClassLoader();
ClassLoaderLogInfo clLogInfo = getClassLoaderInfo(classLoader);
resetLoggers(clLogInfo);
// Do not call super.reset(). It should be a NO-OP as all loggers
should
@@ -401,16 +398,17 @@ public class ClassLoaderLogManager extends LogManager {
/**
* Retrieve the configuration associated with the specified classloader. If
- * it does not exist, it will be created.
+ * it does not exist, it will be created. If no class loader is specified,
+ * the class loader used to load this class is used.
*
- * @param classLoader The classloader for which we will retrieve or build
the
- *configuration
+ * @param classLoader The class loader for which we will retrieve or build
+ *the configuration
* @return the log configuration
*/
protected synchronized ClassLoaderLogInfo getClassLoaderInfo(ClassLoader
classLoader) {
if (classLoader == null) {
-classLoader = ClassLoader.getSystemClassLoader();
+classLoader = this.getClass().getClassLoader();
}
ClassLoaderLogInfo info = classLoaderLoggers.get(classLoader);
if (info == null) {
@@ -665,7 +663,7 @@
[tomcat] branch main updated: Add workaround to allow running testsuite on Java 8
This is an automated email from the ASF dual-hosted git repository. remm 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 164da63 Add workaround to allow running testsuite on Java 8 164da63 is described below commit 164da636d9414c85b29549c76bc13e09535052f7 Author: remm AuthorDate: Thu Feb 10 21:15:04 2022 +0100 Add workaround to allow running testsuite on Java 8 For previous branches obviously, not for Tomcat 10.1+ (commit for consistency). In build.properties, add: opens.javalang=-Dtest10=1 opens.javaio=-Dtest11=1 opens.sunrmi=-Dtest12=1 opens.javautil=-Dtest13=1 opens.javautilconcurrent=-Dtest14=1 --- build.xml | 15 ++- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/build.xml b/build.xml index 4417a9c..9aef252 100644 --- a/build.xml +++ b/build.xml @@ -1950,6 +1950,11 @@ + + + + + - - - - - + + + + + - 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 accidental add
This is an automated email from the ASF dual-hosted git repository. remm 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 c7b3502 Remove accidental add c7b3502 is described below commit c7b35020b37c4cdcc91238edc85900ba261c36b0 Author: remm AuthorDate: Thu Feb 10 21:25:42 2022 +0100 Remove accidental add --- build.xml | 1 - 1 file changed, 1 deletion(-) diff --git a/build.xml b/build.xml index b88f86b..cfbf19a 100644 --- a/build.xml +++ b/build.xml @@ -1915,7 +1915,6 @@ - - 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: Based on #464. Avoid using null for class loader during lookups
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/8.5.x by this push:
new 442bc00 Based on #464. Avoid using null for class loader during
lookups
442bc00 is described below
commit 442bc00a7adad3e08c70c8ee21f4e09231683c7d
Author: Mark Thomas
AuthorDate: Thu Feb 10 19:53:15 2022 +
Based on #464. Avoid using null for class loader during lookups
Fall back to the class loader used to load JULI when the thread context
class loader is not set. In a normal Tomcat configuration, this will be
the system class loader.
Based on a pull request by jackshirazi.
---
java/org/apache/juli/ClassLoaderLogManager.java | 49 +
java/org/apache/juli/FileHandler.java | 2 +-
webapps/docs/changelog.xml | 6 +++
3 files changed, 40 insertions(+), 17 deletions(-)
diff --git a/java/org/apache/juli/ClassLoaderLogManager.java
b/java/org/apache/juli/ClassLoaderLogManager.java
index 421fce0..2243e57 100644
--- a/java/org/apache/juli/ClassLoaderLogManager.java
+++ b/java/org/apache/juli/ClassLoaderLogManager.java
@@ -150,8 +150,7 @@ public class ClassLoaderLogManager extends LogManager {
final String loggerName = logger.getName();
-ClassLoader classLoader =
-Thread.currentThread().getContextClassLoader();
+ClassLoader classLoader = getClassLoader();
ClassLoaderLogInfo info = getClassLoaderInfo(classLoader);
if (info.loggers.containsKey(loggerName)) {
return false;
@@ -244,8 +243,7 @@ public class ClassLoaderLogManager extends LogManager {
*/
@Override
public synchronized Logger getLogger(final String name) {
-ClassLoader classLoader = Thread.currentThread()
-.getContextClassLoader();
+ClassLoader classLoader = getClassLoader();
return getClassLoaderInfo(classLoader).loggers.get(name);
}
@@ -256,8 +254,7 @@ public class ClassLoaderLogManager extends LogManager {
*/
@Override
public synchronized Enumeration getLoggerNames() {
-ClassLoader classLoader = Thread.currentThread()
-.getContextClassLoader();
+ClassLoader classLoader = getClassLoader();
return
Collections.enumeration(getClassLoaderInfo(classLoader).loggers.keySet());
}
@@ -300,7 +297,7 @@ public class ClassLoaderLogManager extends LogManager {
private synchronized String findProperty(String name) {
-ClassLoader classLoader =
Thread.currentThread().getContextClassLoader();
+ClassLoader classLoader = getClassLoader();
ClassLoaderLogInfo info = getClassLoaderInfo(classLoader);
String result = info.props.getProperty(name);
// If the property was not found, and the current classloader had no
@@ -333,7 +330,7 @@ public class ClassLoaderLogManager extends LogManager {
checkAccess();
-readConfiguration(Thread.currentThread().getContextClassLoader());
+readConfiguration(getClassLoader());
}
@@ -344,7 +341,7 @@ public class ClassLoaderLogManager extends LogManager {
checkAccess();
reset();
-readConfiguration(is, Thread.currentThread().getContextClassLoader());
+readConfiguration(is, getClassLoader());
}
@@ -357,7 +354,7 @@ public class ClassLoaderLogManager extends LogManager {
// because we have our own shutdown hook
return;
}
-ClassLoader classLoader = thread.getContextClassLoader();
+ClassLoader classLoader = getClassLoader();
ClassLoaderLogInfo clLogInfo = getClassLoaderInfo(classLoader);
resetLoggers(clLogInfo);
// Do not call super.reset(). It should be a NO-OP as all loggers
should
@@ -409,16 +406,17 @@ public class ClassLoaderLogManager extends LogManager {
/**
* Retrieve the configuration associated with the specified classloader. If
- * it does not exist, it will be created.
+ * it does not exist, it will be created. If no class loader is specified,
+ * the class loader used to load this class is used.
*
- * @param classLoader The classloader for which we will retrieve or build
the
- *configuration
+ * @param classLoader The class loader for which we will retrieve or build
+ *the configuration
* @return the log configuration
*/
protected synchronized ClassLoaderLogInfo getClassLoaderInfo(ClassLoader
classLoader) {
if (classLoader == null) {
-classLoader = ClassLoader.getSystemClassLoader();
+classLoader = this.getClass().getClassLoader();
}
ClassLoaderLogInfo info = classLoaderLoggers.get(classLoader);
if (info == null) {
@@ -679,7 +677,7 @@
[GitHub] [tomcat] markt-asf closed pull request #464: avoid null classloader when thread context CL is null
markt-asf closed pull request #464: URL: https://github.com/apache/tomcat/pull/464 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch 10.0.x updated: Add workaround to allow running testsuite on Java 8
This is an automated email from the ASF dual-hosted git repository. remm 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 31e9585 Add workaround to allow running testsuite on Java 8 31e9585 is described below commit 31e95854f35c69007fc213e0663989a9f9d2d22d Author: remm AuthorDate: Thu Feb 10 21:15:04 2022 +0100 Add workaround to allow running testsuite on Java 8 In build.properties, add: opens.javalang=-Dtest10=1 opens.javaio=-Dtest11=1 opens.sunrmi=-Dtest12=1 opens.javautil=-Dtest13=1 opens.javautilconcurrent=-Dtest14=1 --- build.xml | 17 - 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/build.xml b/build.xml index 2c83901..b88f86b 100644 --- a/build.xml +++ b/build.xml @@ -1915,6 +1915,13 @@ + + + + + + + @@ -1940,11 +1947,11 @@ - - - - - + + + + + - 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 accidental add
This is an automated email from the ASF dual-hosted git repository. remm 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 dcd881e Remove accidental add dcd881e is described below commit dcd881e8d80f9f14ee3d4ac9fa1468b74f8e1ac8 Author: remm AuthorDate: Thu Feb 10 21:25:42 2022 +0100 Remove accidental add --- build.xml | 1 - 1 file changed, 1 deletion(-) diff --git a/build.xml b/build.xml index df3a0fb..d641dc4 100644 --- a/build.xml +++ b/build.xml @@ -1897,7 +1897,6 @@ - - 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: Based on #464. Avoid using null for class loader during lookups
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 f162ef3 Based on #464. Avoid using null for class loader during
lookups
f162ef3 is described below
commit f162ef32aef08c808d0568eb090ffa12c31f24f1
Author: Mark Thomas
AuthorDate: Thu Feb 10 19:53:15 2022 +
Based on #464. Avoid using null for class loader during lookups
Fall back to the class loader used to load JULI when the thread context
class loader is not set. In a normal Tomcat configuration, this will be
the system class loader.
Based on a pull request by jackshirazi.
---
java/org/apache/juli/ClassLoaderLogManager.java | 49 +
java/org/apache/juli/FileHandler.java | 2 +-
webapps/docs/changelog.xml | 6 +++
3 files changed, 40 insertions(+), 17 deletions(-)
diff --git a/java/org/apache/juli/ClassLoaderLogManager.java
b/java/org/apache/juli/ClassLoaderLogManager.java
index 227b4e1..b290972 100644
--- a/java/org/apache/juli/ClassLoaderLogManager.java
+++ b/java/org/apache/juli/ClassLoaderLogManager.java
@@ -145,8 +145,7 @@ public class ClassLoaderLogManager extends LogManager {
final String loggerName = logger.getName();
-ClassLoader classLoader =
-Thread.currentThread().getContextClassLoader();
+ClassLoader classLoader = getClassLoader();
ClassLoaderLogInfo info = getClassLoaderInfo(classLoader);
if (info.loggers.containsKey(loggerName)) {
return false;
@@ -236,8 +235,7 @@ public class ClassLoaderLogManager extends LogManager {
*/
@Override
public synchronized Logger getLogger(final String name) {
-ClassLoader classLoader = Thread.currentThread()
-.getContextClassLoader();
+ClassLoader classLoader = getClassLoader();
return getClassLoaderInfo(classLoader).loggers.get(name);
}
@@ -248,8 +246,7 @@ public class ClassLoaderLogManager extends LogManager {
*/
@Override
public synchronized Enumeration getLoggerNames() {
-ClassLoader classLoader = Thread.currentThread()
-.getContextClassLoader();
+ClassLoader classLoader = getClassLoader();
return
Collections.enumeration(getClassLoaderInfo(classLoader).loggers.keySet());
}
@@ -292,7 +289,7 @@ public class ClassLoaderLogManager extends LogManager {
private synchronized String findProperty(String name) {
-ClassLoader classLoader =
Thread.currentThread().getContextClassLoader();
+ClassLoader classLoader = getClassLoader();
ClassLoaderLogInfo info = getClassLoaderInfo(classLoader);
String result = info.props.getProperty(name);
// If the property was not found, and the current classloader had no
@@ -325,7 +322,7 @@ public class ClassLoaderLogManager extends LogManager {
checkAccess();
-readConfiguration(Thread.currentThread().getContextClassLoader());
+readConfiguration(getClassLoader());
}
@@ -336,7 +333,7 @@ public class ClassLoaderLogManager extends LogManager {
checkAccess();
reset();
-readConfiguration(is, Thread.currentThread().getContextClassLoader());
+readConfiguration(is, getClassLoader());
}
@@ -349,7 +346,7 @@ public class ClassLoaderLogManager extends LogManager {
// because we have our own shutdown hook
return;
}
-ClassLoader classLoader = thread.getContextClassLoader();
+ClassLoader classLoader = getClassLoader();
ClassLoaderLogInfo clLogInfo = getClassLoaderInfo(classLoader);
resetLoggers(clLogInfo);
// Do not call super.reset(). It should be a NO-OP as all loggers
should
@@ -401,16 +398,17 @@ public class ClassLoaderLogManager extends LogManager {
/**
* Retrieve the configuration associated with the specified classloader. If
- * it does not exist, it will be created.
+ * it does not exist, it will be created. If no class loader is specified,
+ * the class loader used to load this class is used.
*
- * @param classLoader The classloader for which we will retrieve or build
the
- *configuration
+ * @param classLoader The class loader for which we will retrieve or build
+ *the configuration
* @return the log configuration
*/
protected synchronized ClassLoaderLogInfo getClassLoaderInfo(ClassLoader
classLoader) {
if (classLoader == null) {
-classLoader = ClassLoader.getSystemClassLoader();
+classLoader = this.getClass().getClassLoader();
}
ClassLoaderLogInfo info = classLoaderLoggers.get(classLoader);
if (info == null) {
@@ -665,7 +663,7 @@
[tomcat] branch main updated: Based on #464. Avoid using null for class loader during lookups
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 e276edc Based on #464. Avoid using null for class loader during
lookups
e276edc is described below
commit e276edcee0dd2747909ac7f87e10f3bcf3182413
Author: Mark Thomas
AuthorDate: Thu Feb 10 19:53:15 2022 +
Based on #464. Avoid using null for class loader during lookups
Fall back to the class loader used to load JULI when the thread context
class loader is not set. In a normal Tomcat configuration, this will be
the system class loader.
Based on a pull request by jackshirazi.
---
java/org/apache/juli/ClassLoaderLogManager.java | 49 +
java/org/apache/juli/FileHandler.java | 2 +-
webapps/docs/changelog.xml | 6 +++
3 files changed, 40 insertions(+), 17 deletions(-)
diff --git a/java/org/apache/juli/ClassLoaderLogManager.java
b/java/org/apache/juli/ClassLoaderLogManager.java
index 91d8871..fc51067 100644
--- a/java/org/apache/juli/ClassLoaderLogManager.java
+++ b/java/org/apache/juli/ClassLoaderLogManager.java
@@ -133,8 +133,7 @@ public class ClassLoaderLogManager extends LogManager {
final String loggerName = logger.getName();
-ClassLoader classLoader =
-Thread.currentThread().getContextClassLoader();
+ClassLoader classLoader = getClassLoader();
ClassLoaderLogInfo info = getClassLoaderInfo(classLoader);
if (info.loggers.containsKey(loggerName)) {
return false;
@@ -224,8 +223,7 @@ public class ClassLoaderLogManager extends LogManager {
*/
@Override
public synchronized Logger getLogger(final String name) {
-ClassLoader classLoader = Thread.currentThread()
-.getContextClassLoader();
+ClassLoader classLoader = getClassLoader();
return getClassLoaderInfo(classLoader).loggers.get(name);
}
@@ -236,8 +234,7 @@ public class ClassLoaderLogManager extends LogManager {
*/
@Override
public synchronized Enumeration getLoggerNames() {
-ClassLoader classLoader = Thread.currentThread()
-.getContextClassLoader();
+ClassLoader classLoader = getClassLoader();
return
Collections.enumeration(getClassLoaderInfo(classLoader).loggers.keySet());
}
@@ -280,7 +277,7 @@ public class ClassLoaderLogManager extends LogManager {
private synchronized String findProperty(String name) {
-ClassLoader classLoader =
Thread.currentThread().getContextClassLoader();
+ClassLoader classLoader = getClassLoader();
ClassLoaderLogInfo info = getClassLoaderInfo(classLoader);
String result = info.props.getProperty(name);
// If the property was not found, and the current classloader had no
@@ -313,7 +310,7 @@ public class ClassLoaderLogManager extends LogManager {
checkAccess();
-readConfiguration(Thread.currentThread().getContextClassLoader());
+readConfiguration(getClassLoader());
}
@@ -324,7 +321,7 @@ public class ClassLoaderLogManager extends LogManager {
checkAccess();
reset();
-readConfiguration(is, Thread.currentThread().getContextClassLoader());
+readConfiguration(is, getClassLoader());
}
@@ -337,7 +334,7 @@ public class ClassLoaderLogManager extends LogManager {
// because we have our own shutdown hook
return;
}
-ClassLoader classLoader = thread.getContextClassLoader();
+ClassLoader classLoader = getClassLoader();
ClassLoaderLogInfo clLogInfo = getClassLoaderInfo(classLoader);
resetLoggers(clLogInfo);
// Do not call super.reset(). It should be a NO-OP as all loggers
should
@@ -389,16 +386,17 @@ public class ClassLoaderLogManager extends LogManager {
/**
* Retrieve the configuration associated with the specified classloader. If
- * it does not exist, it will be created.
+ * it does not exist, it will be created. If no class loader is specified,
+ * the class loader used to load this class is used.
*
- * @param classLoader The classloader for which we will retrieve or build
the
- *configuration
+ * @param classLoader The class loader for which we will retrieve or build
+ *the configuration
* @return the log configuration
*/
protected synchronized ClassLoaderLogInfo getClassLoaderInfo(ClassLoader
classLoader) {
if (classLoader == null) {
-classLoader = ClassLoader.getSystemClassLoader();
+classLoader = this.getClass().getClassLoader();
}
ClassLoaderLogInfo info = classLoaderLoggers.get(classLoader);
if (info == null) {
@@ -652,7 +650,7 @@ public
[tomcat] branch 9.0.x updated: Add workaround to allow running testsuite on Java 8
This is an automated email from the ASF dual-hosted git repository. remm 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 31f880c Add workaround to allow running testsuite on Java 8 31f880c is described below commit 31f880cb520b7760e914426f4a03c5d178f7f221 Author: remm AuthorDate: Thu Feb 10 21:15:04 2022 +0100 Add workaround to allow running testsuite on Java 8 In build.properties, add: opens.javalang=-Dtest10=1 opens.javaio=-Dtest11=1 opens.sunrmi=-Dtest12=1 opens.javautil=-Dtest13=1 opens.javautilconcurrent=-Dtest14=1 --- build.xml | 17 - 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/build.xml b/build.xml index 748d1cb..df3a0fb 100644 --- a/build.xml +++ b/build.xml @@ -1897,6 +1897,13 @@ + + + + + + + @@ -1922,11 +1929,11 @@ - - - - - + + + + + - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Running unit tests with JVMs < 11
On Thu, Feb 10, 2022 at 4:25 PM Rainer Jung wrote: > > Am 10.02.2022 um 14:41 schrieb Rémy Maucherat: > > On Thu, Feb 10, 2022 at 2:11 PM Mark Thomas wrote: > >> > >> On 09/02/2022 14:04, Rainer Jung wrote: > >>> Hi all, > >>> > >>> I wonder how important it is to be able to run the unit tests for TC > >>> 8.5-10.0 with JVM versions 8 or even 7 (TC 8.5). > >>> > >>> The unconditional addition of unit test jvmargs "--add-opens=..." in > >>> 2b6e19e971a980e38bcd30f05c554d9b798666c0 (TC 10, backported) means, we > >>> can no longer run the tests with Java 8 or even 7. > >>> > >>> If I remove those jvmargs from build.xml, I run into a second problem > >>> when testing TC 8.5 with Java 7. In > >>> 014f4746176c7f27cda7c04b2c6036a539c385ff EasyMock was updated from 3.x > >>> to 4.x, which needs Java 8 to run. Several tests fail with > >>> UnsupportedClassVersionError in a few EasyMock classes. > >>> > >>> I can work around all of these myself for running the tests here, but I > >>> wonder, what kind of setup for testing we want to support out-of-the box. > >>> > >>> Note that by reintroducing the previous compatibility trick for jvmargs > >>> (TC 8.5, 9 and 10) and using the old EasyMock branch for TC 8.5 we could > >>> still build and test with Java 11, but would also allow testing on the > >>> other target Java versions we support. > >>> > >>> WDYT? > >> > >> I've been thinking about this since I read this yesterday. > >> > >> I like that the move to Java 11 allowed us to reduce the differences > >> between the build scripts. > > > > +1 > > > >> So the decision is: Is the benefit of being able to test with older Java > >> versions worth the differences this would re-introduce? > >> > >> We don't test the JreCompat code so there aren't any Java version > >> specific tests. So what do we gain by testing on Java 7 / Java 8? I can > >> think of a few things: > >> - that we really have coded to the correct Java version > >> - that the code runs as expected on that Java version > > For example TLS behavior. There are functionalities that have the same > API in different Java versions, but implementations provide different > behavior. > > >> Both of those things are more testing Java than testing Tomcat but I can > >> see that there is some benefit - especially if one is concerned about > >> the possibility of the code behaving differently on Java 7/8 compared to > >> Java 11. > > > > It is a good point that this is about testing Java (the bytecode > > produced should target Java 8 exclusively - talking about Tomcat 9, > > for example -, and the behavior of the API should be the same; if > > either becomes false, then this is actually a Java bug). > > Concerning the same API behavior, wouldn't changes in TLS support > qualify as such a change which is not a bug? Of course, most of the time > Tomcat would be agnostic in itself to such changes, but I wouldn't be so > sure that we would not every now and then find a relevant difference via > the unit tests. > > > To be honest, I only partially trust that "release" feature right now. > > Although it seems to work, it might simply need time to get used to > > the concept ! I did point out the testsuite problem when this was > > initially discussed. And I understand the lack of trust if Tomcat is > > simply not tested on Java 8 (or 7, cough; let's pretend it is really > > tested ;) ) before release. > > > > Instead of completely reverting to the previous properties hack and > > reintroducing differences between branches, I would simply advocate > > moving the offending values to hardcoded property values in build.xml > > (which can then be replaced by Java 8 compatible placeholder values in > > your own build.properties - same idea than for the dependencies which > > can be replaced by Java 8 compatible ones). > > Sounds like a reasonable compromise. If we would find over time more > reasons to actually run the tests, would it be easy to define JVM > version specific build.properties lines on CI nodes? No idea, whether we > already overwrite build properties on CI. I added the workaround, in build.properties, you can add: opens.javalang=-Dtest10=1 opens.javaio=-Dtest11=1 opens.sunrmi=-Dtest12=1 opens.javautil=-Dtest13=1 opens.javautilconcurrent=-Dtest14=1 I would build everything first with Java 11 (although the release flag is ignored if you try on Java 8) with "ant test". Then using "ant test" again with Java 8 and the properties worked for me. Rémy > > >> I'm not concerned about that but neither am I too concerned about the > >> re-introduction of a few differences. I guess I am -0 overall provided > >> that any change does not impact the existing CI systems. > > > > Rémy > > Thanks and regards! > > Rainer > > - > 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: Add workaround to allow running testsuite on Java 8
This is an automated email from the ASF dual-hosted git repository. remm pushed a commit to branch 8.5.x in repository https://gitbox.apache.org/repos/asf/tomcat.git The following commit(s) were added to refs/heads/8.5.x by this push: new 1912714 Add workaround to allow running testsuite on Java 8 1912714 is described below commit 1912714307925c82b1ee015d7644bba044acf1f9 Author: remm AuthorDate: Thu Feb 10 21:15:04 2022 +0100 Add workaround to allow running testsuite on Java 8 In build.properties, add: opens.javalang=-Dtest10=1 opens.javaio=-Dtest11=1 opens.sunrmi=-Dtest12=1 opens.javautil=-Dtest13=1 opens.javautilconcurrent=-Dtest14=1 --- build.xml | 16 +++- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/build.xml b/build.xml index a297f1b..a640e20 100644 --- a/build.xml +++ b/build.xml @@ -1508,6 +1508,12 @@ + + + + + + @@ -1535,11 +1541,11 @@ - - - - - + + + + + - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] markt-asf commented on pull request #464: avoid null classloader when thread context CL is null
markt-asf commented on pull request #464: URL: https://github.com/apache/tomcat/pull/464#issuecomment-1035427384 Broader patch with refactoring applied. Credit given for your inspiration. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Running unit tests with JVMs < 11
Thanks Rémy! - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Add support for additional user attributes in TomcatPrincipal
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. 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. @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. Carsten - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [tomcat] branch main updated: Add support for additional user attributes in TomcatPrincipal
On Thu, Feb 10, 2022 at 2:48 PM Mark Thomas wrote: > > On 10/02/2022 12:01, Rémy Maucherat wrote: > > On Tue, Feb 8, 2022 at 2:13 PM wrote: > >> > >> This is an automated email from the ASF dual-hosted git repository. > >> > >> michaelo 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 c3edf43 Add support for additional user attributes in > >> TomcatPrincipal > >> c3edf43 is described below > >> > >> commit c3edf437da20af0f11edc0ad6d893399b01e6287 > >> Author: Carsten Klein > >> AuthorDate: Wed Jan 12 15:06:42 2022 +0100 > >> > >> Add support for additional user attributes in TomcatPrincipal > >> > >> This closes #463 > > > > -1 (veto) > > > > Unfortunately, the more I think about this, the more it seems wrong to me. > > ACK. I won't tag until this has been reverted or you retract the veto. > > I will note that if I get to the point where I am ready to tag and this > is the only issue remaining to resolve I will revert the change prior to > the tag - although I'd prefer that michaelo did that before then. > > > This makes the realm an object store, which is not a good idea at all, > > as it will: > > - Encourage all sorts of bad hacks and practices for users > > - Force us to gradually add more and more features to this generic > > object store feature > > - Have massive discrepancies between realms, with the users trying to > > figure out pros and cons between realms > > - Is a proprietary feature which ties up users to Tomcat > > Just trying to understand the concern at this point. > > You think the risk is that developers will push data in the Realm and > then access from the authenticated principal? This will seem convenient. If building an e-commerce thingie, you could imagine storing payment info, all misc info, shopping cart. The only thing preventing this is that there is no setAttribute(String name, Object value), but what if someone comes along with a PR later ? Is that a "no" then ? Why would this be more a "no" than for this one ? This is clearly my concern now [following lingering thoughts about how this could be (ab)used]. Again I would like to apologize for taking so long until reaching this position, I know this wastes a lot of time ... My wider concerns were initially triggered when there was suddenly some intent to add the feature to the UserDatabase (which is a R/W database for users, so there the feature clearly becomes general object storage - even though the PR only contained a mock impl). > > More importantly: application data storage should remain the > > prerogative of the application, Tomcat should not attempt to do that. > > The only thing Tomcat should provide is a principal name, which is > > then the primary key for the third party object store. There is zero > > reason to integrate this within the realm or principal API. It also > > feels like it can never be complete or appropriate. > > Looking at the proposed change for the DataSourceRealm I can see it has > clearly defined limits. I can see how we might get requests to push back > those limits in the future. > > I have a general concern / unease that doing this in an application > specific manner would be relatively easy but doing it generically could > get complex. > > > I apologize for having to do this, after initially helping out with > > this PR, but this is one of those situations where the more I look > > about it, the worse I feel about it. > > I do recognise the use case of wanting to expose data that is already in > the Realm to the application. Yes, that was what I was thinking about too. But then it still encourages putting more stuff in the realm backend (since now you can, you probably know how it works by now). The use case in the PR is the DataSourceRealm (not the LDAP one), so let's assume it is a JDBC thing. Retrieving the data from another data source connecting to the same backend in the application might (supposedly) be less efficient (but if you pull a lot of data from the DB, it will suddenly be impossible to tell the difference), but is portable and you could store things properly (and more importantly avoid asking us to add code to automagically map to Java collections ... that's just for starters obviously). > I wonder whether this is a case where we can just provide some hooks > and, if the developer chooses to (ab)use them, that is up to them. > > I'm thinking something like a getUserAttributes() method on Realm that > returns null by default. If an application wants to populate additional > attributes, then it needs to provide a custom realm that overrides that > method. The issue with this approach is it requires a custom realm. > > Given your concerns, backing this out while we discuss it some more > seems like the right approach to me. If there's firm consensus that this feature is final for there, then I could drop the veto
[Bug 65776] "Duplicate accept detected" error from a subsequent request with the same local port
https://bz.apache.org/bugzilla/show_bug.cgi?id=65776 --- Comment #15 from Mark Thomas --- The large drop in throughput I reported in comment #7 has been bothering me so I wanted to look at this again. Testing this is tricky as you can quickly run out of ephemeral ports. Once that happens, test results become unstable. I think this may have happened when I tested this previously. I definitely saw ephemeral port exhaustion when I tried to re-test this. By reducing the test to a single client thread and temporarily reducing the period sockets spend in TIME_WAIT I was able to get the test results to stabilise. I was then able to test the following changes: - skip the check completely on Windows - add a check for 'duplicates must occur within 1s" to avoid false positives from things like heartbeats on idle systems The results were very promising. The performance difference was (probably) marginally lower but I'd need to repeat the test quite a few times (and brush up on my stats) to provide a definitive answer. My engineering estimate view of the results is that the performance impact is negligible so I'll be apply the change shortly. -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Running unit tests with JVMs < 11
On Thu, Feb 10, 2022 at 4:25 PM Rainer Jung wrote: > > Am 10.02.2022 um 14:41 schrieb Rémy Maucherat: > > On Thu, Feb 10, 2022 at 2:11 PM Mark Thomas wrote: > >> > >> On 09/02/2022 14:04, Rainer Jung wrote: > >>> Hi all, > >>> > >>> I wonder how important it is to be able to run the unit tests for TC > >>> 8.5-10.0 with JVM versions 8 or even 7 (TC 8.5). > >>> > >>> The unconditional addition of unit test jvmargs "--add-opens=..." in > >>> 2b6e19e971a980e38bcd30f05c554d9b798666c0 (TC 10, backported) means, we > >>> can no longer run the tests with Java 8 or even 7. > >>> > >>> If I remove those jvmargs from build.xml, I run into a second problem > >>> when testing TC 8.5 with Java 7. In > >>> 014f4746176c7f27cda7c04b2c6036a539c385ff EasyMock was updated from 3.x > >>> to 4.x, which needs Java 8 to run. Several tests fail with > >>> UnsupportedClassVersionError in a few EasyMock classes. > >>> > >>> I can work around all of these myself for running the tests here, but I > >>> wonder, what kind of setup for testing we want to support out-of-the box. > >>> > >>> Note that by reintroducing the previous compatibility trick for jvmargs > >>> (TC 8.5, 9 and 10) and using the old EasyMock branch for TC 8.5 we could > >>> still build and test with Java 11, but would also allow testing on the > >>> other target Java versions we support. > >>> > >>> WDYT? > >> > >> I've been thinking about this since I read this yesterday. > >> > >> I like that the move to Java 11 allowed us to reduce the differences > >> between the build scripts. > > > > +1 > > > >> So the decision is: Is the benefit of being able to test with older Java > >> versions worth the differences this would re-introduce? > >> > >> We don't test the JreCompat code so there aren't any Java version > >> specific tests. So what do we gain by testing on Java 7 / Java 8? I can > >> think of a few things: > >> - that we really have coded to the correct Java version > >> - that the code runs as expected on that Java version > > For example TLS behavior. There are functionalities that have the same > API in different Java versions, but implementations provide different > behavior. > > >> Both of those things are more testing Java than testing Tomcat but I can > >> see that there is some benefit - especially if one is concerned about > >> the possibility of the code behaving differently on Java 7/8 compared to > >> Java 11. > > > > It is a good point that this is about testing Java (the bytecode > > produced should target Java 8 exclusively - talking about Tomcat 9, > > for example -, and the behavior of the API should be the same; if > > either becomes false, then this is actually a Java bug). > > Concerning the same API behavior, wouldn't changes in TLS support > qualify as such a change which is not a bug? Of course, most of the time > Tomcat would be agnostic in itself to such changes, but I wouldn't be so > sure that we would not every now and then find a relevant difference via > the unit tests. I believe the TLS impl in Java 8 has to remain compatible *and* functional, or it's a bug. If suddenly no clients can connect to it, then Oracle will have to fix it. For Java 7, it will be EOL in 6 months, so it's not looking very good. If/when it becomes less functional, we cannot do anything about it. > > To be honest, I only partially trust that "release" feature right now. > > Although it seems to work, it might simply need time to get used to > > the concept ! I did point out the testsuite problem when this was > > initially discussed. And I understand the lack of trust if Tomcat is > > simply not tested on Java 8 (or 7, cough; let's pretend it is really > > tested ;) ) before release. > > > > Instead of completely reverting to the previous properties hack and > > reintroducing differences between branches, I would simply advocate > > moving the offending values to hardcoded property values in build.xml > > (which can then be replaced by Java 8 compatible placeholder values in > > your own build.properties - same idea than for the dependencies which > > can be replaced by Java 8 compatible ones). > > Sounds like a reasonable compromise. If we would find over time more > reasons to actually run the tests, would it be easy to define JVM > version specific build.properties lines on CI nodes? No idea, whether we > already overwrite build properties on CI. Yes, build.properties has to be done to set the amount of threads and other important things like that. I'll make the changes in build.xml (likely tomorrow). Rémy > >> I'm not concerned about that but neither am I too concerned about the > >> re-introduction of a few differences. I guess I am -0 overall provided > >> that any change does not impact the existing CI systems. > > > > Rémy > > Thanks and regards! > > Rainer > > - > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail:
Re: [tomcat] branch main updated: Fix BZ 65776. Reduce false positives detecting duplicate accept bug
I know, I know. I missed the "!" in the OS test.
I'm just re-chcking the performance data before I fix it.
Mark
On 10/02/2022 16:34, ma...@apache.org wrote:
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 113be8f Fix BZ 65776. Reduce false positives detecting duplicate
accept bug
113be8f is described below
commit 113be8f7e133af77a81d381769bacc0584e29eb8
Author: Mark Thomas
AuthorDate: Thu Feb 10 16:02:05 2022 +
Fix BZ 65776. Reduce false positives detecting duplicate accept bug
https://bz.apache.org/bugzilla/show_bug.cgi?id=65776
---
java/org/apache/tomcat/util/net/Nio2Endpoint.java | 18 +-
java/org/apache/tomcat/util/net/NioEndpoint.java | 18 +-
webapps/docs/changelog.xml| 4
3 files changed, 30 insertions(+), 10 deletions(-)
diff --git a/java/org/apache/tomcat/util/net/Nio2Endpoint.java
b/java/org/apache/tomcat/util/net/Nio2Endpoint.java
index 687665b..11bd099 100644
--- a/java/org/apache/tomcat/util/net/Nio2Endpoint.java
+++ b/java/org/apache/tomcat/util/net/Nio2Endpoint.java
@@ -44,6 +44,7 @@ import org.apache.juli.logging.Log;
import org.apache.juli.logging.LogFactory;
import org.apache.tomcat.util.ExceptionUtils;
import org.apache.tomcat.util.collections.SynchronizedStack;
+import org.apache.tomcat.util.compat.JrePlatform;
import org.apache.tomcat.util.net.AbstractEndpoint.Handler.SocketState;
import org.apache.tomcat.util.net.Acceptor.AcceptorState;
import org.apache.tomcat.util.net.jsse.JSSESupport;
@@ -85,9 +86,10 @@ public class Nio2Endpoint extends
AbstractJsseEndpoint nioChannels;
private SocketAddress previousAcceptedSocketRemoteAddress = null;
+private long previouspreviousAcceptedSocketNanoTime = 0;
-// - Public Methods
+// - Public Methods
/**
* Number of keep-alive sockets.
@@ -362,11 +364,17 @@ public class Nio2Endpoint extends
AbstractJsseEndpoint
-SocketAddress currentRemoteAddress = result.getRemoteAddress();
-if (currentRemoteAddress.equals(previousAcceptedSocketRemoteAddress)) {
-throw new
IOException(sm.getString("endpoint.err.duplicateAccept"));
+// Bug does not affect Windows. Skip the check on that platform.
+if (JrePlatform.IS_WINDOWS) {
+SocketAddress currentRemoteAddress = result.getRemoteAddress();
+long currentNanoTime = System.nanoTime();
+if (currentRemoteAddress.equals(previousAcceptedSocketRemoteAddress)
&&
+currentNanoTime - previouspreviousAcceptedSocketNanoTime <
1000) {
+throw new
IOException(sm.getString("endpoint.err.duplicateAccept"));
+}
+previousAcceptedSocketRemoteAddress = currentRemoteAddress;
+previouspreviousAcceptedSocketNanoTime = currentNanoTime;
}
-previousAcceptedSocketRemoteAddress = currentRemoteAddress;
return result;
}
diff --git a/java/org/apache/tomcat/util/net/NioEndpoint.java
b/java/org/apache/tomcat/util/net/NioEndpoint.java
index d5a79cd..4baab31 100644
--- a/java/org/apache/tomcat/util/net/NioEndpoint.java
+++ b/java/org/apache/tomcat/util/net/NioEndpoint.java
@@ -58,6 +58,7 @@ import org.apache.tomcat.util.ExceptionUtils;
import org.apache.tomcat.util.collections.SynchronizedQueue;
import org.apache.tomcat.util.collections.SynchronizedStack;
import org.apache.tomcat.util.compat.JreCompat;
+import org.apache.tomcat.util.compat.JrePlatform;
import org.apache.tomcat.util.net.AbstractEndpoint.Handler.SocketState;
import org.apache.tomcat.util.net.Acceptor.AcceptorState;
import org.apache.tomcat.util.net.jsse.JSSESupport;
@@ -109,11 +110,11 @@ public class NioEndpoint extends
AbstractJsseEndpoint
private SynchronizedStack nioChannels;
private SocketAddress previousAcceptedSocketRemoteAddress = null;
+private long previouspreviousAcceptedSocketNanoTime = 0;
// - Properties
-
/**
* Use System.inheritableChannel to obtain channel from stdin/stdout.
*/
@@ -516,11 +517,18 @@ public class NioEndpoint extends
AbstractJsseEndpoint
@Override
protected SocketChannel serverSocketAccept() throws Exception {
SocketChannel result = serverSock.accept();
-SocketAddress currentRemoteAddress = result.getRemoteAddress();
-if (currentRemoteAddress.equals(previousAcceptedSocketRemoteAddress)) {
-throw new
IOException(sm.getString("endpoint.err.duplicateAccept"));
+
+// Bug does not affect Windows. Skip the check on that
[tomcat] branch main updated: Fix copy/paste error
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 c3bd12d Fix copy/paste error
c3bd12d is described below
commit c3bd12dad649a211f981a0674625d2a45b768491
Author: Mark Thomas
AuthorDate: Thu Feb 10 17:11:56 2022 +
Fix copy/paste error
---
java/org/apache/tomcat/util/net/Nio2Endpoint.java | 6 +++---
java/org/apache/tomcat/util/net/NioEndpoint.java | 6 +++---
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/java/org/apache/tomcat/util/net/Nio2Endpoint.java
b/java/org/apache/tomcat/util/net/Nio2Endpoint.java
index 01f3c46..a5ad04a 100644
--- a/java/org/apache/tomcat/util/net/Nio2Endpoint.java
+++ b/java/org/apache/tomcat/util/net/Nio2Endpoint.java
@@ -86,7 +86,7 @@ public class Nio2Endpoint extends
AbstractJsseEndpoint nioChannels;
private SocketAddress previousAcceptedSocketRemoteAddress = null;
-private long previouspreviousAcceptedSocketNanoTime = 0;
+private long previousAcceptedSocketNanoTime = 0;
// - Public Methods
@@ -369,11 +369,11 @@ public class Nio2Endpoint extends
AbstractJsseEndpoint
private SynchronizedStack nioChannels;
private SocketAddress previousAcceptedSocketRemoteAddress = null;
-private long previouspreviousAcceptedSocketNanoTime = 0;
+private long previousAcceptedSocketNanoTime = 0;
// - Properties
@@ -523,11 +523,11 @@ public class NioEndpoint extends
AbstractJsseEndpoint
SocketAddress currentRemoteAddress = result.getRemoteAddress();
long currentNanoTime = System.nanoTime();
if
(currentRemoteAddress.equals(previousAcceptedSocketRemoteAddress) &&
-currentNanoTime - previouspreviousAcceptedSocketNanoTime <
1000) {
+currentNanoTime - previousAcceptedSocketNanoTime < 1000) {
throw new
IOException(sm.getString("endpoint.err.duplicateAccept"));
}
previousAcceptedSocketRemoteAddress = currentRemoteAddress;
-previouspreviousAcceptedSocketNanoTime = currentNanoTime;
+previousAcceptedSocketNanoTime = currentNanoTime;
}
return result;
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
