On Mon, 9 Sep 2024 15:19:28 GMT, Artur Barashev <[email protected]> wrote:
>> test/jdk/java/security/SecureRandom/ThreadSafe.java line 77:
>>
>>> 75: //Bad. Alias of S2, should fail because S2 is marked as
>>> ThreadSafe
>>> 76: put("alg.Alias.SecureRandom.AliasS2", "S2");
>>> 77:
>>
>> What about an alias for S1? I assume since the attribute is by default false
>> the test will still pass. Just want to see it confirmed again.
>>
>> Also, please add an alias for S4. Let's see if the test still pass for the
>> new-service-with-an-alias case.
>
> Good idea, added more tests. Alternatively we could re-work the `GetInstance`
> utility class to store the real algo name, but then we should update all the
> callers of that class to use the real algorithm and not the (possible) alias
> name. Not sure how safe it would be, some callers probably use the alias name
> for logging and debugging.
Great. Some simple comments:
1. `List.of(alias)` is shorter.
2. There is a `jdk.test.lib.Utils` method that will simplify the exception
check:
Utils.runAndCheckException(
() -> NoSync.test(SecureRandom.getInstance("S2", p), 5, 5),
RuntimeException.class);
Otherwise, all good.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20916#discussion_r1750521038
