RFR: 8290368: Introduce LDAP and RMI protocol-specific object factory filters to JNDI implementation [v2]

[Aleksei Efimov]

On Thu, 6 Oct 2022 16:10:37 GMT, Roger Riggs <rriggs at openjdk.org> wrote:

>> 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
>
> src/java.naming/share/classes/com/sun/naming/internal/ObjectFactoriesFilter.java line 91:
> 
>> 89:     }
>> 90: 
>> 91:     private static boolean checkInput(String scheme, FactoryInfo factoryInfo) {
> 
> This construct in which the supplied lambda fills in the serialClass is pretty obscure. 
> Perhaps the variable name can be "serialClass" to match the only non-default method in ObjectInputFilter would give a better hint.

Thanks - `serialClass` name reads better.

> src/java.naming/share/classes/com/sun/naming/internal/ObjectFactoriesFilter.java line 92:
> 
>> 90: 
>> 91:     private static boolean checkInput(String scheme, FactoryInfo factoryInfo) {
>> 92:         Status result = switch(scheme) {
> 
> The handling of the selection of the filter could be more direct.
> You can change the argument to checkInput to be ObjectInputFilter and pass the desired filter; LDAP_FILTER, RMI_FITLER, or GLOBAL_FILTER.
> And make the check of the GLOBAL_FILTER conditional on it not having already been evaluated.
> Then it will be the case that there must always be a specific filter.
> 
> The callers are all local to the class and would change to pass the desired filter.

Thank you - refactored as suggested.

> src/java.naming/share/classes/com/sun/naming/internal/ObjectFactoriesFilter.java line 173:
> 
>> 171:                                            DEFAULT_GLOBAL_SP_VALUE));
>> 172: 
>> 173:     // A system-wide LDAP specific object factories filter constructed from the system
> 
> Where does the IllegalArgumentException from ObjectInputFilter.create get handled or reported?
> If the property value is illformed, the error should be enough to diagnose and correct the property.

That is a good point - the current state of reporting for a malformed filter pattern can be improved:
First filter check with `check*Filter` throws `java.lang.ExceptionInInitializerError` with a cause set to `java.lang.IllegalArgumentException` with filter pattern error. But subsequent checks throw `java.lang.NoClassDefFoundError: Could not initialize class com.sun.naming.internal.ObjectFactoriesFilter`.

To address that one interface and two new records have been added to represent a factory filter state:
 - `ConfiguredFilter` - interface for representing a filter created with `ObjectInputFilter.create` from a pattern.
 - `ValidFilter implements ConfiguredFilter` - stores `ObjectInputFilter` constructed from a valid filter pattern.
 - `InvalidFilter implements ConfiguredFilter` - stores `IllegalArgumentException` generated by parsing of an invalid filter pattern. The stored `IAE` is used to report malformed filter pattern each time specific filter is consulted.

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

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