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

Tighten rules around profile naming #43176

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Tighten rules around profile naming #43176

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
eab2b45
Add validateProfiles
YangSiJun528 Nov 15, 2024
225382f
Fix resolveProfileSpecificReturnsProfileSpecificFiles test to success
YangSiJun528 Nov 15, 2024
2e99672
Add test about validateProfiles
YangSiJun528 Nov 15, 2024
c999851
Fix verifyProfiles to allow non-ASCII
YangSiJun528 Nov 24, 2024
038de1e
Fix test
YangSiJun528 Nov 24, 2024
16f87e5
Add test to allow non-ASCII
YangSiJun528 Nov 24, 2024
cf6acc0
Remove activeProfileWithComma
YangSiJun528 Nov 24, 2024
05e813b
Fix testPropertyValuesShouldTakePrecedenceWhenInlinedPropertiesPresen…
YangSiJun528 Nov 24, 2024
262111e
Fix tests in SpringApplicationTests by removing profiles with commas
YangSiJun528 Nov 24, 2024
b7b43af
Polishing
YangSiJun528 Dec 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions ...main/java/org/springframework/boot/context/config/StandardConfigDataLocationResolver.java
View file
Open in desktop
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,21 @@ private Set<StandardConfigDataReference> getProfileSpecificReferences(ConfigData
return references;
}

private void validateProfiles(Profiles profiles) {
Pattern validProfilePattern = Pattern.compile("^[a-zA-Z0-9_\\-]+$");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be simplified to Pattern validProfilePattern = Pattern.compile("^[\\w-]+$");.

Assert.notNull(profiles, "Profiles must not be null");
for (String profile : profiles) {
Assert.notNull(profile, "Profile must not be null");
Assert.hasText(profile, "Profile must not be empty");
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));
Assert.state(validProfilePattern.matcher(profile).matches(), () -> String
.format("Invalid profile '%s': must only contain alphanumeric characters, '-', or '_'", profile));
}
}

private String getResourceLocation(ConfigDataLocationResolverContext context,
ConfigDataLocation configDataLocation) {
String resourceLocation = configDataLocation.getNonPrefixedValue(PREFIX);
Expand Down
74 changes: 72 additions & 2 deletions ...java/org/springframework/boot/context/config/StandardConfigDataLocationResolverTests.java
View file
Open in desktop
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,75 @@ 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 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 only contain alphanumeric characters, '-', or '_'");
}

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