spring-framework icon indicating copy to clipboard operation
spring-framework copied to clipboard

ConfigurationClassParser won't add PropertySources of the same name correctly

Open djechelon opened this issue 3 years ago • 0 comments

Thanks to the team for taking the time.

Affects: 5.3.18 and main branch

Affected code

https://github.com/spring-projects/spring-framework/blob/28ca0dd64209a003dcd577212ad06624dfbc2684/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java#L482-L502

Discussion

The addPropertySource method is affected by a bug IMO where the effect of the if statement is reverted by the fact that PropertySources are hashed using their name property.

When code reaches the affected if-statement, it means that a PropertySource of name foo is already registered in the Spring context, and that PropertySource is assigned to the existing variable. The idea is to replace (L499) the old property source with name foo with the CompositePropertySource containing both.

The problem is, given that

  • CompositePropertySource holds a Set of PropertySources
  • PropertySources are hashed by their getName(), which is not extended currently
  • Both PropertySources to compose have same name

In the end, the second PropertySource won't be added and CompositePropertySource will hold only one PropertySource of name foo

Affected case

We found a problem when multiple teams work on different Java libraries, each declaring some property sources from classpath files.

In the end, we discovered that if two libraries hold the same env.yaml in different packages, but the associated PropertySource name is computed by the file name (not fully qualified path), two libraries define a PropertySource named env.yaml and clash.

We expect this to happen even if teams define any kind of MapPropertySource with the same non-unique name. We have implemented a workaround by using the fully qualified resource URL.

Suggested solution

Have CompositePropertySource use List than Set?

Test case

Not yet available

djechelon avatar Jul 29 '22 09:07 djechelon