On Thu, Jul 20, 2023 at 12:30 PM Felix Schumacher
<felix.schumac...@internetallee.de> wrote:
>
>
> 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".

Pretty much.

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

Ok, I read the code one level up in Connection.readReply, and the
IOException in LdapRequest gets wrapped as a CommunicationException.
So the workaround is not needed in Tomcat 11 (trunk) since it requires
Java 21. However, Java 17 (and 11 obviously) don't have the fix, so
the change is needed in 10.1 and 9.0.

Rémy

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

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

Reply via email to