RFR [9] 8011645: CopyOnWriteArrayList.COWSubList.subList does not validate range properly

Martin Buchholz martinrb at google.com
Fri Jan 31 18:01:27 UTC 2014


jsr166 CVS is now updated with this fix, and also adds some missing tck
tests.

Index: src/jdk7/java/util/concurrent/CopyOnWriteArrayList.java
===================================================================
RCS file:
/export/home/jsr166/jsr166/jsr166/src/jdk7/java/util/concurrent/CopyOnWriteArrayList.java,v
retrieving revision 1.6
diff -u -r1.6 CopyOnWriteArrayList.java
--- src/jdk7/java/util/concurrent/CopyOnWriteArrayList.java 4 Nov 2013
00:00:39 -0000 1.6
+++ src/jdk7/java/util/concurrent/CopyOnWriteArrayList.java 31 Jan 2014
17:50:14 -0000
@@ -1232,7 +1232,7 @@
             lock.lock();
             try {
                 checkForComodification();
-                if (fromIndex < 0 || toIndex > size)
+                if (fromIndex < 0 || toIndex > size || fromIndex > toIndex)
                     throw new IndexOutOfBoundsException();
                 return new COWSubList<E>(l, fromIndex + offset,
                                          toIndex + offset);
Index: src/main/java/util/concurrent/CopyOnWriteArrayList.java
===================================================================
RCS file:
/export/home/jsr166/jsr166/jsr166/src/main/java/util/concurrent/CopyOnWriteArrayList.java,v
retrieving revision 1.114
diff -u -r1.114 CopyOnWriteArrayList.java
--- src/main/java/util/concurrent/CopyOnWriteArrayList.java 4 Nov 2013
00:00:39 -0000 1.114
+++ src/main/java/util/concurrent/CopyOnWriteArrayList.java 31 Jan 2014
17:50:15 -0000
@@ -1371,7 +1371,7 @@
             lock.lock();
             try {
                 checkForComodification();
-                if (fromIndex < 0 || toIndex > size)
+                if (fromIndex < 0 || toIndex > size || fromIndex > toIndex)
                     throw new IndexOutOfBoundsException();
                 return new COWSubList<E>(l, fromIndex + offset,
                                          toIndex + offset);
Index: src/test/tck/CopyOnWriteArrayListTest.java
===================================================================
RCS file:
/export/home/jsr166/jsr166/jsr166/src/test/tck/CopyOnWriteArrayListTest.java,v
retrieving revision 1.29
diff -u -r1.29 CopyOnWriteArrayListTest.java
--- src/test/tck/CopyOnWriteArrayListTest.java 30 May 2013 03:28:55 -0000
1.29
+++ src/test/tck/CopyOnWriteArrayListTest.java 31 Jan 2014 17:50:15 -0000
@@ -500,10 +500,10 @@
      * can not store the objects inside the list
      */
     public void testToArray_ArrayStoreException() {
+        CopyOnWriteArrayList c = new CopyOnWriteArrayList();
+        c.add("zfasdfsdf");
+        c.add("asdadasd");
         try {
-            CopyOnWriteArrayList c = new CopyOnWriteArrayList();
-            c.add("zfasdfsdf");
-            c.add("asdadasd");
             c.toArray(new Long[5]);
             shouldThrow();
         } catch (ArrayStoreException success) {}
@@ -513,167 +513,196 @@
      * get throws an IndexOutOfBoundsException on a negative index
      */
     public void testGet1_IndexOutOfBoundsException() {
-        try {
-            CopyOnWriteArrayList c = new CopyOnWriteArrayList();
-            c.get(-1);
-            shouldThrow();
-        } catch (IndexOutOfBoundsException success) {}
+        CopyOnWriteArrayList c = populatedArray(5);
+        List[] lists = { c, c.subList(1, c.size() - 1) };
+        for (List list : lists) {
+            try {
+                list.get(-1);
+                shouldThrow();
+            } catch (IndexOutOfBoundsException success) {}
+        }
     }

     /**
      * get throws an IndexOutOfBoundsException on a too high index
      */
     public void testGet2_IndexOutOfBoundsException() {
-        try {
-            CopyOnWriteArrayList c = new CopyOnWriteArrayList();
-            c.add("asdasd");
-            c.add("asdad");
-            c.get(100);
-            shouldThrow();
-        } catch (IndexOutOfBoundsException success) {}
+        CopyOnWriteArrayList c = populatedArray(5);
+        List[] lists = { c, c.subList(1, c.size() - 1) };
+        for (List list : lists) {
+            try {
+                list.get(list.size());
+                shouldThrow();
+            } catch (IndexOutOfBoundsException success) {}
+        }
     }

     /**
      * set throws an IndexOutOfBoundsException on a negative index
      */
     public void testSet1_IndexOutOfBoundsException() {
-        try {
-            CopyOnWriteArrayList c = new CopyOnWriteArrayList();
-            c.set(-1,"qwerty");
-            shouldThrow();
-        } catch (IndexOutOfBoundsException success) {}
+        CopyOnWriteArrayList c = populatedArray(5);
+        List[] lists = { c, c.subList(1, c.size() - 1) };
+        for (List list : lists) {
+            try {
+                list.set(-1, "qwerty");
+                shouldThrow();
+            } catch (IndexOutOfBoundsException success) {}
+        }
     }

     /**
      * set throws an IndexOutOfBoundsException on a too high index
      */
     public void testSet2() {
-        try {
-            CopyOnWriteArrayList c = new CopyOnWriteArrayList();
-            c.add("asdasd");
-            c.add("asdad");
-            c.set(100, "qwerty");
-            shouldThrow();
-        } catch (IndexOutOfBoundsException success) {}
+        CopyOnWriteArrayList c = populatedArray(5);
+        List[] lists = { c, c.subList(1, c.size() - 1) };
+        for (List list : lists) {
+            try {
+                list.set(list.size(), "qwerty");
+                shouldThrow();
+            } catch (IndexOutOfBoundsException success) {}
+        }
     }

     /**
      * add throws an IndexOutOfBoundsException on a negative index
      */
     public void testAdd1_IndexOutOfBoundsException() {
-        try {
-            CopyOnWriteArrayList c = new CopyOnWriteArrayList();
-            c.add(-1,"qwerty");
-            shouldThrow();
-        } catch (IndexOutOfBoundsException success) {}
+        CopyOnWriteArrayList c = populatedArray(5);
+        List[] lists = { c, c.subList(1, c.size() - 1) };
+        for (List list : lists) {
+            try {
+                list.add(-1,"qwerty");
+                shouldThrow();
+            } catch (IndexOutOfBoundsException success) {}
+        }
     }

     /**
      * add throws an IndexOutOfBoundsException on a too high index
      */
     public void testAdd2_IndexOutOfBoundsException() {
-        try {
-            CopyOnWriteArrayList c = new CopyOnWriteArrayList();
-            c.add("asdasd");
-            c.add("asdasdasd");
-            c.add(100, "qwerty");
-            shouldThrow();
-        } catch (IndexOutOfBoundsException success) {}
+        CopyOnWriteArrayList c = populatedArray(5);
+        List[] lists = { c, c.subList(1, c.size() - 1) };
+        for (List list : lists) {
+            try {
+                list.add(list.size() + 1, "qwerty");
+                shouldThrow();
+            } catch (IndexOutOfBoundsException success) {}
+        }
     }

     /**
      * remove throws an IndexOutOfBoundsException on a negative index
      */
     public void testRemove1_IndexOutOfBounds() {
-        try {
-            CopyOnWriteArrayList c = new CopyOnWriteArrayList();
-            c.remove(-1);
-            shouldThrow();
-        } catch (IndexOutOfBoundsException success) {}
+        CopyOnWriteArrayList c = populatedArray(5);
+        List[] lists = { c, c.subList(1, c.size() - 1) };
+        for (List list : lists) {
+            try {
+                list.remove(-1);
+                shouldThrow();
+            } catch (IndexOutOfBoundsException success) {}
+        }
     }

     /**
      * remove throws an IndexOutOfBoundsException on a too high index
      */
     public void testRemove2_IndexOutOfBounds() {
-        try {
-            CopyOnWriteArrayList c = new CopyOnWriteArrayList();
-            c.add("asdasd");
-            c.add("adasdasd");
-            c.remove(100);
-            shouldThrow();
-        } catch (IndexOutOfBoundsException success) {}
+        CopyOnWriteArrayList c = populatedArray(5);
+        List[] lists = { c, c.subList(1, c.size() - 1) };
+        for (List list : lists) {
+            try {
+                list.remove(list.size());
+                shouldThrow();
+            } catch (IndexOutOfBoundsException success) {}
+        }
     }

     /**
      * addAll throws an IndexOutOfBoundsException on a negative index
      */
     public void testAddAll1_IndexOutOfBoundsException() {
-        try {
-            CopyOnWriteArrayList c = new CopyOnWriteArrayList();
-            c.addAll(-1,new LinkedList());
-            shouldThrow();
-        } catch (IndexOutOfBoundsException success) {}
+        CopyOnWriteArrayList c = populatedArray(5);
+        List[] lists = { c, c.subList(1, c.size() - 1) };
+        for (List list : lists) {
+            try {
+                list.addAll(-1,new LinkedList());
+                shouldThrow();
+            } catch (IndexOutOfBoundsException success) {}
+        }
     }

     /**
      * addAll throws an IndexOutOfBoundsException on a too high index
      */
     public void testAddAll2_IndexOutOfBoundsException() {
-        try {
-            CopyOnWriteArrayList c = new CopyOnWriteArrayList();
-            c.add("asdasd");
-            c.add("asdasdasd");
-            c.addAll(100, new LinkedList());
-            shouldThrow();
-        } catch (IndexOutOfBoundsException success) {}
+        CopyOnWriteArrayList c = populatedArray(5);
+        List[] lists = { c, c.subList(1, c.size() - 1) };
+        for (List list : lists) {
+            try {
+                list.addAll(list.size() + 1, new LinkedList());
+                shouldThrow();
+            } catch (IndexOutOfBoundsException success) {}
+        }
     }

     /**
      * listIterator throws an IndexOutOfBoundsException on a negative index
      */
     public void testListIterator1_IndexOutOfBoundsException() {
-        try {
-            CopyOnWriteArrayList c = new CopyOnWriteArrayList();
-            c.listIterator(-1);
-            shouldThrow();
-        } catch (IndexOutOfBoundsException success) {}
+        CopyOnWriteArrayList c = populatedArray(5);
+        List[] lists = { c, c.subList(1, c.size() - 1) };
+        for (List list : lists) {
+            try {
+                list.listIterator(-1);
+                shouldThrow();
+            } catch (IndexOutOfBoundsException success) {}
+        }
     }

     /**
      * listIterator throws an IndexOutOfBoundsException on a too high index
      */
     public void testListIterator2_IndexOutOfBoundsException() {
-        try {
-            CopyOnWriteArrayList c = new CopyOnWriteArrayList();
-            c.add("adasd");
-            c.add("asdasdas");
-            c.listIterator(100);
-            shouldThrow();
-        } catch (IndexOutOfBoundsException success) {}
+        CopyOnWriteArrayList c = populatedArray(5);
+        List[] lists = { c, c.subList(1, c.size() - 1) };
+        for (List list : lists) {
+            try {
+                list.listIterator(list.size() + 1);
+                shouldThrow();
+            } catch (IndexOutOfBoundsException success) {}
+        }
     }

     /**
      * subList throws an IndexOutOfBoundsException on a negative index
      */
     public void testSubList1_IndexOutOfBoundsException() {
-        try {
-            CopyOnWriteArrayList c = new CopyOnWriteArrayList();
-            c.subList(-1,100);
-            shouldThrow();
-        } catch (IndexOutOfBoundsException success) {}
+        CopyOnWriteArrayList c = populatedArray(5);
+        List[] lists = { c, c.subList(1, c.size() - 1) };
+        for (List list : lists) {
+            try {
+                list.subList(-1, list.size());
+                shouldThrow();
+            } catch (IndexOutOfBoundsException success) {}
+        }
     }

     /**
      * subList throws an IndexOutOfBoundsException on a too high index
      */
     public void testSubList2_IndexOutOfBoundsException() {
-        try {
-            CopyOnWriteArrayList c = new CopyOnWriteArrayList();
-            c.add("asdasd");
-            c.subList(1,100);
-            shouldThrow();
-        } catch (IndexOutOfBoundsException success) {}
+        CopyOnWriteArrayList c = populatedArray(5);
+        List[] lists = { c, c.subList(1, c.size() - 1) };
+        for (List list : lists) {
+            try {
+                list.subList(0, list.size() + 1);
+                shouldThrow();
+            } catch (IndexOutOfBoundsException success) {}
+        }
     }

     /**
@@ -681,11 +710,14 @@
      * is lower then the first
      */
     public void testSubList3_IndexOutOfBoundsException() {
-        try {
-            CopyOnWriteArrayList c = new CopyOnWriteArrayList();
-            c.subList(3,1);
-            shouldThrow();
-        } catch (IndexOutOfBoundsException success) {}
+        CopyOnWriteArrayList c = populatedArray(5);
+        List[] lists = { c, c.subList(1, c.size() - 1) };
+        for (List list : lists) {
+            try {
+                list.subList(list.size() - 1, 1);
+                shouldThrow();
+            } catch (IndexOutOfBoundsException success) {}
+        }
     }

     /**



On Fri, Jan 31, 2014 at 8:23 AM, Chris Hegarty <chris.hegarty at oracle.com>wrote:

> Trivial change to CopyOnWriteArrayList.COWSubList.subList to catch the
> case where the fromIndex is greater that the toIndex.
>
> http://cr.openjdk.java.net/~chegar/8011645/webrev.00/webrev/
>
> -Chris.
>



More information about the core-libs-dev mailing list