Skip to content

Commit 3295289

Browse files
committed
Fix placeholder resolution in PropertySourcesPlaceholderConfigurer
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, #27947 was addressed in Spring Framework 5.3.16, and due to #34315 and #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 gh-27947 See gh-34326 See gh-34862 Closes gh-34861
1 parent 457e876 commit 3295289

File tree

2 files changed

+165
-23
lines changed

2 files changed

+165
-23
lines changed

spring-context/src/main/java/org/springframework/context/support/PropertySourcesPlaceholderConfigurer.java

+18-22
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,12 @@
2424
import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
2525
import org.springframework.beans.factory.config.PlaceholderConfigurerSupport;
2626
import org.springframework.context.EnvironmentAware;
27+
import org.springframework.core.env.CompositePropertySource;
2728
import org.springframework.core.env.ConfigurableEnvironment;
2829
import org.springframework.core.env.ConfigurablePropertyResolver;
2930
import org.springframework.core.env.Environment;
3031
import org.springframework.core.env.MutablePropertySources;
3132
import org.springframework.core.env.PropertiesPropertySource;
32-
import org.springframework.core.env.PropertyResolver;
3333
import org.springframework.core.env.PropertySource;
3434
import org.springframework.core.env.PropertySources;
3535
import org.springframework.core.env.PropertySourcesPropertyResolver;
@@ -133,28 +133,24 @@ public void postProcessBeanFactory(ConfigurableListableBeanFactory beanFactory)
133133
if (this.propertySources == null) {
134134
this.propertySources = new MutablePropertySources();
135135
if (this.environment != null) {
136-
PropertyResolver propertyResolver = this.environment;
137-
// If the ignoreUnresolvablePlaceholders flag is set to true, we have to create a
138-
// local PropertyResolver to enforce that setting, since the Environment is most
139-
// likely not configured with ignoreUnresolvablePlaceholders set to true.
140-
// See https://github.com/spring-projects/spring-framework/issues/27947
141-
if (this.ignoreUnresolvablePlaceholders &&
142-
(this.environment instanceof ConfigurableEnvironment configurableEnvironment)) {
143-
PropertySourcesPropertyResolver resolver =
144-
new PropertySourcesPropertyResolver(configurableEnvironment.getPropertySources());
145-
resolver.setIgnoreUnresolvableNestedPlaceholders(true);
146-
propertyResolver = resolver;
136+
PropertySource<?> environmentPropertySource;
137+
if (this.environment instanceof ConfigurableEnvironment configurableEnvironment) {
138+
environmentPropertySource = new CompositePropertySource(ENVIRONMENT_PROPERTIES_PROPERTY_SOURCE_NAME,
139+
configurableEnvironment.getPropertySources());
147140
}
148-
PropertyResolver propertyResolverToUse = propertyResolver;
149-
this.propertySources.addLast(
150-
new PropertySource<>(ENVIRONMENT_PROPERTIES_PROPERTY_SOURCE_NAME, this.environment) {
151-
@Override
152-
@Nullable
153-
public String getProperty(String key) {
154-
return propertyResolverToUse.getProperty(key);
155-
}
156-
}
157-
);
141+
else {
142+
// Fallback code path that should never apply in a regular scenario, since the
143+
// Environment in the ApplicationContext should always be a ConfigurableEnvironment.
144+
environmentPropertySource =
145+
new PropertySource<>(ENVIRONMENT_PROPERTIES_PROPERTY_SOURCE_NAME, this.environment) {
146+
@Override
147+
@Nullable
148+
public Object getProperty(String key) {
149+
return super.source.getProperty(key);
150+
}
151+
};
152+
}
153+
this.propertySources.addLast(environmentPropertySource);
158154
}
159155
try {
160156
PropertySource<?> localPropertySource =

spring-context/src/test/java/org/springframework/context/support/PropertySourcesPlaceholderConfigurerTests.java

+147-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2024 the original author or authors.
2+
* Copyright 2002-2025 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -19,11 +19,13 @@
1919
import java.util.Optional;
2020
import java.util.Properties;
2121

22+
import org.junit.jupiter.api.Nested;
2223
import org.junit.jupiter.api.Test;
2324

2425
import org.springframework.beans.factory.BeanCreationException;
2526
import org.springframework.beans.factory.BeanDefinitionStoreException;
2627
import org.springframework.beans.factory.annotation.Value;
28+
import org.springframework.beans.factory.config.BeanDefinition;
2729
import org.springframework.beans.factory.support.DefaultListableBeanFactory;
2830
import org.springframework.beans.testfixture.beans.TestBean;
2931
import org.springframework.context.annotation.AnnotationConfigApplicationContext;
@@ -432,6 +434,150 @@ void optionalPropertyWithoutValue() {
432434
}
433435

434436

437+
/**
438+
* Tests that use the escape character (or disable it) with nested placeholder
439+
* resolution.
440+
*/
441+
@Nested
442+
class EscapedNestedPlaceholdersTests {
443+
444+
@Test // gh-34861
445+
void singleEscapeWithDefaultEscapeCharacter() {
446+
MockEnvironment env = new MockEnvironment()
447+
.withProperty("user.home", "admin")
448+
.withProperty("my.property", "\\DOMAIN\\${user.home}");
449+
450+
DefaultListableBeanFactory bf = createBeanFactory();
451+
PropertySourcesPlaceholderConfigurer ppc = new PropertySourcesPlaceholderConfigurer();
452+
ppc.setEnvironment(env);
453+
ppc.postProcessBeanFactory(bf);
454+
455+
// \DOMAIN\${user.home} resolves to \DOMAIN${user.home} instead of \DOMAIN\admin
456+
assertThat(bf.getBean(TestBean.class).getName()).isEqualTo("\\DOMAIN${user.home}");
457+
}
458+
459+
@Test // gh-34861
460+
void singleEscapeWithCustomEscapeCharacter() {
461+
MockEnvironment env = new MockEnvironment()
462+
.withProperty("user.home", "admin\\~${nested}")
463+
.withProperty("my.property", "DOMAIN\\${user.home}\\~${enigma}");
464+
465+
DefaultListableBeanFactory bf = createBeanFactory();
466+
PropertySourcesPlaceholderConfigurer ppc = new PropertySourcesPlaceholderConfigurer();
467+
ppc.setEnvironment(env);
468+
// Set custom escape character.
469+
ppc.setEscapeCharacter('~');
470+
ppc.postProcessBeanFactory(bf);
471+
472+
assertThat(bf.getBean(TestBean.class).getName()).isEqualTo("DOMAIN\\admin\\${nested}\\${enigma}");
473+
}
474+
475+
@Test // gh-34861
476+
void singleEscapeWithEscapeCharacterDisabled() {
477+
MockEnvironment env = new MockEnvironment()
478+
.withProperty("user.home", "admin\\")
479+
.withProperty("my.property", "\\DOMAIN\\${user.home}");
480+
481+
DefaultListableBeanFactory bf = createBeanFactory();
482+
PropertySourcesPlaceholderConfigurer ppc = new PropertySourcesPlaceholderConfigurer();
483+
ppc.setEnvironment(env);
484+
// Disable escape character.
485+
ppc.setEscapeCharacter(null);
486+
ppc.postProcessBeanFactory(bf);
487+
488+
// \DOMAIN\${user.home} resolves to \DOMAIN\admin
489+
assertThat(bf.getBean(TestBean.class).getName()).isEqualTo("\\DOMAIN\\admin\\");
490+
}
491+
492+
@Test // gh-34861
493+
void tripleEscapeWithDefaultEscapeCharacter() {
494+
MockEnvironment env = new MockEnvironment()
495+
.withProperty("user.home", "admin\\\\\\")
496+
.withProperty("my.property", "DOMAIN\\\\\\${user.home}#${user.home}");
497+
498+
DefaultListableBeanFactory bf = createBeanFactory();
499+
PropertySourcesPlaceholderConfigurer ppc = new PropertySourcesPlaceholderConfigurer();
500+
ppc.setEnvironment(env);
501+
ppc.postProcessBeanFactory(bf);
502+
503+
assertThat(bf.getBean(TestBean.class).getName()).isEqualTo("DOMAIN\\\\${user.home}#admin\\\\\\");
504+
}
505+
506+
@Test // gh-34861
507+
void tripleEscapeWithCustomEscapeCharacter() {
508+
MockEnvironment env = new MockEnvironment()
509+
.withProperty("user.home", "admin\\~${enigma}")
510+
.withProperty("my.property", "DOMAIN~~~${user.home}#${user.home}");
511+
512+
DefaultListableBeanFactory bf = createBeanFactory();
513+
PropertySourcesPlaceholderConfigurer ppc = new PropertySourcesPlaceholderConfigurer();
514+
ppc.setEnvironment(env);
515+
// Set custom escape character.
516+
ppc.setEscapeCharacter('~');
517+
ppc.postProcessBeanFactory(bf);
518+
519+
assertThat(bf.getBean(TestBean.class).getName()).isEqualTo("DOMAIN~~${user.home}#admin\\${enigma}");
520+
}
521+
522+
@Test // gh-34861
523+
void singleEscapeWithDefaultEscapeCharacterAndIgnoreUnresolvablePlaceholders() {
524+
MockEnvironment env = new MockEnvironment()
525+
.withProperty("user.home", "${enigma}")
526+
.withProperty("my.property", "\\${DOMAIN}${user.home}");
527+
528+
DefaultListableBeanFactory bf = createBeanFactory();
529+
PropertySourcesPlaceholderConfigurer ppc = new PropertySourcesPlaceholderConfigurer();
530+
ppc.setEnvironment(env);
531+
ppc.setIgnoreUnresolvablePlaceholders(true);
532+
ppc.postProcessBeanFactory(bf);
533+
534+
assertThat(bf.getBean(TestBean.class).getName()).isEqualTo("${DOMAIN}${enigma}");
535+
}
536+
537+
@Test // gh-34861
538+
void singleEscapeWithCustomEscapeCharacterAndIgnoreUnresolvablePlaceholders() {
539+
MockEnvironment env = new MockEnvironment()
540+
.withProperty("user.home", "${enigma}")
541+
.withProperty("my.property", "~${DOMAIN}\\${user.home}");
542+
543+
DefaultListableBeanFactory bf = createBeanFactory();
544+
PropertySourcesPlaceholderConfigurer ppc = new PropertySourcesPlaceholderConfigurer();
545+
ppc.setEnvironment(env);
546+
// Set custom escape character.
547+
ppc.setEscapeCharacter('~');
548+
ppc.setIgnoreUnresolvablePlaceholders(true);
549+
ppc.postProcessBeanFactory(bf);
550+
551+
assertThat(bf.getBean(TestBean.class).getName()).isEqualTo("${DOMAIN}\\${enigma}");
552+
}
553+
554+
@Test // gh-34861
555+
void tripleEscapeWithDefaultEscapeCharacterAndIgnoreUnresolvablePlaceholders() {
556+
MockEnvironment env = new MockEnvironment()
557+
.withProperty("user.home", "${enigma}")
558+
.withProperty("my.property", "X:\\\\\\${DOMAIN}${user.home}");
559+
560+
DefaultListableBeanFactory bf = createBeanFactory();
561+
PropertySourcesPlaceholderConfigurer ppc = new PropertySourcesPlaceholderConfigurer();
562+
ppc.setEnvironment(env);
563+
ppc.setIgnoreUnresolvablePlaceholders(true);
564+
ppc.postProcessBeanFactory(bf);
565+
566+
assertThat(bf.getBean(TestBean.class).getName()).isEqualTo("X:\\\\${DOMAIN}${enigma}");
567+
}
568+
569+
private static DefaultListableBeanFactory createBeanFactory() {
570+
BeanDefinition beanDefinition = genericBeanDefinition(TestBean.class)
571+
.addPropertyValue("name", "${my.property}")
572+
.getBeanDefinition();
573+
DefaultListableBeanFactory bf = new DefaultListableBeanFactory();
574+
bf.registerBeanDefinition("testBean",beanDefinition);
575+
return bf;
576+
}
577+
578+
}
579+
580+
435581
private static class OptionalTestBean {
436582

437583
private Optional<String> name;

0 commit comments

Comments
 (0)