TreeMap.get throws NPE when the key is present and any value is null

[ahammel]

TreeMap.get will throw a null pointer exception when trying to do a lookup on a map with nulls in the values. HashMap works as expected under the same circumstances.

void bug() {
    final HashMap<String, String> hashMap = HashMap.of(
            "Thisworks", "fine",
            "evenwith", null);

    hashMap.get("nokey");     // Option.none()
    hashMap.get("Thisworks"); // Option.some("fine")
    hashMap.get("evenwith");  // Option.some(null)

    final TreeMap<String, String> treeMap = TreeMap.of(
            "thisbreaks", "themap",
            "lolhax", null);

    treeMap.get("nokey");       // Option.none()
    treeMap.get("thisbreaks"); // NPE
    treeMap.get("lolhax");     // NPE
}

I'm using javaslang 2.0.3

Stack trace:

java.lang.NullPointerException
    at javaslang.Tuple2.compareTo(Tuple2.java:80)
    at javaslang.Tuple2.compareTo(Tuple2.java:96)
    at javaslang.Tuple2.compareTo(Tuple2.java:28)
    at javaslang.collection.RedBlackTreeModule$Node.find(RedBlackTree.java:418)
    at javaslang.collection.TreeMap.get(TreeMap.java:420)

[ahammel]

Looks fixed on master :)

[ahammel]

Although now I can't even get those tests to fail on 2.0.3, so maybe I'm crazy ¯_(ツ)_/¯

I'll have another look in the morning.

[danieldietrich]

Hi @ahammel, thank you for reporting! I can reproduce it on 2.0.3 and confirm that it is fixed on the current SNAPSHOT. I will release Bugfix Release 2.0.4 neartime.

[danieldietrich]

@ruslansennov do you know/remember with which PR we fixed it?

[ruslansennov]

do you know/remember with which PR we fixed it?

sorry 😄

[danieldietrich]

I think it is not fixed. Maybe the example now works on the current SNAPSHOT because the order of Map elements changed!? (A Map is unordered.)

The root error is thrown by the Tuple2.compareTo method, which calls this code:

    @SuppressWarnings("unchecked")
    private static <U1 extends Comparable<? super U1>, U2 extends Comparable<? super U2>> int compareTo(Tuple2<?, ?> o1, Tuple2<?, ?> o2) {
        final Tuple2<U1, U2> t1 = (Tuple2<U1, U2>) o1;
        final Tuple2<U1, U2> t2 = (Tuple2<U1, U2>) o2;

        final int check1 = t1._1.compareTo(t2._1);
        if (check1 != 0) {
            return check1;
        }

        final int check2 = t1._2.compareTo(t2._2);
        if (check2 != 0) {
            return check2;
        }

        // all components are equal
        return 0;
    }

Especially these throw:

• final int check1 = t1._1.compareTo(t2._1); if t1._1 is null
• final int check2 = t1._2.compareTo(t2._2); if t1._2 is null

Solution:

The generated Tuple code needs to take null values into account when comparing, similar to Comparator.nullsFirst(Comparator) resp. Comparators.NullComparator.

private static <U1 extends Comparable<? super U1>, U2 extends Comparable<? super U2>, U3 extends Comparable<? super U3>, U4 extends Comparable<? super U4>> int compareTo(Tuple4<?, ?, ?, ?> o1, Tuple4<?, ?, ?, ?> o2) {
    final Tuple2<U1, U2, U3, U4> t1 = (Tuple2<U1, U2, U3, U4>) o1;
    final Tuple2<U1, U2; U3, U4> t2 = (Tuple2<U1, U2, U3, U4>) o2;
    if (t1 == null) {
        return (t2 == null) ? 0 : -1;
    } else if (t2 == null) {
        return 1;
    } else {
        int check;
        if ((check = compare(t1._1, t2._1) == 0) {
          if ((check = compare(t1._2, t2._2) == 0) {
            if ((check = compare(t1._3, t2._3) == 0) {
              check = compare(t1._4, t2._4);
            }
          }
        }
        return check;
    }
}

private static <U extends Comparable<? super U>> compare(U u1, U u2) {
    if (u1 == null) {
        return (u2 == null) ? 0 : -1;
    } if (u2 == null) {
        return 1;
    } else {
        return u1.compareTo(u2);
    }
}

Note: The Tuple*.comparator(Comparator, Comparator, ...) methods need also take null into account.