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 2016/07/12 19:06:40 UTC
svn commit: r1752330 - in /commons/proper/configuration/trunk/src:
main/java/org/apache/commons/configuration2/spring/ConfigurationPropertiesFactoryBean.java
test/java/org/apache/commons/configuration2/spring/TestConfigurationPropertiesFactoryBean.java
Author: oheger
Date: Tue Jul 12 19:06:40 2016
New Revision: 1752330
URL: http://svn.apache.org/viewvc?rev=1752330&view=rev
Log:
Made defensive copies of arrays.
This fixes some findbugs warnings about exposing mutable internal
state.
Modified:
commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration2/spring/ConfigurationPropertiesFactoryBean.java
commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration2/spring/TestConfigurationPropertiesFactoryBean.java
Modified: commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration2/spring/ConfigurationPropertiesFactoryBean.java
URL: http://svn.apache.org/viewvc/commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration2/spring/ConfigurationPropertiesFactoryBean.java?rev=1752330&r1=1752329&r2=1752330&view=diff
==============================================================================
--- commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration2/spring/ConfigurationPropertiesFactoryBean.java (original)
+++ commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration2/spring/ConfigurationPropertiesFactoryBean.java Tue Jul 12 19:06:40 2016
@@ -135,7 +135,7 @@ public class ConfigurationPropertiesFact
public Configuration[] getConfigurations()
{
- return configurations;
+ return defensiveCopy(configurations);
}
/**
@@ -145,12 +145,12 @@ public class ConfigurationPropertiesFact
*/
public void setConfigurations(Configuration[] configurations)
{
- this.configurations = configurations;
+ this.configurations = defensiveCopy(configurations);
}
public Resource[] getLocations()
{
- return locations;
+ return defensiveCopy(locations);
}
/**
@@ -162,7 +162,7 @@ public class ConfigurationPropertiesFact
*/
public void setLocations(Resource[] locations)
{
- this.locations = locations;
+ this.locations = defensiveCopy(locations);
}
public boolean isThrowExceptionOnMissing()
@@ -186,4 +186,16 @@ public class ConfigurationPropertiesFact
return compositeConfiguration;
}
+ /**
+ * Creates a defensive copy of the specified array. Handles null values
+ * correctly.
+ *
+ * @param src the source array
+ * @param <T> the type of the array
+ * @return the defensive copy of the array
+ */
+ private static <T> T[] defensiveCopy(T[] src)
+ {
+ return (src != null) ? src.clone() : null;
+ }
}
Modified: commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration2/spring/TestConfigurationPropertiesFactoryBean.java
URL: http://svn.apache.org/viewvc/commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration2/spring/TestConfigurationPropertiesFactoryBean.java?rev=1752330&r1=1752329&r2=1752330&view=diff
==============================================================================
--- commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration2/spring/TestConfigurationPropertiesFactoryBean.java (original)
+++ commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration2/spring/TestConfigurationPropertiesFactoryBean.java Tue Jul 12 19:06:40 2016
@@ -16,6 +16,9 @@
*/
package org.apache.commons.configuration2.spring;
+import static org.junit.Assert.assertArrayEquals;
+import static org.junit.Assert.assertNull;
+
import java.io.StringReader;
import java.util.Properties;
@@ -23,6 +26,7 @@ import org.apache.commons.configuration2
import org.apache.commons.configuration2.Configuration;
import org.apache.commons.configuration2.PropertiesConfiguration;
import org.apache.commons.configuration2.PropertiesConfigurationLayout;
+import org.apache.commons.configuration2.XMLConfiguration;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
@@ -105,4 +109,67 @@ public class TestConfigurationProperties
configurationFactory.afterPropertiesSet();
Assert.assertNotNull(configurationFactory.getConfiguration());
}
+
+ @Test
+ public void testSetLocationsDefensiveCopy()
+ {
+ Resource[] locations = {
+ new ClassPathResource("f1"), new ClassPathResource("f2")
+ };
+ Resource[] locationsUpdate = locations.clone();
+
+ configurationFactory.setLocations(locationsUpdate);
+ locationsUpdate[0] = new ClassPathResource("other");
+ assertArrayEquals("Locations were changed", locations,
+ configurationFactory.getLocations());
+ }
+
+ @Test
+ public void testSetLocationsNull()
+ {
+ configurationFactory.setLocations(null);
+ assertNull("Got locations", configurationFactory.getLocations());
+ }
+
+ @Test
+ public void testGetLocationsDefensiveCopy()
+ {
+ Resource[] locations = {
+ new ClassPathResource("f1"), new ClassPathResource("f2")
+ };
+ configurationFactory.setLocations(locations);
+
+ Resource[] locationsGet = configurationFactory.getLocations();
+ locationsGet[1] = null;
+ assertArrayEquals("Locations were changed", locations,
+ configurationFactory.getLocations());
+ }
+
+ @Test
+ public void testSetConfigurationsDefensiveCopy()
+ {
+ Configuration[] configs = {
+ new PropertiesConfiguration(), new XMLConfiguration()
+ };
+ Configuration[] configsUpdate = configs.clone();
+
+ configurationFactory.setConfigurations(configsUpdate);
+ configsUpdate[0] = null;
+ assertArrayEquals("Configurations were changed", configs,
+ configurationFactory.getConfigurations());
+ }
+
+ @Test
+ public void testGetConfigurationDefensiveCopy()
+ {
+ Configuration[] configs = {
+ new PropertiesConfiguration(), new XMLConfiguration()
+ };
+ configurationFactory.setConfigurations(configs);
+
+ Configuration[] configsGet = configurationFactory.getConfigurations();
+ configsGet[0] = null;
+ assertArrayEquals("Configurations were changed", configs,
+ configurationFactory.getConfigurations());
+ }
}