From 45ec2a7b55e435ae72d90d1f9121aed24af5196f Mon Sep 17 00:00:00 2001 From: Gary Gregory Date: Mon, 11 May 2026 08:57:32 -0400 Subject: [PATCH] Detect and avoid processing cycles in YAML input (YAMLConfiguration). --- .../AbstractYAMLBasedConfiguration.java | 48 ++++++++++++------- .../configuration2/YAMLConfiguration.java | 20 ++------ .../configuration2/io/FileHandler.java | 3 -- .../configuration2/TestYAMLConfiguration.java | 16 ++++++- .../commons/configuration2/yaml/cycle.yaml | 17 +++++++ src/test/resources/test.yaml | 3 +- 6 files changed, 69 insertions(+), 38 deletions(-) create mode 100644 src/test/resources/org/apache/commons/configuration2/yaml/cycle.yaml diff --git a/src/main/java/org/apache/commons/configuration2/AbstractYAMLBasedConfiguration.java b/src/main/java/org/apache/commons/configuration2/AbstractYAMLBasedConfiguration.java index 1e730c3f87..a1573ec1f2 100644 --- a/src/main/java/org/apache/commons/configuration2/AbstractYAMLBasedConfiguration.java +++ b/src/main/java/org/apache/commons/configuration2/AbstractYAMLBasedConfiguration.java @@ -21,13 +21,16 @@ import java.util.Collection; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.stream.Collectors; import org.apache.commons.configuration2.ex.ConfigurationException; import org.apache.commons.configuration2.io.ConfigurationLogger; import org.apache.commons.configuration2.tree.ImmutableNode; +import org.apache.commons.lang3.StringUtils; /** *

@@ -69,44 +72,51 @@ private static void addEntry(final Map map, final String key, fi } /** - * Creates a part of the hierarchical nodes structure of the resulting configuration. The passed in element is converted - * into one or multiple configuration nodes. (If list structures are involved, multiple nodes are returned.) + * Creates a part of the hierarchical nodes structure of the resulting configuration. The passed in element is converted into one or multiple configuration + * nodes. (If list structures are involved, multiple nodes are returned.) * - * @param key the key of the new node(s) - * @param elem the element to be processed + * @param key the key of the new node(s). + * @param elem the element to be processed. + * @param visited the set of visited objects. * @return a list with configuration nodes representing the element */ - private static List constructHierarchy(final String key, final Object elem) { + private static List constructHierarchy(final String key, final Object elem, final Set visited) { if (elem instanceof Map) { - return parseMap((Map) elem, key); + return isVisisted(elem, visited) ? Collections.emptyList() : parseMap((Map) elem, key, visited); } if (elem instanceof Collection) { - return parseCollection((Collection) elem, key); + return isVisisted(elem, visited) ? Collections.emptyList() : parseCollection((Collection) elem, key, visited); } return Collections.singletonList(new ImmutableNode.Builder().name(key).value(elem).create()); } + private static boolean isVisisted(final Object elem, final Set visited) { + return !visited.add(System.identityHashCode(elem)); + } + /** * Parses a collection structure. The elements of the collection are processed recursively. * - * @param col the collection to be processed - * @param key the key under which this collection is to be stored - * @return a node representing this collection + * @param col the collection to be processed. + * @param key the key under which this collection is to be stored. + * @param visited the set of visited objects. + * @return a node representing this collection. */ - private static List parseCollection(final Collection col, final String key) { - return col.stream().flatMap(elem -> constructHierarchy(key, elem).stream()).collect(Collectors.toList()); + private static List parseCollection(final Collection col, final String key, final Set visited) { + return col.stream().flatMap(elem -> constructHierarchy(key, elem, visited).stream()).collect(Collectors.toList()); } /** * Parses a map structure. The single keys of the map are processed recursively. * - * @param map the map to be processed - * @param key the key under which this map is to be stored + * @param map the map to be processed. + * @param key the key under which this map is to be stored. + * @param visited the set of visited objects. * @return a node representing this map */ - private static List parseMap(final Map map, final String key) { + private static List parseMap(final Map map, final String key, final Set visited) { final ImmutableNode.Builder subtree = new ImmutableNode.Builder().name(key); - map.forEach((k, v) -> constructHierarchy(k, v).forEach(subtree::addChild)); + map.forEach((k, v) -> constructHierarchy(k, v, visited).forEach(subtree::addChild)); return Collections.singletonList(subtree.create()); } @@ -159,7 +169,9 @@ protected Map constructMap(final ImmutableNode node) { * @param map the map to be processed */ protected void load(final Map map) { - final List roots = constructHierarchy("", map); - getNodeModel().setRootNode(roots.get(0)); + final List roots = constructHierarchy(StringUtils.EMPTY, map, new HashSet<>()); + if (!roots.isEmpty()) { + getNodeModel().setRootNode(roots.get(0)); + } } } diff --git a/src/main/java/org/apache/commons/configuration2/YAMLConfiguration.java b/src/main/java/org/apache/commons/configuration2/YAMLConfiguration.java index 72125edbc2..6ca91be29d 100644 --- a/src/main/java/org/apache/commons/configuration2/YAMLConfiguration.java +++ b/src/main/java/org/apache/commons/configuration2/YAMLConfiguration.java @@ -21,7 +21,6 @@ import java.io.InputStream; import java.io.Reader; import java.io.Writer; -import java.util.Map; import org.apache.commons.configuration2.ex.ConfigurationException; import org.apache.commons.configuration2.io.InputStreamSupport; @@ -68,8 +67,7 @@ public YAMLConfiguration(final HierarchicalConfiguration c) { public void dump(final Writer out, final DumperOptions options) throws ConfigurationException, IOException { - final Yaml yaml = new Yaml(options); - yaml.dump(constructMap(getNodeModel().getNodeHandler().getRootNode()), out); + new Yaml(options).dump(constructMap(getNodeModel().getNodeHandler().getRootNode()), out); } /** @@ -81,9 +79,7 @@ public void dump(final Writer out, final DumperOptions options) @Override public void read(final InputStream in) throws ConfigurationException { try { - final Yaml yaml = createYamlForReading(new LoaderOptions()); - final Map map = yaml.load(in); - load(map); + load(createYamlForReading(new LoaderOptions()).load(in)); } catch (final Exception e) { rethrowException(e); } @@ -91,9 +87,7 @@ public void read(final InputStream in) throws ConfigurationException { public void read(final InputStream in, final LoaderOptions options) throws ConfigurationException { try { - final Yaml yaml = createYamlForReading(options); - final Map map = yaml.load(in); - load(map); + load(createYamlForReading(options).load(in)); } catch (final Exception e) { rethrowException(e); } @@ -102,9 +96,7 @@ public void read(final InputStream in, final LoaderOptions options) throws Confi @Override public void read(final Reader in) throws ConfigurationException { try { - final Yaml yaml = createYamlForReading(new LoaderOptions()); - final Map map = yaml.load(in); - load(map); + load(createYamlForReading(new LoaderOptions()).load(in)); } catch (final Exception e) { rethrowException(e); } @@ -112,9 +104,7 @@ public void read(final Reader in) throws ConfigurationException { public void read(final Reader in, final LoaderOptions options) throws ConfigurationException { try { - final Yaml yaml = createYamlForReading(options); - final Map map = yaml.load(in); - load(map); + load(createYamlForReading(options).load(in)); } catch (final Exception e) { rethrowException(e); } diff --git a/src/main/java/org/apache/commons/configuration2/io/FileHandler.java b/src/main/java/org/apache/commons/configuration2/io/FileHandler.java index 0f28f36b6a..6042f546b8 100644 --- a/src/main/java/org/apache/commons/configuration2/io/FileHandler.java +++ b/src/main/java/org/apache/commons/configuration2/io/FileHandler.java @@ -586,7 +586,6 @@ public void load(final File file) throws ConfigurationException { } catch (final MalformedURLException e1) { throw new ConfigurationException("Cannot create URL from file %s", file); } - load(url); } @@ -687,7 +686,6 @@ public void load(final URL url) throws ConfigurationException { */ private void load(final URL url, final FileLocator locator) throws ConfigurationException { InputStream in = null; - try { final FileSystem fileSystem = FileLocatorUtils.getFileSystem(locator); final URLConnectionOptions urlConnectionOptions = locator.getURLConnectionOptions(); @@ -732,7 +730,6 @@ private void loadFromStream(final InputStream in, final String encoding, final U syncSupport.lock(LockMode.WRITE); try { injectFileLocator(url); - if (getContent() instanceof InputStreamSupport) { loadFromStreamDirectly(in); } else { diff --git a/src/test/java/org/apache/commons/configuration2/TestYAMLConfiguration.java b/src/test/java/org/apache/commons/configuration2/TestYAMLConfiguration.java index 9ba7c319d4..c4f2521c5d 100644 --- a/src/test/java/org/apache/commons/configuration2/TestYAMLConfiguration.java +++ b/src/test/java/org/apache/commons/configuration2/TestYAMLConfiguration.java @@ -34,6 +34,7 @@ import java.util.Map; import org.apache.commons.configuration2.ex.ConfigurationException; +import org.apache.commons.configuration2.io.FileHandler; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; @@ -68,9 +69,22 @@ void testCopyConstructor() { assertEquals("bar", yamlConfiguration.getString("foo")); } + @Test + void testCycle() throws ConfigurationException { + final YAMLConfiguration configuration = new YAMLConfiguration(); + final FileHandler handler = new FileHandler(configuration); + handler.load(new File("src/test/resources/org/apache/commons/configuration2/yaml/cycle.yaml")); + } + @Test void testDoubleStringValues() { - final Object property = yamlConfiguration.getProperty("key5.example"); + final Object property = yamlConfiguration.getProperty("key5.example2"); + assertEquals(Arrays.asList("a", "a", "value"), property); + } + + @Test + void testDoubleStringEmptyValues() { + final Object property = yamlConfiguration.getProperty("key5.example1"); assertEquals(Arrays.asList("", "", "value"), property); } diff --git a/src/test/resources/org/apache/commons/configuration2/yaml/cycle.yaml b/src/test/resources/org/apache/commons/configuration2/yaml/cycle.yaml new file mode 100644 index 0000000000..a00023265b --- /dev/null +++ b/src/test/resources/org/apache/commons/configuration2/yaml/cycle.yaml @@ -0,0 +1,17 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +root: &cycle + - *cycle diff --git a/src/test/resources/test.yaml b/src/test/resources/test.yaml index 6052e50aa2..ee972d3dd1 100644 --- a/src/test/resources/test.yaml +++ b/src/test/resources/test.yaml @@ -26,4 +26,5 @@ martin: skill: Elite key5: - example: [ '', '', value] \ No newline at end of file + example1: [ '', '', value] + example2: [ 'a', 'a', value] \ No newline at end of file