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

[Daniel Fuchs]

On Mon, 10 Oct 2022 12:07:38 GMT, Aleksei Efimov <aefimov at openjdk.org> wrote:

>> 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?
>
>> 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?
> 
> Yes - there is no point, and that will reduce number of `checkInput` calls on a specific filter, if it is not the global one. That's how it will look like:
> 
>     private static boolean checkInput(ConfiguredFilter filter, FactoryInfo serialClass) {
>         var globalFilter = GLOBAL_FILTER.filter();
>         var specificFilter = filter.filter();
>         Status globalResult = globalFilter.checkInput(serialClass);
> 
>         // Check if a specific filter is the global one
>         if (filter == GLOBAL_FILTER) {
>             return globalResult == Status.ALLOWED;
>         }
>         return switch (globalResult) {
>             case ALLOWED -> specificFilter.checkInput(serialClass) != Status.REJECTED;
>             case REJECTED -> false;
>             case UNDECIDED -> specificFilter.checkInput(serialClass) == Status.ALLOWED;
>         };
>     }
> 
> 
> The `if (filter == GLOBAL_FILTER) {` check can be also removed but without it the `checkInput` will be called twice on global filter for the case where `specific == global`, ie call from the `checkGlobalFilter`.

The code above LGTM. An alternative could be:


    private static boolean checkInput(ConfiguredFilter filter, FactoryInfo serialClass) {
        var globalFilter = GLOBAL_FILTER.filter();
        var specificFilter = filter.filter();
        Status globalResult = globalFilter.checkInput(serialClass);

        return switch (globalResult) {
            case ALLOWED -> filter == GLOBAL || specificFilter.checkInput(serialClass) != Status.REJECTED;
            case REJECTED -> false;
            case UNDECIDED -> specificFilter.checkInput(serialClass) == Status.ALLOWED;
        };
    }


but I believe your proposed code reads better.

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

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