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 9b59357a1 Reject invalid load factor in hashed/reference map
doReadObject (#691)
9b59357a1 is described below
commit 9b59357a14f547402f5e536f8a08525c080b5a3b
Author: Dexter.k <[email protected]>
AuthorDate: Tue Jun 23 19:01:33 2026 +0000
Reject invalid load factor in hashed/reference map doReadObject (#691)
* reject invalid load factor in hashed map doReadObject
the constructors forbid a non-positive or NaN load factor; reapply that
check in AbstractHashedMap and AbstractReferenceMap doReadObject so a crafted
stream cannot deserialize a map with a corrupt threshold.
* use parameterized test for invalid load factor cases
* move reference map load factor test into ReferenceMapTest
* add invalid load factor tests for remaining hashed/reference map
subclasses
---
.../commons/collections4/map/AbstractHashedMap.java | 4 ++++
.../commons/collections4/map/AbstractReferenceMap.java | 4 ++++
.../collections4/map/CaseInsensitiveMapTest.java | 16 ++++++++++++++++
.../apache/commons/collections4/map/HashedMapTest.java | 17 +++++++++++++++++
.../org/apache/commons/collections4/map/LRUMapTest.java | 14 ++++++++++++++
.../apache/commons/collections4/map/LinkedMapTest.java | 15 +++++++++++++++
.../collections4/map/ReferenceIdentityMapTest.java | 15 +++++++++++++++
.../commons/collections4/map/ReferenceMapTest.java | 15 +++++++++++++++
8 files changed, 100 insertions(+)
diff --git
a/src/main/java/org/apache/commons/collections4/map/AbstractHashedMap.java
b/src/main/java/org/apache/commons/collections4/map/AbstractHashedMap.java
index aee103d8b..be761abde 100644
--- a/src/main/java/org/apache/commons/collections4/map/AbstractHashedMap.java
+++ b/src/main/java/org/apache/commons/collections4/map/AbstractHashedMap.java
@@ -17,6 +17,7 @@
package org.apache.commons.collections4.map;
import java.io.IOException;
+import java.io.InvalidObjectException;
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
import java.util.AbstractCollection;
@@ -943,6 +944,9 @@ public class AbstractHashedMap<K, V> extends AbstractMap<K,
V> implements Iterab
@SuppressWarnings("unchecked")
protected void doReadObject(final ObjectInputStream in) throws
IOException, ClassNotFoundException {
loadFactor = in.readFloat();
+ if (loadFactor <= 0.0f || Float.isNaN(loadFactor)) {
+ throw new InvalidObjectException("Load factor must be greater than
0");
+ }
final int capacity = in.readInt();
final int size = in.readInt();
init();
diff --git
a/src/main/java/org/apache/commons/collections4/map/AbstractReferenceMap.java
b/src/main/java/org/apache/commons/collections4/map/AbstractReferenceMap.java
index 42f84c294..e67829dc1 100644
---
a/src/main/java/org/apache/commons/collections4/map/AbstractReferenceMap.java
+++
b/src/main/java/org/apache/commons/collections4/map/AbstractReferenceMap.java
@@ -17,6 +17,7 @@
package org.apache.commons.collections4.map;
import java.io.IOException;
+import java.io.InvalidObjectException;
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
import java.lang.ref.Reference;
@@ -823,6 +824,9 @@ public abstract class AbstractReferenceMap<K, V> extends
AbstractHashedMap<K, V>
valueType = ReferenceStrength.resolve(in.readInt());
purgeValues = in.readBoolean();
loadFactor = in.readFloat();
+ if (loadFactor <= 0.0f || Float.isNaN(loadFactor)) {
+ throw new InvalidObjectException("Load factor must be greater than
0");
+ }
final int capacity = in.readInt();
init();
data = new HashEntry[capacity];
diff --git
a/src/test/java/org/apache/commons/collections4/map/CaseInsensitiveMapTest.java
b/src/test/java/org/apache/commons/collections4/map/CaseInsensitiveMapTest.java
index f72d0356c..1b51d3dfd 100644
---
a/src/test/java/org/apache/commons/collections4/map/CaseInsensitiveMapTest.java
+++
b/src/test/java/org/apache/commons/collections4/map/CaseInsensitiveMapTest.java
@@ -18,8 +18,10 @@ package org.apache.commons.collections4.map;
import static org.junit.jupiter.api.Assertions.assertEquals;
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.HashMap;
import java.util.Locale;
import java.util.Map;
@@ -27,6 +29,8 @@ import java.util.Set;
import org.apache.commons.lang3.StringUtils;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
/**
* Tests for the {@link CaseInsensitiveMap} implementation.
@@ -46,6 +50,18 @@ public class CaseInsensitiveMapTest<K, V> extends
AbstractIterableMapTest<K, V>
return new CaseInsensitiveMap<>();
}
+ /**
+ * A crafted stream can carry a load factor the constructor rejects.
AbstractHashedMap.doReadObject
+ * must reapply that contract on read.
+ */
+ @ParameterizedTest
+ @ValueSource(floats = {0.0f, -1.0f, Float.NaN})
+ void testDeserializeRejectsInvalidLoadFactor(final float badLoadFactor) {
+ final CaseInsensitiveMap<K, V> map = new CaseInsensitiveMap<>();
+ map.loadFactor = badLoadFactor;
+ assertThrows(InvalidObjectException.class, () ->
serializeDeserialize(map));
+ }
+
@Test
@SuppressWarnings("unchecked")
void testCaseInsensitive() {
diff --git
a/src/test/java/org/apache/commons/collections4/map/HashedMapTest.java
b/src/test/java/org/apache/commons/collections4/map/HashedMapTest.java
index 24d024fc3..2bbf69533 100644
--- a/src/test/java/org/apache/commons/collections4/map/HashedMapTest.java
+++ b/src/test/java/org/apache/commons/collections4/map/HashedMapTest.java
@@ -18,8 +18,13 @@ package org.apache.commons.collections4.map;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertSame;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import java.io.InvalidObjectException;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
/**
* JUnit tests.
@@ -85,4 +90,16 @@ public class HashedMapTest<K, V> extends
AbstractIterableMapTest<K, V> {
// the threshold has changed due to calling ensureCapacity
assertEquals(96, map.threshold);
}
+
+ /**
+ * A crafted stream can carry a load factor the constructor rejects.
AbstractHashedMap.doReadObject
+ * must reapply that contract on read.
+ */
+ @ParameterizedTest
+ @ValueSource(floats = {0.0f, -1.0f, Float.NaN})
+ void testDeserializeRejectsInvalidLoadFactor(final float badLoadFactor) {
+ final HashedMap<K, V> map = new HashedMap<>();
+ map.loadFactor = badLoadFactor;
+ assertThrows(InvalidObjectException.class, () ->
serializeDeserialize(map));
+ }
}
diff --git a/src/test/java/org/apache/commons/collections4/map/LRUMapTest.java
b/src/test/java/org/apache/commons/collections4/map/LRUMapTest.java
index 15fe791dc..8312a9e64 100644
--- a/src/test/java/org/apache/commons/collections4/map/LRUMapTest.java
+++ b/src/test/java/org/apache/commons/collections4/map/LRUMapTest.java
@@ -37,6 +37,8 @@ import org.apache.commons.collections4.MapIterator;
import org.apache.commons.collections4.OrderedMap;
import org.apache.commons.collections4.ResettableIterator;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
/**
* JUnit tests.
@@ -167,6 +169,18 @@ public class LRUMapTest<K, V> extends
AbstractOrderedMapTest<K, V> {
return new LRUMap<>();
}
+ /**
+ * A crafted stream can carry a load factor the constructor rejects.
AbstractHashedMap.doReadObject
+ * must reapply that contract on read.
+ */
+ @ParameterizedTest
+ @ValueSource(floats = {0.0f, -1.0f, Float.NaN})
+ void testDeserializeRejectsInvalidLoadFactor(final float badLoadFactor) {
+ final LRUMap<K, V> map = new LRUMap<>();
+ map.loadFactor = badLoadFactor;
+ assertThrows(InvalidObjectException.class, () ->
serializeDeserialize(map));
+ }
+
@Test
void testAccessOrder() {
if (!isPutAddSupported() || !isPutChangeSupported()) {
diff --git
a/src/test/java/org/apache/commons/collections4/map/LinkedMapTest.java
b/src/test/java/org/apache/commons/collections4/map/LinkedMapTest.java
index 479fd5714..e7a6bcc5b 100644
--- a/src/test/java/org/apache/commons/collections4/map/LinkedMapTest.java
+++ b/src/test/java/org/apache/commons/collections4/map/LinkedMapTest.java
@@ -21,6 +21,7 @@ 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 java.io.InvalidObjectException;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
@@ -33,6 +34,8 @@ import org.apache.commons.collections4.ResettableIterator;
import org.apache.commons.collections4.list.AbstractListTest;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
/**
* JUnit tests.
@@ -116,6 +119,18 @@ public class LinkedMapTest<K, V> extends
AbstractOrderedMapTest<K, V> {
return new LinkedMap<>();
}
+ /**
+ * A crafted stream can carry a load factor the constructor rejects.
AbstractHashedMap.doReadObject
+ * must reapply that contract on read.
+ */
+ @ParameterizedTest
+ @ValueSource(floats = {0.0f, -1.0f, Float.NaN})
+ void testDeserializeRejectsInvalidLoadFactor(final float badLoadFactor) {
+ final LinkedMap<K, V> map = new LinkedMap<>();
+ map.loadFactor = badLoadFactor;
+ assertThrows(InvalidObjectException.class, () ->
serializeDeserialize(map));
+ }
+
@Test
@SuppressWarnings("unchecked")
void testClone() {
diff --git
a/src/test/java/org/apache/commons/collections4/map/ReferenceIdentityMapTest.java
b/src/test/java/org/apache/commons/collections4/map/ReferenceIdentityMapTest.java
index bb107e91c..ef65f1015 100644
---
a/src/test/java/org/apache/commons/collections4/map/ReferenceIdentityMapTest.java
+++
b/src/test/java/org/apache/commons/collections4/map/ReferenceIdentityMapTest.java
@@ -25,6 +25,7 @@ import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
+import java.io.InvalidObjectException;
import java.lang.ref.WeakReference;
import java.util.Iterator;
import java.util.Map;
@@ -32,6 +33,8 @@ import java.util.Map;
import org.apache.commons.collections4.IterableMap;
import
org.apache.commons.collections4.map.AbstractReferenceMap.ReferenceStrength;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
/**
* Tests for ReferenceIdentityMap.
@@ -235,6 +238,18 @@ public class ReferenceIdentityMapTest<K, V> extends
AbstractIterableMapTest<K, V
return new ReferenceIdentityMap<>(ReferenceStrength.WEAK,
ReferenceStrength.WEAK);
}
+ /**
+ * A crafted stream can carry a load factor the constructor rejects.
AbstractReferenceMap.doReadObject
+ * (its own override) must reapply that contract on read.
+ */
+ @ParameterizedTest
+ @ValueSource(floats = {0.0f, -1.0f, Float.NaN})
+ void testDeserializeRejectsInvalidLoadFactor(final float badLoadFactor) {
+ final ReferenceIdentityMap<K, V> map = new ReferenceIdentityMap<>();
+ map.loadFactor = badLoadFactor;
+ assertThrows(InvalidObjectException.class, () ->
serializeDeserialize(map));
+ }
+
@Test
@SuppressWarnings("unchecked")
void testBasics() {
diff --git
a/src/test/java/org/apache/commons/collections4/map/ReferenceMapTest.java
b/src/test/java/org/apache/commons/collections4/map/ReferenceMapTest.java
index a2413c613..1f8889d1e 100644
--- a/src/test/java/org/apache/commons/collections4/map/ReferenceMapTest.java
+++ b/src/test/java/org/apache/commons/collections4/map/ReferenceMapTest.java
@@ -27,6 +27,7 @@ import static org.junit.jupiter.api.Assertions.fail;
import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
+import java.io.InvalidObjectException;
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
import java.io.Serializable;
@@ -41,6 +42,8 @@ import
org.apache.commons.collections4.map.AbstractHashedMap.HashEntry;
import org.apache.commons.collections4.map.AbstractReferenceMap.ReferenceEntry;
import
org.apache.commons.collections4.map.AbstractReferenceMap.ReferenceStrength;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
/**
* Tests for ReferenceMap.
@@ -309,6 +312,18 @@ public class ReferenceMapTest<K, V> extends
AbstractIterableMapTest<K, V> {
}
+ /**
+ * A crafted stream can carry a load factor the constructor rejects.
AbstractReferenceMap.doReadObject
+ * (its own override) must reapply that contract on read.
+ */
+ @ParameterizedTest
+ @ValueSource(floats = {0.0f, -1.0f, Float.NaN})
+ void testDeserializeRejectsInvalidLoadFactor(final float badLoadFactor) {
+ final ReferenceMap<K, V> map = new ReferenceMap<>();
+ map.loadFactor = badLoadFactor;
+ assertThrows(InvalidObjectException.class, () ->
serializeDeserialize(map));
+ }
+
/**
* Test whether remove is not removing last entry after calling hasNext.
* <p>
