Skip to content

PropertySourcesPlaceholderConfigurer placeholder resolution fails in several scenarios #34861

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
sbrannen opened this issue May 7, 2025 · 1 comment
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: bug A general bug
Milestone

Comments

@sbrannen
Copy link
Member

sbrannen commented May 7, 2025

Overview

In summary, the implementation of the placeholder resolution algorithm in PropertySourcesPlaceholderConfigurer fails in several scenarios, and the root cause for this category of failures has existed since PropertySourcesPlaceholderConfigurer was introduced in Spring Framework 3.1.

if (this.environment != null) {
this.propertySources.addLast(
new PropertySource<Environment>(ENVIRONMENT_PROPERTIES_PROPERTY_SOURCE_NAME, this.environment) {
@Override
public String getProperty(String key) {
return this.source.getProperty(key);
}
}
);
}

In the original code above, we see that a PropertySource is implemented as an anonymous inner class which delegates to Environment#getProperty, which in turn delegates to an internal PropertySourcesPropertyResolver, which in turn performs placeholder resolution.

And... that anonymous PropertySource is added to the MutablePropertySources managed by PropertySourcesPlaceholderConfigurer, which wraps those MutablePropertySources in its own PropertySourcesPropertyResolver, which in turn performs placeholder resolution.

Thus, we effectively have always had a top-level PropertySourcesPropertyResolver that indirectly delegates to a nested PropertySourcesPropertyResolver, which results in double placeholder parsing and resolution.

And that is what leads to a whole category of bugs.

In #27947, I realized that we were lacking proper support for ignoreUnresolvablePlaceholders in the nested PropertySourcesPropertyResolver, and due to #34315 and #34326 it became apparent that we are currently (as of Spring Framework 6.2.6) lacking support for the configured escapeCharacter in the nested PropertySourcesPropertyResolver as well.

Analysis

My research into #34315 and #34326 led me to realize that the reported bug for escaped placeholders being evaluated despite the escaping was in fact due to the same underlying flaw in the core logic of PropertySourcesPlaceholderConfigurer.

Namely, we should never have indirectly used or directly created a nested PropertySourcesPropertyResolver.

Instead, properties from property sources from the Environment should be accessed directly without duplicate/nested placeholder resolution, since the top-level PropertySourcesPropertyResolver already handles placeholder resolution.

Related Issues

@sbrannen sbrannen added this to the 6.2.7 milestone May 7, 2025
@sbrannen sbrannen self-assigned this May 7, 2025
@sbrannen sbrannen added type: bug A general bug in: core Issues in core modules (aop, beans, core, context, expression) labels May 7, 2025
sbrannen pushed a commit that referenced this issue May 9, 2025
sbrannen added a commit to sbrannen/spring-framework that referenced this issue May 10, 2025
Currently, the placeholder resolution algorithm in
PropertySourcesPlaceholderConfigurer fails in several scenarios, and
the root cause for this category of failures has actually existed since
PropertySourcesPlaceholderConfigurer was introduced in Spring Framework
3.1.

Specifically, PropertySourcesPlaceholderConfigurer creates its own
PropertySourcesPropertyResolver that indirectly delegates to another
"nested" PropertySourcesPropertyResolver to interact with
PropertySources from the Environment, which results in double
placeholder parsing and resolution attempts, and that behavior leads to
a whole category of bugs.

For example, spring-projects#27947 was addressed in Spring Framework 5.3.16, and due
to spring-projects#34315 and spring-projects#34326 we have recently realized that additional bugs
exist with placeholder resolution: nested placeholder resolution can
fail when escape characters are used, and it is currently impossible
to disable the escape character support for nested resolution.

To address this category of bugs, we no longer indirectly use or
directly create a "nested" PropertySourcesPropertyResolver in
PropertySourcesPlaceholderConfigurer. Instead, properties from property
sources from the Environment are now accessed directly without
duplicate/nested placeholder resolution.

See spring-projectsgh-27947
See spring-projectsgh-34326
See spring-projectsgh-34862
Closes spring-projectsgh-34861
@sbrannen
Copy link
Member Author

Reopening to address build failures in Spring Boot.

Specifically, the switch to using a CompositePropertySource in PropertySourcesPlaceholderConfigurer resulted in exposure of the property source as an EnumerablePropertySource, and CompositePropertySource.getPropertyNames() throws an IllegalStateException for a non-EnumerablePropertySource, which is a breaking change for the behavior of PropertySourcesPlaceholderConfigurer.

@sbrannen sbrannen reopened this May 11, 2025
sbrannen added a commit that referenced this issue May 11, 2025
…ySource

Although it's unlikely that the implementation of getPropertySources()
in a ConfigurableEnvironment would be overridden to return a different
MutablePropertySources instance than the one that the
ConfigurableEnvironment typically acts on, it is in fact possible.

In light of that possibility, this commit refactors
ConfigurableEnvironmentPropertySource so that it always obtains a fresh
PropertySources reference.

See gh-34861
sbrannen added a commit that referenced this issue May 11, 2025
This commit overrides containsProperty() in
FallbackEnvironmentPropertySource for consistency with the
implementation of ConfigurableEnvironmentPropertySource.

See gh-34861
sbrannen added a commit that referenced this issue May 11, 2025
This new test serves as a "regression test" for behavior tested in
Spring Boot.

See gh-34861
sbrannen added a commit to sbrannen/spring-framework that referenced this issue May 13, 2025
Spring Framework 6.2 introduced support for an escape character for
property placeholders (by default '\'). However, as of Spring Framework
6.2.6, there was no way to either escape the escape character or disable
escape character support.

For example, given a `username` property configured with the value of
`Jane.Smith` and a `DOMAIN\${username}` configuration string, property
placeholder replacement used to result in `DOMAIN\Jane.Smith` prior to
6.2 but now results in `DOMAIN${username}`. Similarly, an attempt to
escape the escape character via `DOMAIN\\${username}` results in
`DOMAIN\${username}`.

In theory, one should be able to disable use of an escape character
altogether, and that is currently possible by invoking
setEscapeCharacter(null) on AbstractPropertyResolver and
PlaceholderConfigurerSupport (the superclass of
PropertySourcesPlaceholderConfigurer).

However, in reality, there are two hurdles.

- As of 6.2.6, an invocation of setEscapeCharacter(null) on a
  PropertySourcesPlaceholderConfigurer applied to its internal
  top-level PropertySourcesPropertyResolver but not to any nested
  PropertySourcesPropertyResolver, which means that the `null` escape
  character could not be effectively applied.

- Users may not have an easy way to explicitly set the escape character
  to `null` for a PropertyResolver or
  PropertySourcesPlaceholderConfigurer. For example, Spring Boot
  auto-configures a PropertySourcesPlaceholderConfigurer with the
  default escape character enabled.

This first issue above has recently been addressed by spring-projectsgh-34861.

This commit therefore addresses the second issue as follows.

- To allow developers to easily revert to the pre-6.2 behavior without
  changes to code or configuration strings, this commit introduces a
  `spring.placeholder.escapeCharacter.default` property for use with
  SpringProperties which globally sets the default escape character that
  is automatically configured in AbstractPropertyResolver and
  PlaceholderConfigurerSupport.

- Setting the property to an empty string sets the default escape
  character to `null`, effectively disabling the default support for
  escape characters.

    spring.placeholder.escapeCharacter.default =

- Setting the property to any other character sets the default escape
  character to that specific character.

    spring.placeholder.escapeCharacter.default = ~

- Setting the property to a string containing more than one character
  results in an exception.

- Developers are still able to configure an explicit escape character
  in AbstractPropertyResolver and PlaceholderConfigurerSupport if they
  choose to do so.

- Third-party components that wish to rely on the same feature can
  invoke AbstractPropertyResolver.getDefaultEscapeCharacter() to obtain
  the globally configured default escape character.

See spring-projectsgh-9628
See spring-projectsgh-34315
See spring-projectsgh-34861
Closes spring-projectsgh-34865
sbrannen added a commit that referenced this issue May 13, 2025
Spring Framework 6.2 introduced support for an escape character for
property placeholders (by default '\'). However, as of Spring Framework
6.2.6, there was no way to either escape the escape character or disable
escape character support.

For example, given a `username` property configured with the value of
`Jane.Smith` and a `DOMAIN\${username}` configuration string, property
placeholder replacement used to result in `DOMAIN\Jane.Smith` prior to
6.2 but now results in `DOMAIN${username}`. Similarly, an attempt to
escape the escape character via `DOMAIN\\${username}` results in
`DOMAIN\${username}`.

In theory, one should be able to disable use of an escape character
altogether, and that is currently possible by invoking
setEscapeCharacter(null) on AbstractPropertyResolver and
PlaceholderConfigurerSupport (the superclass of
PropertySourcesPlaceholderConfigurer).

However, in reality, there are two hurdles.

- As of 6.2.6, an invocation of setEscapeCharacter(null) on a
  PropertySourcesPlaceholderConfigurer applied to its internal
  top-level PropertySourcesPropertyResolver but not to any nested
  PropertySourcesPropertyResolver, which means that the `null` escape
  character could not be effectively applied.

- Users may not have an easy way to explicitly set the escape character
  to `null` for a PropertyResolver or
  PropertySourcesPlaceholderConfigurer. For example, Spring Boot
  auto-configures a PropertySourcesPlaceholderConfigurer with the
  default escape character enabled.

This first issue above has recently been addressed by gh-34861.

This commit therefore addresses the second issue as follows.

- To allow developers to easily revert to the pre-6.2 behavior without
  changes to code or configuration strings, this commit introduces a
  `spring.placeholder.escapeCharacter.default` property for use with
  SpringProperties which globally sets the default escape character that
  is automatically configured in AbstractPropertyResolver and
  PlaceholderConfigurerSupport.

- Setting the property to an empty string sets the default escape
  character to `null`, effectively disabling the default support for
  escape characters.

    spring.placeholder.escapeCharacter.default =

- Setting the property to any other character sets the default escape
  character to that specific character.

    spring.placeholder.escapeCharacter.default = ~

- Setting the property to a string containing more than one character
  results in an exception.

- Developers are still able to configure an explicit escape character
  in AbstractPropertyResolver and PlaceholderConfigurerSupport if they
  choose to do so.

- Third-party components that wish to rely on the same feature can
  invoke AbstractPropertyResolver.getDefaultEscapeCharacter() to obtain
  the globally configured default escape character.

See gh-9628
See gh-34315
See gh-34861
Closes gh-34865
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: bug A general bug
Projects
None yet
Development

No branches or pull requests

1 participant