You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by re...@apache.org on 2020/10/07 20:34:54 UTC

[tomcat] branch 8.5.x updated (7c50a4f -> 03c4e62)

This is an automated email from the ASF dual-hosted git repository.

remm pushed a change to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git.


    from 7c50a4f  Fix BZ 55559. Add local JNDI support to the UserDatabaseRealm
     new 50de36b  Add connection pool to JNDI realm
     new 25d1f18  Remove leftover debug
     new 539377f  Remove unnecessary code and avoid potential NPEs. Thanks to SpotBugs.
     new 03c4e62  Remove leftover debug

The 4 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 java/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, 272 insertions(+), 188 deletions(-)


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[tomcat] 03/04: Remove unnecessary code and avoid potential NPEs. Thanks to SpotBugs.

Posted by re...@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 539377f7e2b831356dce7c5e68720e88085b78ee
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Fri Mar 6 12:17:03 2020 +0000

    Remove unnecessary code and avoid potential NPEs. Thanks to SpotBugs.
---
 java/org/apache/catalina/realm/JNDIRealm.java | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/java/org/apache/catalina/realm/JNDIRealm.java b/java/org/apache/catalina/realm/JNDIRealm.java
index d7553b6..1c5d06e 100644
--- a/java/org/apache/catalina/realm/JNDIRealm.java
+++ b/java/org/apache/catalina/realm/JNDIRealm.java
@@ -1329,9 +1329,6 @@ public class JNDIRealm extends RealmBase {
             // Log the problem for posterity
             containerLog.error(sm.getString("jndiRealm.exception"), e);
 
-            // Close the connection so that it gets reopened next time
-            close(connection);
-
             // Return "not authenticated" for this request
             if (containerLog.isDebugEnabled())
                 containerLog.debug("Returning null principal.");
@@ -2310,9 +2307,6 @@ System.out.println("getUserBySearch " + username);
             // Log the problem for posterity
             containerLog.error(sm.getString("jndiRealm.exception"), e);
 
-            // Close the connection so that it gets reopened next time
-            close(connection);
-
             // Return "not authenticated" for this request
             return null;
 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[tomcat] 04/04: Remove leftover debug

Posted by re...@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 03c4e624e9e6f5dae1a4216bea5d757c6f9ddde4
Author: remm <re...@apache.org>
AuthorDate: Mon Oct 5 13:39:53 2020 +0200

    Remove leftover debug
---
 java/org/apache/catalina/realm/JNDIRealm.java | 1 -
 1 file changed, 1 deletion(-)

diff --git a/java/org/apache/catalina/realm/JNDIRealm.java b/java/org/apache/catalina/realm/JNDIRealm.java
index 1c5d06e..72087ab 100644
--- a/java/org/apache/catalina/realm/JNDIRealm.java
+++ b/java/org/apache/catalina/realm/JNDIRealm.java
@@ -1667,7 +1667,6 @@ public class JNDIRealm extends RealmBase {
         if (attrIds == null)
             attrIds = new String[0];
         constraints.setReturningAttributes(attrIds);
-System.out.println("getUserBySearch " + username);
 
         NamingEnumeration<SearchResult> results =
                 connection.context.search(userBase, filter, constraints);


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: [tomcat] 01/04: Add connection pool to JNDI realm

Posted by Rémy Maucherat <re...@apache.org>.
On Tue, Oct 13, 2020 at 8:26 PM Michael Osipov <mi...@apache.org> wrote:

> Am 2020-10-13 um 13:49 schrieb Rémy Maucherat:
> > On Tue, Oct 13, 2020 at 11:33 AM Michael Osipov <mi...@apache.org>
> wrote:
> >
> >> Am 2020-10-07 um 22:34 schrieb remm@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 <re...@apache.org>
> >>> 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
> org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:87)
> >         at
> org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:343)
> >         at
> org.apache.coyote.http11.Http11Processor.service(Http11Processor.java:615)
> >         at
> org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:65)
> >         at
> org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:818)
> >         at org.apache.tomcat.util.net
> .AprEndpoint$SocketProcessor.doRun(AprEndpoint.java:2070)
> >         at org.apache.tomcat.util.net
> .SocketProcessorBase.run(SocketProcessorBase.java:49)
> >         at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
> >         at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
> >         at
> org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
> >         at java.lang.Thread.run(Thread.java:748)
> >     Caused by: java.net.SocketException: Connection reset
> >         at java.net.SocketInputStream.read(SocketInputStream.java:211)
> >         at java.net.SocketInputStream.read(SocketInputStream.java:141)
> >         at java.io.BufferedInputStream.fill(BufferedInputStream.java:246)
> >         at
> java.io.BufferedInputStream.read1(BufferedInputStream.java:286)
> >         at java.io.BufferedInputStream.read(BufferedInputStream.java:345)
> >         at
> com.sun.jndi.ldap.sasl.SaslInputStream.readFully(SaslInputStream.java:166)
> >         at
> com.sun.jndi.ldap.sasl.SaslInputStream.fill(SaslInputStream.java:123)
> >         at
> com.sun.jndi.ldap.sasl.SaslInputStream.read(SaslInputStream.java:90)
> >         at com.sun.jndi.ldap.Connection.run(Connection.java:846)
> >         ... 1 more
> > 2020-10-12T23:13:06 FEIN [https-openssl-apr-8444-exec-5]
> net.sf.michaelo.tomcat.realm.ActiveDirectoryRealm.close Closing directory
> server connection
> > 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
> org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:87)
> >         at
> org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:343)
> >         at
> org.apache.coyote.http11.Http11Processor.service(Http11Processor.java:615)
> >         at
> org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:65)
> >         at
> org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:818)
> >         at org.apache.tomcat.util.net
> .AprEndpoint$SocketProcessor.doRun(AprEndpoint.java:2070)
> >         at org.apache.tomcat.util.net
> .SocketProcessorBase.run(SocketProcessorBase.java:49)
> >         at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
> >         at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
> >         at
> org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
> >         at java.lang.Thread.run(Thread.java:748)
> >     Caused by: java.net.SocketException: Connection reset
> >         at java.net.SocketInputStream.read(SocketInputStream.java:211)
> >         at java.net.SocketInputStream.read(SocketInputStream.java:141)
> >         at java.io.BufferedInputStream.fill(BufferedInputStream.java:246)
> >         at
> java.io.BufferedInputStream.read1(BufferedInputStream.java:286)
> >         at java.io.BufferedInputStream.read(BufferedInputStream.java:345)
> >         at
> com.sun.jndi.ldap.sasl.SaslInputStream.readFully(SaslInputStream.java:166)
> >         at
> com.sun.jndi.ldap.sasl.SaslInputStream.fill(SaslInputStream.java:123)
> >         at
> com.sun.jndi.ldap.sasl.SaslInputStream.read(SaslInputStream.java:90)
> >         at com.sun.jndi.ldap.Connection.run(Connection.java:846)
> >         ... 1 more
> > 2020-10-12T23:13:06 FEIN [https-openssl-apr-8444-exec-5]
> net.sf.michaelo.tomcat.realm.ActiveDirectoryRealm.close Closing directory
> server connection
> > 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
> org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:87)
> >         at
> org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:343)
> >         at
> org.apache.coyote.http11.Http11Processor.service(Http11Processor.java:615)
> >         at
> org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:65)
> >         at
> org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:818)
> >         at org.apache.tomcat.util.net
> .AprEndpoint$SocketProcessor.doRun(AprEndpoint.java:2070)
> >         at org.apache.tomcat.util.net
> .SocketProcessorBase.run(SocketProcessorBase.java:49)
> >         at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
> >         at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
> >         at
> org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
> >         at java.lang.Thread.run(Thread.java:748)
> >     Caused by: java.net.SocketException: Connection reset
> >         at java.net.SocketInputStream.read(SocketInputStream.java:211)
> >         at java.net.SocketInputStream.read(SocketInputStream.java:141)
> >         at java.io.BufferedInputStream.fill(BufferedInputStream.java:246)
> >         at
> java.io.BufferedInputStream.read1(BufferedInputStream.java:286)
> >         at java.io.BufferedInputStream.read(BufferedInputStream.java:345)
> >         at
> com.sun.jndi.ldap.sasl.SaslInputStream.readFully(SaslInputStream.java:166)
> >         at
> com.sun.jndi.ldap.sasl.SaslInputStream.fill(SaslInputStream.java:123)
> >         at
> com.sun.jndi.ldap.sasl.SaslInputStream.read(SaslInputStream.java:90)
> >         at com.sun.jndi.ldap.Connection.run(Connection.java:846)
> >         ... 1 more
> > 2020-10-12T23:13:06 FEIN [https-openssl-apr-8444-exec-5]
> net.sf.michaelo.tomcat.realm.ActiveDirectoryRealm.close Closing directory
> server connection
> > 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
> org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:87)
> >         at
> org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:343)
> >         at
> org.apache.coyote.http11.Http11Processor.service(Http11Processor.java:615)
> >         at
> org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:65)
> >         at
> org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:818)
> >         at org.apache.tomcat.util.net
> .AprEndpoint$SocketProcessor.doRun(AprEndpoint.java:2070)
> >         at org.apache.tomcat.util.net
> .SocketProcessorBase.run(SocketProcessorBase.java:49)
> >         at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
> >         at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
> >         at
> org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
> >         at java.lang.Thread.run(Thread.java:748)
> >     Caused by: java.net.SocketException: Connection reset
> >         at java.net.SocketInputStream.read(SocketInputStream.java:211)
> >         at java.net.SocketInputStream.read(SocketInputStream.java:141)
> >         at java.io.BufferedInputStream.fill(BufferedInputStream.java:246)
> >         at
> java.io.BufferedInputStream.read1(BufferedInputStream.java:286)
> >         at java.io.BufferedInputStream.read(BufferedInputStream.java:345)
> >         at
> com.sun.jndi.ldap.sasl.SaslInputStream.readFully(SaslInputStream.java:166)
> >         at
> com.sun.jndi.ldap.sasl.SaslInputStream.fill(SaslInputStream.java:123)
> >         at
> com.sun.jndi.ldap.sasl.SaslInputStream.read(SaslInputStream.java:90)
> >         at com.sun.jndi.ldap.Connection.run(Connection.java:846)
> >         ... 1 more
> > 2020-10-12T23:13:06 FEIN [https-openssl-apr-8444-exec-5]
> net.sf.michaelo.tomcat.realm.ActiveDirectoryRealm.close Closing directory
> server connection
> > 2020-10-12T23:13:06 FEIN [https-openssl-apr-8444-exec-5]
> net.sf.michaelo.tomcat.realm.ActiveDirectoryRealm.open Opening new
> directory server connection
> > 2020-10-12T23:13:06 FEIN [https-openssl-apr-8444-exec-8]
> net.sf.michaelo.tomcat.realm.ActiveDirectoryRealm.acquire Acquiring
> directory server connection from pool
> > 2020-10-12T23:13:06 FEIN [https-openssl-apr-8444-exec-8]
> net.sf.michaelo.tomcat.realm.ActiveDirectoryRealm.open Opening new
> directory server connection
> > 2020-10-12T23:13:06 FEIN [https-openssl-apr-8444-exec-10]
> net.sf.michaelo.tomcat.realm.ActiveDirectoryRealm.acquire Acquiring
> directory server connection from pool
> > 2020-10-12T23:13:06 FEIN [https-openssl-apr-8444-exec-10]
> net.sf.michaelo.tomcat.realm.ActiveDirectoryRealm.open Opening new
> directory server connection
> > 2020-10-12T23:13:06 FEIN [https-openssl-apr-8444-exec-4]
> net.sf.michaelo.tomcat.realm.ActiveDirectoryRealm.acquire Acquiring
> directory server connection from pool
> > 2020-10-12T23:13:06 FEIN [https-openssl-apr-8444-exec-4]
> net.sf.michaelo.tomcat.realm.ActiveDirectoryRealm.open Opening new
> directory server connection
> > 2020-10-12T23:13:07 FEIN [https-openssl-apr-8444-exec-10]
> net.sf.michaelo.tomcat.realm.ActiveDirectoryRealm.getUser Searching for
> username 'osipovmi' in base 'DC=ad001,DC=siemens,DC=net' and attribute
> 'sAMAccountName' with mapper 'SamAccountNameRfc2247Mapper'
>
>
>
> >
> 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 ?
>
> Correct!
>
> > A solution could be to create a new connection for the pooled case, to
> make
> > up for that.
>
> That is one, but also consider that other connections in the stack are
> broken too in such a case. So the next authenticate request would run in
> into the same issue.
>
> >> * 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<SearchResult> 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.
>
> It has a cost (like "select 1 from dual"), but consider that better than
> solving the connection issue in authenticate() and having the user auth
> fail at all.
>

The idea here was to do the exact same behavior + pooling as before and I
think it does that.

It's ok if the changes now make you want to improve the JNDIRealm further.
Feel free to do anything you like if that works better for real ldap
servers. I don't have any real world uses on hand personally [my test
localhost ldap is not a good representation of anything].

If you want to track connection activity because you *know* it's going to
be timedout, then it's going to be much more efficient for sure. The only
problem is it requires accurate user configuration since it's server
dependent. Annoying. I'm still skeptical about an active connection
validation, but maybe it's also better for the real world.

Rémy

Re: [tomcat] 01/04: Add connection pool to JNDI realm

Posted by Michael Osipov <mi...@apache.org>.
Am 2020-10-13 um 13:49 schrieb Rémy Maucherat:
> On Tue, Oct 13, 2020 at 11:33 AM Michael Osipov <mi...@apache.org> wrote:
> 
>> Am 2020-10-07 um 22:34 schrieb remm@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 <re...@apache.org>
>>> 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 org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:87)
>         at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:343)
>         at org.apache.coyote.http11.Http11Processor.service(Http11Processor.java:615)
>         at org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:65)
>         at org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:818)
>         at org.apache.tomcat.util.net.AprEndpoint$SocketProcessor.doRun(AprEndpoint.java:2070)
>         at org.apache.tomcat.util.net.SocketProcessorBase.run(SocketProcessorBase.java:49)
>         at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
>         at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
>         at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
>         at java.lang.Thread.run(Thread.java:748)
>     Caused by: java.net.SocketException: Connection reset
>         at java.net.SocketInputStream.read(SocketInputStream.java:211)
>         at java.net.SocketInputStream.read(SocketInputStream.java:141)
>         at java.io.BufferedInputStream.fill(BufferedInputStream.java:246)
>         at java.io.BufferedInputStream.read1(BufferedInputStream.java:286)
>         at java.io.BufferedInputStream.read(BufferedInputStream.java:345)
>         at com.sun.jndi.ldap.sasl.SaslInputStream.readFully(SaslInputStream.java:166)
>         at com.sun.jndi.ldap.sasl.SaslInputStream.fill(SaslInputStream.java:123)
>         at com.sun.jndi.ldap.sasl.SaslInputStream.read(SaslInputStream.java:90)
>         at com.sun.jndi.ldap.Connection.run(Connection.java:846)
>         ... 1 more
> 2020-10-12T23:13:06 FEIN [https-openssl-apr-8444-exec-5] net.sf.michaelo.tomcat.realm.ActiveDirectoryRealm.close Closing directory server connection
> 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 org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:87)
>         at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:343)
>         at org.apache.coyote.http11.Http11Processor.service(Http11Processor.java:615)
>         at org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:65)
>         at org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:818)
>         at org.apache.tomcat.util.net.AprEndpoint$SocketProcessor.doRun(AprEndpoint.java:2070)
>         at org.apache.tomcat.util.net.SocketProcessorBase.run(SocketProcessorBase.java:49)
>         at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
>         at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
>         at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
>         at java.lang.Thread.run(Thread.java:748)
>     Caused by: java.net.SocketException: Connection reset
>         at java.net.SocketInputStream.read(SocketInputStream.java:211)
>         at java.net.SocketInputStream.read(SocketInputStream.java:141)
>         at java.io.BufferedInputStream.fill(BufferedInputStream.java:246)
>         at java.io.BufferedInputStream.read1(BufferedInputStream.java:286)
>         at java.io.BufferedInputStream.read(BufferedInputStream.java:345)
>         at com.sun.jndi.ldap.sasl.SaslInputStream.readFully(SaslInputStream.java:166)
>         at com.sun.jndi.ldap.sasl.SaslInputStream.fill(SaslInputStream.java:123)
>         at com.sun.jndi.ldap.sasl.SaslInputStream.read(SaslInputStream.java:90)
>         at com.sun.jndi.ldap.Connection.run(Connection.java:846)
>         ... 1 more
> 2020-10-12T23:13:06 FEIN [https-openssl-apr-8444-exec-5] net.sf.michaelo.tomcat.realm.ActiveDirectoryRealm.close Closing directory server connection
> 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 org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:87)
>         at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:343)
>         at org.apache.coyote.http11.Http11Processor.service(Http11Processor.java:615)
>         at org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:65)
>         at org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:818)
>         at org.apache.tomcat.util.net.AprEndpoint$SocketProcessor.doRun(AprEndpoint.java:2070)
>         at org.apache.tomcat.util.net.SocketProcessorBase.run(SocketProcessorBase.java:49)
>         at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
>         at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
>         at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
>         at java.lang.Thread.run(Thread.java:748)
>     Caused by: java.net.SocketException: Connection reset
>         at java.net.SocketInputStream.read(SocketInputStream.java:211)
>         at java.net.SocketInputStream.read(SocketInputStream.java:141)
>         at java.io.BufferedInputStream.fill(BufferedInputStream.java:246)
>         at java.io.BufferedInputStream.read1(BufferedInputStream.java:286)
>         at java.io.BufferedInputStream.read(BufferedInputStream.java:345)
>         at com.sun.jndi.ldap.sasl.SaslInputStream.readFully(SaslInputStream.java:166)
>         at com.sun.jndi.ldap.sasl.SaslInputStream.fill(SaslInputStream.java:123)
>         at com.sun.jndi.ldap.sasl.SaslInputStream.read(SaslInputStream.java:90)
>         at com.sun.jndi.ldap.Connection.run(Connection.java:846)
>         ... 1 more
> 2020-10-12T23:13:06 FEIN [https-openssl-apr-8444-exec-5] net.sf.michaelo.tomcat.realm.ActiveDirectoryRealm.close Closing directory server connection
> 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 org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:87)
>         at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:343)
>         at org.apache.coyote.http11.Http11Processor.service(Http11Processor.java:615)
>         at org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:65)
>         at org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:818)
>         at org.apache.tomcat.util.net.AprEndpoint$SocketProcessor.doRun(AprEndpoint.java:2070)
>         at org.apache.tomcat.util.net.SocketProcessorBase.run(SocketProcessorBase.java:49)
>         at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
>         at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
>         at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
>         at java.lang.Thread.run(Thread.java:748)
>     Caused by: java.net.SocketException: Connection reset
>         at java.net.SocketInputStream.read(SocketInputStream.java:211)
>         at java.net.SocketInputStream.read(SocketInputStream.java:141)
>         at java.io.BufferedInputStream.fill(BufferedInputStream.java:246)
>         at java.io.BufferedInputStream.read1(BufferedInputStream.java:286)
>         at java.io.BufferedInputStream.read(BufferedInputStream.java:345)
>         at com.sun.jndi.ldap.sasl.SaslInputStream.readFully(SaslInputStream.java:166)
>         at com.sun.jndi.ldap.sasl.SaslInputStream.fill(SaslInputStream.java:123)
>         at com.sun.jndi.ldap.sasl.SaslInputStream.read(SaslInputStream.java:90)
>         at com.sun.jndi.ldap.Connection.run(Connection.java:846)
>         ... 1 more
> 2020-10-12T23:13:06 FEIN [https-openssl-apr-8444-exec-5] net.sf.michaelo.tomcat.realm.ActiveDirectoryRealm.close Closing directory server connection
> 2020-10-12T23:13:06 FEIN [https-openssl-apr-8444-exec-5] net.sf.michaelo.tomcat.realm.ActiveDirectoryRealm.open Opening new directory server connection
> 2020-10-12T23:13:06 FEIN [https-openssl-apr-8444-exec-8] net.sf.michaelo.tomcat.realm.ActiveDirectoryRealm.acquire Acquiring directory server connection from pool
> 2020-10-12T23:13:06 FEIN [https-openssl-apr-8444-exec-8] net.sf.michaelo.tomcat.realm.ActiveDirectoryRealm.open Opening new directory server connection
> 2020-10-12T23:13:06 FEIN [https-openssl-apr-8444-exec-10] net.sf.michaelo.tomcat.realm.ActiveDirectoryRealm.acquire Acquiring directory server connection from pool
> 2020-10-12T23:13:06 FEIN [https-openssl-apr-8444-exec-10] net.sf.michaelo.tomcat.realm.ActiveDirectoryRealm.open Opening new directory server connection
> 2020-10-12T23:13:06 FEIN [https-openssl-apr-8444-exec-4] net.sf.michaelo.tomcat.realm.ActiveDirectoryRealm.acquire Acquiring directory server connection from pool
> 2020-10-12T23:13:06 FEIN [https-openssl-apr-8444-exec-4] net.sf.michaelo.tomcat.realm.ActiveDirectoryRealm.open Opening new directory server connection
> 2020-10-12T23:13:07 FEIN [https-openssl-apr-8444-exec-10] net.sf.michaelo.tomcat.realm.ActiveDirectoryRealm.getUser Searching for username 'osipovmi' in base 'DC=ad001,DC=siemens,DC=net' and attribute 'sAMAccountName' with mapper 'SamAccountNameRfc2247Mapper'



> 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 ?

Correct!

> A solution could be to create a new connection for the pooled case, to make
> up for that.

That is one, but also consider that other connections in the stack are 
broken too in such a case. So the next authenticate request would run in 
into the same issue.

>> * 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<SearchResult> 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.

It has a cost (like "select 1 from dual"), but consider that better than 
solving the connection issue in authenticate() and having the user auth 
fail at all.

M


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: [tomcat] 01/04: Add connection pool to JNDI realm

Posted by Rémy Maucherat <re...@apache.org>.
On Tue, Oct 13, 2020 at 1:49 PM Rémy Maucherat <re...@apache.org> 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

Re: [tomcat] 01/04: Add connection pool to JNDI realm

Posted by Rémy Maucherat <re...@apache.org>.
On Tue, Oct 13, 2020 at 11:33 AM Michael Osipov <mi...@apache.org> wrote:

> Am 2020-10-07 um 22:34 schrieb remm@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 <re...@apache.org>
> > 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<SearchResult> 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();
> >
> >                       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 = null;
> >                                       }
> >                               }
> >                       } else {
> >                               connection = new DirContextConnection();
> >                               open(connection);
> >                       }
> >               }
>
>
> WDYT?
>

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.

Rémy

Re: [tomcat] 01/04: Add connection pool to JNDI realm

Posted by Michael Osipov <mi...@apache.org>.
Am 2020-10-07 um 22:34 schrieb remm@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 <re...@apache.org>
> 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<SearchResult> 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 = null;
> 					}
> 				}
> 			} else {
> 				connection = new DirContextConnection();
> 				open(connection);
> 			}
> 		}


WDYT?


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[tomcat] 01/04: Add connection pool to JNDI realm

Posted by re...@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 <re...@apache.org>
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(-)

diff --git a/java/org/apache/catalina/realm/JNDIRealm.java b/java/org/apache/catalina/realm/JNDIRealm.java
index a9565dc..a22e3c6 100644
--- a/java/org/apache/catalina/realm/JNDIRealm.java
+++ b/java/org/apache/catalina/realm/JNDIRealm.java
@@ -32,6 +32,8 @@ import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Set;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
 
 import javax.naming.AuthenticationException;
 import javax.naming.CommunicationException;
@@ -61,6 +63,7 @@ import javax.net.ssl.SSLSession;
 import javax.net.ssl.SSLSocketFactory;
 
 import org.apache.catalina.LifecycleException;
+import org.apache.tomcat.util.collections.SynchronizedStack;
 import org.ietf.jgss.GSSCredential;
 import org.ietf.jgss.GSSName;
 
@@ -166,10 +169,6 @@ import org.ietf.jgss.GSSName;
  *     directory server itself.</li>
  * </ul>
  *
- * <p><strong>TODO</strong> - Support connection pooling (including message
- * format objects) so that <code>authenticate()</code> does not have to be
- * synchronized.</p>
- *
  * <p><strong>WARNING</strong> - There is a reported bug against the Netscape
  * provider code (com.netscape.jndi.ldap.LdapContextFactory) with respect to
  * successfully authenticated a non-existing user. The
@@ -209,12 +208,6 @@ public class JNDIRealm extends RealmBase {
 
 
     /**
-     * The directory context linking us to our directory server.
-     */
-    protected DirContext context = null;
-
-
-    /**
      * The JNDI context factory used to acquire our InitialContext.  By
      * default, assumes use of an LDAP server using the standard JNDI LDAP
      * provider.
@@ -291,13 +284,6 @@ public class JNDIRealm extends RealmBase {
 
 
     /**
-     * The MessageFormat object associated with the current
-     * <code>userSearch</code>.
-     */
-    protected MessageFormat userSearchFormat = null;
-
-
-    /**
      * Should we search the entire subtree for matching users?
      */
     protected boolean userSubtree = false;
@@ -337,32 +323,12 @@ public class JNDIRealm extends RealmBase {
 
 
     /**
-     * An array of MessageFormat objects associated with the current
-     * <code>userPatternArray</code>.
-     */
-    protected MessageFormat[] userPatternFormatArray = null;
-
-    /**
      * The base element for role searches.
      */
     protected String roleBase = "";
 
 
     /**
-     * The MessageFormat object associated with the current
-     * <code>roleBase</code>.
-     */
-    protected MessageFormat roleBaseFormat = null;
-
-
-    /**
-     * The MessageFormat object associated with the current
-     * <code>roleSearch</code>.
-     */
-    protected MessageFormat roleFormat = null;
-
-
-    /**
      * The name of an attribute in the user's entry containing
      * roles for that user
      */
@@ -508,6 +474,30 @@ public class JNDIRealm extends RealmBase {
     private boolean forceDnHexEscape = false;
 
 
+    /**
+     * Non pooled connection to our directory server.
+     */
+    protected JNDIConnection singleConnection = new JNDIConnection();
+
+
+    /**
+     * The lock to ensure single connection thread safety.
+     */
+    protected final Lock singleConnectionLock = new ReentrantLock();
+
+
+    /**
+     * Connection pool.
+     */
+    protected SynchronizedStack<JNDIConnection> connectionPool = null;
+
+
+    /**
+     * The pool size limit. If 1, pooling is not used.
+     */
+    protected int connectionPoolSize = 1;
+
+
     // ------------------------------------------------------------- Properties
 
     public boolean getForceDnHexEscape() {
@@ -736,13 +726,8 @@ public class JNDIRealm extends RealmBase {
      * @param userSearch The new user search pattern
      */
     public void setUserSearch(String userSearch) {
-
         this.userSearch = userSearch;
-        if (userSearch == null)
-            userSearchFormat = null;
-        else
-            userSearchFormat = new MessageFormat(userSearch);
-
+        singleConnection = create();
     }
 
 
@@ -815,13 +800,8 @@ public class JNDIRealm extends RealmBase {
      * @param roleBase The new base element
      */
     public void setRoleBase(String roleBase) {
-
         this.roleBase = roleBase;
-        if (roleBase == null)
-            roleBaseFormat = null;
-        else
-            roleBaseFormat = new MessageFormat(roleBase);
-
+        singleConnection = create();
     }
 
 
@@ -863,13 +843,8 @@ public class JNDIRealm extends RealmBase {
      * @param roleSearch The new role search pattern
      */
     public void setRoleSearch(String roleSearch) {
-
         this.roleSearch = roleSearch;
-        if (roleSearch == null)
-            roleFormat = null;
-        else
-            roleFormat = new MessageFormat(roleSearch);
-
+        singleConnection = create();
     }
 
 
@@ -979,18 +954,12 @@ public class JNDIRealm extends RealmBase {
      * @param userPattern The new user pattern
      */
     public void setUserPattern(String userPattern) {
-
         this.userPattern = userPattern;
-        if (userPattern == null)
+        if (userPattern == null) {
             userPatternArray = null;
-        else {
+        } else {
             userPatternArray = parseUserPatternString(userPattern);
-            int len = this.userPatternArray.length;
-            userPatternFormatArray = new MessageFormat[len];
-            for (int i=0; i < len; i++) {
-                userPatternFormatArray[i] =
-                    new MessageFormat(userPatternArray[i]);
-            }
+            singleConnection = create();
         }
     }
 
@@ -1171,6 +1140,22 @@ public class JNDIRealm extends RealmBase {
     }
 
     /**
+     * @return the connection pool size, or the default value 1 if pooling
+     *   is disabled
+     */
+    public int getConnectionPoolSize() {
+        return connectionPoolSize;
+    }
+
+    /**
+     * Set the connection pool size
+     * @param connectionPoolSize the new pool size
+     */
+    public void setConnectionPoolSize(int connectionPoolSize) {
+        this.connectionPoolSize = connectionPoolSize;
+    }
+
+    /**
      * @return name of the {@link HostnameVerifier} class used for connections
      *         using StartTLS, or the empty string, if the default verifier
      *         should be used.
@@ -1288,20 +1273,20 @@ public class JNDIRealm extends RealmBase {
     @Override
     public Principal authenticate(String username, String credentials) {
 
-        DirContext context = null;
+        JNDIConnection connection = null;
         Principal principal = null;
 
         try {
 
             // Ensure that we have a directory context available
-            context = open();
+            connection = get();
 
             // Occasionally the directory context will timeout.  Try one more
             // time before giving up.
             try {
 
                 // Authenticate the specified username if possible
-                principal = authenticate(context, username, credentials);
+                principal = authenticate(connection, username, credentials);
 
             } catch (NullPointerException | NamingException e) {
                 /*
@@ -1323,19 +1308,18 @@ public class JNDIRealm extends RealmBase {
                 containerLog.info(sm.getString("jndiRealm.exception.retry"), e);
 
                 // close the connection so we know it will be reopened.
-                if (context != null)
-                    close(context);
+                close(connection);
 
                 // open a new directory context.
-                context = open();
+                connection = get();
 
                 // Try the authentication again.
-                principal = authenticate(context, username, credentials);
+                principal = authenticate(connection, username, credentials);
             }
 
 
             // Release this context
-            release(context);
+            release(connection);
 
             // Return the authenticated Principal (if any)
             return principal;
@@ -1346,8 +1330,7 @@ public class JNDIRealm extends RealmBase {
             containerLog.error(sm.getString("jndiRealm.exception"), e);
 
             // Close the connection so that it gets reopened next time
-            if (context != null)
-                close(context);
+            close(connection);
 
             // Return "not authenticated" for this request
             if (containerLog.isDebugEnabled())
@@ -1369,7 +1352,7 @@ public class JNDIRealm extends RealmBase {
      * Return the Principal associated with the specified username and
      * credentials, if there is one; otherwise return <code>null</code>.
      *
-     * @param context The directory context
+     * @param connection The directory context
      * @param username Username of the Principal to look up
      * @param credentials Password or other credentials to use in
      *  authenticating this username
@@ -1377,7 +1360,7 @@ public class JNDIRealm extends RealmBase {
      *
      * @exception NamingException if a directory server error occurs
      */
-    public synchronized Principal authenticate(DirContext context,
+    public Principal authenticate(JNDIConnection connection,
                                                String username,
                                                String credentials)
         throws NamingException {
@@ -1391,16 +1374,16 @@ public class JNDIRealm extends RealmBase {
 
         if (userPatternArray != null) {
             for (int curUserPattern = 0;
-                 curUserPattern < userPatternFormatArray.length;
+                 curUserPattern < userPatternArray.length;
                  curUserPattern++) {
                 // Retrieve user information
-                User user = getUser(context, username, credentials, curUserPattern);
+                User user = getUser(connection, username, credentials, curUserPattern);
                 if (user != null) {
                     try {
                         // Check the user's credentials
-                        if (checkCredentials(context, user, credentials)) {
+                        if (checkCredentials(connection.context, user, credentials)) {
                             // Search for additional roles
-                            List<String> roles = getRoles(context, user);
+                            List<String> roles = getRoles(connection, user);
                             if (containerLog.isDebugEnabled()) {
                                 containerLog.debug("Found roles: " + roles.toString());
                             }
@@ -1419,16 +1402,16 @@ public class JNDIRealm extends RealmBase {
             return null;
         } else {
             // Retrieve user information
-            User user = getUser(context, username, credentials);
+            User user = getUser(connection, username, credentials);
             if (user == null)
                 return null;
 
             // Check the user's credentials
-            if (!checkCredentials(context, user, credentials))
+            if (!checkCredentials(connection.context, user, credentials))
                 return null;
 
             // Search for additional roles
-            List<String> roles = getRoles(context, user);
+            List<String> roles = getRoles(connection, user);
             if (containerLog.isDebugEnabled()) {
                 containerLog.debug("Found roles: " + roles.toString());
             }
@@ -1444,17 +1427,17 @@ public class JNDIRealm extends RealmBase {
      * with the specified username, if found in the directory;
      * otherwise return <code>null</code>.
      *
-     * @param context The directory context
+     * @param connection The directory context
      * @param username Username to be looked up
      * @return the User object
      * @exception NamingException if a directory server error occurs
      *
-     * @see #getUser(DirContext, String, String, int)
+     * @see #getUser(JNDIConnection, String, String, int)
      */
-    protected User getUser(DirContext context, String username)
+    protected User getUser(JNDIConnection connection, String username)
         throws NamingException {
 
-        return getUser(context, username, null, -1);
+        return getUser(connection, username, null, -1);
     }
 
 
@@ -1463,18 +1446,18 @@ public class JNDIRealm extends RealmBase {
      * with the specified username, if found in the directory;
      * otherwise return <code>null</code>.
      *
-     * @param context The directory context
+     * @param connection The directory context
      * @param username Username to be looked up
      * @param credentials User credentials (optional)
      * @return the User object
      * @exception NamingException if a directory server error occurs
      *
-     * @see #getUser(DirContext, String, String, int)
+     * @see #getUser(JNDIConnection, String, String, int)
      */
-    protected User getUser(DirContext context, String username, String credentials)
+    protected User getUser(JNDIConnection connection, String username, String credentials)
         throws NamingException {
 
-        return getUser(context, username, credentials, -1);
+        return getUser(connection, username, credentials, -1);
     }
 
 
@@ -1489,14 +1472,14 @@ public class JNDIRealm extends RealmBase {
      * configuration attribute is specified, all values of that
      * attribute are retrieved from the directory entry.
      *
-     * @param context The directory context
+     * @param connection The directory context
      * @param username Username to be looked up
      * @param credentials User credentials (optional)
      * @param curUserPattern Index into userPatternFormatArray
      * @return the User object
      * @exception NamingException if a directory server error occurs
      */
-    protected User getUser(DirContext context, String username,
+    protected User getUser(JNDIConnection connection, String username,
                            String credentials, int curUserPattern)
         throws NamingException {
 
@@ -1515,8 +1498,8 @@ public class JNDIRealm extends RealmBase {
         list.toArray(attrIds);
 
         // Use pattern or search for user entry
-        if (userPatternFormatArray != null && curUserPattern >= 0) {
-            user = getUserByPattern(context, username, credentials, attrIds, curUserPattern);
+        if (userPatternArray != null && curUserPattern >= 0) {
+            user = getUserByPattern(connection, username, credentials, attrIds, curUserPattern);
             if (containerLog.isDebugEnabled()) {
                 containerLog.debug("Found user by pattern [" + user + "]");
             }
@@ -1524,12 +1507,12 @@ public class JNDIRealm extends RealmBase {
             boolean thisUserSearchAsUser = isUserSearchAsUser();
             try {
                 if (thisUserSearchAsUser) {
-                    userCredentialsAdd(context, username, credentials);
+                    userCredentialsAdd(connection.context, username, credentials);
                 }
-                user = getUserBySearch(context, username, attrIds);
+                user = getUserBySearch(connection, username, attrIds);
             } finally {
                 if (thisUserSearchAsUser) {
-                    userCredentialsRemove(context);
+                    userCredentialsRemove(connection.context);
                 }
             }
             if (containerLog.isDebugEnabled()) {
@@ -1606,7 +1589,7 @@ public class JNDIRealm extends RealmBase {
      * username and return a User object; otherwise return
      * <code>null</code>.
      *
-     * @param context The directory context
+     * @param connection The directory context
      * @param username The username
      * @param credentials User credentials (optional)
      * @param attrIds String[]containing names of attributes to
@@ -1615,7 +1598,7 @@ public class JNDIRealm extends RealmBase {
      * @exception NamingException if a directory server error occurs
      * @see #getUserByPattern(DirContext, String, String[], String)
      */
-    protected User getUserByPattern(DirContext context,
+    protected User getUserByPattern(JNDIConnection connection,
                                     String username,
                                     String credentials,
                                     String[] attrIds,
@@ -1624,25 +1607,25 @@ public class JNDIRealm extends RealmBase {
 
         User user = null;
 
-        if (username == null || userPatternFormatArray[curUserPattern] == null)
+        if (username == null || userPatternArray[curUserPattern] == null)
             return null;
 
         // Form the dn from the user pattern
-        String dn = userPatternFormatArray[curUserPattern].format(new String[] { username });
+        String dn = connection.userPatternFormatArray[curUserPattern].format(new String[] { username });
 
         try {
-            user = getUserByPattern(context, username, attrIds, dn);
+            user = getUserByPattern(connection.context, username, attrIds, dn);
         } catch (NameNotFoundException e) {
             return null;
         } catch (NamingException e) {
             // If the getUserByPattern() call fails, try it again with the
             // credentials of the user that we're searching for
             try {
-                userCredentialsAdd(context, dn, credentials);
+                userCredentialsAdd(connection.context, dn, credentials);
 
-                user = getUserByPattern(context, username, attrIds, dn);
+                user = getUserByPattern(connection.context, username, attrIds, dn);
             } finally {
-                userCredentialsRemove(context);
+                userCredentialsRemove(connection.context);
             }
         }
         return user;
@@ -1654,22 +1637,22 @@ public class JNDIRealm extends RealmBase {
      * information about the user with the specified username, if
      * found in the directory; otherwise return <code>null</code>.
      *
-     * @param context The directory context
+     * @param connection The directory context
      * @param username The username
      * @param attrIds String[]containing names of attributes to retrieve.
      * @return the User object
      * @exception NamingException if a directory server error occurs
      */
-    protected User getUserBySearch(DirContext context,
+    protected User getUserBySearch(JNDIConnection connection,
                                    String username,
                                    String[] attrIds)
         throws NamingException {
 
-        if (username == null || userSearchFormat == null)
+        if (username == null || connection.userSearchFormat == null)
             return null;
 
         // Form the search filter
-        String filter = userSearchFormat.format(new String[] { username });
+        String filter = connection.userSearchFormat.format(new String[] { username });
 
         // Set up the search controls
         SearchControls constraints = new SearchControls();
@@ -1687,9 +1670,10 @@ public class JNDIRealm extends RealmBase {
         if (attrIds == null)
             attrIds = new String[0];
         constraints.setReturningAttributes(attrIds);
+System.out.println("getUserBySearch " + username);
 
         NamingEnumeration<SearchResult> results =
-            context.search(userBase, filter, constraints);
+                connection.context.search(userBase, filter, constraints);
 
         try {
             // Fail if no entries found
@@ -1710,8 +1694,9 @@ public class JNDIRealm extends RealmBase {
             // Check no further entries were found
             try {
                 if (results.hasMore()) {
-                    if(containerLog.isInfoEnabled())
-                        containerLog.info("username " + username + " has multiple entries");
+                    if (containerLog.isInfoEnabled()) {
+                        containerLog.info(sm.getString("jndiRealm.multipleEntries", username));
+                    }
                     return null;
                 }
             } catch (PartialResultException ex) {
@@ -1719,7 +1704,7 @@ public class JNDIRealm extends RealmBase {
                     throw ex;
             }
 
-            String dn = getDistinguishedName(context, userBase, result);
+            String dn = getDistinguishedName(connection.context, userBase, result);
 
             if (containerLog.isTraceEnabled())
                 containerLog.trace("  entry found for " + username + " with dn " + dn);
@@ -1741,6 +1726,7 @@ public class JNDIRealm extends RealmBase {
 
             // Retrieve values of userRoleName attribute
             ArrayList<String> roles = null;
+System.out.println("userRoleName " + userRoleName + " " + attrs.get(userRoleName));
             if (userRoleName != null)
                 roles = addAttributeValues(userRoleName, attrs, roles);
 
@@ -1918,12 +1904,12 @@ public class JNDIRealm extends RealmBase {
      * a directory search. If no roles are associated with this user,
      * a zero-length List is returned.
      *
-     * @param context The directory context we are searching
+     * @param connection The directory context we are searching
      * @param user The User to be checked
      * @return the list of role names
      * @exception NamingException if a directory server error occurs
      */
-    protected List<String> getRoles(DirContext context, User user)
+    protected List<String> getRoles(JNDIConnection connection, User user)
         throws NamingException {
 
         if (user == null)
@@ -1954,11 +1940,11 @@ public class JNDIRealm extends RealmBase {
         }
 
         // Are we configured to do role searches?
-        if ((roleFormat == null) || (roleName == null))
+        if ((connection.roleFormat == null) || (roleName == null))
             return list;
 
         // Set up parameters for an appropriate search
-        String filter = roleFormat.format(new String[] { doRFC2254Encoding(dn), username, userRoleId });
+        String filter = connection.roleFormat.format(new String[] { doRFC2254Encoding(dn), username, userRoleId });
         SearchControls controls = new SearchControls();
         if (roleSubtree)
             controls.setSearchScope(SearchControls.SUBTREE_SCOPE);
@@ -1967,20 +1953,20 @@ public class JNDIRealm extends RealmBase {
         controls.setReturningAttributes(new String[] {roleName});
 
         String base = null;
-        if (roleBaseFormat != null) {
-            NameParser np = context.getNameParser("");
+        if (connection.roleBaseFormat != null) {
+            NameParser np = connection.context.getNameParser("");
             Name name = np.parse(dn);
             String nameParts[] = new String[name.size()];
             for (int i = 0; i < name.size(); i++) {
                 nameParts[i] = name.get(i);
             }
-            base = roleBaseFormat.format(nameParts);
+            base = connection.roleBaseFormat.format(nameParts);
         } else {
             base = "";
         }
 
         // Perform the configured search and process the results
-        NamingEnumeration<SearchResult> results = searchAsUser(context, user, base, filter, controls,
+        NamingEnumeration<SearchResult> results = searchAsUser(connection.context, user, base, filter, controls,
                 isRoleSearchAsUser());
 
         if (results == null)
@@ -1993,7 +1979,7 @@ public class JNDIRealm extends RealmBase {
                 Attributes attrs = result.getAttributes();
                 if (attrs == null)
                     continue;
-                String dname = getDistinguishedName(context, roleBase, result);
+                String dname = getDistinguishedName(connection.context, roleBase, result);
                 String name = getAttributeValue(roleName, attrs);
                 if (name != null && dname != null) {
                     groupMap.put(dname, name);
@@ -2026,14 +2012,14 @@ public class JNDIRealm extends RealmBase {
                 Map<String, String> newThisRound = new HashMap<>(); // Stores the groups we find in this iteration
 
                 for (Entry<String, String> group : newGroups.entrySet()) {
-                    filter = roleFormat.format(new String[] { doRFC2254Encoding(group.getKey()),
+                    filter = connection.roleFormat.format(new String[] { doRFC2254Encoding(group.getKey()),
                             group.getValue(), group.getValue() });
 
                     if (containerLog.isTraceEnabled()) {
                         containerLog.trace("Perform a nested group search with base "+ roleBase + " and filter " + filter);
                     }
 
-                    results = searchAsUser(context, user, roleBase, filter, controls,
+                    results = searchAsUser(connection.context, user, roleBase, filter, controls,
                             isRoleSearchAsUser());
 
                     try {
@@ -2042,7 +2028,7 @@ public class JNDIRealm extends RealmBase {
                             Attributes attrs = result.getAttributes();
                             if (attrs == null)
                                 continue;
-                            String dname = getDistinguishedName(context, roleBase, result);
+                            String dname = getDistinguishedName(connection.context, roleBase, result);
                             String name = getAttributeValue(roleName, attrs);
                             if (name != null && dname != null && !groupMap.keySet().contains(dname)) {
                                 groupMap.put(dname, name);
@@ -2185,12 +2171,12 @@ public class JNDIRealm extends RealmBase {
     /**
      * Close any open connection to the directory server for this Realm.
      *
-     * @param context The directory context to be closed
+     * @param connection The directory context to be closed
      */
-    protected void close(DirContext context) {
+    protected void close(JNDIConnection connection) {
 
         // Do nothing if there is no opened connection
-        if (context == null)
+        if (connection.context == null)
             return;
 
         // Close tls startResponse if used
@@ -2205,11 +2191,15 @@ public class JNDIRealm extends RealmBase {
         try {
             if (containerLog.isDebugEnabled())
                 containerLog.debug("Closing directory context");
-            context.close();
+            connection.context.close();
         } catch (NamingException e) {
             containerLog.error(sm.getString("jndiRealm.close"), e);
         }
-        this.context = null;
+        connection.context = null;
+        // The lock will be reacquired before any manipulation of the connection
+        if (connectionPool == null) {
+            singleConnectionLock.unlock();
+        }
 
     }
 
@@ -2234,7 +2224,7 @@ public class JNDIRealm extends RealmBase {
         }
 
         try {
-            User user = getUser(open(), username, null);
+            User user = getUser(get(), username, null);
             if (user == null) {
                 // User should be found...
                 return null;
@@ -2278,20 +2268,20 @@ public class JNDIRealm extends RealmBase {
     protected Principal getPrincipal(String username,
             GSSCredential gssCredential) {
 
-        DirContext context = null;
+        JNDIConnection connection = null;
         Principal principal = null;
 
         try {
 
             // Ensure that we have a directory context available
-            context = open();
+            connection = get();
 
             // Occasionally the directory context will timeout.  Try one more
             // time before giving up.
             try {
 
                 // Authenticate the specified username if possible
-                principal = getPrincipal(context, username, gssCredential);
+                principal = getPrincipal(connection, username, gssCredential);
 
             } catch (CommunicationException | ServiceUnavailableException e) {
 
@@ -2299,20 +2289,19 @@ public class JNDIRealm extends RealmBase {
                 containerLog.info(sm.getString("jndiRealm.exception.retry"), e);
 
                 // close the connection so we know it will be reopened.
-                if (context != null)
-                    close(context);
+                close(connection);
 
                 // open a new directory context.
-                context = open();
+                connection = get();
 
                 // Try the authentication again.
-                principal = getPrincipal(context, username, gssCredential);
+                principal = getPrincipal(connection, username, gssCredential);
 
             }
 
 
             // Release this context
-            release(context);
+            release(connection);
 
             // Return the authenticated Principal (if any)
             return principal;
@@ -2323,8 +2312,7 @@ public class JNDIRealm extends RealmBase {
             containerLog.error(sm.getString("jndiRealm.exception"), e);
 
             // Close the connection so that it gets reopened next time
-            if (context != null)
-                close(context);
+            close(connection);
 
             // Return "not authenticated" for this request
             return null;
@@ -2337,19 +2325,20 @@ public class JNDIRealm extends RealmBase {
 
     /**
      * Get the principal associated with the specified certificate.
-     * @param context The directory context
+     * @param connection The directory context
      * @param username The user name
      * @param gssCredential The credentials
      * @return the Principal associated with the given certificate.
      * @exception NamingException if a directory server error occurs
      */
-    protected synchronized Principal getPrincipal(DirContext context,
+    protected Principal getPrincipal(JNDIConnection connection,
             String username, GSSCredential gssCredential)
         throws NamingException {
 
         User user = null;
         List<String> roles = null;
         Hashtable<?, ?> preservedEnvironment = null;
+        DirContext context = connection.context;
 
         try {
             if (gssCredential != null && isUseDelegatedCredential()) {
@@ -2365,9 +2354,9 @@ public class JNDIRealm extends RealmBase {
                 // Note: Subject already set in SPNEGO authenticator so no need
                 //       for Subject.doAs() here
             }
-            user = getUser(context, username);
+            user = getUser(connection, username);
             if (user != null) {
-                roles = getRoles(context, user);
+                roles = getRoles(connection, user);
             }
         } finally {
             if (gssCredential != null && isUseDelegatedCredential()) {
@@ -2404,50 +2393,100 @@ public class JNDIRealm extends RealmBase {
     /**
      * Open (if necessary) and return a connection to the configured
      * directory server for this Realm.
-     * @return the directory context
+     * @return the connection
      * @exception NamingException if a directory server error occurs
      */
-    protected DirContext open() throws NamingException {
+    protected JNDIConnection get() throws NamingException {
+        JNDIConnection connection = null;
+        // Use the pool if available, otherwise use the single connection
+        if (connectionPool != null) {
+            connection = connectionPool.pop();
+            if (connection == null) {
+                connection = create();
+            }
+        } else {
+            singleConnectionLock.lock();
+            connection = singleConnection;
+        }
+        if (connection.context == null) {
+            open(connection);
+        }
+        return connection;
+    }
 
-        // Do nothing if there is a directory server connection already open
-        if (context != null)
-            return context;
+    /**
+     * Release our use of this connection so that it can be recycled.
+     *
+     * @param connection The directory context to release
+     */
+    protected void release(JNDIConnection connection) {
+        if (connectionPool != null) {
+            if (!connectionPool.push(connection)) {
+                // Any connection that doesn't end back to the pool must be closed
+                close(connection);
+            }
+        } else {
+            singleConnectionLock.unlock();
+        }
+    }
 
-        try {
+    /**
+     * Create a new connection wrapper, along with the
+     * message formats.
+     * @return the new connection
+     */
+    protected JNDIConnection create() {
+        JNDIConnection connection = new JNDIConnection();
+        if (userSearch != null) {
+            connection.userSearchFormat = new MessageFormat(userSearch);
+        }
+        if (userPattern != null) {
+            int len = userPatternArray.length;
+            connection.userPatternFormatArray = new MessageFormat[len];
+            for (int i = 0; i < len; i++) {
+                connection.userPatternFormatArray[i] =
+                        new MessageFormat(userPatternArray[i]);
+            }
+        }
+        if (roleBase != null) {
+            connection.roleBaseFormat = new MessageFormat(roleBase);
+        }
+        if (roleSearch != null) {
+            connection.roleFormat = new MessageFormat(roleSearch);
+        }
+        return connection;
+    }
 
+    /**
+     * Create a new connection to the directory server.
+     * @param connection The directory server connection wrapper
+     * @throws NamingException if a directory server error occurs
+     */
+    protected void open(JNDIConnection connection) throws NamingException {
+        try {
             // Ensure that we have a directory context available
-            context = createDirContext(getDirectoryContextEnvironment());
-
+            connection.context = createDirContext(getDirectoryContextEnvironment());
         } catch (Exception e) {
             if (alternateURL == null || alternateURL.length() == 0) {
                 // No alternate URL. Re-throw the exception.
                 throw e;
             }
-
             connectionAttempt = 1;
-
             // log the first exception.
             containerLog.info(sm.getString("jndiRealm.exception.retry"), e);
-
             // Try connecting to the alternate url.
-            context = createDirContext(getDirectoryContextEnvironment());
-
+            connection.context = createDirContext(getDirectoryContextEnvironment());
         } finally {
-
             // reset it in case the connection times out.
             // the primary may come back.
             connectionAttempt = 0;
-
         }
-
-        return context;
-
     }
 
     @Override
     public boolean isAvailable() {
         // Simple best effort check
-        return (context != null);
+        return (connectionPool != null || singleConnection.context != null);
     }
 
     private DirContext createDirContext(Hashtable<String, String> env) throws NamingException {
@@ -2600,18 +2639,6 @@ public class JNDIRealm extends RealmBase {
     }
 
 
-    /**
-     * Release our use of this connection so that it can be recycled.
-     *
-     * @param context The directory context to release
-     */
-    protected void release(DirContext context) {
-
-        // NO-OP since we are not pooling anything
-
-    }
-
-
     // ------------------------------------------------------ Lifecycle Methods
 
 
@@ -2626,15 +2653,22 @@ public class JNDIRealm extends RealmBase {
     @Override
     protected void startInternal() throws LifecycleException {
 
+        if (connectionPoolSize != 1) {
+            connectionPool = new SynchronizedStack<>(SynchronizedStack.DEFAULT_SIZE, connectionPoolSize);
+        }
+
         // Check to see if the connection to the directory can be opened
+        JNDIConnection connection = null;
         try {
-            open();
+            connection = get();
         } catch (NamingException e) {
             // A failure here is not fatal as the directory may be unavailable
             // now but available later. Unavailability of the directory is not
             // fatal once the Realm has started so there is no reason for it to
             // be fatal when the Realm starts.
             containerLog.error(sm.getString("jndiRealm.open"), e);
+        } finally {
+            release(connection);
         }
 
         super.startInternal();
@@ -2651,12 +2685,18 @@ public class JNDIRealm extends RealmBase {
      */
      @Override
     protected void stopInternal() throws LifecycleException {
-
         super.stopInternal();
-
         // Close any open directory server connection
-        close(this.context);
-
+        if (connectionPool == null) {
+            singleConnectionLock.lock();
+            close(singleConnection);
+        } else {
+            JNDIConnection connection = null;
+            while ((connection = connectionPool.pop()) != null) {
+                close(connection);
+            }
+            connectionPool = null;
+        }
     }
 
     /**
@@ -2933,5 +2973,43 @@ public class JNDIRealm extends RealmBase {
             return userRoleId;
         }
     }
+
+    /**
+     * Class holding the connection to the directory plus the associated
+     * non thread safe message formats.
+     */
+    protected static class JNDIConnection {
+
+        /**
+         * The MessageFormat object associated with the current
+         * <code>userSearch</code>.
+         */
+        protected MessageFormat userSearchFormat = null;
+
+        /**
+         * An array of MessageFormat objects associated with the current
+         * <code>userPatternArray</code>.
+         */
+        protected MessageFormat[] userPatternFormatArray = null;
+
+        /**
+         * The MessageFormat object associated with the current
+         * <code>roleBase</code>.
+         */
+        protected MessageFormat roleBaseFormat = null;
+
+        /**
+         * The MessageFormat object associated with the current
+         * <code>roleSearch</code>.
+         */
+        protected MessageFormat roleFormat = null;
+
+        /**
+         * The directory context linking us to our directory server.
+         */
+        protected DirContext context = null;
+
+    }
+
 }
 
diff --git a/java/org/apache/catalina/realm/LocalStrings.properties b/java/org/apache/catalina/realm/LocalStrings.properties
index 52f6e42..8b4f5ca 100644
--- a/java/org/apache/catalina/realm/LocalStrings.properties
+++ b/java/org/apache/catalina/realm/LocalStrings.properties
@@ -66,6 +66,7 @@ jndiRealm.exception.retry=Exception performing authentication. Retrying...
 jndiRealm.invalidHostnameVerifier=[{0}] not a valid class name for a HostnameVerifier
 jndiRealm.invalidSslProtocol=Given protocol [{0}] is invalid. It has to be one of [{1}]
 jndiRealm.invalidSslSocketFactory=[{0}] not a valid class name for an SSLSocketFactory
+jndiRealm.multipleEntries=User name [{0}] has multiple entries
 jndiRealm.negotiatedTls=Negotiated tls connection using protocol [{0}]
 jndiRealm.open=Exception opening directory server connection
 jndiRealm.tlsClose=Exception closing tls response
diff --git a/test/org/apache/catalina/realm/TestJNDIRealm.java b/test/org/apache/catalina/realm/TestJNDIRealm.java
index 35383fd..203e794 100644
--- a/test/org/apache/catalina/realm/TestJNDIRealm.java
+++ b/test/org/apache/catalina/realm/TestJNDIRealm.java
@@ -117,9 +117,12 @@ public class TestJNDIRealm {
         realm.setContainer(context);
         realm.setUserSearch("");
 
-        Field field = JNDIRealm.class.getDeclaredField("context");
+        // Usually everything is created in create() but that's not the case here
+        Field field = JNDIRealm.class.getDeclaredField("singleConnection");
         field.setAccessible(true);
-        field.set(realm, mockDirContext(mockSearchResults(password)));
+        Field field2 = JNDIRealm.JNDIConnection.class.getDeclaredField("context");
+        field2.setAccessible(true);
+        field2.set(field.get(realm), mockDirContext(mockSearchResults(password)));
 
         realm.start();
 
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 69ff0fb..49ef2e2 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -54,6 +54,9 @@
         context. This option is only useful when the Realm is defined on the
         Context. (markt)
       </fix>
+      <update>
+        Add connection pooling to JNDI realm. (remm)
+      </update>
     </changelog>
   </subsection>
 </section>
diff --git a/webapps/docs/config/realm.xml b/webapps/docs/config/realm.xml
index db60315..ba8c217 100644
--- a/webapps/docs/config/realm.xml
+++ b/webapps/docs/config/realm.xml
@@ -312,6 +312,13 @@
         property.</p>
       </attribute>
 
+      <attribute name="connectionPoolSize" required="false">
+        <p>The JNDI realm can use a pool of connections to the directory server
+        to avoid blocking on a single connection. This attribute value is the
+        maximum pool size. If not specified, it will use <code>1</code>, which
+        means a single connection will be used.</p>
+      </attribute>
+
       <attribute name="connectionTimeout" required="false">
         <p>The timeout in milliseconds to use when establishing the connection
         to the LDAP directory. If not specified, a value of 5000 (5 seconds) is


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[tomcat] 02/04: Remove leftover debug

Posted by re...@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 25d1f181a40698ae3e07cc5fca30f42a19c62eae
Author: remm <re...@apache.org>
AuthorDate: Thu Jan 30 18:02:43 2020 +0100

    Remove leftover debug
---
 java/org/apache/catalina/realm/JNDIRealm.java | 1 -
 1 file changed, 1 deletion(-)

diff --git a/java/org/apache/catalina/realm/JNDIRealm.java b/java/org/apache/catalina/realm/JNDIRealm.java
index a22e3c6..d7553b6 100644
--- a/java/org/apache/catalina/realm/JNDIRealm.java
+++ b/java/org/apache/catalina/realm/JNDIRealm.java
@@ -1726,7 +1726,6 @@ System.out.println("getUserBySearch " + username);
 
             // Retrieve values of userRoleName attribute
             ArrayList<String> roles = null;
-System.out.println("userRoleName " + userRoleName + " " + attrs.get(userRoleName));
             if (userRoleName != null)
                 roles = addAttributeValues(userRoleName, attrs, roles);
 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org