From ca34304a0ee691260f143ccbcff1abb86ed4d76d Mon Sep 17 00:00:00 2001 From: Maria Galbis Date: Tue, 14 Jul 2026 10:13:35 +0200 Subject: [PATCH 1/7] Enhance flattening logic to handle duplicate scalar values in collections --- .../convert/AbstractListDelimiterHandler.java | 32 +++++++++++-------- ...estAbstractConfigurationBasicFeatures.java | 28 ++++++++++++++++ .../TestPropertiesConfiguration.java | 4 +-- 3 files changed, 49 insertions(+), 15 deletions(-) diff --git a/src/main/java/org/apache/commons/configuration2/convert/AbstractListDelimiterHandler.java b/src/main/java/org/apache/commons/configuration2/convert/AbstractListDelimiterHandler.java index 0c0aa542ef..613a6afcf2 100644 --- a/src/main/java/org/apache/commons/configuration2/convert/AbstractListDelimiterHandler.java +++ b/src/main/java/org/apache/commons/configuration2/convert/AbstractListDelimiterHandler.java @@ -20,6 +20,7 @@ import java.nio.file.Path; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.Iterator; import java.util.LinkedList; import java.util.Set; @@ -38,28 +39,27 @@ * @since 2.0 */ public abstract class AbstractListDelimiterHandler implements ListDelimiterHandler { - + static Collection flatten(final ListDelimiterHandler handler, final Object value, final int limit, final Set dejaVu) { if (value instanceof String) { return handler.split((String) value, true); + } else if (!isRecursiveContainer(value)) { + return value != null ? Collections.singletonList(value) : Collections.emptyList(); } dejaVu.add(value); final Collection result = new LinkedList<>(); - if (value instanceof Path) { - // Don't handle as an Iterable. - result.add(value); - } else if (value instanceof Iterable) { - flattenIterator(handler, result, ((Iterable) value).iterator(), limit, dejaVu); - } else if (value instanceof Iterator) { - flattenIterator(handler, result, (Iterator) value, limit, dejaVu); - } else if (value != null) { - if (value.getClass().isArray()) { + try { + if (value instanceof Iterable) { + flattenIterator(handler, result, ((Iterable) value).iterator(), limit, dejaVu); + } else if (value instanceof Iterator) { + flattenIterator(handler, result, (Iterator) value, limit, dejaVu); + } else if (value.getClass().isArray()) { for (int len = Array.getLength(value), idx = 0, size = 0; idx < len && size < limit; idx++, size = result.size()) { result.addAll(handler.flatten(Array.get(value, idx), limit - size)); } - } else { - result.add(value); } + } finally { + dejaVu.remove(value); } return result; } @@ -74,7 +74,7 @@ static Collection flatten(final ListDelimiterHandler handler, final Object va * @param dejaVue Previously visited objects. */ static void flattenIterator(final ListDelimiterHandler handler, final Collection target, final Iterator iterator, final int limit, - final Set dejaVue) { + final Set dejaVue) { int size = target.size(); while (size < limit && iterator.hasNext()) { final Object next = iterator.next(); @@ -85,6 +85,12 @@ static void flattenIterator(final ListDelimiterHandler handler, final Collection } } + private static boolean isRecursiveContainer(final Object value) { + return value instanceof Iterator + || value instanceof Iterable && !(value instanceof Path) + || value != null && value.getClass().isArray(); + } + /** * Constructs a new instance. */ diff --git a/src/test/java/org/apache/commons/configuration2/TestAbstractConfigurationBasicFeatures.java b/src/test/java/org/apache/commons/configuration2/TestAbstractConfigurationBasicFeatures.java index 604661e481..91223128c4 100644 --- a/src/test/java/org/apache/commons/configuration2/TestAbstractConfigurationBasicFeatures.java +++ b/src/test/java/org/apache/commons/configuration2/TestAbstractConfigurationBasicFeatures.java @@ -26,6 +26,8 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -676,6 +678,32 @@ void testGetList() { assertEquals(expected, result); } + /** + * Tests typed list conversion for delimited values with duplicates. + */ + @Test + void testGetListTypedWithDuplicatesAndDelimiterHandling() { + final BaseConfiguration config = new BaseConfiguration(); + config.setListDelimiterHandler(new DefaultListDelimiterHandler(',')); + + config.addProperty("list.strings", Arrays.asList("a", "b", "a")); + config.addProperty("list.strings2", Arrays.asList("", "", "a")); + config.addProperty("list.ints", Arrays.asList(1, 2, 1)); + config.addProperty("list.booleans", Arrays.asList(true, false, true)); + config.addProperty("list.doubles", Arrays.asList(1.5, 2.5, 1.5)); + config.addProperty("list.paths", Arrays.asList(Paths.get("path1"), Paths.get("path2"), Paths.get("path1"))); + + assertEquals(Arrays.asList("a", "b", "a"), config.getList(String.class, "list.strings")); + assertEquals(Arrays.asList("", "", "a"), config.getList(String.class, "list.strings2")); + assertEquals(Arrays.asList(1, 2, 1), config.getList(Integer.class, "list.ints")); + assertEquals(Arrays.asList(Boolean.TRUE, Boolean.FALSE, Boolean.TRUE), config.getList(Boolean.class, "list.booleans")); + assertEquals(Arrays.asList(1.5d, 2.5d, 1.5d), config.getList(Double.class, "list.doubles")); + assertEquals( + Arrays.asList(Paths.get("path1"), Paths.get("path2"), Paths.get("path1")), + config.getList(Path.class, "list.paths") + ); + } + /** * Tests getList() for single non-string values. */ diff --git a/src/test/java/org/apache/commons/configuration2/TestPropertiesConfiguration.java b/src/test/java/org/apache/commons/configuration2/TestPropertiesConfiguration.java index ced8e3968b..b8a0698f3e 100644 --- a/src/test/java/org/apache/commons/configuration2/TestPropertiesConfiguration.java +++ b/src/test/java/org/apache/commons/configuration2/TestPropertiesConfiguration.java @@ -511,13 +511,13 @@ void testCompress840ArrayList(final int size) { void testCompress840ArrayListCycle(final int size) { final ArrayList object = new ArrayList<>(); for (int i = 0; i < size; i++) { - object.add(i); + object.add(String.valueOf(i)); object.add(object); object.add(new ArrayList<>(object)); } final Collection result = testCompress840(object); assertNotNull(result); - assertEquals(size, result.size()); + assertEquals((1 << (size + 1)) - 2, result.size()); object.add(object); testCompress840(object); } From 93ae5e79c1ee0a30557a7db3f2e77dfdda242a10 Mon Sep 17 00:00:00 2001 From: Maria Galbis Date: Tue, 14 Jul 2026 15:26:16 +0200 Subject: [PATCH 2/7] Fix invalid whitespace and tabs --- .../configuration2/convert/AbstractListDelimiterHandler.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/apache/commons/configuration2/convert/AbstractListDelimiterHandler.java b/src/main/java/org/apache/commons/configuration2/convert/AbstractListDelimiterHandler.java index 613a6afcf2..e37562bc06 100644 --- a/src/main/java/org/apache/commons/configuration2/convert/AbstractListDelimiterHandler.java +++ b/src/main/java/org/apache/commons/configuration2/convert/AbstractListDelimiterHandler.java @@ -39,7 +39,7 @@ * @since 2.0 */ public abstract class AbstractListDelimiterHandler implements ListDelimiterHandler { - + static Collection flatten(final ListDelimiterHandler handler, final Object value, final int limit, final Set dejaVu) { if (value instanceof String) { return handler.split((String) value, true); @@ -74,7 +74,7 @@ static Collection flatten(final ListDelimiterHandler handler, final Object va * @param dejaVue Previously visited objects. */ static void flattenIterator(final ListDelimiterHandler handler, final Collection target, final Iterator iterator, final int limit, - final Set dejaVue) { + final Set dejaVue) { int size = target.size(); while (size < limit && iterator.hasNext()) { final Object next = iterator.next(); From 6be3cf69319a5b3372f85aafb460d97d70a1c570 Mon Sep 17 00:00:00 2001 From: Maria Galbis Date: Wed, 15 Jul 2026 00:48:37 +0200 Subject: [PATCH 3/7] Restore the comment explaining why Path is treated as a scalar --- .../convert/AbstractListDelimiterHandler.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/apache/commons/configuration2/convert/AbstractListDelimiterHandler.java b/src/main/java/org/apache/commons/configuration2/convert/AbstractListDelimiterHandler.java index e37562bc06..1d4dd36414 100644 --- a/src/main/java/org/apache/commons/configuration2/convert/AbstractListDelimiterHandler.java +++ b/src/main/java/org/apache/commons/configuration2/convert/AbstractListDelimiterHandler.java @@ -86,8 +86,12 @@ static void flattenIterator(final ListDelimiterHandler handler, final Collection } private static boolean isRecursiveContainer(final Object value) { + if (value instanceof Path) { + // Don't handle as an Iterable. + return false; + } return value instanceof Iterator - || value instanceof Iterable && !(value instanceof Path) + || value instanceof Iterable || value != null && value.getClass().isArray(); } From 9e40af37cc3e4eabed320429349017443d48d2e9 Mon Sep 17 00:00:00 2001 From: Maria Galbis Date: Wed, 15 Jul 2026 00:50:41 +0200 Subject: [PATCH 4/7] Reuse the active recursion-path set when flattening array elements, centralize cycle checks for recursive containers, and keep scalar values outside cycle detection --- .../convert/AbstractListDelimiterHandler.java | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/apache/commons/configuration2/convert/AbstractListDelimiterHandler.java b/src/main/java/org/apache/commons/configuration2/convert/AbstractListDelimiterHandler.java index 1d4dd36414..e041ed6ca4 100644 --- a/src/main/java/org/apache/commons/configuration2/convert/AbstractListDelimiterHandler.java +++ b/src/main/java/org/apache/commons/configuration2/convert/AbstractListDelimiterHandler.java @@ -46,7 +46,9 @@ static Collection flatten(final ListDelimiterHandler handler, final Object va } else if (!isRecursiveContainer(value)) { return value != null ? Collections.singletonList(value) : Collections.emptyList(); } - dejaVu.add(value); + if (!dejaVu.add(value)) { + return Collections.emptyList(); + } final Collection result = new LinkedList<>(); try { if (value instanceof Iterable) { @@ -55,7 +57,7 @@ static Collection flatten(final ListDelimiterHandler handler, final Object va flattenIterator(handler, result, (Iterator) value, limit, dejaVu); } else if (value.getClass().isArray()) { for (int len = Array.getLength(value), idx = 0, size = 0; idx < len && size < limit; idx++, size = result.size()) { - result.addAll(handler.flatten(Array.get(value, idx), limit - size)); + result.addAll(flatten(handler, Array.get(value, idx), limit - size, dejaVu)); } } } finally { @@ -77,11 +79,8 @@ static void flattenIterator(final ListDelimiterHandler handler, final Collection final Set dejaVue) { int size = target.size(); while (size < limit && iterator.hasNext()) { - final Object next = iterator.next(); - if (!dejaVue.contains(next)) { - target.addAll(flatten(handler, next, limit - size, dejaVue)); - size = target.size(); - } + target.addAll(flatten(handler, iterator.next(), limit - size, dejaVue)); + size = target.size(); } } From 80a6d3ccc1cb08c1574c07085aad901a110728b9 Mon Sep 17 00:00:00 2001 From: Maria Galbis Date: Wed, 15 Jul 2026 00:51:18 +0200 Subject: [PATCH 5/7] add coverage for array and mixed-container cycles --- .../TestDefaultListDelimiterHandler.java | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/test/java/org/apache/commons/configuration2/convert/TestDefaultListDelimiterHandler.java b/src/test/java/org/apache/commons/configuration2/convert/TestDefaultListDelimiterHandler.java index 5a97c98eda..5d40362a20 100644 --- a/src/test/java/org/apache/commons/configuration2/convert/TestDefaultListDelimiterHandler.java +++ b/src/test/java/org/apache/commons/configuration2/convert/TestDefaultListDelimiterHandler.java @@ -23,6 +23,7 @@ import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.List; @@ -175,4 +176,29 @@ void testSplitSingleElement() { void testSplitUnexpectedEscape() { checkSplit("\\x, \\,y, \\", true, "\\x", ",y", "\\"); } + + /** + * Tests whether flatten() skips a recursive array reference while keeping the reachable leaf values. + */ + @Test + void testFlattenArrayCycle() { + final Object[] array = new Object[2]; + array[0] = "value1,value2"; + array[1] = array; + + assertIterableEquals(Arrays.asList("value1", "value2"), handler.flatten(array, Integer.MAX_VALUE)); + } + + /** + * Tests whether flatten() skips a recursive list-array cycle while keeping the reachable leaf values. + */ + @Test + void testFlattenMixedListAndArrayCycle() { + final List list = new ArrayList<>(); + final Object[] array = {list}; + list.add("value1,value2"); + list.add(array); + + assertIterableEquals(Arrays.asList("value1", "value2"), handler.flatten(list, Integer.MAX_VALUE)); + } } From e3228086bc786b453aeac481abe7fc1749259462 Mon Sep 17 00:00:00 2001 From: Maria Galbis Date: Wed, 15 Jul 2026 00:52:02 +0200 Subject: [PATCH 6/7] document the expected size calculation in the cyclic list test --- .../commons/configuration2/TestPropertiesConfiguration.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/test/java/org/apache/commons/configuration2/TestPropertiesConfiguration.java b/src/test/java/org/apache/commons/configuration2/TestPropertiesConfiguration.java index b8a0698f3e..0fdb120a49 100644 --- a/src/test/java/org/apache/commons/configuration2/TestPropertiesConfiguration.java +++ b/src/test/java/org/apache/commons/configuration2/TestPropertiesConfiguration.java @@ -517,6 +517,8 @@ void testCompress840ArrayListCycle(final int size) { } final Collection result = testCompress840(object); assertNotNull(result); + // Each iteration doubles the previous flattened values and adds two occurrences + // of the new scalar: f(n) = 2 * f(n - 1) + 2, with f(0) = 0. assertEquals((1 << (size + 1)) - 2, result.size()); object.add(object); testCompress840(object); From 480d7ed96cb8637e84477316be8857921e9c1b0f Mon Sep 17 00:00:00 2001 From: Maria Galbis Date: Wed, 15 Jul 2026 01:05:44 +0200 Subject: [PATCH 7/7] clarify the size calculation formula in the flattening test for scalar values --- .../commons/configuration2/TestPropertiesConfiguration.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/test/java/org/apache/commons/configuration2/TestPropertiesConfiguration.java b/src/test/java/org/apache/commons/configuration2/TestPropertiesConfiguration.java index 0fdb120a49..6ea513bc37 100644 --- a/src/test/java/org/apache/commons/configuration2/TestPropertiesConfiguration.java +++ b/src/test/java/org/apache/commons/configuration2/TestPropertiesConfiguration.java @@ -517,8 +517,9 @@ void testCompress840ArrayListCycle(final int size) { } final Collection result = testCompress840(object); assertNotNull(result); - // Each iteration doubles the previous flattened values and adds two occurrences - // of the new scalar: f(n) = 2 * f(n - 1) + 2, with f(0) = 0. + // At each iteration, the previous flattened values and the new scalar appear twice: + // once in the original list and once in its copy. Therefore, f(n) = 2 * (f(n - 1) + 1), + // with f(0) = 0, which gives f(n) = 2^(n + 1) - 2. assertEquals((1 << (size + 1)) - 2, result.size()); object.add(object); testCompress840(object);