Placeholders are not resolved when a property in the Environment has a String[] value

[darkman97i]

Hi

Tested with:

• from version 2.4.0 to version 2.4.13
• Java OpenJDK 1.8.203
• tomcat-8.5.69
• executing war file into the tomcat

After upgrading from version 2.3.12.RELEASE to version 2.4.0 I have found this issue. I have also tested with the latest version 2.4.13 and also it happens. During the loading of the configuration properties file is not able to replace ${catalina.home} with the path of the tomcat.

I have read upgrading-from-spring-boot-23 and Spring-Boot-Config-Data-Migration-Guide but I have not been able to find the reason why it happens. Finally, I have also added in the application.properties file the parameter spring.config.use-legacy-processing=true and nothing was solved. I think has not relation with the issue.

Code sample

import org.springframework.boot.SpringApplication;
import org.springframework.boot.autoconfigure.SpringBootApplication;
import org.springframework.boot.autoconfigure.jms.JmsAutoConfiguration;
import org.springframework.boot.autoconfigure.solr.SolrAutoConfiguration;
import org.springframework.boot.builder.SpringApplicationBuilder;
import org.springframework.boot.web.servlet.support.SpringBootServletInitializer;
import org.springframework.cache.annotation.EnableCaching;
import org.springframework.context.annotation.ImportResource;

import java.util.Properties;

@EnableCaching
@SpringBootApplication(exclude = {JmsAutoConfiguration.class, SolrAutoConfiguration.class})
@ImportResource("${okm.authentication.config}")
public class MainAppConfig extends SpringBootServletInitializer {
    public static void main(String[] args) {
        SpringApplication.run(MainAppConfig.class, args);
    }

	public MainAppConfig() {
		super();
		setRegisterErrorPageFilter(false);
	}

    @Override
    protected SpringApplicationBuilder configure(SpringApplicationBuilder application) {
        return application.sources(MainAppConfig.class).properties(getProperties());
    }

    static Properties getProperties() {
        Properties props = new Properties();
        props.put("spring.config.location", new String[]{"classpath:/application.properties", "file:${catalina.home}/openkm.properties"});
        return props;
    }
}

When the line

props.put("spring.config.location", new String[]{"classpath:/application.properties", "file:${catalina.home}/openkm.properties"});

is replaced by file system path the application finds the file and startup as expected

props.put("spring.config.location", new String[]{"classpath:/application.properties", "file:/desarrollo/tomcat/tomcat-8.5.69-boot-2/openkm.properties"});

Take from here the startup log with the error startup-error.log

Thanks for your time
Josep

[mbhave]

@darkman97i Spring Boot 2.4.x has reached end of OSS support. Can you try with Spring Boot version 2.5.9 or 2.6.3 to see if the problem still exists?

From what I can tell by looking at the code, the property is bound using the binder which should take care of placeholder resolution.

[darkman97i]

I can confirm it happens with 2.5.9 I will try to debug the proposed class looking for the point of what it changes.

[darkman97i]

I have debug the code and I'm locked because I'm not totally sure in which step the ${catalina.home} should be replaced by the file sytem path ( I supposed is what happens in background? ). I attach some screenshots.

In the next screenshot the value of the location still contains ${catalina.home}, I have debug into the two resolvers are in the list of this.resolvers variable and they do not replace the ${catalina.home} with file system path ( not sure in what point of the code should be done ? I have compared with previous code in version 2.3.X and this has been radically changed)

In the next screenshot is trying to load the file, because not changed the value with file system path it raises the error.

Hope this will help in some manner.

[wilkinsona]

The problem's caused by the rather unusual presence of a String[] in a property source. That's set up here:

props.put("spring.config.location", new String[]{"classpath:/application.properties", "file:${catalina.home}/openkm.properties"});

The problem doesn't occur if a more conventional comma-separated string is used instead:

props.put("spring.config.location", "classpath:/application.properties,file:${catalina.home}/openkm.properties");

When a String[] is used, placeholder resolution is skipped because the property's value is a String[] and we're looking for a String:

spring-boot/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/PropertySourcesPlaceholdersResolver.java

Lines 54 to 60 in 8bc792f

	@Override
	public Object resolvePlaceholders(Object value) {
	if (value instanceof String) {
	return this.helper.replacePlaceholders((String) value, this::resolvePlaceholder);
	}
	return value;
	}

It would appear to be straightforward to look for a String[] and perhaps a Collection<String> here as well but I'm not sure that's the right thing to do. I'd like to get some input from @mbhave and @philwebb before deciding what, if anything, to do.

[darkman97i]

Thanks, @wilkinsona and @mbhave for your quick answer. Confirmed that replacing the String[] by a single String comma-separated it works.

For your information, I got the configuration with String[] from version 2.0.3.RELEASE to 2.3.12.RELEASE ).In my opinion, one way to add the parameters is better than two ways, for me comma-separated is comfortable and clear to be used. The only scenario I can imagine could have sense working with String[] will be when a single String contains in the middle some comma that should not be used for splitting ( rare case ).

[mbhave]

Looking at the code in 2.3.x, it looks like this worked specifically for this property because we didn't use the Binder for it. It wouldn't have worked for spring.profiles.active which was obtained using the Binder.

While adding support for String[] and Collection<String> looks straightforward, I'm not sure if we should given the Spring Framework's PropertySourcesPlaceholderResolver also checks if the value is a String before resolving placeholders.

[snicoll]

I think we should align to what Framework does.

[wilkinsona]

Alright, let's leave things as they are which already aligns with Framework.

[KiraResari]

I think I ran into this after running System.clearProperty("mail.local.address") as part of a test that checks if changes to Environment Variables are correctly updated. Here's how I run that test:

@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
class AppSettingsAltEnvrionmentTest {
	public static final String SERVER_ADDRESS_KEY = "mail.local.address";
	
	static final String TEST_ADDRESS = "http://test.address";

    @Autowired
    private AppSettings appSettings;
	
    @BeforeAll
    public static void setup() {
        System.setProperty(SERVER_ADDRESS_KEY, TEST_ADDRESS);
    }

    @Test
    void mailLocalAddressShouldHaveChangedValueIfEnvironemntVariableIsSet(){
        assertEquals(TEST_ADDRESS, appSettings.getLocalAddress());
    }
    
    @AfterAll
    public static void cleanup() {
    	System.clearProperty(SERVER_ADDRESS_KEY);
    }
}

In the AppSettings, the value is assigned like this:

    @Value("${mail.local.address}")
    private String localAddress;

It worked once, but all the tests (even completely unrelated ones in a different SpringBootContext) started failing with the error:

Could not resolve placeholder 'mail.local.address' in value "${mail.local.address}"