EXP11-J. Be careful of autoboxing when removing elements from a Collection

A boxing conversion converts the value of a primitive type to the corresponding value of the reference type, for instance, from int to the type Integer [JLS 2005, Section 5.1.7, "Boxing Conversions"]. This is convenient in many cases where an object parameter is desired, such as with collection classes like Map and List. Another use case is to pass object references to methods, as opposed to primitive types that are always passed by value. The resulting wrapper types also help to reduce clutter in code.

Noncompliant Code Example

This noncompliant code example prints 100 as the size of the HashSet while it is expected to print 1. The combination of values of types short and int in the operation i-1 leads to autoboxing of the result into an object of type Integer, rather than Short. (See guideline [INT10-J. Be aware of integer promotion behavior].) The HashSet contains values of only one type Short, whereas the code attempts to remove objects of type Integer. As a result, the remove() operation accomplishes nothing. The compiler enforces type checking so that only Short values are inserted. However, a programmer is free to remove an object of any type without triggering any exceptions because Collections<E>.remove() accepts an argument of type Object and not E. Such behavior can result in unintended object retention or memory leaks [[Techtalk 2007]].

public class ShortSet {
  public static void main(String[] args) {
    HashSet<Short> s = new HashSet<Short>();
      for(short i = 0; i < 100; i++) {
        s.add(i);
        s.remove(i - 1);
      }
    System.out.println(s.size());
  }
}

Compliant Solution

Objects removed from a collection should always share the type of the elements of the collection. Numeric promotion and autoboxing can produce unexpected object types. This compliant solution uses an explicit cast to short that parallels the intended boxed type.

public class ShortSet {
  public static void main(String[] args) {
    HashSet<Short> s = new HashSet<Short>();
      for(short i = 0; i < 100; i++) {
        s.add(i);
        s.remove((short)(i - 1)); // cast to short 
      }
    System.out.println(s.size());
  }
}

Risk Assessment

Numeric promotion and autoboxing while removing elements from a Collection can cause operations on the Collection to fail silently.

Automated Detection

Detection of invocations of Collection.remove() whose operand fails to match the type of the elements of the underlying collection is straightforward. It is possible, albeit unlikely, that some of these invocations could be intended. The remainder are heuristically likely to be in error.

Related Vulnerabilities

Search for vulnerabilities resulting from the violation of this guideline on the CERT website.

Bibliography

[[Core Java 2004]] Chapter 5
[[JLS 2005]] Section 5.1.7
[[Techtalk 2007]] "The Joy of Sets"

EXP10-J. Avoid side-effects in assertions      04. Expressions (EXP)      EXP12-J. Avoid dereferencing null pointers