EdwardsCurve's "contains" method can cause NPE

[ypypy28]

Hi, I've read in the README that before making code related PR's I must create a discussion about it. So this is it.

File: jjwt/impl/src/main/java/io/jsonwebtoken/impl/security/EdwardsCurve.java

The bug is simple. Method findByKey which is called under the hood can return null, and then calling equals on null will cause NullPointerException

    @Override
    public boolean contains(Key key) {
        EdwardsCurve curve = findByKey(key);
        return curve.equals(this);
    }

I suggest changing return statement to

        return equals(curve);

equals method of AbstractCurve class, which EdwardsCurve is extending, can successfully handle null as an argument.

[lhazlewood]

Hi @ypypy28! Thanks SO much for reading the README first, I really appreciate it 😄. And thanks for creating the discussion!

I agree with you that this can be made cleaner, but it does not appear to be a current bug within JJWT: there is no path in the codebase (verified with static analysis and test cases) that would ever pass null as an argument to the contains method, so an NPE doesn't appear to be possible.

Excluding test code, the only time the EdwardsCurve contains method is ever invoked is in the EcdhKeyAlgorithm class:

jjwt/impl/src/main/java/io/jsonwebtoken/impl/security/EcdhKeyAlgorithm.java

Line 223 in e3fff12

if (!curve.contains(epk.toKey())) {

and the epk value has already been validated to contain a non-null Key instance. (An elliptic curve Jwk instance cannot even be created if its associated Java Key instance cannot be obtained).

That said, if you'd like to create a pull request with the desired change (and an associated test case), I'd be happy to accept it because it is indeed cleaner/simpler and prevents any potential NPE that might arise in the future! Thank you! 🙏