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 fb2b16456 Clamp overflow in AbstractMapBag and AbstractMapMultiSet add
(#705)
fb2b16456 is described below
commit fb2b164569a7daf193b40542a2705fc59ebfed5a
Author: Naveed Khan <[email protected]>
AuthorDate: Fri Jul 10 12:53:44 2026 +0000
Clamp overflow in AbstractMapBag and AbstractMapMultiSet add (#705)
* clamp overflow in AbstractMapBag and AbstractMapMultiSet add
add(Object, int) accumulated size and the element count into int fields
with no cap, so adding past Integer.MAX_VALUE overflowed and size()/getCount()
returned negative values. Clamp both to Integer.MAX_VALUE.
* track exact size in a long and clamp on return in size()
clamping the stored size field lost information once the true size
crossed Integer.MAX_VALUE: later removals dropped size() below
Integer.MAX_VALUE while the real cardinality was still above it. keep
the exact total in a long, saturate only the per-element count (stored
as an int), accrue only the applied delta so size always equals the sum
of counts, and clamp on return in size(). extends the regression tests
to cover removal while the true size still exceeds Integer.MAX_VALUE.
---
src/changes/changes.xml | 1 +
.../commons/collections4/bag/AbstractMapBag.java | 17 +++++++++++------
.../collections4/multiset/AbstractMapMultiSet.java | 15 +++++++++------
.../apache/commons/collections4/bag/HashBagTest.java | 20 ++++++++++++++++++++
.../collections4/multiset/HashMultiSetTest.java | 18 ++++++++++++++++++
5 files changed, 59 insertions(+), 12 deletions(-)
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 17d574879..02a83f518 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, 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="Gary Gregory"
issue="COLLECTIONS-776">Wrapping PassiveExpiringMap in
Collections.synchronizedMap breaks expiration.</action>
<action type="fix" dev="ggregory" due-to="Naveed Khan, Gary
Gregory">[javadoc] Document that BloomFilter merge operations are non-atomic
and a filter that throws during a merge should be considered invalid.</action>
<action type="fix" dev="ggregory" due-to="Gary
Gregory">org.apache.commons.collections4.map.AbstractReferenceMap.ReferenceEntry.toReference(ReferenceStrength,
T, int) now throws IllegalArgumentException instead of Error.</action>
diff --git
a/src/main/java/org/apache/commons/collections4/bag/AbstractMapBag.java
b/src/main/java/org/apache/commons/collections4/bag/AbstractMapBag.java
index ddbd039f1..6a424b6c5 100644
--- a/src/main/java/org/apache/commons/collections4/bag/AbstractMapBag.java
+++ b/src/main/java/org/apache/commons/collections4/bag/AbstractMapBag.java
@@ -146,8 +146,8 @@ public abstract class AbstractMapBag<E> implements Bag<E> {
/** The map to use to store the data */
private transient Map<E, MutableInteger> map;
- /** The current total size of the bag */
- private int size;
+ /** The current total size of the bag; kept exact past {@link
Integer#MAX_VALUE}, {@link #size()} saturates */
+ private long size;
/** The modification count for fail fast iterators */
private transient int modCount;
@@ -196,6 +196,8 @@ public abstract class AbstractMapBag<E> implements Bag<E> {
/**
* Adds a new element to the bag, incrementing its count in the map.
+ * The count of an element saturates at {@code Integer.MAX_VALUE}; copies
+ * that would take it past that limit are not added.
*
* @param object the object to search for
* @param nCopies the number of copies to add
@@ -206,12 +208,14 @@ public abstract class AbstractMapBag<E> implements Bag<E>
{
modCount++;
if (nCopies > 0) {
final MutableInteger mut = map.get(object);
- size += nCopies;
if (mut == null) {
map.put(object, new MutableInteger(nCopies));
+ size += nCopies;
return true;
}
- mut.value += nCopies;
+ final int applied = Math.min(nCopies, Integer.MAX_VALUE -
mut.value);
+ mut.value += applied;
+ size += applied;
}
return false;
}
@@ -523,13 +527,14 @@ public abstract class AbstractMapBag<E> implements Bag<E>
{
}
/**
- * Returns the number of elements in this bag.
+ * Returns the number of elements in this bag, or {@code Integer.MAX_VALUE}
+ * if the bag contains more than {@code Integer.MAX_VALUE} elements.
*
* @return current size of the bag
*/
@Override
public int size() {
- return size;
+ return (int) Math.min(size, Integer.MAX_VALUE);
}
/**
diff --git
a/src/main/java/org/apache/commons/collections4/multiset/AbstractMapMultiSet.java
b/src/main/java/org/apache/commons/collections4/multiset/AbstractMapMultiSet.java
index 4d62bc47a..2b2245e2d 100644
---
a/src/main/java/org/apache/commons/collections4/multiset/AbstractMapMultiSet.java
+++
b/src/main/java/org/apache/commons/collections4/multiset/AbstractMapMultiSet.java
@@ -280,8 +280,8 @@ public abstract class AbstractMapMultiSet<E> extends
AbstractMultiSet<E> {
/** The map to use to store the data */
private transient Map<E, MutableInteger> map;
- /** The current total size of the multiset */
- private transient int size;
+ /** The current total size of the multiset; kept exact past {@link
Integer#MAX_VALUE}, {@link #size()} saturates */
+ private transient long size;
/** The modification count for fail fast iterators */
private transient int modCount;
@@ -326,11 +326,13 @@ public abstract class AbstractMapMultiSet<E> extends
AbstractMultiSet<E> {
if (occurrences > 0) {
modCount++;
- size += occurrences;
if (mut == null) {
map.put(object, new MutableInteger(occurrences));
+ size += occurrences;
} else {
- mut.value += occurrences;
+ final int applied = Math.min(occurrences, Integer.MAX_VALUE -
mut.value);
+ mut.value += applied;
+ size += applied;
}
}
return oldCount;
@@ -523,13 +525,14 @@ public abstract class AbstractMapMultiSet<E> extends
AbstractMultiSet<E> {
}
/**
- * Returns the number of elements in this multiset.
+ * Returns the number of elements in this multiset, or {@code
Integer.MAX_VALUE}
+ * if the multiset contains more than {@code Integer.MAX_VALUE} elements.
*
* @return current size of the multiset
*/
@Override
public int size() {
- return size;
+ return (int) Math.min(size, Integer.MAX_VALUE);
}
/**
diff --git a/src/test/java/org/apache/commons/collections4/bag/HashBagTest.java
b/src/test/java/org/apache/commons/collections4/bag/HashBagTest.java
index 31837664c..b8671e98a 100644
--- a/src/test/java/org/apache/commons/collections4/bag/HashBagTest.java
+++ b/src/test/java/org/apache/commons/collections4/bag/HashBagTest.java
@@ -16,6 +16,7 @@
*/
package org.apache.commons.collections4.bag;
+import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import java.io.InvalidObjectException;
@@ -57,6 +58,25 @@ public class HashBagTest<T> extends AbstractBagTest<T> {
}
}
+ @Test
+ void testAddClampsCountAndSizeToIntegerMaxValue() {
+ final HashBag<String> bag = new HashBag<>();
+ bag.add("X", Integer.MAX_VALUE - 1);
+ bag.add("Y", Integer.MAX_VALUE - 1);
+ assertEquals(Integer.MAX_VALUE, bag.size());
+ bag.add("X", 10);
+ assertEquals(Integer.MAX_VALUE, bag.getCount("X"));
+ assertEquals(Integer.MAX_VALUE, bag.size());
+ assertEquals(2, bag.uniqueSet().size());
+ // true size is 2 * Integer.MAX_VALUE - 11 after this, so size() must
stay saturated
+ bag.remove("X", 10);
+ assertEquals(Integer.MAX_VALUE - 10, bag.getCount("X"));
+ assertEquals(Integer.MAX_VALUE, bag.size());
+ // size() only drops below Integer.MAX_VALUE once the true size does
+ bag.remove("Y");
+ assertEquals(Integer.MAX_VALUE - 10, bag.size());
+ }
+
// void testCreate() throws Exception {
// Bag<T> bag = makeObject();
// writeExternalFormToDisk((java.io.Serializable) bag,
"src/test/resources/data/test/HashBag.emptyCollection.version4.obj");
diff --git
a/src/test/java/org/apache/commons/collections4/multiset/HashMultiSetTest.java
b/src/test/java/org/apache/commons/collections4/multiset/HashMultiSetTest.java
index 139d95c04..da57c20f8 100644
---
a/src/test/java/org/apache/commons/collections4/multiset/HashMultiSetTest.java
+++
b/src/test/java/org/apache/commons/collections4/multiset/HashMultiSetTest.java
@@ -68,6 +68,24 @@ public class HashMultiSetTest<T> extends
AbstractMultiSetTest<T> {
}
}
+ @Test
+ void testAddClampsCountAndSizeToIntegerMaxValue() {
+ final HashMultiSet<String> set = new HashMultiSet<>();
+ set.add("X", Integer.MAX_VALUE);
+ set.add("X", 1);
+ assertEquals(Integer.MAX_VALUE, set.getCount("X"));
+ assertEquals(Integer.MAX_VALUE, set.size());
+ set.add("Y", Integer.MAX_VALUE);
+ assertEquals(Integer.MAX_VALUE, set.size());
+ // true size is 2 * Integer.MAX_VALUE - 2 after this, so size() must
stay saturated
+ set.remove("X", 2);
+ assertEquals(Integer.MAX_VALUE - 2, set.getCount("X"));
+ assertEquals(Integer.MAX_VALUE, set.size());
+ // size() only drops below Integer.MAX_VALUE once the true size does
+ set.remove("Y", Integer.MAX_VALUE);
+ assertEquals(Integer.MAX_VALUE - 2, set.size());
+ }
+
// void testCreate() throws Exception {
// MultiSet<T> multiset = makeObject();
// writeExternalFormToDisk((java.io.Serializable) multiset,
"src/test/resources/data/test/HashMultiSet.emptyCollection.version4.1.obj");
