One way to order objects of a class relative to one another is to implement the Comparable
interface. Library classes such as TreeSet
and TreeMap
accept Comparable
objects and use their compareTo()
methods to sort them. However, a class that implements the compareTo()
method in an unexpected way can cause undesirable results.
The general usage contract for compareTo()
from Java SE 6 API [[API 2006]] states that
The implementor must ensure
sgn(x.compareTo(y)) == -sgn(y.compareTo(x))
for all x and y. (This implies thatx.compareTo(y)
must throw an exception iffy.compareTo(x)
throws an exception.)The implementor must also ensure that the relation is transitive:
(x.compareTo(y) >0 && y.compareTo(z)>0)
impliesx.compareTo(z)>0
.Finally, the implementor must ensure that
x.compareTo(y) ==0
implies thatsgn(x.compareTo(z)) == sgn(y.compareTo(z))
, for allz
.It is strongly recommended, but not strictly required that
(x.compareTo(y) ==0) == (x.equals(y) )
. Generally speaking, any class that implements the Comparable interface and violates this condition should clearly indicate this fact. The recommended language is "Note: this class has a natural ordering that is inconsistent with equals."In the foregoing description, the notation
sgn(expression)
designates the mathematical signum function, which is defined to return either -1, 0, or 1 depending on whether the value of the expression is negative, zero or positive.
Do not violate any of the first three conditions when implementing the compareTo()
method. Implementing the fourth condition is strongly recommended but is not necessary.
Noncompliant Code Example
This noncompliant code example violates the third condition (transitivity) in the contract. This requirement states that the objects that compareTo()
considers equal (by returning 0) must be ordered the same with respect to other objects. For example, a card may require to be compared against any other card to check whether both belong to the same suit or have the same rank. If neither of these conditions is true, compareTo()
is expected to order the cards based on rank alone. This situation may arise in a game like Uno or Crazy Eights, where one can only place a card on the pile that shares a suit or rank with the top most card on the pile.
public final class Card implements Comparable { private String suit; private int rank; public Card(String s, int r) { if (s == null) { throw new NullPointerException(); } suit = s; rank = r; } public boolean equals(Object o) { if (o instanceof Card) { Card c = (Card)o; return suit.equals(c.suit) || (rank == c.rank); // Bad } return false; } // This method violates its contract public int compareTo(Object o) { if (o instanceof Card) { Card c = (Card)o; if(suit.equals(c.suit) ) return 0; if((c.rank >= rank + Integer.MIN_VALUE) && (c.rank <= rank + Integer.MAX_VALUE) ) // Check for integer overflow return c.rank - rank; // Order based on rank } throw new ClassCastException(); } public static void main(String[] args) { Card a = new Card("Clubs", 2); Card b = new Card("Clubs", 10); Card c = new Card("Hearts", 7); System.out.println(a.compareTo(b)); // Returns 0 System.out.println(a.compareTo(c)); // Returns a negative number System.out.println(b.compareTo(c)); // Returns a positive number } }
Here, the comparison between (a,c)
yields that c
is larger. However, the comparison (b,c)
yields b
as larger. This means b
must be larger than a
. However, comparing (a,b)
results in the value 0
implying that both a
and b
compare equal.
Compliant Solution
This compliant solution ensures that the compareTo()
contract is satisfied, and the corresponding equals()
method is consistent with compareTo()
.
public final class Card implements Comparable{ private String suit; private int rank; public Card(String s, int r) { if (s == null) { throw new NullPointerException(); } suit = s; rank = r; } public boolean equals(Object o) { if (o instanceof Card) { Card c=(Card)o; return suit.equals(c.suit) && (rank == c.rank); // Good } return false; } // This method fulfills its contract public int compareTo(Object o) { if (o instanceof Card) { Card c=(Card)o; if(suit.equals(c.suit) && (c.rank >= rank + Integer.MIN_VALUE) && (c.rank <= rank + Integer.MAX_VALUE) ) return c.rank - rank; return suit.compareTo(c.suit); } throw new ClassCastException(); } public static void main(String[] args) { Card a = new Card("Clubs", 2); Card b = new Card("Clubs", 10); Card c = new Card("Hearts", 7); System.out.println(a.compareTo(b)); // Returns 0 System.out.println(a.compareTo(c)); // Returns a negative number System.out.println(b.compareTo(c)); // Returns a negative number } }
As required by the ordering, c
is larger than both a
and b
and the comparison (a,b)
produces an equal result. This maintains the compareTo()
method's contract.
Risk Assessment
Violating the general contract when implementing the compareTo()
method can result in unexpected results, possibly leading to invalid comparisons and information disclosure.
Guideline |
Severity |
Likelihood |
Remediation Cost |
Priority |
Level |
---|---|---|---|---|---|
MET14-J |
medium |
unlikely |
medium |
P4 |
L3 |
Automated Detection
The Coverity Prevent Version 5.0 MUTABLE_COMPARISON checker can detect the instances where compareTo method is reading from a non constant field. If the non-constant field is modified, the value of compareTo might change, which may break program invariants.
Related Vulnerabilities
Search for vulnerabilities resulting from the violation of this guideline on the CERT website.
Other Languages
This guideline appears in the C++ Secure Coding Standard as ARR40-CPP. Use a valid ordering rule.
Bibliography
[[API 2006]] method compareTo()
[[JLS 2005]]
MET13-J. Classes that define an equals() method must also define a hashCode() method 05. Methods (MET) MET15-J. Do not use deprecated or obsolete methods