On Fri, 13 Feb 2026 11:15:00 GMT, Jaikiran Pai <[email protected]> wrote:
> Can I please get a review of this change which removes the unused > `src/java.naming/share/classes/com/sun/jndi/ldap/jndiprovider.properties` > file? This addresses the issue noted in > https://bugs.openjdk.org/browse/JDK-8296183. > > This `jndiprovider.properties` file lists 3 properties > `java.naming.factory.control`, `java.naming.factory.object` and > `java.naming.factory.state`. The semantics of these environment properties > are explained in > https://docs.oracle.com/javase/jndi/tutorial/beyond/env/provider.html. The > classes configured in `com/sun/jndi/ldap/jndiprovider.properties` for each of > these properties are non-existent in the JDK. Looking at the version control > history of the JDK, these classes haven't been around for several releases > (not even in JDK 8). The `jndiprovier.properties` gets looked up by an JDK > internal class in the `java.naming` module - > `com.sun.naming.internal.ResourceManager`. The > `ResourceManager.getFactories(...)` method is the one that gets called to > load the classes configured for those 3 environment properties. The javadoc > of `ResourceManager.getFactories(...)` says this: > > > Retrieves an enumeration of factory classes/object specified by a property. > > The property is gotten from the environment and the provider resource file > associated with the given context and concatenated. > ... The resulting property value is a list of class names. > This method then loads each class using the current thread's context class > loader and keeps them in a list. > Any class that cannot be loaded is ignored. > ... > > > The implementation of `ResourceManager.getFactories(...)` matches that > javadoc. Because the implementation ignores such missing classes, the > reference to these non-existent classes in the > `com/sun/jndi/ldap/jndiprovider.properties` has gone unnoticed all this > while. Manual experiments of exercising this code path does indeed show that > a `ClassNotFoundException` gets raised (and ignored) for the classes > referenced in that `jnidprovider.properties` file. > > The commit in this PR removes the `jndiprovier.properties`. It also removes a > javadoc reference to one of these non-existent classes. Given the nature of > the change no new tests have been introduced and the existing tests in tier1, > tier2 and tier3 continue to pass. Generally looks good to me. Good to avoid those unneeded CNFE. If backported, some more checks might be required on JDK 8 - as CORBA is still supported there. src/java.naming/share/classes/com/sun/jndi/ldap/EntryChangeResponseControl.java line 54: > 52: * > 53: * @see PersistentSearchControl > 54: */ Not that it matters much, but I see that `DefaultResponseControlFactory` and `PersistentSearchControl` both have an `@see EntryChangeResponseControl` so I'd suggest to replace: - * @see com.sun.jndi.ldap.ctl.ResponseControlFactory ResponseControlFactory + * @see DefaultResponseControlFactory ------------- PR Review: https://git.openjdk.org/jdk/pull/29712#pullrequestreview-3796864479 PR Review Comment: https://git.openjdk.org/jdk/pull/29712#discussion_r2803848914
