On Thu, 6 Oct 2022 16:10:37 GMT, Roger Riggs <rri...@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
