This is an automated email from the ASF dual-hosted git repository.

davsclaus pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/camel.git


The following commit(s) were added to refs/heads/main by this push:
     new 62eed4b38acc CAMEL-26437: camel-util - FilterIterator: remove after a 
failed next throws IllegalStateException (#28784)
62eed4b38acc is described below

commit 62eed4b38acc47acbcc83139b9f06ba9a136ed65
Author: allthingssecurity <[email protected]>
AuthorDate: Sun Nov 1 13:50:15 2026 +0530

    CAMEL-26437: camel-util - FilterIterator: remove after a failed next throws 
IllegalStateException (#28784)
    
    Co-Authored-By: Claude Opus 5.5 <[email protected]>
---
 .../java/org/apache/camel/util/FilterIterator.java | 10 ++-
 .../camel/util/FilterIteratorRemoveTest.java       | 76 +++++++++++++++++++++-
 .../ROOT/pages/camel-4x-upgrade-guide-4_24.adoc    |  6 ++
 3 files changed, 88 insertions(+), 4 deletions(-)

diff --git 
a/core/camel-util/src/main/java/org/apache/camel/util/FilterIterator.java 
b/core/camel-util/src/main/java/org/apache/camel/util/FilterIterator.java
index 5482f7188eff..dc720c2dca1e 100644
--- a/core/camel-util/src/main/java/org/apache/camel/util/FilterIterator.java
+++ b/core/camel-util/src/main/java/org/apache/camel/util/FilterIterator.java
@@ -81,9 +81,10 @@ public class FilterIterator<T> implements Iterator<T>, 
Closeable {
     /**
      * Removes the last element returned by {@link #next()} from the 
underlying iterator.
      *
-     * @throws IllegalStateException if {@link #next()} has not been called, 
{@link #remove()} has already been called
-     *                               after the last call to {@link #next()}, 
or {@link #hasNext()} has been called after
-     *                               it (the look-ahead has advanced the 
underlying iterator past that element)
+     * @throws IllegalStateException if {@link #next()} has not been called, 
if {@link #remove()} has already been
+     *                               called after the last call to {@link 
#next()}, if {@link #hasNext()} was called
+     *                               after it, or if a later {@link #next()} 
failed after it looked at further elements
+     *                               (they were all rejected or null, or the 
filter or the underlying iterator threw)
      */
     @Override
     public void remove() {
@@ -96,6 +97,9 @@ public class FilterIterator<T> implements Iterator<T>, 
Closeable {
 
     protected T checkNext() {
         while (!closed && it.hasNext()) {
+            // moving the underlying iterator past the element returned by 
next() means it can no longer be removed,
+            // also when no accepted element follows (next() then throws 
NoSuchElementException) or the filter throws
+            canRemove = false;
             T ep = it.next();
             if (ep != null && (filter == null || filter.test(ep))) {
                 return ep;
diff --git 
a/core/camel-util/src/test/java/org/apache/camel/util/FilterIteratorRemoveTest.java
 
b/core/camel-util/src/test/java/org/apache/camel/util/FilterIteratorRemoveTest.java
index 71c2f189a1fb..c2257d888923 100644
--- 
a/core/camel-util/src/test/java/org/apache/camel/util/FilterIteratorRemoveTest.java
+++ 
b/core/camel-util/src/test/java/org/apache/camel/util/FilterIteratorRemoveTest.java
@@ -17,7 +17,9 @@
 package org.apache.camel.util;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
+import java.util.NoSuchElementException;
 
 import org.junit.jupiter.api.Test;
 
@@ -28,7 +30,8 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
 
 /**
  * remove() removes the element last returned by next(); hasNext() looks ahead 
in the underlying iterator, so after it
- * that element can no longer be removed and remove() must fail instead of 
removing the element looked at.
+ * that element can no longer be removed and remove() must fail instead of 
removing the element looked at. The same
+ * holds for a next() that fails after it looked at further elements (they 
were all rejected, or the filter threw).
  */
 class FilterIteratorRemoveTest {
 
@@ -94,4 +97,75 @@ class FilterIteratorRemoveTest {
         it.remove();
         assertEquals(List.of("a", "b"), list);
     }
+
+    @Test
+    void removeAfterNextFoundOnlyRejectedElementsFailsAndRemovesNothing() {
+        List<String> list = new ArrayList<>(List.of("a", "b", "c"));
+        FilterIterator<String> it = new FilterIterator<>(list.iterator(), 
"a"::equals);
+
+        assertEquals("a", it.next());
+        assertThrows(NoSuchElementException.class, it::next);
+        assertThrows(IllegalStateException.class, it::remove);
+        assertEquals(List.of("a", "b", "c"), list);
+        assertFalse(it.hasNext());
+    }
+
+    @Test
+    void removeAfterTheFilterThrowsFailsAndRemovesNothing() {
+        List<String> list = new ArrayList<>(List.of("a", "b", "c"));
+        FilterIterator<String> it = new FilterIterator<>(list.iterator(), s -> 
{
+            if ("b".equals(s)) {
+                throw new IllegalArgumentException("cannot filter b");
+            }
+            return true;
+        });
+
+        assertEquals("a", it.next());
+        assertThrows(IllegalArgumentException.class, it::next);
+        assertThrows(IllegalStateException.class, it::remove);
+        assertEquals(List.of("a", "b", "c"), list);
+
+        // the iterator goes on after the element the filter failed on, and 
the next element can be removed
+        assertEquals("c", it.next());
+        it.remove();
+        assertEquals(List.of("a", "b"), list);
+    }
+
+    @Test
+    void removeAfterNextSkippedANullElementFailsAndRemovesNothing() {
+        List<String> list = new ArrayList<>(Arrays.asList("a", null));
+        FilterIterator<String> it = new FilterIterator<>(list.iterator());
+
+        assertEquals("a", it.next());
+        assertThrows(NoSuchElementException.class, it::next);
+        assertThrows(IllegalStateException.class, it::remove);
+        assertEquals(Arrays.asList("a", null), list);
+    }
+
+    // control: at the end of the underlying iterator next() fails without 
moving it, so the element returned last can
+    // still be removed (as with the iterator of a list)
+    @Test
+    void removeAfterNextAtTheEndRemovesTheLastReturnedElement() {
+        List<String> list = new ArrayList<>(List.of("a", "b"));
+        FilterIterator<String> it = new FilterIterator<>(list.iterator());
+
+        assertEquals("a", it.next());
+        assertEquals("b", it.next());
+        assertThrows(NoSuchElementException.class, it::next);
+        it.remove();
+        assertEquals(List.of("a"), list);
+    }
+
+    // control: hasNext() looked at the rejected elements before next() failed
+    @Test
+    void removeAfterHasNextFoundOnlyRejectedElementsFails() {
+        List<String> list = new ArrayList<>(List.of("a", "b"));
+        FilterIterator<String> it = new FilterIterator<>(list.iterator(), 
"a"::equals);
+
+        assertEquals("a", it.next());
+        assertFalse(it.hasNext());
+        assertThrows(NoSuchElementException.class, it::next);
+        assertThrows(IllegalStateException.class, it::remove);
+        assertEquals(List.of("a", "b"), list);
+    }
 }
diff --git 
a/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_24.adoc 
b/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_24.adoc
index c919c687b9a1..a827b2f9e442 100644
--- a/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_24.adoc
+++ b/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_24.adoc
@@ -279,6 +279,12 @@ removed the element after the one `next()` returned, and a 
`remove()` before any
 `remove()` now throws `IllegalStateException` unless it directly follows 
`next()`. Code that called `hasNext()`
 between `next()` and `remove()` must call `remove()` before `hasNext()`.
 
+The same applies to a `remove()` after a `next()` that failed after it looked 
at further elements: a
+`NoSuchElementException` because the filter rejected all the elements that 
were left (or they were null), or an
+exception thrown by the filter or by the iterator it wraps. Such a `remove()` 
could remove an element that `next()`
+never returned; it now throws `IllegalStateException` and removes nothing. A 
`next()` that fails at the end of the
+wrapped iterator, without looking at any element, still leaves the element 
returned before it removable.
+
 === camel-util - CaseInsensitiveMap merges keys that differ only in the case 
of a supplementary letter
 
 `CaseInsensitiveMap`, which holds the message headers, now treats two keys 
that differ only in the case of a letter

Reply via email to