spring-projects / spring-boot

Spring Boot helps you to create Spring-powered, production-grade applications and services with absolute minimum fuss.
https://spring.io/projects/spring-boot
Apache License 2.0
75.35k stars 40.72k forks source link

Tighten rules around profile naming #43176

Open YangSiJun528 opened 1 week ago

YangSiJun528 commented 1 week ago

I added the validateProfiles() method to enforce stricter rules for Spring profile names.

Currently, only alphanumeric characters, hyphens (-), and underscores (_) are allowed. Please let me know if additional characters should be permitted - I'll update accordingly.

I've verified validation works in application.properties and @ActiveProfiles.

While @Profile doesn't perform validation, this seems acceptable since invalid profiles will fail during activation. Perhaps we could explore adding warnings as a potential enhancement in the future.

Note: this change may break applications using non-conforming profile names.

Fixes gh-34062

mhalbritter commented 4 days ago

Hello! First of all, thank you for the PR!

This currently breaks:

I think we can remove activeProfileWithComma and use a profile without a comma in testPropertyValuesShouldTakePrecedenceWhenInlinedPropertiesPresentAndProfilesActive.

It also breaks:

mhalbritter commented 4 days ago

Flagging this for team meeting as I want to talk about if the positive list is a good approach. Right now, it's very "western centric" by allowing only A-Z and digits.

I wonder if we should approach it like this:

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

    private void validateProfile(String profile) {
        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));
        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));
        });
    }

This would also block ,, ( etc. but lets more non-ascii letters and digits through.

YangSiJun528 commented 1 day ago

I incorporated the feedback provided. @mhalbritter