[
https://issues.apache.org/jira/browse/FELIX-6862?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Kevan Jahanshahi updated FELIX-6862:
------------------------------------
Issue Type: Bug (was: Task)
> ConfigInstaller adopts and deletes configuration files that another
> ArtifactInstaller handles
> ---------------------------------------------------------------------------------------------
>
> Key: FELIX-6862
> URL: https://issues.apache.org/jira/browse/FELIX-6862
> Project: Felix
> Issue Type: Bug
> Components: File Install
> Affects Versions: fileinstall-3.7.4
> Reporter: Kevan Jahanshahi
> Priority: Major
>
> h2. Summary
> {{ConfigInstaller}} adopts every configuration that records
> {{felix.fileinstall.filename}}, whatever the format of the file. The
> {{CM_DELETED}} path then deletes the file that the resulting {{pidToFile}}
> entry names. An installer therefore deletes a configuration file that another
> {{ArtifactInstaller}} handles.
> h2. Where this comes from
> FELIX-5832 added a {{canHandle}} filter to the {{CM_UPDATED}} path in 2018,
> and the subject of that commit states the rule:
> bq. Only handle ConfigurationEvents for config objects managed by us
> Three sites read or write {{pidToFile}}, and FELIX-5832 covered one of them.
> The other two sites are still unfiltered on {{master}} today.
> * {{init()}} puts every configuration that records a file name into
> {{pidToFile}}, with no filter.
> * The {{CM_DELETED}} path removes the pid from {{pidToFile}} and deletes the
> file, with no filter.
> h2. Failure scenario
> A container runs a second {{ArtifactInstaller}} for another configuration
> format. Jahia ships one for {{.yml}} and {{.yaml}}, and that installer
> records {{felix.fileinstall.filename}} the same way any installer does.
> # The second installer creates a configuration from {{example.yml}} and
> records that file name.
> # The container restarts, and {{ConfigInstaller.init()}} adopts the pid,
> because no filter rejects the format.
> # Something deletes that configuration through Configuration Admin.
> # {{ConfigInstaller}} deletes {{example.yml}}, although it never handled that
> file.
> The defect needs a restart, because {{init()}} is the only unfiltered writer
> of {{pidToFile}}. The guarded {{CM_UPDATED}} path rejects the foreign format,
> so a configuration created while the container runs never enters the map.
> A guard in the second installer cannot prevent the deletion. That installer
> can decline to delete the file itself, and {{ConfigInstaller}} deletes the
> file anyway on the same event.
> h2. Proposed fix
> Apply the filter FELIX-5832 already wrote, at both remaining sites.
> {code:java}
> // in init()
> if (fileName != null && canHandle(new File(fileName))) {
> pidToFile.put(config.getPid(), fileName);
> }
> // in the CM_DELETED path
> if (file != null && file.isFile() && canHandle(file)) {
> {code}
> The filter at the deletion site matters on its own. Deleting the file is the
> act that loses data, and a filter at each writer makes the invariant depend
> on every future writer repeating it.
> {{init()}} passes {{new File(fileName)}} rather than
> {{fromConfigKey(fileName)}}. {{canHandle}} reads the file name only, and
> {{fromConfigKey}} calls {{URI.create}}, which throws on a value that is not a
> URI. The catch around that loop would then leave {{pidToFile}} half-built.
> Behaviour for {{.cfg}} and {{.config}} files is unchanged.
> h2. Patch
> A pull request follows. It carries two tests, and the first fails when both
> filters are removed.
> * {{testCmDeletedKeepsAFileOfAnotherInstallersFormat}} asserts that a
> {{.yml}} file survives {{CM_DELETED}}.
> * {{testCmDeletedStillRemovesAFileOfItsOwnFormat}} asserts that a {{.cfg}}
> file is still deleted.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
