Detect and avoid processing cycles in YAML input (YAMLConfiguration)

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;
 
 /**
  * <p>
@@ -69,44 +72,51 @@ private static void addEntry(final Map<String, Object> 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<ImmutableNode> constructHierarchy(final String key, final Object elem) {
+    private static List<ImmutableNode> constructHierarchy(final String key, final Object elem, final Set<Object> visited) {
         if (elem instanceof Map) {
-            return parseMap((Map<String, Object>) elem, key);
+            return isVisisted(elem, visited) ? Collections.emptyList() : parseMap((Map<String, Object>) elem, key, visited);
         }
         if (elem instanceof Collection) {
-            return parseCollection((Collection<Object>) elem, key);
+            return isVisisted(elem, visited) ? Collections.emptyList() : parseCollection((Collection<Object>) elem, key, visited);
         }
         return Collections.singletonList(new ImmutableNode.Builder().name(key).value(elem).create());
     }
 
+    private static boolean isVisisted(final Object elem, final Set<Object> 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<ImmutableNode> parseCollection(final Collection<Object> col, final String key) {
-        return col.stream().flatMap(elem -> constructHierarchy(key, elem).stream()).collect(Collectors.toList());
+    private static List<ImmutableNode> parseCollection(final Collection<Object> col, final String key, final Set<Object> 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<ImmutableNode> parseMap(final Map<String, Object> map, final String key) {
+    private static List<ImmutableNode> parseMap(final Map<String, Object> map, final String key, final Set<Object> 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<String, Object> constructMap(final ImmutableNode node) {
      * @param map the map to be processed
      */
     protected void load(final Map<String, Object> map) {
-        final List<ImmutableNode> roots = constructHierarchy("", map);
-        getNodeModel().setRootNode(roots.get(0));
+        final List<ImmutableNode> roots = constructHierarchy(StringUtils.EMPTY, map, new HashSet<>());
+        if (!roots.isEmpty()) {
+            getNodeModel().setRootNode(roots.get(0));
+        }
     }
 }

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<ImmutableNode> 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,19 +79,15 @@ 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<String, Object> map = yaml.load(in);
-            load(map);
+            load(createYamlForReading(new LoaderOptions()).load(in));
         } catch (final Exception e) {
             rethrowException(e);
         }
     }
 
     public void read(final InputStream in, final LoaderOptions options) throws ConfigurationException {
         try {
-            final Yaml yaml = createYamlForReading(options);
-            final Map<String, Object> map = yaml.load(in);
-            load(map);
+            load(createYamlForReading(options).load(in));
         } catch (final Exception e) {
             rethrowException(e);
         }
@@ -102,19 +96,15 @@ 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<String, Object> map = yaml.load(in);
-            load(map);
+            load(createYamlForReading(new LoaderOptions()).load(in));
         } catch (final Exception e) {
             rethrowException(e);
         }
     }
 
     public void read(final Reader in, final LoaderOptions options) throws ConfigurationException {
         try {
-            final Yaml yaml = createYamlForReading(options);
-            final Map<String, Object> map = yaml.load(in);
-            load(map);
+            load(createYamlForReading(options).load(in));
         } catch (final Exception e) {
             rethrowException(e);
         }

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 {

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);
     }
 

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

src/test/resources/test.yaml

@@ -26,4 +26,5 @@ martin:
     skill: Elite
 
 key5:
-  example: [ '', '', value]
+  example1: [ '', '', value]
+  example2: [ 'a', 'a', value]