This is an automated email from the ASF dual-hosted git repository.
garydgregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-collections.git
The following commit(s) were added to refs/heads/master by this push:
new 21c7d0500 Reject out-of-range index before mutating in indexed add
(#713)
21c7d0500 is described below
commit 21c7d050043fed83dbb27d818bcb109c87efdbdf
Author: Naveed Khan <[email protected]>
AuthorDate: Tue Jul 14 17:51:19 2026 +0000
Reject out-of-range index before mutating in indexed add (#713)
* reject out-of-range index before mutating in indexed add
SetUniqueList and ListOrderedSet updated the uniqueness set before the
index-validating backing collection, so a bad index threw but left the element
in the set. Range-check the index first, as ListOrderedMap.put(int, K, V)
already does.
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI
<[email protected]>
---------
Co-authored-by: Gary Gregory <[email protected]>
Co-authored-by: Copilot Autofix powered by AI
<[email protected]>
---
src/changes/changes.xml | 1 +
.../commons/collections4/list/SetUniqueList.java | 6 ++++++
.../commons/collections4/set/ListOrderedSet.java | 6 ++++++
.../collections4/list/SetUniqueListTest.java | 21 +++++++++++++++++++
.../collections4/set/ListOrderedSetTest.java | 24 ++++++++++++++++++++++
5 files changed, 58 insertions(+)
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index b13fc1e2c..9c0408fff 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -24,6 +24,7 @@
<body>
<release version="4.6.0" date="YYYY-MM-DD" description="This is a feature
and maintenance release. Java 8 or later is required.">
<!-- FIX -->
+ <action type="fix" dev="ggregory" due-to="Naveed Khan">SetUniqueList and
ListOrderedSet leave the backing collection inconsistent when add(int, Object)
or addAll(int, Collection) is called with an out-of-range index.</action>
<action type="fix" dev="ggregory" due-to="Naveed Khan, Gary
Gregory">DualHashBidiMap, DualLinkedHashBidiMap, and DualTreeBidiMap entrySet()
Map.Entry.setValue(Object) returns the new value instead of the previous
value.</action>
<action type="fix" dev="ggregory" due-to="Naveed Khan, Gary
Gregory">AbstractMapBag and AbstractMapMultiSet.add(Object, int) overflow the
size and element count past Integer.MAX_VALUE.</action>
<action type="fix" dev="ggregory" due-to="Naveed
Khan">CompositeCollection, CompositeSet, CompositeMap, AbstractMultiValuedMap
and AbstractMultiSet size() overflow int when the total exceeds
Integer.MAX_VALUE.</action>
diff --git
a/src/main/java/org/apache/commons/collections4/list/SetUniqueList.java
b/src/main/java/org/apache/commons/collections4/list/SetUniqueList.java
index a63e18714..4548eac41 100644
--- a/src/main/java/org/apache/commons/collections4/list/SetUniqueList.java
+++ b/src/main/java/org/apache/commons/collections4/list/SetUniqueList.java
@@ -219,6 +219,9 @@ public class SetUniqueList<E> extends
AbstractSerializableListDecorator<E> {
*/
@Override
public void add(final int index, final E object) {
+ if (index < 0 || index > size()) {
+ throw new IndexOutOfBoundsException("Index: " + index + ", Size: "
+ size());
+ }
// adds element if it is not contained already
if (!set.contains(object)) {
set.add(object);
@@ -261,6 +264,9 @@ public class SetUniqueList<E> extends
AbstractSerializableListDecorator<E> {
*/
@Override
public boolean addAll(final int index, final Collection<? extends E> coll)
{
+ if (index < 0 || index > size()) {
+ throw new IndexOutOfBoundsException("Index: " + index + ", Size: "
+ size());
+ }
final List<E> temp = new ArrayList<>();
for (final E e : coll) {
if (set.add(e)) {
diff --git
a/src/main/java/org/apache/commons/collections4/set/ListOrderedSet.java
b/src/main/java/org/apache/commons/collections4/set/ListOrderedSet.java
index eb96dc9fe..664d97411 100644
--- a/src/main/java/org/apache/commons/collections4/set/ListOrderedSet.java
+++ b/src/main/java/org/apache/commons/collections4/set/ListOrderedSet.java
@@ -232,6 +232,9 @@ public class ListOrderedSet<E>
* @see List#add(int, Object)
*/
public void add(final int index, final E object) {
+ if (index < 0 || index > setOrder.size()) {
+ throw new IndexOutOfBoundsException("Index: " + index + ", Size: "
+ setOrder.size());
+ }
if (!contains(object)) {
decorated().add(object);
setOrder.add(index, object);
@@ -259,6 +262,9 @@ public class ListOrderedSet<E>
* @see List#addAll(int, Collection)
*/
public boolean addAll(final int index, final Collection<? extends E> coll)
{
+ if (index < 0 || index > setOrder.size()) {
+ throw new IndexOutOfBoundsException("Index: " + index + ", Size: "
+ setOrder.size());
+ }
boolean changed = false;
// collect all elements to be added for performance reasons
final List<E> toAdd = new ArrayList<>();
diff --git
a/src/test/java/org/apache/commons/collections4/list/SetUniqueListTest.java
b/src/test/java/org/apache/commons/collections4/list/SetUniqueListTest.java
index b966fb401..a8d3eda0b 100644
--- a/src/test/java/org/apache/commons/collections4/list/SetUniqueListTest.java
+++ b/src/test/java/org/apache/commons/collections4/list/SetUniqueListTest.java
@@ -339,6 +339,27 @@ public class SetUniqueListTest<E> extends
AbstractListTest<E> {
assertEquals(thirdNewElement, list.get(0), "Third new element should
be at index 0");
}
+ @Test
+ void testIntCollectionAddAllOutOfBoundsIndex() {
+ final SetUniqueList<Integer> list = new SetUniqueList<>(new
ArrayList<>(), new HashSet<>());
+ list.add(Integer.valueOf(1));
+ final Integer newElement = Integer.valueOf(2);
+
+ // an out-of-range index must be rejected before the uniqueness set is
mutated
+ assertThrows(IndexOutOfBoundsException.class, () -> list.add(5,
newElement));
+ assertFalse(list.contains(newElement), "rejected element leaked into
the uniqueness set");
+ assertEquals(list.size(), list.asSet().size(), "list and uniqueness
set diverged");
+ // otherwise the element is silently dropped on the next add
+ assertTrue(list.add(newElement));
+ assertEquals(newElement, list.get(1));
+
+ assertThrows(IndexOutOfBoundsException.class,
+ () -> list.addAll(9, Arrays.asList(Integer.valueOf(3),
Integer.valueOf(4))));
+ assertFalse(list.contains(Integer.valueOf(3)));
+ assertFalse(list.contains(Integer.valueOf(4)));
+ assertEquals(list.size(), list.asSet().size(), "list and uniqueness
set diverged");
+ }
+
@Test
@SuppressWarnings("unchecked")
void testListIterator() {
diff --git
a/src/test/java/org/apache/commons/collections4/set/ListOrderedSetTest.java
b/src/test/java/org/apache/commons/collections4/set/ListOrderedSetTest.java
index 95553f8ac..a01b2d843 100644
--- a/src/test/java/org/apache/commons/collections4/set/ListOrderedSetTest.java
+++ b/src/test/java/org/apache/commons/collections4/set/ListOrderedSetTest.java
@@ -17,12 +17,14 @@
package org.apache.commons.collections4.set;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertSame;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import java.io.InvalidObjectException;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Collection;
import java.util.HashSet;
import java.util.Iterator;
@@ -171,6 +173,28 @@ public class ListOrderedSetTest<E>
assertSame(ONE, set.get(3));
}
+ @Test
+ @SuppressWarnings("unchecked")
+ void testListAddIndexedOutOfBounds() {
+ final ListOrderedSet<E> set = makeObject();
+ set.add((E) ZERO);
+
+ // an out-of-range index is rejected before the backing set is touched
+ assertThrows(IndexOutOfBoundsException.class, () -> set.add(5, (E)
ONE));
+ assertFalse(set.contains(ONE), "rejected element leaked into the
backing set");
+ assertEquals(set.size(), set.asList().size(), "backing set and order
list diverged");
+ // the element is still insertable afterwards
+ assertTrue(set.add((E) ONE));
+ assertSame(ONE, set.get(1));
+
+ final ListOrderedSet<E> other = makeObject();
+ other.add((E) ZERO);
+ assertThrows(IndexOutOfBoundsException.class, () -> other.addAll(9,
Arrays.asList((E) ONE, (E) TWO)));
+ assertFalse(other.contains(ONE));
+ assertFalse(other.contains(TWO));
+ assertEquals(other.size(), other.asList().size(), "backing set and
order list diverged");
+ }
+
@Test
@SuppressWarnings("unchecked")
void testListAddRemove() {
