FELIX-3229 Fix permission checks (and clean code up a bit) for getConfiguration(String, String) and createFactoryConfiguration(String, String)

git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1202621 13f79535-47bb-0310-9956-ffa450edef68
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 63babbe..a48ba67 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
@@ -74,7 +74,9 @@
         configurationManager.log( LogService.LOG_DEBUG, "createFactoryConfiguration(factoryPid={0})", new Object[]
             { factoryPid } );
 
-        return this.wrap( configurationManager.createFactoryConfiguration( this, factoryPid ) );
+        ConfigurationImpl config = configurationManager.createFactoryConfiguration( factoryPid, this.getBundle()
+            .getLocation() );
+        return this.wrap( config );
     }
 
 
@@ -88,9 +90,10 @@
                 { factoryPid, location } );
 
         // CM 1.4 / 104.13.2.3
-        this.checkPermission( location );
+        this.checkPermission( ( location == null ) ? "*" : location );
 
-        return this.wrap( configurationManager.createFactoryConfiguration( factoryPid, location ) );
+        ConfigurationImpl config = configurationManager.createFactoryConfiguration( factoryPid, location );
+        return this.wrap( config );
     }
 
 
@@ -102,20 +105,26 @@
         configurationManager.log( LogService.LOG_DEBUG, "getConfiguration(pid={0})", new Object[]
             { pid } );
 
-        ConfigurationImpl config = configurationManager.getConfiguration( pid, getBundle().getLocation() );
-
-        if ( config.getBundleLocation() == null )
+        ConfigurationImpl config = configurationManager.getConfiguration( pid );
+        if ( config == null )
         {
-            configurationManager.log( LogService.LOG_DEBUG, "Binding configuration {0} (isNew: {1}) to bundle {2}",
-                new Object[]
-                    { config.getPid(), Boolean.valueOf( config.isNew() ), getBundle().getLocation() } );
-
-            config.setStaticBundleLocation( this.getBundle().getLocation() );
+            config = configurationManager.createConfiguration( pid, getBundle().getLocation() );
         }
-        else if ( !config.getBundleLocation().equals( this.getBundle().getLocation() ) )
+        else
         {
-            // CM 1.4 / 104.13.2.3
-            this.checkPermission( config.getBundleLocation() );
+            if ( config.getBundleLocation() == null )
+            {
+                configurationManager.log( LogService.LOG_DEBUG, "Binding configuration {0} (isNew: {1}) to bundle {2}",
+                    new Object[]
+                        { config.getPid(), Boolean.valueOf( config.isNew() ), this.getBundle().getLocation() } );
+
+                config.setStaticBundleLocation( this.getBundle().getLocation() );
+            }
+            else
+            {
+                // CM 1.4 / 104.13.2.3
+                this.checkPermission( config.getBundleLocation() );
+            }
         }
 
         return this.wrap( config );
@@ -131,13 +140,17 @@
             { pid, location } );
 
         // CM 1.4 / 104.13.2.3
-        this.checkPermission( location );
+        this.checkPermission( ( location == null ) ? "*" : location );
 
-        ConfigurationImpl config = configurationManager.getConfiguration( pid, location );
-        if ( config.getBundleLocation() != null )
+        ConfigurationImpl config = configurationManager.getConfiguration( pid );
+        if ( config == null )
         {
-            // CM 1.4 / 104.13.2.3
-            this.checkPermission( config.getBundleLocation() );
+            config = configurationManager.createConfiguration( pid, location );
+        }
+        else
+        {
+            final String configLocation = config.getBundleLocation();
+            this.checkPermission( ( configLocation == null ) ? "*" : configLocation );
         }
 
         return this.wrap( config );
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 3794599..1e74bdd 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
@@ -416,20 +416,23 @@
     }
 
 
-    ConfigurationImpl createFactoryConfiguration( ConfigurationAdminImpl configurationAdmin, String factoryPid )
-        throws IOException
-    {
-        return createFactoryConfiguration( factoryPid, configurationAdmin.getBundle().getLocation() );
-    }
-
-
     ConfigurationImpl createFactoryConfiguration( String factoryPid, String location ) throws IOException
     {
         return createConfiguration( createPid( factoryPid ), factoryPid, location );
     }
 
 
-    ConfigurationImpl getExistingConfiguration( String pid ) throws IOException
+    /**
+     * Returns the {@link ConfigurationImpl} with the given PID if
+     * available in the internal cache or from any persistence manager.
+     * Otherwise <code>null</code> is returned.
+     *
+     * @param pid The PID for which to return the configuration
+     * @return The configuration or <code>null</code> if non exists
+     * @throws IOException If an error occurrs reading from a persistence
+     *      manager.
+     */
+    ConfigurationImpl getConfiguration( String pid ) throws IOException
     {
         ConfigurationImpl config = getCachedConfiguration( pid );
         if ( config != null )
@@ -457,10 +460,26 @@
     }
 
 
-    ConfigurationImpl getConfiguration( String pid, String bundleLocation ) throws IOException
+    /**
+     * Creates a regular (non-factory) configuration for the given PID
+     * setting the bundle location accordingly.
+     * <p>
+     * This method assumes the configuration to not exist yet and will
+     * create it without further checking.
+     *
+     * @param pid The PID of the new configuration
+     * @param bundleLocation The location to set on the new configuration.
+     *      This may be <code>null</code> to not bind the configuration
+     *      yet.
+     * @return The new configuration persisted in the first persistence
+     *      manager.
+     * @throws IOException If an error occurrs writing the configuration
+     *      to the persistence.
+     */
+    ConfigurationImpl createConfiguration( String pid, String bundleLocation ) throws IOException
     {
         // check for existing (cached or persistent) configuration
-        ConfigurationImpl config = getExistingConfiguration( pid );
+        ConfigurationImpl config = getConfiguration( pid );
         if ( config != null )
         {
             return config;
@@ -1318,7 +1337,7 @@
             long lastModificationTime = -1;
             try
             {
-                config = getExistingConfiguration( pid );
+                config = getConfiguration( pid );
                 if ( config != null )
                 {
                     synchronized ( config )
@@ -1468,7 +1487,7 @@
                         ConfigurationImpl cfg;
                         try
                         {
-                            cfg = getExistingConfiguration( pid );
+                            cfg = getConfiguration( pid );
                         }
                         catch ( IOException ioe )
                         {