[MSHARED-417] Infinite loop when loading self-referencing properties
Submitted-by: Gabriel Belingueres <belingueres@gmail.com>
This closes #17
git-svn-id: https://svn.apache.org/repos/asf/maven/shared/trunk@1791421 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/src/main/java/org/apache/maven/shared/filtering/BaseFilter.java b/src/main/java/org/apache/maven/shared/filtering/BaseFilter.java
index cb04db2..63a3966 100644
--- a/src/main/java/org/apache/maven/shared/filtering/BaseFilter.java
+++ b/src/main/java/org/apache/maven/shared/filtering/BaseFilter.java
@@ -208,7 +208,7 @@
try
{
File propFile = FileUtils.resolveFile( basedir, filterFile );
- Properties properties = PropertyUtils.loadPropertyFile( propFile, workProperties );
+ Properties properties = PropertyUtils.loadPropertyFile( propFile, workProperties, getLogger() );
filterProperties.putAll( properties );
workProperties.putAll( properties );
}
diff --git a/src/main/java/org/apache/maven/shared/filtering/PropertyUtils.java b/src/main/java/org/apache/maven/shared/filtering/PropertyUtils.java
index d03277c..3c8c9a5 100644
--- a/src/main/java/org/apache/maven/shared/filtering/PropertyUtils.java
+++ b/src/main/java/org/apache/maven/shared/filtering/PropertyUtils.java
@@ -23,10 +23,13 @@
import java.io.FileInputStream;
import java.io.FileNotFoundException;
import java.io.IOException;
+import java.util.LinkedList;
+import java.util.List;
import java.util.Properties;
import org.apache.maven.shared.utils.StringUtils;
import org.apache.maven.shared.utils.io.IOUtil;
+import org.codehaus.plexus.logging.Logger;
/**
* @author <a href="mailto:kenney@neonics.com">Kenney Westerhof</a>
@@ -57,6 +60,27 @@
public static Properties loadPropertyFile( File propFile, Properties baseProps )
throws IOException
{
+ return loadPropertyFile( propFile, baseProps, null );
+ }
+
+ /**
+ * Reads a property file, resolving all internal variables, using the supplied base properties.
+ * <p>
+ * The properties are resolved iteratively, so if the value of property A refers to property B, then after
+ * resolution the value of property B will contain the value of property B.
+ * </p>
+ *
+ * @param propFile The property file to load.
+ * @param baseProps Properties containing the initial values to substitute into the properties file.
+ * @param logger Logger instance
+ * @return Properties object containing the properties in the file with their values fully resolved.
+ * @throws IOException if profile does not exist, or cannot be read.
+ *
+ * @since 3.1.2
+ */
+ public static Properties loadPropertyFile( File propFile, Properties baseProps, Logger logger )
+ throws IOException
+ {
if ( !propFile.exists() )
{
throw new FileNotFoundException( propFile.toString() );
@@ -92,7 +116,7 @@
for ( Object o : fileProps.keySet() )
{
final String k = (String) o;
- final String propValue = getPropertyValue( k, combinedProps );
+ final String propValue = getPropertyValue( k, combinedProps, logger );
fileProps.setProperty( k, propValue );
}
@@ -111,6 +135,24 @@
public static Properties loadPropertyFile( File propfile, boolean fail, boolean useSystemProps )
throws IOException
{
+ return loadPropertyFile( propfile, fail, useSystemProps, null );
+ }
+
+ /**
+ * Reads a property file, resolving all internal variables.
+ *
+ * @param propfile The property file to load
+ * @param fail whether to throw an exception when the file cannot be loaded or to return null
+ * @param useSystemProps whether to incorporate System.getProperties settings into the returned Properties object.
+ * @param logger Logger instance
+ * @return the loaded and fully resolved Properties object
+ * @throws IOException if profile does not exist, or cannot be read.
+ *
+ * @since 3.1.2
+ */
+ public static Properties loadPropertyFile( File propfile, boolean fail, boolean useSystemProps, Logger logger )
+ throws IOException
+ {
final Properties baseProps = new Properties();
@@ -122,7 +164,7 @@
final Properties resolvedProps = new Properties();
try
{
- resolvedProps.putAll( loadPropertyFile( propfile, baseProps ) );
+ resolvedProps.putAll( loadPropertyFile( propfile, baseProps, logger ) );
}
catch ( FileNotFoundException e )
{
@@ -147,15 +189,21 @@
*
* @param k
* @param p
+ * @param logger Logger instance
* @return The filtered property value.
*/
- private static String getPropertyValue( String k, Properties p )
+ private static String getPropertyValue( String k, Properties p, Logger logger )
{
// This can also be done using InterpolationFilterReader,
// but it requires reparsing the file over and over until
// it doesn't change.
+ // for cycle detection
+ List<String> valueChain = new LinkedList<String>();
+ valueChain.add( k );
+
String v = p.getProperty( k );
+ String defaultValue = v;
String ret = "";
int idx, idx2;
@@ -180,26 +228,57 @@
v = v.substring( idx2 + 1 );
String nv = p.getProperty( nk );
- // try global environment..
- if ( nv == null && !StringUtils.isEmpty( nk ) )
+ if ( valueChain.contains( nk ) )
{
- nv = System.getProperty( nk );
- }
-
- // if the key cannot be resolved,
- // leave it alone ( and don't parse again )
- // else prefix the original string with the
- // resolved property ( so it can be parsed further )
- // taking recursion into account.
- if ( nv == null || nv.equals( k ) || k.equals( nk ) )
- {
- ret += "${" + nk + "}";
+ if ( logger != null )
+ {
+ logCircularDetection( valueChain, nk, logger );
+ }
+ return defaultValue;
}
else
{
- v = nv + v;
+ valueChain.add( nk );
+
+ // try global environment..
+ if ( nv == null && !StringUtils.isEmpty( nk ) )
+ {
+ nv = System.getProperty( nk );
+ }
+
+ // if the key cannot be resolved,
+ // leave it alone ( and don't parse again )
+ // else prefix the original string with the
+ // resolved property ( so it can be parsed further )
+ // taking recursion into account.
+ if ( nv == null || nv.equals( k ) || k.equals( nk ) )
+ {
+ ret += "${" + nk + "}";
+ }
+ else
+ {
+ v = nv + v;
+ }
}
}
+
return ret + v;
}
+
+ /**
+ * Logs the detected cycle in properties resolution
+ * @param valueChain the secuence of properties resolved so fa
+ * @param nk the key the closes the cycle
+ * @param logger Logger instance
+ */
+ private static void logCircularDetection( List<String> valueChain, String nk, Logger logger )
+ {
+ StringBuilder sb = new StringBuilder( "Circular reference between properties detected: " );
+ for ( String key : valueChain )
+ {
+ sb.append( key ).append( " => " );
+ }
+ sb.append( nk );
+ logger.warn( sb.toString() );
+ }
}
diff --git a/src/test/java/org/apache/maven/shared/filtering/PropertyUtilsTest.java b/src/test/java/org/apache/maven/shared/filtering/PropertyUtilsTest.java
index c86f72b..a9e651c 100644
--- a/src/test/java/org/apache/maven/shared/filtering/PropertyUtilsTest.java
+++ b/src/test/java/org/apache/maven/shared/filtering/PropertyUtilsTest.java
@@ -21,9 +21,12 @@
import java.io.File;
import java.io.FileWriter;
+import java.io.IOException;
+import java.util.ArrayList;
import java.util.Properties;
import org.codehaus.plexus.PlexusTestCase;
+import org.codehaus.plexus.logging.Logger;
/**
* @author Olivier Lamy
@@ -111,4 +114,199 @@
assertEquals( "realVersion", interpolated.get( "bar" ) );
assertEquals( "none filtered", interpolated.get( "none" ) );
}
+
+ /**
+ * Test case to reproduce MSHARED-417
+ *
+ * @throws IOException if problem writing file
+ */
+ public void testCircularReferences()
+ throws IOException
+ {
+ File basicProp = new File( testDirectory, "circular.properties" );
+
+ if ( basicProp.exists() )
+ {
+ basicProp.delete();
+ }
+
+ basicProp.createNewFile();
+ FileWriter writer = new FileWriter( basicProp );
+
+ writer.write( "test=${test2}\n" );
+ writer.write( "test2=${test2}\n" );
+ writer.flush();
+ writer.close();
+
+ MockLogger logger = new MockLogger();
+
+ Properties prop = PropertyUtils.loadPropertyFile( basicProp, null, logger );
+ assertEquals( "${test2}", prop.getProperty( "test" ) );
+ assertEquals( "${test2}", prop.getProperty( "test2" ) );
+ assertEquals( 2, logger.warnMsgs.size() );
+ assertWarn( "Circular reference between properties detected: test2 => test2", logger );
+ assertWarn( "Circular reference between properties detected: test => test2 => test2", logger );
+ }
+
+ /**
+ * Test case to reproduce MSHARED-417
+ *
+ * @throws IOException if problem writing file
+ */
+ public void testCircularReferences3Vars()
+ throws IOException
+ {
+ File basicProp = new File( testDirectory, "circular.properties" );
+
+ if ( basicProp.exists() )
+ {
+ basicProp.delete();
+ }
+
+ basicProp.createNewFile();
+ FileWriter writer = new FileWriter( basicProp );
+
+ writer.write( "test=${test2}\n" );
+ writer.write( "test2=${test3}\n" );
+ writer.write( "test3=${test}\n" );
+ writer.flush();
+ writer.close();
+
+ MockLogger logger = new MockLogger();
+
+ Properties prop = PropertyUtils.loadPropertyFile( basicProp, null, logger );
+ assertEquals( "${test2}", prop.getProperty( "test" ) );
+ assertEquals( "${test3}", prop.getProperty( "test2" ) );
+ assertEquals( "${test}", prop.getProperty( "test3" ) );
+ assertEquals( 3, logger.warnMsgs.size() );
+ assertWarn( "Circular reference between properties detected: test3 => test => test2 => test3", logger );
+ assertWarn( "Circular reference between properties detected: test2 => test3 => test => test2", logger );
+ assertWarn( "Circular reference between properties detected: test => test2 => test3 => test", logger );
+ }
+
+ private void assertWarn( String expected, MockLogger logger )
+ {
+ assertTrue( logger.warnMsgs.contains( expected ) );
+ }
+
+ private static class MockLogger
+ implements Logger
+ {
+
+ ArrayList<String> warnMsgs = new ArrayList<String>();
+
+ @Override
+ public void debug( String message )
+ {
+ // nothing
+ }
+
+ @Override
+ public void debug( String message, Throwable throwable )
+ {
+ // nothing
+ }
+
+ @Override
+ public boolean isDebugEnabled()
+ {
+ return false;
+ }
+
+ @Override
+ public void info( String message )
+ {
+ // nothing
+ }
+
+ @Override
+ public void info( String message, Throwable throwable )
+ {
+ // nothing
+ }
+
+ @Override
+ public boolean isInfoEnabled()
+ {
+ return false;
+ }
+
+ @Override
+ public void warn( String message )
+ {
+ warnMsgs.add( message );
+ }
+
+ @Override
+ public void warn( String message, Throwable throwable )
+ {
+ // nothing
+ }
+
+ @Override
+ public boolean isWarnEnabled()
+ {
+ return false;
+ }
+
+ @Override
+ public void error( String message )
+ {
+ // nothing
+ }
+
+ @Override
+ public void error( String message, Throwable throwable )
+ {
+ // nothing
+ }
+
+ @Override
+ public boolean isErrorEnabled()
+ {
+ return false;
+ }
+
+ @Override
+ public void fatalError( String message )
+ {
+ // nothing
+ }
+
+ @Override
+ public void fatalError( String message, Throwable throwable )
+ {
+ // nothing
+ }
+
+ @Override
+ public boolean isFatalErrorEnabled()
+ {
+ return false;
+ }
+
+ @Override
+ public int getThreshold()
+ {
+ return 0;
+ }
+
+ @Override
+ public void setThreshold( int threshold )
+ {
+ // nothing
+ }
+
+ @Override
+ public Logger getChildLogger( String name )
+ {
+ return null;
+ }
+
+ @Override
+ public String getName()
+ {
+ return null;
+ }
+ }
}