From 8521a353daca1303a4c60bff481e59fc54999547 Mon Sep 17 00:00:00 2001 From: Jefiya M J Date: Thu, 13 Aug 2026 09:35:16 +0530 Subject: [PATCH 1/2] fix(fileinstall): skip setConfig() and deleteConfig() when file path does not match registered source --- .../fileinstall/internal/ConfigInstaller.java | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/fileinstall/src/main/java/org/apache/felix/fileinstall/internal/ConfigInstaller.java b/fileinstall/src/main/java/org/apache/felix/fileinstall/internal/ConfigInstaller.java index cf0a4b539f..a766302d3a 100644 --- a/fileinstall/src/main/java/org/apache/felix/fileinstall/internal/ConfigInstaller.java +++ b/fileinstall/src/main/java/org/apache/felix/fileinstall/internal/ConfigInstaller.java @@ -388,6 +388,17 @@ boolean setConfig(final File f) throws Exception clearReadOnlyIfWritable(pid, config, f); Dictionary props = config.getProperties(); + + // Only update if this file is the registered source of the configuration, + // or if no source has been registered yet (new configuration). + // Skips duplicate files that share the same PID but originate from a different path. + if (props != null) { + String registeredFileName = (String) props.get(DirectoryWatcher.FILENAME); + if (registeredFileName != null && !registeredFileName.equals(toConfigKey(f))) { + return false; + } + } + Hashtable old = props != null ? new Hashtable(new DictionaryAsMap<>(props)) : null; if (old != null) { old.remove( DirectoryWatcher.FILENAME ); @@ -433,6 +444,17 @@ boolean deleteConfig(File f) throws Exception { String pid[] = parsePid(f.getName()); Configuration config = getConfiguration(toConfigKey(f), pid[0], pid[1]); + + // Only delete if this file is the registered source of the configuration. + // If the registered felix.fileinstall.filename does not match the file being deleted, skip deletion to protect the live config. + Dictionary props = config.getProperties(); + if (props != null) { + String registeredFileName = (String) props.get(DirectoryWatcher.FILENAME); + if (registeredFileName != null && !registeredFileName.equals(toConfigKey(f))) { + return false; + } + } + Util.log(context, Logger.LOG_INFO, "Deleting configuration {" + config.getPid() + "} from " + f.getAbsolutePath(), null); From 3a1be31469d5ade954878d14c7e07a70df06b4ba Mon Sep 17 00:00:00 2001 From: Jefiya M J Date: Thu, 13 Aug 2026 11:42:44 +0530 Subject: [PATCH 2/2] Prevent duplicate *.cfg files in subdirectories from corrupting live OSGi configurations --- .../internal/ConfigInstallerTest.java | 122 ++++++++++++++++++ .../resources/watched/backup/firstcfg.cfg | 19 +++ 2 files changed, 141 insertions(+) create mode 100644 fileinstall/src/test/resources/watched/backup/firstcfg.cfg diff --git a/fileinstall/src/test/java/org/apache/felix/fileinstall/internal/ConfigInstallerTest.java b/fileinstall/src/test/java/org/apache/felix/fileinstall/internal/ConfigInstallerTest.java index 35ef49f2b3..507d50e9b9 100644 --- a/fileinstall/src/test/java/org/apache/felix/fileinstall/internal/ConfigInstallerTest.java +++ b/fileinstall/src/test/java/org/apache/felix/fileinstall/internal/ConfigInstallerTest.java @@ -218,6 +218,7 @@ public void testDeleteConfig() throws Exception .andReturn(null); EasyMock.expect(mockConfigurationAdmin.getConfiguration("pid", "?" )) .andReturn(mockConfiguration); + EasyMock.expect(mockConfiguration.getProperties()).andReturn(null); EasyMock.replay(mockConfiguration, mockConfigurationAdmin, mockBundleContext, mockBundle); ConfigInstaller ci = new ConfigInstaller( mockBundleContext, mockConfigurationAdmin, new FileInstall() ); @@ -227,6 +228,127 @@ public void testDeleteConfig() throws Exception EasyMock.verify(mockConfiguration, mockConfigurationAdmin, mockBundleContext); } + // verify that deleteConfig() skips deletion when the file being deleted + // is a duplicate (different path) from the registered live configuration. + public void testDeleteConfigSkipsWhenPathDiffersFromRegisteredSource() throws Exception + { + File canonicalFile = new File( "src/test/resources/watched/firstcfg.cfg" ); + File backupFile = new File( "src/test/resources/watched/backup/firstcfg.cfg" ); + + Hashtable props = new Hashtable<>(); + props.put( DirectoryWatcher.FILENAME, canonicalFile.getAbsoluteFile().toURI().toString() ); + + EasyMock.expect(mockBundleContext.getBundle()).andReturn(mockBundle).anyTimes(); + EasyMock.expect(mockBundle.loadClass(ConfigurationAttribute.class.getName())).andReturn((Class)ConfigurationAttribute.class).anyTimes(); + EasyMock.expect(mockBundleContext.getProperty((String) EasyMock.anyObject())) + .andReturn(null) + .anyTimes(); + EasyMock.expect(mockConfigurationAdmin.listConfigurations((String) EasyMock.anyObject())) + .andReturn(null); + EasyMock.expect(mockConfigurationAdmin.getConfiguration("firstcfg", "?")) + .andReturn(mockConfiguration); + EasyMock.expect(mockConfiguration.getProperties()).andReturn(props); + EasyMock.replay(mockConfiguration, mockConfigurationAdmin, mockBundleContext, mockBundle); + + ConfigInstaller ci = new ConfigInstaller( mockBundleContext, mockConfigurationAdmin, new FileInstall() ); + + assertFalse( ci.deleteConfig( backupFile ) ); + + EasyMock.verify(mockConfiguration, mockConfigurationAdmin, mockBundleContext); + } + + // verify that deleteConfig() proceeds normally when the file being deleted + // is the registered source of the live configuration. + public void testDeleteConfigProceedsWhenPathMatchesRegisteredSource() throws Exception + { + File canonicalFile = new File( "src/test/resources/watched/firstcfg.cfg" ); + + Hashtable props = new Hashtable<>(); + props.put( DirectoryWatcher.FILENAME, canonicalFile.getAbsoluteFile().toURI().toString() ); + + mockConfiguration.delete(); + EasyMock.expect(mockConfiguration.getPid()).andReturn("firstcfg"); + EasyMock.expect(mockBundleContext.getBundle()).andReturn(mockBundle).anyTimes(); + EasyMock.expect(mockBundle.loadClass(ConfigurationAttribute.class.getName())).andReturn((Class)ConfigurationAttribute.class).anyTimes(); + EasyMock.expect(mockBundleContext.getProperty((String) EasyMock.anyObject())) + .andReturn(null) + .anyTimes(); + EasyMock.expect(mockConfigurationAdmin.listConfigurations((String) EasyMock.anyObject())) + .andReturn(null); + EasyMock.expect(mockConfigurationAdmin.getConfiguration("firstcfg", "?")) + .andReturn(mockConfiguration); + EasyMock.expect(mockConfiguration.getProperties()).andReturn(props); + EasyMock.replay(mockConfiguration, mockConfigurationAdmin, mockBundleContext, mockBundle); + + ConfigInstaller ci = new ConfigInstaller( mockBundleContext, mockConfigurationAdmin, new FileInstall() ); + + assertTrue( ci.deleteConfig( canonicalFile ) ); + + EasyMock.verify(mockConfiguration, mockConfigurationAdmin, mockBundleContext); + } + + // Verify that setConfig() skips the update when the file being set is a duplicate + // (different path) from the registered live configuration. + public void testSetConfigSkipsWhenPathDiffersFromRegisteredSource() throws Exception + { + File canonicalFile = new File( "src/test/resources/watched/firstcfg.cfg" ); + File backupFile = new File( "src/test/resources/watched/backup/firstcfg.cfg" ); + + Hashtable props = new Hashtable<>(); + props.put( DirectoryWatcher.FILENAME, canonicalFile.getAbsoluteFile().toURI().toString() ); + + EasyMock.expect(mockBundleContext.getBundle()).andReturn(mockBundle).anyTimes(); + EasyMock.expect(mockBundle.loadClass(ConfigurationAttribute.class.getName())).andReturn((Class)ConfigurationAttribute.class).anyTimes(); + EasyMock.expect(mockBundleContext.getProperty((String) EasyMock.anyObject())) + .andReturn(null) + .anyTimes(); + EasyMock.expect(mockConfigurationAdmin.listConfigurations((String) EasyMock.anyObject())) + .andReturn(null); + EasyMock.expect(mockConfigurationAdmin.getConfiguration("firstcfg", "?")) + .andReturn(mockConfiguration); + EasyMock.expect(mockConfiguration.getAttributes()).andReturn(Collections.emptySet()).anyTimes(); + EasyMock.expect(mockConfiguration.getProperties()).andReturn(props); + EasyMock.replay(mockConfiguration, mockConfigurationAdmin, mockBundleContext, mockBundle); + + ConfigInstaller ci = new ConfigInstaller( mockBundleContext, mockConfigurationAdmin, new FileInstall() ); + + assertFalse( ci.setConfig( backupFile ) ); + + EasyMock.verify(mockConfiguration, mockConfigurationAdmin, mockBundleContext); + } + + // Verify that setConfig() proceeds normally when the file being set is the + // registered source of the live configuration. + public void testSetConfigProceedsWhenPathMatchesRegisteredSource() throws Exception + { + File canonicalFile = new File( "src/test/resources/watched/firstcfg.cfg" ); + + Hashtable props = new Hashtable<>(); + props.put( DirectoryWatcher.FILENAME, canonicalFile.getAbsoluteFile().toURI().toString() ); + + EasyMock.expect(mockBundleContext.getBundle()).andReturn(mockBundle).anyTimes(); + EasyMock.expect(mockBundle.loadClass(ConfigurationAttribute.class.getName())).andReturn((Class)ConfigurationAttribute.class).anyTimes(); + EasyMock.expect(mockBundleContext.getProperty((String) EasyMock.anyObject())) + .andReturn(null) + .anyTimes(); + EasyMock.expect(mockConfigurationAdmin.listConfigurations((String) EasyMock.anyObject())) + .andReturn(null); + EasyMock.expect(mockConfigurationAdmin.getConfiguration("firstcfg", "?")) + .andReturn(mockConfiguration); + EasyMock.expect(mockConfiguration.getProperties()).andReturn(props).anyTimes(); + EasyMock.expect(mockConfiguration.getAttributes()).andReturn(Collections.emptySet()).times(2); + EasyMock.expect(mockConfiguration.getPid()).andReturn("firstcfg"); + EasyMock.expect(mockConfiguration.updateIfDifferent((Dictionary) EasyMock.anyObject())) + .andReturn(true); + EasyMock.replay(mockConfiguration, mockConfigurationAdmin, mockBundleContext, mockBundle); + + ConfigInstaller ci = new ConfigInstaller( mockBundleContext, mockConfigurationAdmin, new FileInstall() ); + + assertTrue( ci.setConfig( canonicalFile ) ); + + EasyMock.verify(mockConfiguration, mockConfigurationAdmin, mockBundleContext); + } + public void testCreateConfigAndObserveCMDeleted() throws Exception { File file = File.createTempFile("test", ".config"); diff --git a/fileinstall/src/test/resources/watched/backup/firstcfg.cfg b/fileinstall/src/test/resources/watched/backup/firstcfg.cfg new file mode 100644 index 0000000000..80fab5942d --- /dev/null +++ b/fileinstall/src/test/resources/watched/backup/firstcfg.cfg @@ -0,0 +1,19 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# +testkey=testvalue