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

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

Comment From: 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.

Comment From: 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.

Comment From: 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.

Comment From: 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:

https://github.com/spring-projects/spring-boot/blob/8bc792fd00fd25da3d0bfa516391e432921cb8d5/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/PropertySourcesPlaceholdersResolver.java#L54-L60

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.

Comment From: 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 ).

Comment From: 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.

Comment From: snicoll

I think we should align to what Framework does.

Comment From: wilkinsona

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

Comment From: 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}"

Comment From: wilkinsona

@acarlstein It's not clear from what you've described why you think that this issue is related to your problem. Unless something is adding String[] values to the environment, this issue isn't related.

Comment From: acarlstein

@acarlstein It's not clear from what you've described why you think that this issue is related to your problem. Unless something is adding String[] values to the environment, this issue isn't related.

sorry, wrong issue.