You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by oh...@apache.org on 2013/02/16 22:08:08 UTC

svn commit: r1446950 - in /commons/proper/configuration/trunk/src: main/java/org/apache/commons/configuration/builder/combined/ test/java/org/apache/commons/configuration/builder/combined/

Author: oheger
Date: Sat Feb 16 21:08:08 2013
New Revision: 1446950

URL: http://svn.apache.org/r1446950
Log:
MultiFileConfigurationBuilder now prevents infinite loops if the file name pattern cannot be resolved, and the ConfigurationInterpolator used performs a recursive lookup.

Modified:
    commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/builder/combined/MultiFileConfigurationBuilder.java
    commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/builder/combined/AbstractMultiFileConfigurationBuilderTest.java
    commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/builder/combined/TestMultiFileConfigurationBuilder.java

Modified: commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/builder/combined/MultiFileConfigurationBuilder.java
URL: http://svn.apache.org/viewvc/commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/builder/combined/MultiFileConfigurationBuilder.java?rev=1446950&r1=1446949&r2=1446950&view=diff
==============================================================================
--- commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/builder/combined/MultiFileConfigurationBuilder.java (original)
+++ commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/builder/combined/MultiFileConfigurationBuilder.java Sat Feb 16 21:08:08 2013
@@ -94,6 +94,13 @@ public class MultiFileConfigurationBuild
             new AtomicReference<ConfigurationInterpolator>();
 
     /**
+     * A flag for preventing reentrant access to managed builders on
+     * interpolation of the file name pattern.
+     */
+    private final ThreadLocal<Boolean> inInterpolation =
+            new ThreadLocal<Boolean>();
+
+    /**
      * A specialized builder listener which gets registered at all managed
      * builders. This listener just propagates notifications from managed
      * builders to the listeners registered at this
@@ -186,7 +193,7 @@ public class MultiFileConfigurationBuild
         {
             throw new ConfigurationException("No file name pattern is set!");
         }
-        String fileName = constructFileName(multiParams);
+        String fileName = fetchFileName(multiParams);
 
         FileBasedConfigurationBuilder<T> builder =
                 getManagedBuilders().get(fileName);
@@ -429,6 +436,39 @@ public class MultiFileConfigurationBuild
     }
 
     /**
+     * Generates a file name for a managed builder based on the file name
+     * pattern. This method prevents infinite loops which could happen if the
+     * file name pattern cannot be resolved and the
+     * {@code ConfigurationInterpolator} used by this object causes a recursive
+     * lookup to this builder's configuration.
+     *
+     * @param multiParams the current builder parameters
+     * @return the file name for a managed builder
+     */
+    private String fetchFileName(MultiFileBuilderParametersImpl multiParams)
+    {
+        String fileName;
+        Boolean reentrant = inInterpolation.get();
+        if (reentrant != null && reentrant.booleanValue())
+        {
+            fileName = multiParams.getFilePattern();
+        }
+        else
+        {
+            inInterpolation.set(Boolean.TRUE);
+            try
+            {
+                fileName = constructFileName(multiParams);
+            }
+            finally
+            {
+                inInterpolation.set(Boolean.FALSE);
+            }
+        }
+        return fileName;
+    }
+
+    /**
      * Creates a map with parameters for a new managed configuration builder.
      * This method merges the basic parameters set for this builder with the
      * specific parameters object for managed builders (if provided).

Modified: commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/builder/combined/AbstractMultiFileConfigurationBuilderTest.java
URL: http://svn.apache.org/viewvc/commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/builder/combined/AbstractMultiFileConfigurationBuilderTest.java?rev=1446950&r1=1446949&r2=1446950&view=diff
==============================================================================
--- commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/builder/combined/AbstractMultiFileConfigurationBuilderTest.java (original)
+++ commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/builder/combined/AbstractMultiFileConfigurationBuilderTest.java Sat Feb 16 21:08:08 2013
@@ -34,25 +34,37 @@ public class AbstractMultiFileConfigurat
     /** The system property which selects a sub configuration. */
     private static final String PROP = "Id";
 
+    /** The part of the pattern containing the variable. */
+    protected static String PATTERN_VAR = "${sys:Id}";
+
     /** The pattern for file names. */
     protected static String PATTERN =
-            "target/test-classes/testMultiConfiguration_${sys:Id}.xml";
+            "target/test-classes/testMultiConfiguration_" + PATTERN_VAR
+                    + ".xml";
 
     /**
      * Sets a system property for accessing a specific configuration file from
-     * the test builder.
+     * the test builder. The passed in id can be null, then the system property
+     * is removed.
      *
      * @param id the ID of the managed configuration to load
      */
     protected static void switchToConfig(String id)
     {
-        System.setProperty(PROP, id);
+        if (id != null)
+        {
+            System.setProperty(PROP, id);
+        }
+        else
+        {
+            System.getProperties().remove(PROP);
+        }
     }
 
     @After
     public void tearDown() throws Exception
     {
-        System.getProperties().remove(PROP);
+        switchToConfig(null);
     }
 
     /**

Modified: commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/builder/combined/TestMultiFileConfigurationBuilder.java
URL: http://svn.apache.org/viewvc/commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/builder/combined/TestMultiFileConfigurationBuilder.java?rev=1446950&r1=1446949&r2=1446950&view=diff
==============================================================================
--- commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/builder/combined/TestMultiFileConfigurationBuilder.java (original)
+++ commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/builder/combined/TestMultiFileConfigurationBuilder.java Sat Feb 16 21:08:08 2013
@@ -29,8 +29,12 @@ import java.util.Collection;
 import java.util.Collections;
 
 import org.apache.commons.configuration.ConfigurationException;
+import org.apache.commons.configuration.ConfigurationLookup;
+import org.apache.commons.configuration.DynamicCombinedConfiguration;
+import org.apache.commons.configuration.HierarchicalConfiguration;
 import org.apache.commons.configuration.XMLConfiguration;
 import org.apache.commons.configuration.builder.BasicBuilderParameters;
+import org.apache.commons.configuration.builder.BuilderConfigurationWrapperFactory;
 import org.apache.commons.configuration.builder.BuilderListener;
 import org.apache.commons.configuration.builder.BuilderParameters;
 import org.apache.commons.configuration.builder.FileBasedConfigurationBuilder;
@@ -427,4 +431,31 @@ public class TestMultiFileConfigurationB
                 managedBuilder1.getFileHandler(),
                 managedBuilder2.getFileHandler());
     }
+
+    /**
+     * Tests whether infinite loops on constructing the file name using
+     * interpolation can be handled. This can happen if a pattern cannot be
+     * resolved and the {@code ConfigurationInterpolator} causes again a lookup
+     * of the builder's configuration.
+     */
+    @Test
+    public void testRecursiveInterpolation() throws ConfigurationException
+    {
+        DynamicCombinedConfiguration config =
+                new DynamicCombinedConfiguration();
+        config.setKeyPattern(PATTERN_VAR);
+        BasicBuilderParameters params = createTestBuilderParameters(null);
+        ConfigurationInterpolator ci = new ConfigurationInterpolator();
+        ci.addDefaultLookup(new ConfigurationLookup(config));
+        params.setInterpolator(ci);
+        MultiFileConfigurationBuilder<XMLConfiguration> builder =
+                new MultiFileConfigurationBuilder<XMLConfiguration>(
+                        XMLConfiguration.class, null, true);
+        builder.configure(params);
+        BuilderConfigurationWrapperFactory wrapFactory =
+                new BuilderConfigurationWrapperFactory();
+        config.addConfiguration(wrapFactory.createBuilderConfigurationWrapper(
+                HierarchicalConfiguration.class, builder), "Multi");
+        assertTrue("Got configuration data", config.isEmpty());
+    }
 }