felix-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From fmesc...@apache.org
Subject svn commit: r1427230 - in /felix/trunk/configadmin/src: main/java/org/apache/felix/cm/impl/ test/java/org/apache/felix/cm/integration/
Date Mon, 31 Dec 2012 19:46:31 GMT
Author: fmeschbe
Date: Mon Dec 31 19:46:31 2012
New Revision: 1427230

URL: http://svn.apache.org/viewvc?rev=1427230&view=rev
Log:
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.

Modified:
    felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationAdapter.java
    felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationAdminImpl.java
    felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationManager.java
    felix/trunk/configadmin/src/test/java/org/apache/felix/cm/integration/ConfigurationBaseTest.java

Modified: felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationAdapter.java
URL: http://svn.apache.org/viewvc/felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationAdapter.java?rev=1427230&r1=1427229&r2=1427230&view=diff
==============================================================================
--- felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationAdapter.java
(original)
+++ felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationAdapter.java
Mon Dec 31 19:46:31 2012
@@ -76,7 +76,7 @@ public class ConfigurationAdapter implem
         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 @@ public class ConfigurationAdapter implem
         // 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 @@ public class ConfigurationAdapter implem
      * @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"
);
         }
     }

Modified: felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationAdminImpl.java
URL: http://svn.apache.org/viewvc/felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationAdminImpl.java?rev=1427230&r1=1427229&r2=1427230&view=diff
==============================================================================
--- felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationAdminImpl.java
(original)
+++ felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationAdminImpl.java
Mon Dec 31 19:46:31 2012
@@ -71,6 +71,8 @@ public class ConfigurationAdminImpl impl
      */
     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 class ConfigurationAdminImpl impl
      */
     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 class ConfigurationAdminImpl impl
      */
     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 @@ public class ConfigurationAdminImpl impl
             else
             {
                 // CM 1.4 / 104.13.2.3
-                this.checkPermission( config.getBundleLocation() );
+                this.checkPermission( configurationManager, config.getBundleLocation() );
             }
         }
 
@@ -137,11 +143,13 @@ public class ConfigurationAdminImpl impl
      */
     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 @@ public class ConfigurationAdminImpl impl
         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 class ConfigurationAdminImpl impl
      */
     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 @@ public class ConfigurationAdminImpl impl
      * 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 @@ public class ConfigurationAdminImpl impl
      * @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 @@ public class ConfigurationAdminImpl impl
         }
     }
 
+
+    /**
+     * 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;
+    }
 }

Modified: felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationManager.java
URL: http://svn.apache.org/viewvc/felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationManager.java?rev=1427230&r1=1427229&r2=1427230&view=diff
==============================================================================
--- felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationManager.java
(original)
+++ felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationManager.java
Mon Dec 31 19:46:31 2012
@@ -669,8 +669,8 @@ public class ConfigurationManager implem
                 }
 
                 // 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,

Modified: felix/trunk/configadmin/src/test/java/org/apache/felix/cm/integration/ConfigurationBaseTest.java
URL: http://svn.apache.org/viewvc/felix/trunk/configadmin/src/test/java/org/apache/felix/cm/integration/ConfigurationBaseTest.java?rev=1427230&r1=1427229&r2=1427230&view=diff
==============================================================================
--- felix/trunk/configadmin/src/test/java/org/apache/felix/cm/integration/ConfigurationBaseTest.java
(original)
+++ felix/trunk/configadmin/src/test/java/org/apache/felix/cm/integration/ConfigurationBaseTest.java
Mon Dec 31 19:46:31 2012
@@ -179,7 +179,7 @@ public class ConfigurationBaseTest exten
 
 
     @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 @@ public class ConfigurationBaseTest exten
 
 
     @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 @@ public class ConfigurationBaseTest exten
 
 
     @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



Mime
View raw message