On Thu, 25 Aug 2022 15:25:57 GMT, Weijun Wang <[email protected]> wrote:
>> Valerie Peng has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> update test to use SHA256 and DSA throughout.
>
> src/java.base/share/classes/java/security/Security.java line 530:
>
>> 528: // <crypto_service>.<algo_or_type> <attr_name>:<attr_value>
>> 529: key = filter.substring(0, index);
>> 530: value = filter.substring(index + 1);
>
> Do you need to ensure `value` is non-empty here? I am worried that somewhere
> later a `string.indexOf("")` is called and it's always true.
Makes sense, I will add a check. Thanks!
> src/java.base/share/classes/java/security/Security.java line 931:
>
>> 929: // check individual component for match and bail if
>> no match
>> 930: if (prop.indexOf(st.nextToken()) == -1) {
>> 931: return false;
>
> So if `value` has several sub-values, all of them must appear in the `prop`
> value. Do we need to make this clear in the spec?
>
> Also, you use `indexOf` instead of an exact match to a sub-value in `prop`.
> Is this always correct? I am wondering if a value can be substring of a
> different value. I see you support simple class name in the test. Is it worth
> we doing this? I would rather be strict at the beginning.
Yes, I'd expect if multiple sub-values are specified, it means all of them
should appear in `prop` in order to be matched. It does raise an interesting
question as to how to do the filtering based on an "OR" relationship. Perhaps
an union of 2 separate filter result? I'd suspect that "AND" relationship would
be more useful. One alternative is to disallow multiple sub-values, and treat
the value as one sub-value. Thoughts?
As for strict vs loose, I am on the fence, thought that it'd be nice to not
having to enter the entire value. Could switch direction to be strict for now
and loosen it up if requested.
-------------
PR: https://git.openjdk.org/jdk/pull/10008
