>-----Original Message----- >From: Weijun Wang >Sent: Friday, June 5, 2020 8:24 PM >To: Prasadrao Koppula <prasadarao.kopp...@oracle.com> >Cc: Valerie Peng <valerie.p...@oracle.com>; security-dev@openjdk.java.net >Subject: Re: [15] RFR JDK-8246613: Choose the default SecureRandom algo >based on registration ordering > >I don't know who in this world would want to do that, but Prasad's concern is >technically possible. I tried 'p.remove("SecureRandom.a")' in the new test, >and new SecureRandom() fails with >"java.security.NoSuchAlgorithmException: a SecureRandom not available".
I too agree, users may not do this. But technically it causes exception in the flow. > >And people can also remove one entry and add it back in order to move it to >the end. One can even add new implementations this way. > >Unfortunately there is no ConcurrentLinkedHashMap. Yes Max, I'm fine with current approach, if refresh at remove costs performance hit. Thanks, Prasad.K > >--Max > >> On Jun 5, 2020, at 1:44 PM, Prasadrao Koppula ><prasadarao.kopp...@oracle.com> wrote: >> >> Hi, >> >> Looks good to me, one question >> >> If first registered SecureRandom algo gets removed, >getDefaultSecureRandomAlgorithm return stale data, a refresh required in >remove? >> >> Thanks, >> Prasad.K >> >>> -----Original Message----- >>> From: Valerie Peng >>> Sent: Friday, June 5, 2020 2:52 AM >>> To: security-dev@openjdk.java.net >>> Subject: Re: [15] RFR JDK-8246613: Choose the default SecureRandom >>> algo based on registration ordering >>> >>> Hi, Sean, >>> >>> Thanks for the review and feedback. Webrev updated: >>> http://cr.openjdk.java.net/~valeriep/8246613/webrev.01/ >>> >>> getTypeAndAlgorithm(...) was not static due to an instance variable >>> used by debugging output. I have removed it and made both method >static. >>> >>> I will wait for others' review comments also. >>> >>> Thanks, >>> Valerie >>> On 6/4/2020 2:01 PM, Sean Mullan wrote: >>>> On 6/4/20 3:34 PM, Valerie Peng wrote: >>>>> Hi, >>>>> >>>>> Could someone help reviewing this fix? This change keep tracks of >>>>> the first registered SecureRandom algorithm and returns it upon the >>>>> request of SecureRandom class. >>>> >>>> This looks good to me. I would recommend that Max or someone else >>>> review it as well. >>>> >>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8246613 >>>>> >>>>> Webrev: http://cr.openjdk.java.net/~valeriep/8246613/webrev.00/ >>>> >>>> A couple of minor comments, feel free to fix or ignore. >>>> >>>> * SecureRandom.java >>>> >>>> 879 // For SUN provider, we use >>>> SunEntries.DEFF_SECURE_RANDOM_ALGO >>>> >>>> Might as well fix the typo while you are in there: s/DEFF/DEF/ >>>> >>>> * Provider.java >>>> >>>> 1156 private String parseSecureRandomPut(String name, String >>>> value) { >>>> >>>> Could be static if you also make getTypeAndAlgorithm static, I think. >>>> >>>> --Sean >