<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
</head>
<body>
<div dir="ltr">
<div></div>
<div>
<div>Hello,</div>
<div dir="ltr"><br>
</div>
<div dir="ltr">I would agree with the interpretation that the NPE is not mandatory.</div>
<div dir="ltr"><br>
</div>
<div dir="ltr"> But even if we keep it, the actual problem in addAll() should be fixed? There is no point in calling contains(null) on a SecureSet, right?</div>
<div dir="ltr"><br>
</div>
<div dir="ltr">Gruss</div>
<div dir="ltr">Bernd</div>
<div class="ms-outlook-ios-signature" id="ms-outlook-mobile-signature">
<div style="direction: ltr;">-- </div>
<div style="direction: ltr;">http://bernd.eckenfels.net</div>
</div>
</div>
<div id="id-4b236930-9b47-4028-ba20-f3dd7d9ae7ce" class="ms-outlook-mobile-reference-message">
<hr style="display: inline-block; width: 98%; font-family: -webkit-standard; font-size: 12pt; color: rgb(0, 0, 0);" tabindex="-1">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif"><b>Von:</b> security-dev <security-dev-bounces@openjdk.java.net> im Auftrag von Mkrtchyan, Tigran <tigran.mkrtchyan@desy.de><br>
<b>Gesendet:</b> Freitag, April 24, 2020 12:52 PM<br>
<b>An:</b> Weijun Wang<br>
<b>Cc:</b> security-dev<br>
<b>Betreff:</b> Re: NPE is used in javax.security.auth.Subject for flowcontrol
<div> </div>
</font></div>
<meta name="Generator" content="Microsoft Exchange Server">
<!-- converted from text --><style><!-- .EmailQuote { margin-left: 1pt; padding-left: 4pt; border-left: #800000 2px solid; } --></style><font size="2"><span style="font-size:11pt;">
<div class="PlainText">I read it differently:<br>
<br>
Attempting to query the presence of an ineligible element may throw an exception, or it may simply return false; some implementations<br>
will exhibit the former behavior and some will exhibit the latter.<br>
<br>
Implementation might thrown an exception or return false, but sill don't allow null values.<br>
<br>
... would not result in the insertion of an ineligible element into the collection may throw an exception or it may<br>
succeed, at the option of the implementation.<br>
<br>
<br>
For example, TreeSet throws NPE depending on Comparator.<br>
<br>
Well, I understand, this is a corner case that possibly not worth to optimize. Let me write<br>
a benchmark to see how much difference it makes.<br>
<br>
Tigran.<br>
<br>
<br>
----- Original Message -----<br>
> From: "Weijun Wang" <weijun.wang@oracle.com><br>
> To: "Tigran Mkrtchyan" <tigran.mkrtchyan@desy.de><br>
> Cc: "security-dev" <security-dev@openjdk.java.net><br>
> Sent: Friday, April 24, 2020 12:23:48 PM<br>
> Subject: Re: NPE is used in javax.security.auth.Subject for flowcontrol<br>
<br>
> My understanding is by optional you have 2 options while designing the Set:<br>
> <br>
> 1. Allow null<br>
> <br>
> 2. Not allow null<br>
> <br>
> Now that SecureSet has chosen the 2nd way, it is now in the "this set does not<br>
> permit null elements" category and therefore it should "@throws<br>
> NullPointerException if the specified element is null".<br>
> <br>
> --Max<br>
> <br>
>> On Apr 24, 2020, at 6:13 PM, Mkrtchyan, Tigran <tigran.mkrtchyan@desy.de> wrote:<br>
>> <br>
>> Hi Max,<br>
>> <br>
>> that's right, but java doc as well mentions that this exception is *optional*,<br>
>> and has a corresponding note:<br>
>> <br>
>> Some collection implementations have restrictions on the elements that they may<br>
>> contain. For example, some implementations prohibit null elements, and some<br>
>> have restrictions on the types of their elements. Attempting to add an<br>
>> ineligible element throws an unchecked exception, typically<br>
>> NullPointerException or ClassCastException. Attempting to query the presence of<br>
>> an ineligible element may throw an exception, or it may simply return false;<br>
>> some implementations will exhibit the former behavior and some will exhibit the<br>
>> latter. More generally, attempting an operation on an ineligible element whose<br>
>> completion would not result in the insertion of an ineligible element into the<br>
>> collection may throw an exception or it may succeed, at the option of the<br>
>> implementation. Such exceptions are marked as "optional" in the specification<br>
>> for this interface.<br>
>> <br>
>> Thus, returning *false* is justified and spec compliant.<br>
>> <br>
>> <br>
>> Thanks,<br>
>> Tigran.<br>
>> <br>
>> <br>
>> ----- Original Message -----<br>
>>> From: "Weijun Wang" <weijun.wang@oracle.com><br>
>>> To: "Tigran Mkrtchyan" <tigran.mkrtchyan@desy.de><br>
>>> Cc: "security-dev" <security-dev@openjdk.java.net><br>
>>> Sent: Friday, April 24, 2020 11:42:41 AM<br>
>>> Subject: Re: NPE is used in javax.security.auth.Subject for flowcontrol<br>
>> <br>
>>> Hi Tigran,<br>
>>> <br>
>>> In java.util.Set, we have:<br>
>>> <br>
>>> * @throws NullPointerException if the specified element is null and this<br>
>>> * set does not permit null elements<br>
>>> * (<a href="Collection.html#optional-restrictions">optional</a>)<br>
>>> */<br>
>>> boolean contains(Object o);<br>
>>> <br>
>>> As an implementation, SecureSet must follow the spec to throw an NPE. If it<br>
>>> returns null, some unexpected thing might happen when the contains() method is<br>
>>> called somewhere else.<br>
>>> <br>
>>> Thanks,<br>
>>> Max<br>
>>> <br>
>>>> On Apr 24, 2020, at 4:21 PM, Mkrtchyan, Tigran <tigran.mkrtchyan@desy.de> wrote:<br>
>>>> <br>
>>>> <br>
>>>> <br>
>>>> <br>
>>>> Dear Java-SE security developers,<br>
>>>> <br>
>>>> <br>
>>>> Imagine a following code:<br>
>>>> <br>
>>>> ```<br>
>>>> Subject s1 = ... ;<br>
>>>> <br>
>>>> Subject s2 = ... ;<br>
>>>> <br>
>>>> <br>
>>>> s2.getPrincipals().addAll(s1.getPrincipals());<br>
>>>> <br>
>>>> ```<br>
>>>> <br>
>>>> The Subject's SecureSet.addAll checks that provided Set doesn't contains 'null'<br>
>>>> values<br>
>>>> by calling collectionNullClean, which calls SecureSet#contains:<br>
>>>> <br>
>>>> ```<br>
>>>> try {<br>
>>>> hasNullElements = coll.contains(null);<br>
>>>> } catch (NullPointerException npe) {<br>
>>>> <br>
>>>> ```<br>
>>>> <br>
>>>> The SecureSet#contains itself checks for 'null' values, the NPE is always<br>
>>>> generated.<br>
>>>> <br>
>>>> This as introduced by commit e680ab7f208e<br>
>>>> <br>
>>>> <a href="https://hg.openjdk.java.net/jdk/jdk/diff/e680ab7f208e/jdk/src/share/classes/javax/security/auth/Subject.java">
https://hg.openjdk.java.net/jdk/jdk/diff/e680ab7f208e/jdk/src/share/classes/javax/security/auth/Subject.java</a><br>
>>>> <br>
>>>> <br>
>>>> As SecureSet doesn't allow null values, it will be much simpler to return false<br>
>>>> right away:<br>
>>>> <br>
>>>> ```<br>
>>>> <br>
>>>> public boolean contains(Object o) {<br>
>>>> if (o == null) {<br>
>>>> // null values rejected by add<br>
>>>> return false;<br>
>>>> }<br>
>>>> <br>
>>>> ...<br>
>>>> }<br>
>>>> <br>
>>>> ```<br>
>>>> <br>
>>>> <br>
>>>> Thanks in advance,<br>
> >>> Tigran.<br>
</div>
</span></font></div>
</div>
</body>
</html>