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

Doug Lea dl at cs.oswego.edu
Fri Jan 31 18:07:06 UTC 2014


Thanks Martin and Chris!

-Doug


On 01/31/2014 01:01 PM, Martin Buchholz wrote:
> 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.java4 Nov 2013 00:00:39
> -00001.6
> +++ src/jdk7/java/util/concurrent/CopyOnWriteArrayList.java31 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.java4 Nov 2013 00:00:39
> -00001.114
> +++ src/main/java/util/concurrent/CopyOnWriteArrayList.java31 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.java30 May 2013 03:28:55 -00001.29
> +++ src/test/tck/CopyOnWriteArrayListTest.java31 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
> <mailto: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/
>     <http://cr.openjdk.java.net/~chegar/8011645/webrev.00/webrev/>
>
>     -Chris.
>
>




More information about the core-libs-dev mailing list