On Wed, 5 Oct 2022 15:23:43 GMT, Aleksei Efimov <aefi...@openjdk.org> wrote:

> ### Summary of the change
> This change introduces new system and security properties for specifying 
> factory filters for the JNDI/LDAP and the JNDI/RMI JDK provider 
> implementations. 
> 
> These new properties allow more granular control over the set of object 
> factories allowed to reconstruct Java objects from LDAP and RMI contexts.
> 
> The new factory filters are supplementing the existing 
> `jdk.jndi.object.factoriesFilter` global factories filter to determine if a 
> specific object factory is permitted to instantiate objects for the given 
> protocol.
> 
> Links:
> 
> - [CSR with details](https://bugs.openjdk.org/browse/JDK-8291556)
> - [JBS issue](https://bugs.openjdk.org/browse/JDK-8290368)
> 
> ### List of code changes
> 
> - Implementation for two new system and security properties have been added 
> to the `com.sun.naming.internal.ObjectFactoriesFilter` class
> - `java.security` and `module-info.java` files have been updated with a 
> documentation for the new properties
> - To keep API of `javax.naming.spi.NamingManager` and 
> `javax.naming.spi.DirectoryManager` classes unmodified a new internal 
> `com.sun.naming.internal.NamingManagerHelper` class has been introduced. All  
> `getObjectInstance` calls have been updated to use the new helper class.
> 
> #### NamingManagerHelper changes
> Changes performed to construct the `NamingManagerHelper` class:
> - `DirectoryManager.getObjectInstance` -> 
> `NamingManagerHelper.getDirObjectInstance`. Dependant methods were also moved 
> to the `NamingManagerHelper` class
> - `NamingManager.getObjectInstance` -> 
> `NamingManagerHelper.getObjectInstance`. Methods responsible for 
> setting/getting object factory builder were moved to the 
> `NamingManagerHelper` class too.
> 
> 
> ### Test changes
> New tests have been added for checking that new factory filters can be used 
> to restrict reconstruction of Java objects from LDAP and RMI contexts:
> - LDAP protocol specific test: 
> test/jdk/com/sun/jndi/ldap/objects/factory/LdapFactoriesFilterTest.java
> - RMI protocol specific test: 
> test/jdk/com/sun/jndi/rmi/registry/objects/RmiFactoriesFilterTest.java
> Existing `test/jdk/javax/naming/module/RunBasic.java` test has been updated 
> to allow test-specific factories filter used to reconstruct objects from the 
> test LDAP server. 
> 
> ### Testing
> tier1-tier3 and JNDI regression/JCK tests not showing any failures related to 
> this change.
> No failures observed for the modified regression tests.

Marked as reviewed by dfuchs (Reviewer).

I am glad to see this RFE. It looks like a big change but most of it is 
actually reorganisation of internal code, so thanks for explaining its making 
off :-) It helps a lot for the review.  I have looked at the code and I believe 
it looks good. I don't see any alternative to the reorganisation that could 
make the changes smaller - so I'm OK with the proposed solution. Thanks for 
documenting the new properties in their respective module info, as well as in 
the security properties file.

I had only a cursory look at the tests but they seem comprehensive. Good work.

I am approving on the condition to make sure that all JNDI tests (as well as 
tier1, tier2, tier3) are run before integrating.

Also please get an approval from a security-libs maintainer for the changes to 
the security properties file before integrating.

-------------

PR: https://git.openjdk.org/jdk/pull/10578

Reply via email to