RFR: 6447816: Provider filtering (getProviders) is not working with OR'd conditions [v5]

Valerie Peng valeriep at openjdk.org
Tue Sep 6 23:41:46 UTC 2022


On Tue, 6 Sep 2022 14:24:43 GMT, Weijun Wang <weijun at openjdk.org> wrote:

>> Valerie Peng has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   more refactoring.
>
> src/java.base/share/classes/java/security/Security.java line 861:
> 
>> 859:             // check required values
>> 860:             if (serviceName.isEmpty() || algName.isEmpty() ||
>> 861:                     (!attrValue.isEmpty() && attrName.isEmpty())) {
> 
> Can we move the `attrName.isEmpty()` check inside the `else` block above? I find it easier to understand.

Sure, makes sense.

> src/java.base/share/classes/java/security/Security.java line 865:
> 
>> 863:             }
>> 864:         }
>> 865:         void apply(LinkedList<Provider> candidates) {
> 
> `candidates.removeIf(p -> !isCriterionSatisfied(p));`

Ok.

> src/java.base/share/classes/java/security/Security.java line 895:
> 
>> 893:                 if (standardName != null) {
>> 894:                     key = serviceName + "." + standardName +
>> 895:                             (attrName != null ? ' ' + attrName : "");
> 
> I'd rather use `(' ' + attrName)`. Always not sure about operation precedence when `?:` is used.

Ok.

> src/java.base/share/classes/java/security/Security.java line 947:
> 
>> 945:         if (isKnownComposite(attribute)) {
>> 946:             value = value.toUpperCase();
>> 947:             prop = prop.toUpperCase();
> 
> Use `toUpperCase(Locale.ENGLISH)`.

Sure~

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

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



More information about the security-dev mailing list