Re: [DISCUSS] Deprecate and remove RealmBase#stripRealmForGss
Am 2020-10-13 um 12:32 schrieb Mark Thomas: On 13/10/2020 10:48, Michael Osipov wrote: Folks, I'd like to propose to get rid of that config option in 10 and deprecate in previous versions for the following reasons: * It suffers from abstraction: It assumes that the GSS name is always email style w/o checking its OID * The realm part, if any, is an integeral part of the principal. Much like with an email address' domain. You wouldn't stip here too, would you? * It is a surprise for clients having the princippal mutilated by default. I trip over and over again this when I set up UserDatabaseRealms for testing purposes I wonder why michae...@example.com does not work. * In a multi realm environment, it is perfectly fine and valid to have user1@REALMA and user1@REALMB. These are distinct principals, but treated by RealmBase equally, this has implications. * Finally, when doing cert-based auth in an AD envinronment, is it pretty common to extract the msUPN name from the cert's SAN which is almost always email address (enteprise principal) which would end up in michael.osipov, but where is the rest?! Thoughts? No objections to changing the default to false in 10.0.x but I think removal/deprecation is a step too far. OK Not everyone uses the full u...@bigco.com format as the "primary key" in their internal user database. I see lots of variation depending on which system is viewed as the source of truth to which other systems are synchronised. So you will never be able to satisfy all, why try? We have X509UsernameRetriever for certificate authentication. That came out of bug 52500 where the aim was to support a wide variety of formats. The root of that requirement was essentially the same - integration with a variety of systems where the user name could be in in a range of formats. The mapping of that format to an X509 cert just added another layer of complexity. I don't think that this compares (well( because a certificate is a complex object explicitly containing SAN which contain wellknown forms for an entity whereas a GSS name is merely a string with an identifier for its format. Your argumentation could also be applied to an LDAP bind. Active Directory, for instance, accepts a various amounts of input formats for the same entity, yet we don't provide a logic to normalize them in the JNDI Realm. That seems to be unfair to me. It lacks a bit of consistency. If there was user demand for it (I haven't seen any) I'd support a similar interface for SPNEGO. With such an interface in place, deprecation and removal of stripRealmForGss would make sense. See above. Michael - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [tomcat] 01/04: Add connection pool to JNDI realm
On Tue, Oct 13, 2020 at 8:26 PM Michael Osipov wrote: > Am 2020-10-13 um 13:49 schrieb Rémy Maucherat: > > On Tue, Oct 13, 2020 at 11:33 AM Michael Osipov > wrote: > > > >> Am 2020-10-07 um 22:34 schrieb r...@apache.org: > >>> 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 > >>> > >>> commit 50de36b7874da98591345e40b374a1e2dd52c188 > >>> Author: remm > >>> AuthorDate: Thu Jan 30 17:22:51 2020 +0100 > >>> > >>> Add connection pool to JNDI realm > >>> > >>> This implements a TODO from the class javadoc header. > >>> As described in the javadoc, the idea is to use a pool to avoid > >> blocking > >>> on a single connection, which could possibly become a bottleneck > in > >> some > >>> cases. The message formats need to be kept along with the > connection > >>> since they are not thread safe. > >>> Preserve the default behavior: sync without pooling (using a Lock > >> object > >>> which is more flexible). > >>> I may backport this since this is limited to the JNDI realm, but > >> only > >>> once it is confirmed to be regression free. Tested with ApacheDS > >> but my > >>> LDAP skills are very limited. > >>> --- > >>>java/org/apache/catalina/realm/JNDIRealm.java | 442 > >> - > >>>.../apache/catalina/realm/LocalStrings.properties | 1 + > >>>test/org/apache/catalina/realm/TestJNDIRealm.java | 7 +- > >>>webapps/docs/changelog.xml | 3 + > >>>webapps/docs/config/realm.xml | 7 + > >>>5 files changed, 276 insertions(+), 184 deletions(-) > >> > >> Salut Rémy, > >> > >> this is a very very nice improvement to the matter and I gave your idea > >> a spin my public Active Directory realm. Based on my tests with AD I see > >> room for improvement: (Note that I don't use the JNDIRealm because > >> because is it too generic and usable for me) > >> > > > > Ok, I only tested this with a local ldap, so not much. I verified it was > > pooling connections. > > > > > >> > >> * You might want to consider to introduce a maxIdleTime to avoid stale > >> connections, e.g., AD has default value of 15 minutes. Yep, I had had > >> several RSTs resulting in 401s > >> > > > > Oops. So I don't think adding something like this helps since there could > > be plenty of issues besides a timeout. Basically it's supposed to retry > > instead when something fails. Can you give me a stack trace to show > what's > > wrong ? > > I consider it cheaper to test for a timeout and close actively rather > than recover from a resetted connection. Here is a strack trace for > concurrency of 4 (with a pool of max 8). Compare the timestamp of the > first line and the subsequent ones: > > > 2020-10-12T22:55:47 FEIN [https-openssl-apr-8444-exec-8] > net.sf.michaelo.tomcat.realm.ActiveDirectoryRealm.release Releasing > directory server connection to pool > > 2020-10-12T23:13:06 FEIN [https-openssl-apr-8444-exec-5] > net.sf.michaelo.tomcat.realm.ActiveDirectoryRealm.acquire Acquiring > directory server connection from pool > > 2020-10-12T23:13:06 FEIN [https-openssl-apr-8444-exec-5] > net.sf.michaelo.tomcat.realm.ActiveDirectoryRealm.validate Validating > directory server connection from pool > > 2020-10-12T23:13:06 SCHWERWIEGEND [https-openssl-apr-8444-exec-5] > net.sf.michaelo.tomcat.realm.ActiveDirectoryRealm.validate Exception > validating directory server connection > > javax.naming.CommunicationException: Connection reset [Root > exception is java.net.SocketException: Connection reset]; remaining name '' > > at com.sun.jndi.ldap.LdapCtx.doSearch(LdapCtx.java:2030) > > at com.sun.jndi.ldap.LdapCtx.searchAux(LdapCtx.java:1872) > > at com.sun.jndi.ldap.LdapCtx.c_search(LdapCtx.java:1797) > > at > com.sun.jndi.toolkit.ctx.ComponentDirContext.p_search(ComponentDirContext.java:392) > > at > com.sun.jndi.toolkit.ctx.PartialCompositeDirContext.search(PartialCompositeDirContext.java:358) > > at > com.sun.jndi.toolkit.ctx.PartialCompositeDirContext.search(PartialCompositeDirContext.java:341) > > at > javax.naming.directory.InitialDirContext.search(InitialDirContext.java:296) > > at > net.sf.michaelo.tomcat.realm.ActiveDirectoryRealm.validate(ActiveDirectoryRealm.java:316) > > at > net.sf.michaelo.tomcat.realm.ActiveDirectoryRealm.acquire(ActiveDirectoryRealm.java:285) > > at > net.sf.michaelo.tomcat.realm.ActiveDirectoryRealm.getPrincipal(ActiveDirectoryRealm.java:244) > > at > org.apache.catalina.realm.RealmBase.authenticate(RealmBase.java:501) > > at > net.sf.michaelo.tomcat.authenticator.SpnegoAuthenticator.doAuthenticate(SpnegoAuthenticator.java:165) > > at > org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:633) > >
Re: [tomcat] branch 8.5.x updated: Always retry on a new connection, even when pooling
Am 2020-10-13 um 16:05 schrieb r...@apache.org:
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 883600b Always retry on a new connection, even when pooling
883600b is described below
commit 883600b8a77c0be93b3a8dc2404e8d1110970ee7
Author: remm
AuthorDate: Tue Oct 13 14:19:54 2020 +0200
Always retry on a new connection, even when pooling
This keeps the same very simple design as for the single connection
scenario, for now.
---
java/org/apache/catalina/realm/JNDIRealm.java | 22 +++---
webapps/docs/changelog.xml| 5 +
2 files changed, 24 insertions(+), 3 deletions(-)
diff --git a/java/org/apache/catalina/realm/JNDIRealm.java
b/java/org/apache/catalina/realm/JNDIRealm.java
index 72087ab..98007f7 100644
--- a/java/org/apache/catalina/realm/JNDIRealm.java
+++ b/java/org/apache/catalina/realm/JNDIRealm.java
@@ -1311,7 +1311,7 @@ public class JNDIRealm extends RealmBase {
close(connection);
// open a new directory context.
-connection = get();
+connection = get(true);
// Try the authentication again.
principal = authenticate(connection, username, credentials);
@@ -2389,12 +2389,28 @@ public class JNDIRealm extends RealmBase {
* @exception NamingException if a directory server error occurs
*/
protected JNDIConnection get() throws NamingException {
+return get(false);
+}
+
+/**
+ * Open (if necessary) and return a connection to the configured
+ * directory server for this Realm.
+ * @param create when pooling, this forces creation of a new connection,
+ * for example after an error
+ * @return the connection
+ * @exception NamingException if a directory server error occurs
+ */
+protected JNDIConnection get(boolean create) throws NamingException {
JNDIConnection connection = null;
// Use the pool if available, otherwise use the single connection
if (connectionPool != null) {
-connection = connectionPool.pop();
-if (connection == null) {
+if (create) {
connection = create();
+} else {
+connection = connectionPool.pop();
+if (connection == null) {
+connection = create();
+}
}
} else {
singleConnectionLock.lock();
That suitable and simple approach.
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [tomcat] 01/04: Add connection pool to JNDI realm
Am 2020-10-13 um 13:49 schrieb Rémy Maucherat: On Tue, Oct 13, 2020 at 11:33 AM Michael Osipov wrote: Am 2020-10-07 um 22:34 schrieb r...@apache.org: 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 commit 50de36b7874da98591345e40b374a1e2dd52c188 Author: remm AuthorDate: Thu Jan 30 17:22:51 2020 +0100 Add connection pool to JNDI realm This implements a TODO from the class javadoc header. As described in the javadoc, the idea is to use a pool to avoid blocking on a single connection, which could possibly become a bottleneck in some cases. The message formats need to be kept along with the connection since they are not thread safe. Preserve the default behavior: sync without pooling (using a Lock object which is more flexible). I may backport this since this is limited to the JNDI realm, but only once it is confirmed to be regression free. Tested with ApacheDS but my LDAP skills are very limited. --- java/org/apache/catalina/realm/JNDIRealm.java | 442 - .../apache/catalina/realm/LocalStrings.properties | 1 + test/org/apache/catalina/realm/TestJNDIRealm.java | 7 +- webapps/docs/changelog.xml | 3 + webapps/docs/config/realm.xml | 7 + 5 files changed, 276 insertions(+), 184 deletions(-) Salut Rémy, this is a very very nice improvement to the matter and I gave your idea a spin my public Active Directory realm. Based on my tests with AD I see room for improvement: (Note that I don't use the JNDIRealm because because is it too generic and usable for me) Ok, I only tested this with a local ldap, so not much. I verified it was pooling connections. * You might want to consider to introduce a maxIdleTime to avoid stale connections, e.g., AD has default value of 15 minutes. Yep, I had had several RSTs resulting in 401s Oops. So I don't think adding something like this helps since there could be plenty of issues besides a timeout. Basically it's supposed to retry instead when something fails. Can you give me a stack trace to show what's wrong ? I consider it cheaper to test for a timeout and close actively rather than recover from a resetted connection. Here is a strack trace for concurrency of 4 (with a pool of max 8). Compare the timestamp of the first line and the subsequent ones: 2020-10-12T22:55:47 FEIN [https-openssl-apr-8444-exec-8] net.sf.michaelo.tomcat.realm.ActiveDirectoryRealm.release Releasing directory server connection to pool 2020-10-12T23:13:06 FEIN [https-openssl-apr-8444-exec-5] net.sf.michaelo.tomcat.realm.ActiveDirectoryRealm.acquire Acquiring directory server connection from pool 2020-10-12T23:13:06 FEIN [https-openssl-apr-8444-exec-5] net.sf.michaelo.tomcat.realm.ActiveDirectoryRealm.validate Validating directory server connection from pool 2020-10-12T23:13:06 SCHWERWIEGEND [https-openssl-apr-8444-exec-5] net.sf.michaelo.tomcat.realm.ActiveDirectoryRealm.validate Exception validating directory server connection javax.naming.CommunicationException: Connection reset [Root exception is java.net.SocketException: Connection reset]; remaining name '' at com.sun.jndi.ldap.LdapCtx.doSearch(LdapCtx.java:2030) at com.sun.jndi.ldap.LdapCtx.searchAux(LdapCtx.java:1872) at com.sun.jndi.ldap.LdapCtx.c_search(LdapCtx.java:1797) at com.sun.jndi.toolkit.ctx.ComponentDirContext.p_search(ComponentDirContext.java:392) at com.sun.jndi.toolkit.ctx.PartialCompositeDirContext.search(PartialCompositeDirContext.java:358) at com.sun.jndi.toolkit.ctx.PartialCompositeDirContext.search(PartialCompositeDirContext.java:341) at javax.naming.directory.InitialDirContext.search(InitialDirContext.java:296) at net.sf.michaelo.tomcat.realm.ActiveDirectoryRealm.validate(ActiveDirectoryRealm.java:316) at net.sf.michaelo.tomcat.realm.ActiveDirectoryRealm.acquire(ActiveDirectoryRealm.java:285) at net.sf.michaelo.tomcat.realm.ActiveDirectoryRealm.getPrincipal(ActiveDirectoryRealm.java:244) at org.apache.catalina.realm.RealmBase.authenticate(RealmBase.java:501) at net.sf.michaelo.tomcat.authenticator.SpnegoAuthenticator.doAuthenticate(SpnegoAuthenticator.java:165) at org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:633) at org.apache.catalina.valves.rewrite.RewriteValve.invoke(RewriteValve.java:571) at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:139) at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:81) at org.apache.catalina.valves.AbstractAccessLogValve.invoke(AbstractAccessLogValve.java:690) at
Cannot override servlet context init param
Hello, Today I was experimenting with a Servlet ContainerInitializer in which I wanted to modify the value of a particular init param, but found that Tomcat's ServletContext implementation (ApplicationContext) explicitly rejected the override. The Servlet API doesn't seem to suggest that what I'm trying to do should not be supported; would a patch (or patches, thinking of multiple Tomcat versions) to support this behavior, perhaps conditionally, be welcome? If so, what form should such conditional support take? Thanks, Matt
[Bug 60030] Run away CPU with JSSE / OpenSSL with IE8
https://bz.apache.org/bugzilla/show_bug.cgi?id=60030 --- Comment #5 from kivetomani --- Kami Memberikan Bonus Special Mystery Box untuk para member aktif dan setia setiap Hari Sabtu Dan Minggu berhadiah UANG TUNAI! https://199.192.31.67 http://www.dewatogelonline88.com/ https://199.192.26.181 -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 60030] Run away CPU with JSSE / OpenSSL with IE8
https://bz.apache.org/bugzilla/show_bug.cgi?id=60030 kivetomani changed: What|Removed |Added URL||https://199.192.31.67 -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] blueSky1825821 commented on pull request #368: fix: CachedResource add path length
blueSky1825821 commented on pull request #368: URL: https://github.com/apache/tomcat/pull/368#issuecomment-707629467 > I've done some calculations and the absolute upper limit (the actual upper limit will be lower) is that a cache with a 10MB limit on size will actually use a little under 175MB. That isn't great but it isn't terrible either. I'll tweak the resource size calculation so it includes an approximate of the memory required to store the path. > The proposed patch has a couple of issues: > > * It converts the string to bytes to determine the size. This is both expensive and wrong. String length *2 +38 is a much closer approximation and I intend to ignore the 38 as that is already accounted for in the 500 byte overhead for each entry. > * It includes the path size when determining if the resource exceeds the max object size (and hence won't have the content cached) but does not include the path size when performing the same check to determine when deciding whether to cache the content or not. > > I should have a fix for this committed shortly. Let me add that, after jdk8, e.g jdk9 String has been changed private final char value[]; --> private final byte[] value; but length *2 is also fine This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [tomcat] 01/04: Add connection pool to JNDI realm
Am 2020-10-07 um 22:34 schrieb r...@apache.org:
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
commit 50de36b7874da98591345e40b374a1e2dd52c188
Author: remm
AuthorDate: Thu Jan 30 17:22:51 2020 +0100
Add connection pool to JNDI realm
This implements a TODO from the class javadoc header.
As described in the javadoc, the idea is to use a pool to avoid blocking
on a single connection, which could possibly become a bottleneck in some
cases. The message formats need to be kept along with the connection
since they are not thread safe.
Preserve the default behavior: sync without pooling (using a Lock object
which is more flexible).
I may backport this since this is limited to the JNDI realm, but only
once it is confirmed to be regression free. Tested with ApacheDS but my
LDAP skills are very limited.
---
java/org/apache/catalina/realm/JNDIRealm.java | 442 -
.../apache/catalina/realm/LocalStrings.properties | 1 +
test/org/apache/catalina/realm/TestJNDIRealm.java | 7 +-
webapps/docs/changelog.xml | 3 +
webapps/docs/config/realm.xml | 7 +
5 files changed, 276 insertions(+), 184 deletions(-)
Salut Rémy,
this is a very very nice improvement to the matter and I gave your idea
a spin my public Active Directory realm. Based on my tests with AD I see
room for improvement: (Note that I don't use the JNDIRealm because
because is it too generic and usable for me)
* You might want to consider to introduce a maxIdleTime to avoid stale
connections, e.g., AD has default value of 15 minutes. Yep, I had had
several RSTs resulting in 401s
* Validating connections optionally might be a good option to detect
stale connections or broken ones ny networks issues. This works for me
very fast:
protected boolean validate(DirContextConnection connection) {
if (logger.isDebugEnabled())
logger.debug(sm.getString("activeDirectoryRealm.validate"));
SearchControls controls = new SearchControls();
controls.setSearchScope(SearchControls.OBJECT_SCOPE);
controls.setCountLimit(1);
controls.setReturningAttributes(new String[] { "objectClass" });
controls.setTimeLimit(500);
try {
NamingEnumeration results =
connection.context.search("", "objectclass=*",
controls);
if (results.hasMore()) {
close(results);
return true;
}
} catch (NamingException e) {
logger.error(sm.getString("activeDirectoryRealm.validate.namingException"), e);
return false;
}
return false;
}
I do this in the acquire() method
* The acquire(), authenticate(), fail, close(), authenticate(),
release() is brittle and proved to fail here. I have a curl script which
does 4 requests in parallel and 1200 requests in total. Pool size is 8.
4 connections in the pool. Rest for 16 minutes, connections are broken
now. Start requests. authenticate() will fail twice because top two
acquired connections from pool are broken. Requests end with 401. I'd
prefer to to modify acquire() is such a fashion:
protected DirContextConnection acquire() {
if (logger.isDebugEnabled())
logger.debug(sm.getString("activeDirectoryRealm.acquire"));
DirContextConnection connection = null;
while (connection == null) {
connection = connectionPool.pop();
if (connection != null) {
long idleTime = System.currentTimeMillis() -
connection.lastBorrowTime;
if (idleTime > maxIdleTime) {
if (logger.isDebugEnabled())
logger.debug(sm.getString("activeDirectoryRealm.exceedMaxIdleTime"));
close(connection);
connection = null;
} else {
boolean valid = validate(connection);
if (valid) {
if (logger.isDebugEnabled())
logger.debug(sm.getString("activeDirectoryRealm.reuse"));
} else {
close(connection);
connection =
[DISCUSS] Deprecate and remove RealmBase#stripRealmForGss
Folks, I'd like to propose to get rid of that config option in 10 and deprecate in previous versions for the following reasons: * It suffers from abstraction: It assumes that the GSS name is always email style w/o checking its OID * The realm part, if any, is an integeral part of the principal. Much like with an email address' domain. You wouldn't stip here too, would you? * It is a surprise for clients having the princippal mutilated by default. I trip over and over again this when I set up UserDatabaseRealms for testing purposes I wonder why michae...@example.com does not work. * In a multi realm environment, it is perfectly fine and valid to have user1@REALMA and user1@REALMB. These are distinct principals, but treated by RealmBase equally, this has implications. * Finally, when doing cert-based auth in an AD envinronment, is it pretty common to extract the msUPN name from the cert's SAN which is almost always email address (enteprise principal) which would end up in michael.osipov, but where is the rest?! Thoughts? Michael - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 64765] ThreadPoolExecutor#submittedCount wrong after undeploy
https://bz.apache.org/bugzilla/show_bug.cgi?id=64765 --- Comment #4 from m...@bsi-software.com --- I confirm the issue seems to be fixed. Tested with release 9.0.39. -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 64809] New: it's haven't reset connection properties based on connectionpool configuration information when returnConnection
https://bz.apache.org/bugzilla/show_bug.cgi?id=64809 Bug ID: 64809 Summary: it's haven't reset connection properties based on connectionpool configuration information when returnConnection Product: Tomcat 8 Version: 8.5.11 Hardware: All OS: All Status: NEW Severity: normal Priority: P2 Component: Connectors Assignee: dev@tomcat.apache.org Reporter: xiongchao90.1...@163.com Target Milestone: it's haven't reset connection properties based on connectionpool configuration information when returnConnection Causes the connection pool properties to be confused if A thread connection A con.setReadOnly(true) con.setAutoCommit(false)，release connection A。 B thread get connection A execute update throws exception -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 64809] Connection properties not reset to defaults when Connection is returned to the pool
https://bz.apache.org/bugzilla/show_bug.cgi?id=64809
--- Comment #2 from xiongchao ---
protected void returnConnection(PooledConnection con) {
if (this.isClosed()) {
this.release(con);
} else {
if (con != null) {
try {
this.returnedCount.incrementAndGet();
con.lock();
if (con.isSuspect()) {
if (this.poolProperties.isLogAbandoned() &&
log.isInfoEnabled()) {
log.info("Connection(" + con + ") that has been
marked suspect was returned." + " The processing time is " +
(System.currentTimeMillis() - con.getTimestamp()) + " ms.");
}
if (this.jmxPool != null) {
this.jmxPool.notify("SUSPECT CONNECTION RETURNED",
"Connection(" + con + ") that has been marked suspect was returned.");
}
}
if (this.busy.remove(con)) {
if (!this.shouldClose(con, 2)) {
con.setStackTrace((String)null);
con.setTimestamp(System.currentTimeMillis());
if (this.idle.size() >=
this.poolProperties.getMaxIdle() && !this.poolProperties.isPoolSweeperEnabled()
|| !this.idle.offer(con)) {
if (log.isDebugEnabled()) {
log.debug("Connection [" + con + "] will be
closed and not returned to the pool, idle[" + this.idle.size() + "]>=maxIdle["
+ this.poolProperties.getMaxIdle() + "] idle.offer failed.");
}
this.release(con);
}
} else {
if (log.isDebugEnabled()) {
log.debug("Connection [" + con + "] will be
closed and not returned to the pool.");
}
this.release(con);
}
} else {
if (log.isDebugEnabled()) {
log.debug("Connection [" + con + "] will be closed
and not returned to the pool, busy.remove failed.");
}
this.release(con);
}
} finally {
con.unlock();
}
}
}
}
--
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: [tomcat] branch master updated: Partial fix for BZ 63362 provide h2 request statistics
On 09/10/2020 17:43, ma...@apache.org wrote:
> This is an automated email from the ASF dual-hosted git repository.
>
> markt pushed a commit to branch master
> in repository https://gitbox.apache.org/repos/asf/tomcat.git
>
>
> The following commit(s) were added to refs/heads/master by this push:
> new 62a684c Partial fix for BZ 63362 provide h2 request statistics
> 62a684c is described below
>
> commit 62a684cd2b2039a68e74813cf4e20fc95a2cebe4
> Author: Mark Thomas
> AuthorDate: Wed Jun 24 19:24:17 2020 +0100
>
> Partial fix for BZ 63362 provide h2 request statistics
Just a quick heads up that as I work on WebSocket (and a generic
solution for any HTTP upgrade protocol) it is looking as if a chunk of
the changes below are going to be reverted.
Mark
> ---
> java/org/apache/coyote/AbstractProtocol.java | 35 +++
> java/org/apache/coyote/LocalStrings.properties| 1 +
> java/org/apache/coyote/UpgradeProtocol.java | 22
> java/org/apache/coyote/http2/Http2Protocol.java | 41
> +++
> java/org/apache/coyote/http2/StreamProcessor.java | 6
> webapps/docs/changelog.xml| 4 +++
> 6 files changed, 102 insertions(+), 7 deletions(-)
>
> diff --git a/java/org/apache/coyote/AbstractProtocol.java
> b/java/org/apache/coyote/AbstractProtocol.java
> index e214f80..159531e 100644
> --- a/java/org/apache/coyote/AbstractProtocol.java
> +++ b/java/org/apache/coyote/AbstractProtocol.java
> @@ -69,12 +69,6 @@ public abstract class AbstractProtocol implements
> ProtocolHandler,
>
>
> /**
> - * Name of MBean for the Global Request Processor.
> - */
> -protected ObjectName rgOname = null;
> -
> -
> -/**
> * Unique ID for this connector. Only used if the connector is configured
> * to use a random port as the port will change if stop(), start() is
> * called.
> @@ -145,6 +139,14 @@ public abstract class AbstractProtocol implements
> ProtocolHandler,
> // --- Properties managed by the
> ProtocolHandler
>
> /**
> + * Name of MBean for the Global Request Processor.
> + */
> +protected ObjectName rgOname = null;
> +public ObjectName getGlobalRequestProcessorMBeanName() {
> +return rgOname;
> +}
> +
> +/**
> * The adapter provides the link between the ProtocolHandler and the
> * connector.
> */
> @@ -546,7 +548,8 @@ public abstract class AbstractProtocol implements
> ProtocolHandler,
> }
>
> if (this.domain != null) {
> -rgOname = new ObjectName(domain +
> ":type=GlobalRequestProcessor,name=" + getName());
> +ObjectName rgOname = new ObjectName(domain +
> ":type=GlobalRequestProcessor,name=" + getName());
> +this.rgOname = rgOname;
> Registry.getRegistry(null, null).registerComponent(
> getHandler().getGlobal(), rgOname, null);
> }
> @@ -556,6 +559,13 @@ public abstract class AbstractProtocol implements
> ProtocolHandler,
> endpoint.setDomain(domain);
>
> endpoint.init();
> +
> +UpgradeProtocol[] upgradeProtocols = findUpgradeProtocols();
> +for (UpgradeProtocol upgradeProtocol : upgradeProtocols) {
> +// Implementation note: Failure of one upgrade protocol fails the
> +// whole Connector
> +upgradeProtocol.init();
> +}
> }
>
>
> @@ -669,6 +679,16 @@ public abstract class AbstractProtocol implements
> ProtocolHandler,
> logPortOffset();
> }
>
> +UpgradeProtocol[] upgradeProtocols = findUpgradeProtocols();
> +for (UpgradeProtocol upgradeProtocol : upgradeProtocols) {
> +try {
> +upgradeProtocol.destroy();
> +} catch (Throwable t) {
> +ExceptionUtils.handleThrowable(t);
> +
> getLog().error(sm.getString("abstractProtocol.upgradeProtocolDestroyError"),
> t);
> +}
> +}
> +
> try {
> endpoint.destroy();
> } finally {
> @@ -686,6 +706,7 @@ public abstract class AbstractProtocol implements
> ProtocolHandler,
> }
> }
>
> +ObjectName rgOname = getGlobalRequestProcessorMBeanName();
> if (rgOname != null) {
> Registry.getRegistry(null,
> null).unregisterComponent(rgOname);
> }
> diff --git a/java/org/apache/coyote/LocalStrings.properties
> b/java/org/apache/coyote/LocalStrings.properties
> index 83960cb..43c8d64 100644
> --- a/java/org/apache/coyote/LocalStrings.properties
> +++ b/java/org/apache/coyote/LocalStrings.properties
> @@ -36,6 +36,7 @@ abstractProcessor.socket.ssl=Exception getting SSL
> attributes
> abstractProtocol.mbeanDeregistrationFailed=Failed to deregister MBean named
> [{0}] from MBean server [{1}]
>
Re: [DISCUSS] Deprecate and remove RealmBase#stripRealmForGss
On 13/10/2020 10:48, Michael Osipov wrote: > Folks, > > I'd like to propose to get rid of that config option in 10 and deprecate > in previous versions for the following reasons: > > * It suffers from abstraction: It assumes that the GSS name is always > email style w/o checking its OID > * The realm part, if any, is an integeral part of the principal. Much > like with an email address' domain. You wouldn't stip here too, would you? > * It is a surprise for clients having the princippal mutilated by > default. I trip over and over again this when I set up > UserDatabaseRealms for testing purposes I wonder why > michae...@example.com does not work. > * In a multi realm environment, it is perfectly fine and valid to have > user1@REALMA and user1@REALMB. These are distinct principals, but > treated by RealmBase equally, this has implications. > * Finally, when doing cert-based auth in an AD envinronment, is it > pretty common to extract the msUPN name from the cert's SAN which is > almost always email address (enteprise principal) which would end up in > michael.osipov, but where is the rest?! > > Thoughts? No objections to changing the default to false in 10.0.x but I think removal/deprecation is a step too far. Not everyone uses the full u...@bigco.com format as the "primary key" in their internal user database. I see lots of variation depending on which system is viewed as the source of truth to which other systems are synchronised. We have X509UsernameRetriever for certificate authentication. That came out of bug 52500 where the aim was to support a wide variety of formats. The root of that requirement was essentially the same - integration with a variety of systems where the user name could be in in a range of formats. The mapping of that format to an X509 cert just added another layer of complexity. If there was user demand for it (I haven't seen any) I'd support a similar interface for SPNEGO. With such an interface in place, deprecation and removal of stripRealmForGss would make sense. Mark - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [tomcat] 01/04: Add connection pool to JNDI realm
On Tue, Oct 13, 2020 at 11:33 AM Michael Osipov wrote:
> Am 2020-10-07 um 22:34 schrieb r...@apache.org:
> > 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
> >
> > commit 50de36b7874da98591345e40b374a1e2dd52c188
> > Author: remm
> > AuthorDate: Thu Jan 30 17:22:51 2020 +0100
> >
> > Add connection pool to JNDI realm
> >
> > This implements a TODO from the class javadoc header.
> > As described in the javadoc, the idea is to use a pool to avoid
> blocking
> > on a single connection, which could possibly become a bottleneck in
> some
> > cases. The message formats need to be kept along with the connection
> > since they are not thread safe.
> > Preserve the default behavior: sync without pooling (using a Lock
> object
> > which is more flexible).
> > I may backport this since this is limited to the JNDI realm, but
> only
> > once it is confirmed to be regression free. Tested with ApacheDS
> but my
> > LDAP skills are very limited.
> > ---
> > java/org/apache/catalina/realm/JNDIRealm.java | 442
> -
> > .../apache/catalina/realm/LocalStrings.properties | 1 +
> > test/org/apache/catalina/realm/TestJNDIRealm.java | 7 +-
> > webapps/docs/changelog.xml | 3 +
> > webapps/docs/config/realm.xml | 7 +
> > 5 files changed, 276 insertions(+), 184 deletions(-)
>
> Salut Rémy,
>
> this is a very very nice improvement to the matter and I gave your idea
> a spin my public Active Directory realm. Based on my tests with AD I see
> room for improvement: (Note that I don't use the JNDIRealm because
> because is it too generic and usable for me)
>
Ok, I only tested this with a local ldap, so not much. I verified it was
pooling connections.
>
> * You might want to consider to introduce a maxIdleTime to avoid stale
> connections, e.g., AD has default value of 15 minutes. Yep, I had had
> several RSTs resulting in 401s
>
Oops. So I don't think adding something like this helps since there could
be plenty of issues besides a timeout. Basically it's supposed to retry
instead when something fails. Can you give me a stack trace to show what's
wrong ?
https://github.com/apache/tomcat/blob/master/java/org/apache/catalina/realm/JNDIRealm.java#L1306
Looking at the code, it does retry, but it would simply get a new
connection from the pool again. So if it gets a second bad connection, then
it would most likely fail. Is it what happened for you ?
A solution could be to create a new connection for the pooled case, to make
up for that.
> * Validating connections optionally might be a good option to detect
> stale connections or broken ones ny networks issues. This works for me
> very fast:
> > protected boolean validate(DirContextConnection connection) {
> > if (logger.isDebugEnabled())
> >
> logger.debug(sm.getString("activeDirectoryRealm.validate"));
> >
> > SearchControls controls = new SearchControls();
> > controls.setSearchScope(SearchControls.OBJECT_SCOPE);
> > controls.setCountLimit(1);
> > controls.setReturningAttributes(new String[] {
> "objectClass" });
> > controls.setTimeLimit(500);
> >
> > try {
> > NamingEnumeration results =
> connection.context.search("", "objectclass=*",
> > controls);
> >
> > if (results.hasMore()) {
> > close(results);
> > return true;
> > }
> > } catch (NamingException e) {
> >
> logger.error(sm.getString("activeDirectoryRealm.validate.namingException"),
> e);
> >
> > return false;
> > }
> >
> > return false;
> > }
> I do this in the acquire() method
>
Ok, but it has a cost and I fundamentally don't see why it's different from
performing the actual operation.
>
> * The acquire(), authenticate(), fail, close(), authenticate(),
> release() is brittle and proved to fail here. I have a curl script which
> does 4 requests in parallel and 1200 requests in total. Pool size is 8.
> 4 connections in the pool. Rest for 16 minutes, connections are broken
> now. Start requests. authenticate() will fail twice because top two
> acquired connections from pool are broken. Requests end with 401. I'd
> prefer to to modify acquire() is such a fashion:
> > protected DirContextConnection acquire() {
> > if (logger.isDebugEnabled())
> >
> logger.debug(sm.getString("activeDirectoryRealm.acquire"));
> >
> > DirContextConnection connection = null;
> >
> > while (connection == null) {
> > connection = connectionPool.pop();
> >
> >
[Bug 64809] Connection properties not reset to defaults when Connection is returned to the pool
https://bz.apache.org/bugzilla/show_bug.cgi?id=64809 Mark Thomas changed: What|Removed |Added Target Milestone||--- Summary|it's haven't reset |Connection properties not |connection properties based |reset to defaults when |on connectionpool |Connection is returned to |configuration information |the pool |when returnConnection | Version|8.5.11 |unspecified Component|Connectors |jdbc-pool Product|Tomcat 8|Tomcat Modules --- Comment #1 from Mark Thomas --- Correcting product (I'm assuming jdbc-pool on the basis I've confirmed DBCP does reset these properties) -- 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 9.0.x updated: Always retry on a new connection, even when pooling
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 a91a861 Always retry on a new connection, even when pooling
a91a861 is described below
commit a91a861fa269383f0f415e95d40066e7de32d895
Author: remm
AuthorDate: Tue Oct 13 14:19:54 2020 +0200
Always retry on a new connection, even when pooling
This keeps the same very simple design as for the single connection
scenario, for now.
---
java/org/apache/catalina/realm/JNDIRealm.java | 22 +++---
webapps/docs/changelog.xml| 5 +
2 files changed, 24 insertions(+), 3 deletions(-)
diff --git a/java/org/apache/catalina/realm/JNDIRealm.java
b/java/org/apache/catalina/realm/JNDIRealm.java
index b9acc42..95b7d0f 100644
--- a/java/org/apache/catalina/realm/JNDIRealm.java
+++ b/java/org/apache/catalina/realm/JNDIRealm.java
@@ -1303,7 +1303,7 @@ public class JNDIRealm extends RealmBase {
close(connection);
// open a new directory context.
-connection = get();
+connection = get(true);
// Try the authentication again.
principal = authenticate(connection, username, credentials);
@@ -2374,12 +2374,28 @@ public class JNDIRealm extends RealmBase {
* @exception NamingException if a directory server error occurs
*/
protected JNDIConnection get() throws NamingException {
+return get(false);
+}
+
+/**
+ * Open (if necessary) and return a connection to the configured
+ * directory server for this Realm.
+ * @param create when pooling, this forces creation of a new connection,
+ * for example after an error
+ * @return the connection
+ * @exception NamingException if a directory server error occurs
+ */
+protected JNDIConnection get(boolean create) throws NamingException {
JNDIConnection connection = null;
// Use the pool if available, otherwise use the single connection
if (connectionPool != null) {
-connection = connectionPool.pop();
-if (connection == null) {
+if (create) {
connection = create();
+} else {
+connection = connectionPool.pop();
+if (connection == null) {
+connection = create();
+}
}
} else {
singleConnectionLock.lock();
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 7cc0676..0e10f9d 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -58,6 +58,11 @@
64805: Correct imports used by JMXPorxyServlet.
(markt)
+
+Fix JNDIRealm pooling problems retrying on another bad connection. Any
+retries are made on a new connection, just like with the single
+connection scenario. (remm)
+
-
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: Always retry on a new connection, even when pooling
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 883600b Always retry on a new connection, even when pooling
883600b is described below
commit 883600b8a77c0be93b3a8dc2404e8d1110970ee7
Author: remm
AuthorDate: Tue Oct 13 14:19:54 2020 +0200
Always retry on a new connection, even when pooling
This keeps the same very simple design as for the single connection
scenario, for now.
---
java/org/apache/catalina/realm/JNDIRealm.java | 22 +++---
webapps/docs/changelog.xml| 5 +
2 files changed, 24 insertions(+), 3 deletions(-)
diff --git a/java/org/apache/catalina/realm/JNDIRealm.java
b/java/org/apache/catalina/realm/JNDIRealm.java
index 72087ab..98007f7 100644
--- a/java/org/apache/catalina/realm/JNDIRealm.java
+++ b/java/org/apache/catalina/realm/JNDIRealm.java
@@ -1311,7 +1311,7 @@ public class JNDIRealm extends RealmBase {
close(connection);
// open a new directory context.
-connection = get();
+connection = get(true);
// Try the authentication again.
principal = authenticate(connection, username, credentials);
@@ -2389,12 +2389,28 @@ public class JNDIRealm extends RealmBase {
* @exception NamingException if a directory server error occurs
*/
protected JNDIConnection get() throws NamingException {
+return get(false);
+}
+
+/**
+ * Open (if necessary) and return a connection to the configured
+ * directory server for this Realm.
+ * @param create when pooling, this forces creation of a new connection,
+ * for example after an error
+ * @return the connection
+ * @exception NamingException if a directory server error occurs
+ */
+protected JNDIConnection get(boolean create) throws NamingException {
JNDIConnection connection = null;
// Use the pool if available, otherwise use the single connection
if (connectionPool != null) {
-connection = connectionPool.pop();
-if (connection == null) {
+if (create) {
connection = create();
+} else {
+connection = connectionPool.pop();
+if (connection == null) {
+connection = create();
+}
}
} else {
singleConnectionLock.lock();
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index ea25154..7e3c4b9 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -61,6 +61,11 @@
64805: Correct imports used by JMXPorxyServlet.
(markt)
+
+Fix JNDIRealm pooling problems retrying on another bad connection. Any
+retries are made on a new connection, just like with the single
+connection scenario. (remm)
+
-
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: Always retry on a new connection, even when pooling
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 a91a861 Always retry on a new connection, even when pooling
a91a861 is described below
commit a91a861fa269383f0f415e95d40066e7de32d895
Author: remm
AuthorDate: Tue Oct 13 14:19:54 2020 +0200
Always retry on a new connection, even when pooling
This keeps the same very simple design as for the single connection
scenario, for now.
---
java/org/apache/catalina/realm/JNDIRealm.java | 22 +++---
webapps/docs/changelog.xml| 5 +
2 files changed, 24 insertions(+), 3 deletions(-)
diff --git a/java/org/apache/catalina/realm/JNDIRealm.java
b/java/org/apache/catalina/realm/JNDIRealm.java
index b9acc42..95b7d0f 100644
--- a/java/org/apache/catalina/realm/JNDIRealm.java
+++ b/java/org/apache/catalina/realm/JNDIRealm.java
@@ -1303,7 +1303,7 @@ public class JNDIRealm extends RealmBase {
close(connection);
// open a new directory context.
-connection = get();
+connection = get(true);
// Try the authentication again.
principal = authenticate(connection, username, credentials);
@@ -2374,12 +2374,28 @@ public class JNDIRealm extends RealmBase {
* @exception NamingException if a directory server error occurs
*/
protected JNDIConnection get() throws NamingException {
+return get(false);
+}
+
+/**
+ * Open (if necessary) and return a connection to the configured
+ * directory server for this Realm.
+ * @param create when pooling, this forces creation of a new connection,
+ * for example after an error
+ * @return the connection
+ * @exception NamingException if a directory server error occurs
+ */
+protected JNDIConnection get(boolean create) throws NamingException {
JNDIConnection connection = null;
// Use the pool if available, otherwise use the single connection
if (connectionPool != null) {
-connection = connectionPool.pop();
-if (connection == null) {
+if (create) {
connection = create();
+} else {
+connection = connectionPool.pop();
+if (connection == null) {
+connection = create();
+}
}
} else {
singleConnectionLock.lock();
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 7cc0676..0e10f9d 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -58,6 +58,11 @@
64805: Correct imports used by JMXPorxyServlet.
(markt)
+
+Fix JNDIRealm pooling problems retrying on another bad connection. Any
+retries are made on a new connection, just like with the single
+connection scenario. (remm)
+
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [tomcat] 01/04: Add connection pool to JNDI realm
On Tue, Oct 13, 2020 at 1:49 PM Rémy Maucherat wrote: That it's the issue. However, since the realm works well in the single > connection case, I'd just put that loop in authenticate(String username, > String credentials), keep the same simple design and be done with it. > Done. If it doesn't work, feel free to add more refinements. Otherwise, I'll backport. Rémy
[tomcat] branch master updated: Always retry on a new connection, even when pooling
This is an automated email from the ASF dual-hosted git repository.
remm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/master by this push:
new 35b2d79 Always retry on a new connection, even when pooling
35b2d79 is described below
commit 35b2d79ad26535deedcc0c6bea2a057138e1bd30
Author: remm
AuthorDate: Tue Oct 13 14:19:54 2020 +0200
Always retry on a new connection, even when pooling
This keeps the same very simple design as for the single connection
scenario, for now.
---
java/org/apache/catalina/realm/JNDIRealm.java | 22 +++---
webapps/docs/changelog.xml| 5 +
2 files changed, 24 insertions(+), 3 deletions(-)
diff --git a/java/org/apache/catalina/realm/JNDIRealm.java
b/java/org/apache/catalina/realm/JNDIRealm.java
index 9ab4c10..a6012bd 100644
--- a/java/org/apache/catalina/realm/JNDIRealm.java
+++ b/java/org/apache/catalina/realm/JNDIRealm.java
@@ -1303,7 +1303,7 @@ public class JNDIRealm extends RealmBase {
close(connection);
// open a new directory context.
-connection = get();
+connection = get(true);
// Try the authentication again.
principal = authenticate(connection, username, credentials);
@@ -2373,12 +2373,28 @@ public class JNDIRealm extends RealmBase {
* @exception NamingException if a directory server error occurs
*/
protected JNDIConnection get() throws NamingException {
+return get(false);
+}
+
+/**
+ * Open (if necessary) and return a connection to the configured
+ * directory server for this Realm.
+ * @param create when pooling, this forces creation of a new connection,
+ * for example after an error
+ * @return the connection
+ * @exception NamingException if a directory server error occurs
+ */
+protected JNDIConnection get(boolean create) throws NamingException {
JNDIConnection connection = null;
// Use the pool if available, otherwise use the single connection
if (connectionPool != null) {
-connection = connectionPool.pop();
-if (connection == null) {
+if (create) {
connection = create();
+} else {
+connection = connectionPool.pop();
+if (connection == null) {
+connection = create();
+}
}
} else {
singleConnectionLock.lock();
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index d935750..97cf512 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -58,6 +58,11 @@
64805: Correct imports used by JMXPorxyServlet.
(markt)
+
+Fix JNDIRealm pooling problems retrying on another bad connection. Any
+retries are made on a new connection, just like with the single
+connection scenario. (remm)
+
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
