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