JDK-8047795 : Collections.checkedList checking bypassed by List.replaceAll
  • Type: Bug
  • Component: core-libs
  • Sub-Component: java.util:collections
  • Affected Version: 8u5
  • Priority: P2
  • Status: Closed
  • Resolution: Fixed
  • OS: generic
  • CPU: generic
  • Submitted: 2014-06-19
  • Updated: 2017-05-19
  • Resolved: 2014-06-25
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 8 JDK 9
8u20Fixed 9 b22Fixed
Related Reports
Relates :  
Description
FULL PRODUCT VERSION :


A DESCRIPTION OF THE PROBLEM :
The replaceAll method of a list returned by java.util.Collections.checkedList passes the replacement operator directly to the underlying list, enabling violation of the type checking.


REPRODUCIBILITY :
This bug can be reproduced always.

---------- BEGIN SOURCE ----------
import java.util.*;
import java.util.function.UnaryOperator;

class CheckedListViolationBug {
    public static void main(String... args) {
        // Create a runtime-checked List of Integers only
        List<Integer> integersOnly = Collections.checkedList(new ArrayList<>(), Integer.class);
        integersOnly.add(1);
        integersOnly.add(2);
        integersOnly.add(3);
        
        // the UnaryOperator bypasses the check, enabling insertion of arbitrary elements
        integersOnly.replaceAll((UnaryOperator)(e -> "ha!"));
        
        // the list now contains Strings:
        System.out.println(integersOnly);
    }
}
---------- END SOURCE ----------


Comments
SQE-is NOT OK taking this fix this late in the release. If concerned, come back with better justification.
09-07-2014

Need SQE-OK
09-07-2014

This change should be backported to 8u20. Failing to catch errors of usage severely limits the usefulness of this class.
07-07-2014

There is a precedent for non-transactional behaviour; toArray(T[]) can throw an ArrayStoreException with only some of the entries copied to the array.
24-06-2014

Here is the transactional version: @Override public void replaceAll(UnaryOperator<E> operator) { E[] asArray = toArray(zeroLengthElementArray()); Arrays.asList(asArray).replaceAll(e -> typeCheck(operator.apply(e))); ListIterator<E> each = list.listIterator(); for(int i = 0; i < asArray.length; i++) { each.next(); each.set(asArray[i]); } } though I am persuaded that it need not be transactional based upon the argument that the function can throw a RuntimeException for any element.
23-06-2014

Good point, like the CheckedMap.replaceAll impl which is not currently transactional, unlike that of CheckedList/Map.addAll. Given that a call to the function passed to List.replaceAll can result in a runtime exception i think it a reasonable expectation it is not transactional (or "all-or-nothing"), further more most value for checked collections seem to be for debugging purposes to more easily find the source of any pollution.
23-06-2014

Do we also need a clarification to the javadoc to make it clear that if CCE is thrown, then it may be done after some (but not all) occurrences have been replaced?
23-06-2014

The code for CheckedList.replaceAll: @Override public void replaceAll(UnaryOperator<E> operator) { list.replaceAll(operator); } Needs to be replaced with something like: @Override public void replaceAll(UnaryOperator<E> operator) { list.replaceAll(e -> { typeCheck(e); return operator.apply(e); }); } Looks like other checked collections/maps are Ok.
23-06-2014