Copilot commented on code in PR #679:
URL:
https://github.com/apache/commons-collections/pull/679#discussion_r3409508203
##########
src/main/java/org/apache/commons/collections4/multiset/AbstractMapMultiSet.java:
##########
@@ -366,6 +367,9 @@ protected void doReadObject(final ObjectInputStream in)
@SuppressWarnings("unchecked") // This will fail at runtime if the
stream is incorrect
final E obj = (E) in.readObject();
final int count = in.readInt();
+ if (count < 1) {
+ throw new InvalidObjectException("Invalid count for entry: " +
count);
+ }
Review Comment:
The InvalidObjectException message doesn’t state the required range (>= 1).
Including it improves diagnostics for corrupted streams without needing to
stringify the element.
##########
src/main/java/org/apache/commons/collections4/bag/AbstractMapBag.java:
##########
@@ -300,6 +301,9 @@ protected void doReadObject(final Map<E, MutableInteger>
map, final ObjectInputS
@SuppressWarnings("unchecked") // This will fail at runtime if the
stream is incorrect
final E obj = (E) in.readObject();
final int count = in.readInt();
+ if (count < 1) {
+ throw new InvalidObjectException("Invalid count for entry: " +
count);
+ }
Review Comment:
The InvalidObjectException message doesn’t indicate the expected range (>=
1), which makes debugging corrupted streams harder. Consider including the
constraint in the message (without calling obj.toString()).
##########
src/test/java/org/apache/commons/collections4/multiset/HashMultiSetTest.java:
##########
@@ -16,14 +16,37 @@
*/
package org.apache.commons.collections4.multiset;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.InvalidObjectException;
+import java.io.ObjectInputStream;
+import java.io.ObjectOutputStream;
+
import org.apache.commons.collections4.MultiSet;
+import org.junit.jupiter.api.Test;
/**
* Extension of {@link AbstractMultiSetTest} for exercising the
* {@link HashMultiSet} implementation.
*/
public class HashMultiSetTest<T> extends AbstractMultiSetTest<T> {
+ private static void replaceInt(final byte[] bytes, final int from, final
int to) {
+ for (int i = 0; i + 4 <= bytes.length; i++) {
+ if (((bytes[i] & 0xFF) << 24 | (bytes[i + 1] & 0xFF) << 16
+ | (bytes[i + 2] & 0xFF) << 8 | bytes[i + 3] & 0xFF) ==
from) {
+ bytes[i] = (byte) (to >>> 24);
+ bytes[i + 1] = (byte) (to >>> 16);
+ bytes[i + 2] = (byte) (to >>> 8);
+ bytes[i + 3] = (byte) to;
+ return;
+ }
+ }
+ throw new IllegalStateException("marker not found in stream");
+ }
Review Comment:
replaceInt(...) is duplicated (also added in HashBagTest). Extracting this
into a shared test helper would reduce duplication and make the
serialization-tampering logic easier to maintain.
##########
src/test/java/org/apache/commons/collections4/bag/HashBagTest.java:
##########
@@ -16,14 +16,37 @@
*/
package org.apache.commons.collections4.bag;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.InvalidObjectException;
+import java.io.ObjectInputStream;
+import java.io.ObjectOutputStream;
+
import org.apache.commons.collections4.Bag;
+import org.junit.jupiter.api.Test;
/**
* Extension of {@link AbstractBagTest} for exercising the {@link HashBag}
* implementation.
*/
public class HashBagTest<T> extends AbstractBagTest<T> {
+ private static void replaceInt(final byte[] bytes, final int from, final
int to) {
+ for (int i = 0; i + 4 <= bytes.length; i++) {
+ if (((bytes[i] & 0xFF) << 24 | (bytes[i + 1] & 0xFF) << 16
+ | (bytes[i + 2] & 0xFF) << 8 | bytes[i + 3] & 0xFF) ==
from) {
+ bytes[i] = (byte) (to >>> 24);
+ bytes[i + 1] = (byte) (to >>> 16);
+ bytes[i + 2] = (byte) (to >>> 8);
+ bytes[i + 3] = (byte) to;
+ return;
+ }
+ }
+ throw new IllegalStateException("marker not found in stream");
+ }
Review Comment:
The helper method replaceInt(...) is duplicated in multiple test classes in
this PR. Consider extracting it into a shared test utility (or a common base
test class) to avoid repeated byte-manipulation logic and keep future test
changes in one place.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
