jkevan opened a new pull request, #563:
URL: https://github.com/apache/felix-dev/pull/563

   A follow-up on 
[FELIX-6812](https://issues.apache.org/jira/browse/FELIX-6812), and it is 
described in [this 
comment](https://issues.apache.org/jira/browse/FELIX-6812?focusedCommentId=18115641).
 `fileinstall-3.7.5` is not released yet, so the fix can still travel with the 
change it repairs. Tell me if you would rather see a new issue, and I will 
retitle this pull request.
   
   ## What happens
   
   [FELIX-6812](https://issues.apache.org/jira/browse/FELIX-6812) replaced 
`getConfiguration(pid, "?")` with a `listConfigurations` call, and it builds 
the filter by concatenating the pid. A pid is not a filter-safe value. 
`findExistingConfiguration` escapes its own filter value through 
`escapeFilterValue`, nine lines below in the same class, and the new call site 
does not.
   
   Two failures follow, and neither one is visible to the user.
   
   - A pid holding an asterisk builds a substring filter. `listConfigurations` 
then returns configurations the event was not about, and `ConfigInstaller` 
writes the first of them back to that other configuration's file.
   - A pid holding a parenthesis throws `InvalidSyntaxException`. The catch 
reports it as "Unable to save configuration", so the configuration is never 
written back to its file.
   
   ## What changed
   
   The pid is escaped through `escapeFilterValue`, which is the method the 
other call site already uses.
   
   `escapeFilterValue` left the backslash alone, although the OSGi filter 
grammar reserves that character. The escaping added for the pid is incomplete 
without the backslash case, so this pull request covers both. The backslash is 
escaped first, or the escapes added after it would be escaped a second time.
   
   The backslash defect predates FELIX-6812, and it affects 
`findExistingConfiguration` as well. Say the word and I will move that half to 
an issue of its own.
   
   The method now matches literals rather than patterns. `String.replaceAll` 
compiles a pattern on each call, and `escapeFilterValue` compiled four of them. 
[FELIX-6812](https://issues.apache.org/jira/browse/FELIX-6812) moved this 
method from `findExistingConfiguration`, called once per file install, to 
`doConfigurationEvent`, called for every configuration event.
   
   The empty result of `listConfigurations` now logs at debug level. 
`listConfigurations` returns only the configurations the caller may see, and 
`getConfiguration(pid, "?")` did not filter that way. A configuration bound to 
another bundle's location therefore stops being written back to its file once a 
SecurityManager runs. See Configuration Admin 104.13.3. Neither early return 
left a trace before.
   
   ## Tests
   
   `testTheConfigurationEventFilterEscapesAnAsteriskInThePid` and 
`testTheConfigurationEventFilterEscapesABackslashInThePid` raise `CM_UPDATED` 
for such a pid and assert the filter the handler builds. Each test fails when 
its escape is removed.
   
   `mvn test` on the `fileinstall` module passes on JDK 11, except 
`DirectoryWatcherTest.testInvalidTempDir`. That test also fails on an 
unmodified `master` on this machine, which is 
[FELIX-6837](https://issues.apache.org/jira/browse/FELIX-6837).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to