FELIX-3820 Guard access to the ConfigurationManager field in the ConfigurationAdminImpl class (slightly modified patch by Guillaume Nodet, thanks). Also added some more guards in the context of permission checks and added integration tests.
git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1427230 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationAdapter.java b/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationAdapter.java
index 58768ac..c616938 100644
--- a/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationAdapter.java
+++ b/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationAdapter.java
@@ -76,7 +76,7 @@
delegatee.getConfigurationManager().log( LogService.LOG_DEBUG, "getBundleLocation() ==> {0}", new Object[]
{ bundleLocation } );
checkActive();
- configurationAdmin.checkPermission( ( bundleLocation == null ) ? "*" : bundleLocation );
+ configurationAdmin.checkPermission( delegatee.getConfigurationManager(), ( bundleLocation == null ) ? "*" : bundleLocation );
checkDeleted();
return bundleLocation;
}
@@ -95,8 +95,8 @@
// CM 1.4 / 104.13.2.4
checkActive();
final String configLocation = delegatee.getBundleLocation();
- configurationAdmin.checkPermission( ( configLocation == null ) ? "*" : configLocation );
- configurationAdmin.checkPermission( ( bundleLocation == null ) ? "*" : bundleLocation );
+ configurationAdmin.checkPermission( delegatee.getConfigurationManager(), ( configLocation == null ) ? "*" : configLocation );
+ configurationAdmin.checkPermission( delegatee.getConfigurationManager(), ( bundleLocation == null ) ? "*" : bundleLocation );
checkDeleted();
delegatee.setStaticBundleLocation( bundleLocation );
}
@@ -202,20 +202,26 @@
* @throws IllegalStateException If this configuration object is not
* backed by an active ConfigurationManager
*/
- private void checkActive() {
- if (!delegatee.isActive()) {
- throw new IllegalStateException( "Configuration " + delegatee.getPid() + " not backed by an active Configuration Admin Service" );
+ private void checkActive()
+ {
+ if ( !delegatee.isActive() )
+ {
+ throw new IllegalStateException( "Configuration " + delegatee.getPid()
+ + " not backed by an active Configuration Admin Service" );
}
}
+
/**
* Checks whether this configuration object has already been deleted.
*
* @throws IllegalStateException If this configuration object has been
* deleted.
*/
- private void checkDeleted() {
- if (delegatee.isDeleted()) {
+ private void checkDeleted()
+ {
+ if ( delegatee.isDeleted() )
+ {
throw new IllegalStateException( "Configuration " + delegatee.getPid() + " deleted" );
}
}
diff --git a/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationAdminImpl.java b/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationAdminImpl.java
index 09b3222..bf3df93 100644
--- a/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationAdminImpl.java
+++ b/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationAdminImpl.java
@@ -71,6 +71,8 @@
*/
public Configuration createFactoryConfiguration( String factoryPid ) throws IOException
{
+ final ConfigurationManager configurationManager = getConfigurationManager();
+
configurationManager.log( LogService.LOG_DEBUG, "createFactoryConfiguration(factoryPid={0})", new Object[]
{ factoryPid } );
@@ -85,12 +87,14 @@
*/
public Configuration createFactoryConfiguration( String factoryPid, String location ) throws IOException
{
+ final ConfigurationManager configurationManager = getConfigurationManager();
+
configurationManager.log( LogService.LOG_DEBUG, "createFactoryConfiguration(factoryPid={0}, location={1})",
new Object[]
{ factoryPid, location } );
// CM 1.4 / 104.13.2.3
- this.checkPermission( ( location == null ) ? "*" : location );
+ this.checkPermission( configurationManager, ( location == null ) ? "*" : location );
ConfigurationImpl config = configurationManager.createFactoryConfiguration( factoryPid, location );
return this.wrap( config );
@@ -102,6 +106,8 @@
*/
public Configuration getConfiguration( String pid ) throws IOException
{
+ final ConfigurationManager configurationManager = getConfigurationManager();
+
configurationManager.log( LogService.LOG_DEBUG, "getConfiguration(pid={0})", new Object[]
{ pid } );
@@ -124,7 +130,7 @@
else
{
// CM 1.4 / 104.13.2.3
- this.checkPermission( config.getBundleLocation() );
+ this.checkPermission( configurationManager, config.getBundleLocation() );
}
}
@@ -137,11 +143,13 @@
*/
public Configuration getConfiguration( String pid, String location ) throws IOException
{
+ final ConfigurationManager configurationManager = getConfigurationManager();
+
configurationManager.log( LogService.LOG_DEBUG, "getConfiguration(pid={0}, location={1})", new Object[]
{ pid, location } );
// CM 1.4 / 104.13.2.3
- this.checkPermission( ( location == null ) ? "*" : location );
+ this.checkPermission( configurationManager, ( location == null ) ? "*" : location );
ConfigurationImpl config = configurationManager.getConfiguration( pid );
if ( config == null )
@@ -151,7 +159,7 @@
else
{
final String configLocation = config.getBundleLocation();
- this.checkPermission( ( configLocation == null ) ? "*" : configLocation );
+ this.checkPermission( configurationManager, ( configLocation == null ) ? "*" : configLocation );
}
return this.wrap( config );
@@ -163,6 +171,8 @@
*/
public Configuration[] listConfigurations( String filter ) throws IOException, InvalidSyntaxException
{
+ final ConfigurationManager configurationManager = getConfigurationManager();
+
configurationManager.log( LogService.LOG_DEBUG, "listConfigurations(filter={0})", new Object[]
{ filter } );
@@ -194,11 +204,11 @@
* Returns <code>true</code> if the current access control context (call
* stack) has the CONFIGURE permission.
*/
- boolean hasPermission(String name)
+ boolean hasPermission( final ConfigurationManager configurationManager, String name )
{
try
{
- checkPermission(name);
+ checkPermission(configurationManager, name);
return true;
}
catch ( SecurityException se )
@@ -220,7 +230,7 @@
* @throws SecurityException if the access control context does not
* have the appropriate permission
*/
- void checkPermission( String name )
+ void checkPermission( final ConfigurationManager configurationManager, String name )
{
// the caller's permission must be checked
final SecurityManager sm = System.getSecurityManager();
@@ -267,4 +277,23 @@
}
}
+
+ /**
+ * Returns the {@link ConfigurationManager} backing this configuraiton
+ * admin instance or throws {@code IllegalStateException} if already
+ * disposed off.
+ *
+ * @return The {@link ConfigurationManager} instance if still active
+ * @throws IllegalStateException if this instance has been
+ * {@linkplain #dispose() disposed off} already.
+ */
+ private ConfigurationManager getConfigurationManager()
+ {
+ if ( this.configurationManager == null )
+ {
+ throw new IllegalStateException( "Configuration Admin service has been unregistered" );
+ }
+
+ return this.configurationManager;
+ }
}
diff --git a/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationManager.java b/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationManager.java
index 54d78ee..021a57a 100644
--- a/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationManager.java
+++ b/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationManager.java
@@ -669,8 +669,8 @@
}
// CM 1.4 / 104.13.2.3 Permission required
- if ( !configurationAdmin.hasPermission( ( String ) config
- .get( ConfigurationAdmin.SERVICE_BUNDLELOCATION ) ) )
+ if ( !configurationAdmin.hasPermission( this,
+ ( String ) config.get( ConfigurationAdmin.SERVICE_BUNDLELOCATION ) ) )
{
log(
LogService.LOG_DEBUG,
diff --git a/configadmin/src/test/java/org/apache/felix/cm/integration/ConfigurationBaseTest.java b/configadmin/src/test/java/org/apache/felix/cm/integration/ConfigurationBaseTest.java
index dbed3f9..684f514 100644
--- a/configadmin/src/test/java/org/apache/felix/cm/integration/ConfigurationBaseTest.java
+++ b/configadmin/src/test/java/org/apache/felix/cm/integration/ConfigurationBaseTest.java
@@ -179,7 +179,7 @@
@Test
- public void test_configuration_after_getProperties_config_admin_stop() throws BundleException
+ public void test_configuration_getProperties_after_config_admin_stop() throws BundleException
{
final String pid = "test_configuration_after_config_admin_stop";
final Configuration config = configure( pid, null, true );
@@ -240,7 +240,7 @@
@Test
- public void test_configuration_after_getBundleLocation_config_admin_stop() throws BundleException
+ public void test_configuration_getBundleLocation_after_config_admin_stop() throws BundleException
{
final String pid = "test_configuration_after_config_admin_stop";
final Configuration config = configure( pid, null, true );
@@ -386,6 +386,181 @@
@Test
+ public void test_configuration_admin_createFactoryConfiguration_1_after_config_admin_stop() throws BundleException
+ {
+ final ConfigurationAdmin ca = getConfigurationAdmin();
+ TestCase.assertNotNull( "ConfigurationAdmin service is required", ca );
+
+ final Bundle cfgAdminBundle = configAdminTracker.getServiceReference().getBundle();
+ cfgAdminBundle.stop();
+ try
+ {
+ ca.createFactoryConfiguration( "sample" );
+ TestCase.fail( "Expected IllegalStateException for ConfigurationAdmin.createFactoryConfiguration" );
+ }
+ catch ( IllegalStateException ise )
+ {
+ // expected
+ }
+ catch ( Exception e )
+ {
+ TestCase.fail( "Expected IllegalStateException for ConfigurationAdmin.createFactoryConfiguration, got: " + e );
+ }
+ finally
+ {
+ try
+ {
+ cfgAdminBundle.start();
+ }
+ catch ( BundleException be )
+ {
+ // tooo bad
+ }
+ }
+ }
+
+
+ @Test
+ public void test_configuration_admin_createFactoryConfiguration_2_after_config_admin_stop() throws BundleException
+ {
+ final ConfigurationAdmin ca = getConfigurationAdmin();
+ TestCase.assertNotNull( "ConfigurationAdmin service is required", ca );
+
+ final Bundle cfgAdminBundle = configAdminTracker.getServiceReference().getBundle();
+ cfgAdminBundle.stop();
+ try
+ {
+ ca.createFactoryConfiguration( "sample", "location" );
+ TestCase.fail( "Expected IllegalStateException for ConfigurationAdmin.createFactoryConfiguration" );
+ }
+ catch ( IllegalStateException ise )
+ {
+ // expected
+ }
+ catch ( Exception e )
+ {
+ TestCase.fail( "Expected IllegalStateException for ConfigurationAdmin.createFactoryConfiguration, got: " + e );
+ }
+ finally
+ {
+ try
+ {
+ cfgAdminBundle.start();
+ }
+ catch ( BundleException be )
+ {
+ // tooo bad
+ }
+ }
+ }
+
+
+ @Test
+ public void test_configuration_admin_getConfiguration_1_after_config_admin_stop() throws BundleException
+ {
+ final ConfigurationAdmin ca = getConfigurationAdmin();
+ TestCase.assertNotNull( "ConfigurationAdmin service is required", ca );
+
+ final Bundle cfgAdminBundle = configAdminTracker.getServiceReference().getBundle();
+ cfgAdminBundle.stop();
+ try
+ {
+ ca.getConfiguration( "sample" );
+ TestCase.fail( "Expected IllegalStateException for ConfigurationAdmin.getConfiguration" );
+ }
+ catch ( IllegalStateException ise )
+ {
+ // expected
+ }
+ catch ( Exception e )
+ {
+ TestCase.fail( "Expected IllegalStateException for ConfigurationAdmin.getConfiguration, got: " + e );
+ }
+ finally
+ {
+ try
+ {
+ cfgAdminBundle.start();
+ }
+ catch ( BundleException be )
+ {
+ // tooo bad
+ }
+ }
+ }
+
+
+ @Test
+ public void test_configuration_admin_getConfiguration_2_after_config_admin_stop() throws BundleException
+ {
+ final ConfigurationAdmin ca = getConfigurationAdmin();
+ TestCase.assertNotNull( "ConfigurationAdmin service is required", ca );
+
+ final Bundle cfgAdminBundle = configAdminTracker.getServiceReference().getBundle();
+ cfgAdminBundle.stop();
+ try
+ {
+ ca.getConfiguration( "sample", "location" );
+ TestCase.fail( "Expected IllegalStateException for ConfigurationAdmin.getConfiguration" );
+ }
+ catch ( IllegalStateException ise )
+ {
+ // expected
+ }
+ catch ( Exception e )
+ {
+ TestCase.fail( "Expected IllegalStateException for ConfigurationAdmin.getConfiguration, got: " + e );
+ }
+ finally
+ {
+ try
+ {
+ cfgAdminBundle.start();
+ }
+ catch ( BundleException be )
+ {
+ // tooo bad
+ }
+ }
+ }
+
+
+ @Test
+ public void test_configuration_admin_listConfigurations_after_config_admin_stop() throws BundleException
+ {
+ final ConfigurationAdmin ca = getConfigurationAdmin();
+ TestCase.assertNotNull( "ConfigurationAdmin service is required", ca );
+
+ final Bundle cfgAdminBundle = configAdminTracker.getServiceReference().getBundle();
+ cfgAdminBundle.stop();
+ try
+ {
+ ca.listConfigurations( "(service.pid=sample)" );
+ TestCase.fail( "Expected IllegalStateException for ConfigurationAdmin.listConfigurations" );
+ }
+ catch ( IllegalStateException ise )
+ {
+ // expected
+ }
+ catch ( Exception e )
+ {
+ TestCase.fail( "Expected IllegalStateException for ConfigurationAdmin.listConfigurations, got: " + e );
+ }
+ finally
+ {
+ try
+ {
+ cfgAdminBundle.start();
+ }
+ catch ( BundleException be )
+ {
+ // tooo bad
+ }
+ }
+ }
+
+
+ @Test
public void test_configuration_change_counter() throws IOException
{
// 1. create config with pid and locationA