Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Tighten rules around profile naming #34062

Closed
onobc opened this issue Feb 4, 2023 · 7 comments · May be fixed by #43176
Closed

Tighten rules around profile naming #34062

onobc opened this issue Feb 4, 2023 · 7 comments · May be fixed by #43176
Labels
status: superseded An issue that has been superseded by another

Comments

@onobc
Copy link
Contributor

onobc commented Feb 4, 2023

If you set spring.profiles.active to a value that contains a * the app will fail to start.

Steps to reproduce

Spring Boot 3.0.2 simplest application from start.spring.io:

@SpringBootApplication
public class WackyProfileApplication {
	public static void main(String[] args) {
		SpringApplication.run(WackyProfileApplication.class, args);
	}
}

Start it w/ spring.profiles.active set to foo*bar via any normal means such as:

  • sysprop
  • envvar
  • property in application.yml
  • profile property in maven/gradle plugin

App fails to start and reports the following in the log:

com.example.wackyprofile.WackyprofileApplication
23:14:37.244 [main] ERROR org.springframework.boot.SpringApplication - Application run failed
java.lang.IllegalStateException: Location 'file:./application-foo*bar.yaml' must end with '*/'
	at org.springframework.util.Assert.state(Assert.java:97)
	at org.springframework.boot.context.config.LocationResourceLoader.validatePattern(LocationResourceLoader.java:134)
	at org.springframework.boot.context.config.LocationResourceLoader.getResources(LocationResourceLoader.java:95)
	at org.springframework.boot.context.config.StandardConfigDataLocationResolver.resolvePattern(StandardConfigDataLocationResolver.java:313)
	at org.springframework.boot.context.config.StandardConfigDataLocationResolver.resolve(StandardConfigDataLocationResolver.java:299)
	at org.springframework.boot.context.config.StandardConfigDataLocationResolver.resolve(StandardConfigDataLocationResolver.java:251)
	at org.springframework.boot.context.config.StandardConfigDataLocationResolver.resolveProfileSpecific(StandardConfigDataLocationResolver.java:150)
	at org.springframework.boot.context.config.ConfigDataLocationResolvers.lambda$resolve$2(ConfigDataLocationResolvers.java:107)
	at org.springframework.boot.context.config.ConfigDataLocationResolvers.resolve(ConfigDataLocationResolvers.java:113)
	at org.springframework.boot.context.config.ConfigDataLocationResolvers.resolve(ConfigDataLocationResolvers.java:106)
	at org.springframework.boot.context.config.ConfigDataLocationResolvers.resolve(ConfigDataLocationResolvers.java:94)
	at org.springframework.boot.context.config.ConfigDataImporter.resolve(ConfigDataImporter.java:105)
	at org.springframework.boot.context.config.ConfigDataImporter.resolve(ConfigDataImporter.java:97)
	at org.springframework.boot.context.config.ConfigDataImporter.resolveAndLoad(ConfigDataImporter.java:85)
	at org.springframework.boot.context.config.ConfigDataEnvironmentContributors.withProcessedImports(ConfigDataEnvironmentContributors.java:115)
	at org.springframework.boot.context.config.ConfigDataEnvironment.processWithProfiles(ConfigDataEnvironment.java:313)
	at org.springframework.boot.context.config.ConfigDataEnvironment.processAndApply(ConfigDataEnvironment.java:234)
	at org.springframework.boot.context.config.ConfigDataEnvironmentPostProcessor.postProcessEnvironment(ConfigDataEnvironmentPostProcessor.java:96)
	at org.springframework.boot.context.config.ConfigDataEnvironmentPostProcessor.postProcessEnvironment(ConfigDataEnvironmentPostProcessor.java:89)
	at org.springframework.boot.env.EnvironmentPostProcessorApplicationListener.onApplicationEnvironmentPreparedEvent(EnvironmentPostProcessorApplicationListener.java:109)

Cause

The * qualifies it to be processed as a pattern in StandardConfigDataLocationResolver#resolve(StandardConfigDataReference)
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Feb 4, 2023
@onobc
Copy link
Contributor Author

onobc commented Feb 4, 2023

This is an edge-casey situation and arguably a terrible choice to include a wildcard in your profile name. However, there is no place in the Spring Boot nor Spring Framework docs (that I could find) that guide the user to the rules for a profile name. Maybe just adding a small section to the docs would suffice.

@onobc
Copy link
Contributor Author

onobc commented Feb 4, 2023

Another interesting point, there is a validateProfile in Environment whose only requirement is that the profile has text and does not begin w a !. Different consumers of the profile probably have different validation requirements than others.

@philwebb
Copy link
Member

philwebb commented Feb 6, 2023

I think we should tighten the rules for Spring Boot applications. The application-<profile> file is one example of something that will break but I suspect there are others. For example , will probably break the list parsing and !, &, | will break Profiles.of.

@philwebb philwebb added the for: team-meeting An issue we'd like to discuss as a team to make progress label Feb 6, 2023
@wilkinsona
Copy link
Member

We've had an issue related to , in the past: #19537

@philwebb philwebb added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged for: team-meeting An issue we'd like to discuss as a team to make progress labels Feb 15, 2023
@philwebb philwebb added this to the 3.1.x milestone Feb 15, 2023
@philwebb philwebb changed the title Asterisk in profile name prevents app from starting Tighten rules around profile naming Feb 15, 2023
@wilkinsona wilkinsona modified the milestones: 3.1.x, 3.x Apr 17, 2023
@bbulgarelli
Copy link
Contributor

I started working on this issue and I added the following code in the beginning of SpringBootContextLoader.setActiveProfiles:

private void setActiveProfiles(ConfigurableEnvironment environment, String[] profiles, boolean applicationEnvironment){
    ***
    for (String profile : profiles) {
        if (containsProhibitedCharacters(profile)) {
            throw new IllegalArgumentException("Invalid profile: '" + profile + "'. Profile names can't contain '*', '&', '!' or '|'.");
        }
    }
    ***
}

And I also created the following method:

private boolean containsProhibitedCharacters(String str){
	String regex = ".*[*&!|].*";
	return str.matches(regex);
}

I didn't include , in the regex expression, because it would break the following unit test that already exists:

@Test
void activeProfileWithComma() {
	assertThat(getActiveProfiles(ActiveProfileWithComma.class)).containsExactly("profile1,2");
}

I would like to know if the changes I made solve the issue correctly. If they do, what should I do about the comma problem?

P.s.: I also made the unit tests that verify the changes I made.

@philwebb
Copy link
Member

philwebb commented Jul 6, 2023

Thanks for looking at this @bbulgarelli. Looking at commit 7ab2bca it seems like we added the profile test when fixing #19537. I think we added the comma test for completeness, but I think it's fine to remove it. A better exception would have actually helped the reporter of #19537.

Feel free to submit a pull-request if you have something you'd like us to review.

@bbulgarelli bbulgarelli mentioned this issue Jul 6, 2023
Closed
@YangSiJun528 YangSiJun528 mentioned this issue Nov 15, 2024
Open
@mhalbritter
Copy link
Contributor

Superseded by #43176.

@mhalbritter mhalbritter closed this as not planned Won't fix, can't repro, duplicate, stale Nov 20, 2024
@mhalbritter mhalbritter removed this from the 3.x milestone Nov 20, 2024
@mhalbritter mhalbritter added status: superseded An issue that has been superseded by another and removed type: enhancement A general enhancement labels Nov 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
status: superseded An issue that has been superseded by another
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Tighten rules around profile naming YangSiJun528/spring-boot
6 participants
@philwebb @wilkinsona @spring-projects-issues @onobc @bbulgarelli @mhalbritter