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.

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

Tighten rules around profile naming #43176

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
* @author Stephane Nicoll
* @author Scott Frederick
* @author Madhura Bhave
* @author Sijun Yang
*/
class SpringBootContextLoaderTests {

Expand Down Expand Up @@ -127,11 +128,6 @@ void multipleActiveProfiles() {
assertThat(getActiveProfiles(MultipleActiveProfiles.class)).containsExactly("profile1", "profile2");
}

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

@Test // gh-28776
void testPropertyValuesShouldTakePrecedenceWhenInlinedPropertiesPresent() {
TestContext context = new ExposedTestContextManager(SimpleConfig.class).getExposedTestContext();
Expand Down Expand Up @@ -314,14 +310,8 @@ static class MultipleActiveProfiles {

}

@SpringBootTest(classes = Config.class)
@ActiveProfiles({ "profile1,2" })
static class ActiveProfileWithComma {

}

@SpringBootTest(properties = { "key=myValue" }, classes = Config.class)
@ActiveProfiles({ "profile1,2" })
@ActiveProfiles({ "profile1" })
static class ActiveProfileWithInlinedProperties {

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
* @author Madhura Bhave
* @author Phillip Webb
* @author Scott Frederick
* @author Sijun Yang
* @since 2.4.0
*/
public class StandardConfigDataLocationResolver
Expand Down Expand Up @@ -154,6 +155,7 @@ public List<StandardConfigDataResource> resolveProfileSpecific(ConfigDataLocatio
private Set<StandardConfigDataReference> getProfileSpecificReferences(ConfigDataLocationResolverContext context,
ConfigDataLocation[] configDataLocations, Profiles profiles) {
Set<StandardConfigDataReference> references = new LinkedHashSet<>();
validateProfiles(profiles);
for (String profile : profiles) {
for (ConfigDataLocation configDataLocation : configDataLocations) {
String resourceLocation = getResourceLocation(context, configDataLocation);
Expand All @@ -163,6 +165,26 @@ private Set<StandardConfigDataReference> getProfileSpecificReferences(ConfigData
return references;
}

private void validateProfiles(Profiles profiles) {
for (String profile : profiles) {
validateProfile(profile);
}
}

private void validateProfile(String profile) {
Assert.hasText(profile, "Profile must contain text");
Assert.state(!profile.startsWith("-") && !profile.startsWith("_"),
() -> String.format("Invalid profile '%s': must not start with '-' or '_'", profile));
Assert.state(!profile.endsWith("-") && !profile.endsWith("_"),
() -> String.format("Invalid profile '%s': must not end with '-' or '_'", profile));
profile.codePoints().forEach((codePoint) -> {
if (codePoint == '-' || codePoint == '_' || Character.isLetterOrDigit(codePoint)) {
return;
}
throw new IllegalStateException(String.format("Invalid profile '%s': must contain only letters or digits or '-' or '_'", profile));
});
}

private String getResourceLocation(ConfigDataLocationResolverContext context,
ConfigDataLocation configDataLocation) {
String resourceLocation = configDataLocation.getNonPrefixedValue(PREFIX);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@
* @author Moritz Halbritter
* @author Tadaya Tsuyukubo
* @author Yanming Zhou
* @author Sijun Yang
*/
@ExtendWith(OutputCaptureExtension.class)
class SpringApplicationTests {
Expand Down Expand Up @@ -252,13 +253,13 @@ void logsActiveProfilesWithoutProfileAndSingleDefault(CapturedOutput output) {
@Test
void logsActiveProfilesWithoutProfileAndMultipleDefaults(CapturedOutput output) {
MockEnvironment environment = new MockEnvironment();
environment.setDefaultProfiles("p0,p1", "default");
environment.setDefaultProfiles("p0", "default");
SpringApplication application = new SpringApplication(ExampleConfig.class);
application.setWebApplicationType(WebApplicationType.NONE);
application.setEnvironment(environment);
this.context = application.run();
assertThat(output)
.contains("No active profile set, falling back to 2 default profiles: \"p0,p1\", \"default\"");
.contains("No active profile set, falling back to 2 default profiles: \"p0\", \"default\"");
}

@Test
Expand All @@ -273,9 +274,9 @@ void logsActiveProfilesWithSingleProfile(CapturedOutput output) {
void logsActiveProfilesWithMultipleProfiles(CapturedOutput output) {
SpringApplication application = new SpringApplication(ExampleConfig.class);
application.setWebApplicationType(WebApplicationType.NONE);
application.setAdditionalProfiles("p1,p2", "p3");
application.setAdditionalProfiles("p1", "p2");
application.run();
assertThat(output).contains("The following 2 profiles are active: \"p1,p2\", \"p3\"");
assertThat(output).contains("The following 2 profiles are active: \"p1\", \"p2\"");
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
* @author Madhura Bhave
* @author Phillip Webb
* @author Moritz Halbritter
* @author Sijun Yang
*/
class StandardConfigDataLocationResolverTests {

Expand Down Expand Up @@ -254,8 +255,8 @@ void resolveWhenLocationUsesOptionalExtensionSyntaxResolves() throws Exception {
@Test
void resolveProfileSpecificReturnsProfileSpecificFiles() {
ConfigDataLocation location = ConfigDataLocation.of("classpath:/configdata/properties/");
Profiles profiles = mock(Profiles.class);
given(profiles.iterator()).willReturn(Collections.singletonList("dev").iterator());
this.environment.setActiveProfiles("dev");
Profiles profiles = new Profiles(this.environment, this.environmentBinder, Collections.emptyList());
List<StandardConfigDataResource> locations = this.resolver.resolveProfileSpecific(this.context, location,
profiles);
assertThat(locations).hasSize(1);
Expand Down Expand Up @@ -293,6 +294,84 @@ void resolveWhenOptionalAndExtensionIsUnknownShouldNotFail() {
assertThatNoException().isThrownBy(() -> this.resolver.resolve(this.context, location));
}

@Test
void resolveProfileSpecificWhenProfileIsValidShouldNotThrowException() {
ConfigDataLocation location = ConfigDataLocation.of("classpath:/configdata/properties/");
this.environment.setActiveProfiles("dev-test_123");
Profiles profiles = new Profiles(this.environment, this.environmentBinder, Collections.emptyList());
assertThatNoException()
.isThrownBy(() -> this.resolver.resolveProfileSpecific(this.context, location, profiles));
}

@Test
void resolveProfileSpecificWithNonAsciiCharactersShouldNotThrowException() {
ConfigDataLocation location = ConfigDataLocation.of("classpath:/configdata/properties/");
this.environment.setActiveProfiles("dev-테스트_123");
Profiles profiles = new Profiles(this.environment, this.environmentBinder, Collections.emptyList());
assertThatNoException()
.isThrownBy(() -> this.resolver.resolveProfileSpecific(this.context, location, profiles));
}

@Test
void resolveProfileSpecificWithAdditionalValidProfilesShouldNotThrowException() {
ConfigDataLocation location = ConfigDataLocation.of("classpath:/configdata/properties/");
this.environment.setActiveProfiles("dev-test");
Profiles profiles = new Profiles(this.environment, this.environmentBinder, List.of("prod-test", "stage-test"));
assertThatNoException()
.isThrownBy(() -> this.resolver.resolveProfileSpecific(this.context, location, profiles));
}

@Test
void resolveProfileSpecificWhenProfileStartsWithSymbolThrowsException() {
ConfigDataLocation location = ConfigDataLocation.of("classpath:/configdata/properties/");
this.environment.setActiveProfiles("-dev");
Profiles profiles = new Profiles(this.environment, this.environmentBinder, Collections.emptyList());
assertThatIllegalStateException()
.isThrownBy(() -> this.resolver.resolveProfileSpecific(this.context, location, profiles))
.withMessageStartingWith("Invalid profile '-dev': must not start with '-' or '_'");
}

@Test
void resolveProfileSpecificWhenProfileStartsWithUnderscoreThrowsException() {
ConfigDataLocation location = ConfigDataLocation.of("classpath:/configdata/properties/");
this.environment.setActiveProfiles("_dev");
Profiles profiles = new Profiles(this.environment, this.environmentBinder, Collections.emptyList());
assertThatIllegalStateException()
.isThrownBy(() -> this.resolver.resolveProfileSpecific(this.context, location, profiles))
.withMessageStartingWith("Invalid profile '_dev': must not start with '-' or '_'");
}

@Test
void resolveProfileSpecificWhenProfileEndsWithSymbolThrowsException() {
ConfigDataLocation location = ConfigDataLocation.of("classpath:/configdata/properties/");
this.environment.setActiveProfiles("dev-");
Profiles profiles = new Profiles(this.environment, this.environmentBinder, Collections.emptyList());
assertThatIllegalStateException()
.isThrownBy(() -> this.resolver.resolveProfileSpecific(this.context, location, profiles))
.withMessageStartingWith("Invalid profile 'dev-': must not end with '-' or '_'");
}

@Test
void resolveProfileSpecificWhenProfileEndsWithUnderscoreThrowsException() {
ConfigDataLocation location = ConfigDataLocation.of("classpath:/configdata/properties/");
this.environment.setActiveProfiles("dev_");
Profiles profiles = new Profiles(this.environment, this.environmentBinder, Collections.emptyList());
assertThatIllegalStateException()
.isThrownBy(() -> this.resolver.resolveProfileSpecific(this.context, location, profiles))
.withMessageStartingWith("Invalid profile 'dev_': must not end with '-' or '_'");
}

@Test
void resolveProfileSpecificWhenProfileContainsInvalidCharactersThrowsException() {
ConfigDataLocation location = ConfigDataLocation.of("classpath:/configdata/properties/");
this.environment.setActiveProfiles("dev*test");
Profiles profiles = new Profiles(this.environment, this.environmentBinder, Collections.emptyList());
assertThatIllegalStateException()
.isThrownBy(() -> this.resolver.resolveProfileSpecific(this.context, location, profiles))
.withMessageStartingWith(
"Invalid profile 'dev*test': must contain only letters or digits or '-' or '_'");
}

private String filePath(String... components) {
return "file [" + String.join(File.separator, components) + "]";
}
Expand Down