From 0be0bed88cef0fe49d1e1e5ab3ece16b82f90e6c Mon Sep 17 00:00:00 2001 From: sijun-yang Date: Fri, 15 Nov 2024 22:51:11 +0900 Subject: [PATCH] Tighten rules around profile naming Profiles are only allowed to use dashes, underscores, digits or letters. See gh-43176 --- .../context/SpringBootContextLoaderTests.java | 14 +--- .../StandardConfigDataLocationResolver.java | 22 +++++ .../boot/SpringApplicationTests.java | 9 +- ...andardConfigDataLocationResolverTests.java | 83 ++++++++++++++++++- 4 files changed, 110 insertions(+), 18 deletions(-) diff --git a/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/context/SpringBootContextLoaderTests.java b/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/context/SpringBootContextLoaderTests.java index 3f6a4888d1..e59d496734 100644 --- a/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/context/SpringBootContextLoaderTests.java +++ b/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/context/SpringBootContextLoaderTests.java @@ -60,6 +60,7 @@ import static org.assertj.core.api.Assertions.assertThatIllegalStateException; * @author Stephane Nicoll * @author Scott Frederick * @author Madhura Bhave + * @author Sijun Yang */ class SpringBootContextLoaderTests { @@ -127,11 +128,6 @@ class SpringBootContextLoaderTests { 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(); @@ -314,14 +310,8 @@ class SpringBootContextLoaderTests { } - @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 { } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/config/StandardConfigDataLocationResolver.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/config/StandardConfigDataLocationResolver.java index beb076ad86..d29efafa96 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/config/StandardConfigDataLocationResolver.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/config/StandardConfigDataLocationResolver.java @@ -54,6 +54,7 @@ import org.springframework.util.StringUtils; * @author Madhura Bhave * @author Phillip Webb * @author Scott Frederick + * @author Sijun Yang * @since 2.4.0 */ public class StandardConfigDataLocationResolver @@ -154,6 +155,7 @@ public class StandardConfigDataLocationResolver private Set getProfileSpecificReferences(ConfigDataLocationResolverContext context, ConfigDataLocation[] configDataLocations, Profiles profiles) { Set references = new LinkedHashSet<>(); + validateProfiles(profiles); for (String profile : profiles) { for (ConfigDataLocation configDataLocation : configDataLocations) { String resourceLocation = getResourceLocation(context, configDataLocation); @@ -163,6 +165,26 @@ public class StandardConfigDataLocationResolver 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); diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/SpringApplicationTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/SpringApplicationTests.java index b606329686..dc7a64c734 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/SpringApplicationTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/SpringApplicationTests.java @@ -165,6 +165,7 @@ import static org.mockito.Mockito.spy; * @author Moritz Halbritter * @author Tadaya Tsuyukubo * @author Yanming Zhou + * @author Sijun Yang */ @ExtendWith(OutputCaptureExtension.class) class SpringApplicationTests { @@ -252,13 +253,13 @@ class SpringApplicationTests { @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 @@ -273,9 +274,9 @@ class SpringApplicationTests { 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 diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/config/StandardConfigDataLocationResolverTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/config/StandardConfigDataLocationResolverTests.java index 6b221e15f2..61a964a175 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/config/StandardConfigDataLocationResolverTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/config/StandardConfigDataLocationResolverTests.java @@ -44,6 +44,7 @@ import static org.mockito.Mockito.mock; * @author Madhura Bhave * @author Phillip Webb * @author Moritz Halbritter + * @author Sijun Yang */ class StandardConfigDataLocationResolverTests { @@ -254,8 +255,8 @@ class StandardConfigDataLocationResolverTests { @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 locations = this.resolver.resolveProfileSpecific(this.context, location, profiles); assertThat(locations).hasSize(1); @@ -293,6 +294,84 @@ class StandardConfigDataLocationResolverTests { 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) + "]"; }