Am 20.07.23 um 11:40 schrieb Rémy Maucherat:
On Thu, Jul 20, 2023 at 11:11 AM Felix Schumacher
<felix.schumac...@internetallee.de>  wrote:
Hi all,

at work, we have seen the following stacktrace without a retrying log message.

javax.naming.NamingException: LDAP connection has been closed
     at com.sun.jndi.ldap.LdapRequest.getReplyBer(LdapRequest.java:133) 
~[?:1.8.0_342]
     at com.sun.jndi.ldap.Connection.readReply(Connection.java:469) 
~[?:1.8.0_342]
     at com.sun.jndi.ldap.LdapClient.getSearchReply(LdapClient.java:638) 
~[?:1.8.0_342]
     at com.sun.jndi.ldap.LdapClient.search(LdapClient.java:561) ~[?:1.8.0_342]
     at com.sun.jndi.ldap.LdapCtx.doSearch(LdapCtx.java:2013) ~[?:1.8.0_342]
     at com.sun.jndi.ldap.LdapCtx.searchAux(LdapCtx.java:1872) ~[?:1.8.0_342]
     at com.sun.jndi.ldap.LdapCtx.c_search(LdapCtx.java:1797) ~[?:1.8.0_342]
     at 
com.sun.jndi.toolkit.ctx.ComponentDirContext.p_search(ComponentDirContext.java:392)
 ~[?:1.8.0_342]
     at 
com.sun.jndi.toolkit.ctx.PartialCompositeDirContext.search(PartialCompositeDirContext.java:358)
 ~[?:1.8.0_342]
     at 
com.sun.jndi.toolkit.ctx.PartialCompositeDirContext.search(PartialCompositeDirContext.java:341)
 ~[?:1.8.0_342]
     at 
javax.naming.directory.InitialDirContext.search(InitialDirContext.java:267) 
~[?:1.8.0_342]
     at 
org.apache.catalina.realm.JNDIRealm.getUserBySearch(JNDIRealm.java:1610) 
~[catalina.jar:9.0.50.redhat-00007]
     at org.apache.catalina.realm.JNDIRealm.getUser(JNDIRealm.java:1447) 
~[catalina.jar:9.0.50.redhat-00007]
     at org.apache.catalina.realm.JNDIRealm.getUser(JNDIRealm.java:1376) 
~[catalina.jar:9.0.50.redhat-00007]
     at org.apache.catalina.realm.JNDIRealm.getPrincipal(JNDIRealm.java:2348) 
~[catalina.jar:9.0.50.redhat-00007]
     at org.apache.catalina.realm.JNDIRealm.getPrincipal(JNDIRealm.java:2288) 
[catalina.jar:9.0.50.redhat-00007]
     at org.apache.catalina.realm.JNDIRealm.getPrincipal(JNDIRealm.java:2253) 
[catalina.jar:9.0.50.redhat-00007]

That happens, because we are catching CommunicationException and 
ServiceUnavailableException in getPrincipal instead of the more general 
NamingException.

We had a similar issue in Bug 61313. To fix that bug we changed the catch 
clause from CommunicationException to NamingException.

I think we should change the code in getPrincipal to catch the more general 
exception, too. Does anyone know, why we catched those specialized 
NamingExceptions instead of the general one?
I think the rationale was very simple: IO errors are always
recoverable by closing and retrying the connection. Other errors are
"????". Now reading your exception it is "NamingException: LDAP
connection has been closed", where it should have been
"CommunicationException: LDAP connection has been closed". This is
unfortunate.
Your proposed change would mean everything is assumed to be
recoverable which is not good, but unavoidable if everything is
reported as a NamingException.

I read your answer as, "ok, not nice, but let's do it".

And thanks for the explanation.

But to add to the fun, I looked at the source code of a current OpenJDK (https://github.com/openjdk/jdk/blob/94eb44b192ba421692549a178c386ea34164ea50/src/java.naming/share/classes/com/sun/jndi/ldap/LdapRequest.java#L115C26-L115C26).

It looks like we now can expect to even get an IOException in case the LDAP connection has been closed.

Regards

 Felix


Rémy

Regards

  Felix

PS. I will do a PR, if we agree on changing the catch clause.

PPS. The code to catch the exception is the same in current tomcat JNDIRealm 
classes, even if the line numbers changed a bit.
---------------------------------------------------------------------
To unsubscribe, e-mail:dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail:dev-h...@tomcat.apache.org

Attachment: OpenPGP_0xEA6C3728EA91C4AF.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature

Reply via email to