I don't know how complex this is. Will it become another map (set) that needs to be thread-safe and has an order?
If this is not a documented spec and just added for compatibility. How about we just keep the current design but inside getDefaultSecureRandomAlgorithm() if firstPrng is not in the map we return another one? --Max > On Jun 6, 2020, at 2:00 AM, Valerie Peng <valerie.p...@oracle.com> wrote: > > Right, I try to keep the impl simple as I am not aware of any property > removal usage. > > Oh-well, if we have to cater to the removal scenario, then we'd have to add a > List and keep track of ALL secure random algos instead of only the FIRST one. > Alright, I guess it costs for covering all aspect. But one extra benefit of > this is that it should be easy to handle the future JDK property such as > "jdk.securerandom.disabledAlgorithms" if it were to be added. > > Valerie > > On 6/5/2020 7:54 AM, Weijun Wang wrote: >> 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". >> >> 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. >> >> --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