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émyRegards 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
OpenPGP_0xEA6C3728EA91C4AF.asc
Description: OpenPGP public key
OpenPGP_signature
Description: OpenPGP digital signature