On Sun, 9 Oct 2022 11:52:18 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.
>
> Aleksei Efimov has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains six additional 
> commits since the last revision:
> 
>  - Refactor checkInput, better reporting for invalid filter patterns
>  - Merge branch 'master' into JDK-8290368_protocol_specific_factory_filters
>  - Additional comments/formatting cleanup.
>  - More tests clean-up. Code/doc comments cleanup.
>  - Cleanup test comments. Add tests to check that LDAP/RMI filters do not 
> intersect.
>  - 8290368: Introduce LDAP and RMI protocol-specific object factory filters 
> to JNDI implementation

Changes requested by dfuchs (Reviewer).

src/java.naming/share/classes/com/sun/naming/internal/ObjectFactoriesFilter.java
 line 99:

> 97:             return globalResult == Status.ALLOWED;
> 98:         }
> 99: 

If I'm not mistaken there's  no point in checking the specific filter if the 
global filter state is REJECTED. So instead of switching on the specificResult 
below, maybe you should change the logic to switch on the globalResult instead 
and only call the specific filter if needed?

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

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

Reply via email to