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