NPE is used in javax.security.auth.Subject for flowcontrol

[Weijun Wang]

Hi Tigran,

In java.util.Set, we have:

     * @throws NullPointerException if the specified element is null and this
     *         set does not permit null elements
     * (<a href="Collection.html#optional-restrictions">optional</a>)
     */
    boolean contains(Object o);

As an implementation, SecureSet must follow the spec to throw an NPE. If it returns null, some unexpected thing might happen when the contains() method is called somewhere else.

Thanks,
Max

> On Apr 24, 2020, at 4:21 PM, Mkrtchyan, Tigran <tigran.mkrtchyan at desy.de> wrote:
> 
> 
> 
> 
> Dear Java-SE security developers,
> 
> 
> Imagine a following code:
> 
> ```
> Subject s1 = ... ;
> 
> Subject s2 = ... ;
> 
> 
> s2.getPrincipals().addAll(s1.getPrincipals());
> 
> ```
> 
> The Subject's SecureSet.addAll checks that provided Set doesn't contains 'null' values
> by calling collectionNullClean, which calls SecureSet#contains:
> 
> ```
> try {
>    hasNullElements = coll.contains(null);
> } catch (NullPointerException npe) {
> 
> ```
> 
> The SecureSet#contains itself checks for 'null' values, the  NPE is always generated.
> 
> This as introduced by commit e680ab7f208e
> 
> https://hg.openjdk.java.net/jdk/jdk/diff/e680ab7f208e/jdk/src/share/classes/javax/security/auth/Subject.java
> 
> 
> As SecureSet doesn't allow null values, it will be much simpler to return false right away:
> 
> ```
> 
>        public boolean contains(Object o) {
>          if (o == null) {
>               // null values rejected  by add
>               return false;
>          }
> 
>          ...
>        }
> 
> ```
> 
> 
> Thanks in advance,
>   Tigran.