Re: RFR 8243592: Subject$SecureSet::contains(null) is suboptimal

Looks good. --Sean On 6/30/20 11:31 PM, Weijun Wang wrote: Please review an updated fix at https://cr.openjdk.java.net/~weijun/8243592/webrev.03 After some discussion, we think there is no need to call contains(null) at all since it might not be reliable (see the new test). Now, collecti

Re: RFR 8243592: Subject$SecureSet::contains(null) is suboptimal

repo! - Original Message - > From: "Weijun Wang" > To: "Sean Mullan" > Cc: "security-dev" > Sent: Wednesday, July 1, 2020 5:31:54 AM > Subject: Re: RFR 8243592: Subject$SecureSet::contains(null) is suboptimal > Please review an updated fix at > &

Re: RFR 8243592: Subject$SecureSet::contains(null) is suboptimal

Please review an updated fix at https://cr.openjdk.java.net/~weijun/8243592/webrev.03 After some discussion, we think there is no need to call contains(null) at all since it might not be reliable (see the new test). Now, collectionNullClean inspects all items in the input collection and crea

Re: RFR 8243592: Subject$SecureSet::contains(null) is suboptimal

Webrev updated at http://cr.openjdk.java.net/~weijun/8243592/webrev.02/ I removed my old test and rewrite the existing SubjectNullTests.java test in TestNG with quite some new test cases. There are so many behavior changes (although we might say good programs are not affected) that I decide

Re: RFR 8243592: Subject$SecureSet::contains(null) is suboptimal

I don't think containsAll and retainsAll need to call collectionNullClean either because SecureSet.contains(null) returns false now. --Sean On 4/28/20 9:58 AM, Weijun Wang wrote: A new webrev is at http://cr.openjdk.java.net/~weijun/8243592/webrev.01/ I take this chance to do some forma

Re: RFR 8243592: Subject$SecureSet::contains(null) is suboptimal

A new webrev is at http://cr.openjdk.java.net/~weijun/8243592/webrev.01/ I take this chance to do some formatting and add a new test. My first testng test! Thanks, Max > On Apr 28, 2020, at 8:16 PM, Sean Mullan wrote: > > On 4/27/20 10:39 PM, Weijun Wang wrote: >> Reading the Set spec, it

Re: RFR 8243592: Subject$SecureSet::contains(null) is suboptimal

On 4/27/20 10:39 PM, Weijun Wang wrote: Reading the Set spec, it looks like an NPE is still needed for add(), but remove() can be modified. Good point, I agree. --Sean --Max On Apr 27, 2020, at 10:27 PM, Sean Mullan wrote: The fix looks fine to me. For consistency, you could make the s

Re: RFR 8243592: Subject$SecureSet::contains(null) is suboptimal

Reading the Set spec, it looks like an NPE is still needed for add(), but remove() can be modified. --Max > On Apr 27, 2020, at 10:27 PM, Sean Mullan wrote: > > The fix looks fine to me. For consistency, you could make the same change for > null elements in the other SecureSet methods: add, r

Re: RFR 8243592: Subject$SecureSet::contains(null) is suboptimal

The fix looks fine to me. For consistency, you could make the same change for null elements in the other SecureSet methods: add, remove. --Sean On 4/25/20 3:39 AM, Weijun Wang wrote: Please take a review at http://cr.openjdk.java.net/~weijun/8243592/webrev.00/ This is helpful if we do an

RFR 8243592: Subject$SecureSet::contains(null) is suboptimal

Please take a review at http://cr.openjdk.java.net/~weijun/8243592/webrev.00/ This is helpful if we do any set arithmetic between 2 Subject objects. No new regression test, I intend to add a noreg-trivial label. *Tigran*: Please confirm you are OK with the "Contributed-by" line. Thanks, Max