From 378c4c9535a5fb52f981e111c402902a064b8409 Mon Sep 17 00:00:00 2001 From: Madhura Bhave Date: Tue, 15 May 2018 17:44:40 -0700 Subject: [PATCH 1/2] Stop included profiles overriding active profiles Update `ConfigFileApplicationListener` so that `spring.profiles.include` properties do not override higher priority active profiles. This commit also changes when profiles get added to the environment. Profiles are now added to the environment at the time of profile processing so that they get logged in the order that they are processed. Closes gh-13151 --- .../config/ConfigFileApplicationListener.java | 44 ++++++++++--------- ...ationListenerYamlProfileNegationTests.java | 2 +- .../src/test/resources/cascadingprofiles.yml | 14 ++++++ 3 files changed, 39 insertions(+), 21 deletions(-) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/config/ConfigFileApplicationListener.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/config/ConfigFileApplicationListener.java index b651e08b1f..126d5e30d8 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/config/ConfigFileApplicationListener.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/config/ConfigFileApplicationListener.java @@ -328,6 +328,9 @@ public class ConfigFileApplicationListener initializeProfiles(); while (!this.profiles.isEmpty()) { Profile profile = this.profiles.poll(); + if (profile != null && !profile.isDefaultProfile()) { + addProfileToEnvironment(profile.getName()); + } load(profile, this::getPositiveProfileFilter, addToLoaded(MutablePropertySources::addLast, false)); this.processedProfiles.add(profile); @@ -347,15 +350,13 @@ public class ConfigFileApplicationListener // first so that it is processed first and has lowest priority. this.profiles.add(null); Set activatedViaProperty = getProfilesActivatedViaProperty(); - processOtherActiveProfiles(activatedViaProperty); + this.profiles.addAll(getOtherActiveProfiles(activatedViaProperty)); // Any pre-existing active profiles set via property sources (e.g. - // System - // properties) take precedence over those added in config files. + // System properties) take precedence over those added in config files. addActiveProfiles(activatedViaProperty); if (this.profiles.size() == 1) { // only has null profile for (String defaultProfileName : this.environment.getDefaultProfiles()) { - ConfigFileApplicationListener.Profile defaultProfile = new ConfigFileApplicationListener.Profile( - defaultProfileName, true); + Profile defaultProfile = new Profile(defaultProfileName, true); this.profiles.add(defaultProfile); } } @@ -373,19 +374,22 @@ public class ConfigFileApplicationListener return activeProfiles; } - private void processOtherActiveProfiles(Set activatedViaProperty) { - List otherActiveProfiles = Arrays - .stream(this.environment.getActiveProfiles()).map(Profile::new) - .filter((o) -> !activatedViaProperty.contains(o)) + private List getOtherActiveProfiles(Set activatedViaProperty) { + return Arrays.stream(this.environment.getActiveProfiles()).map(Profile::new) + .filter((profile) -> !activatedViaProperty.contains(profile)) .collect(Collectors.toList()); - this.profiles.addAll(otherActiveProfiles); } void addActiveProfiles(Set profiles) { - if (this.activatedProfiles || profiles.isEmpty()) { + if (profiles.isEmpty()) { return; } - addProfiles(profiles); + if (this.activatedProfiles) { + this.logger.debug("Profiles already activated, '" + profiles + + "' will not be applied"); + return; + } + this.profiles.addAll(profiles); if (this.logger.isDebugEnabled()) { this.logger.debug("Activated activeProfiles " + StringUtils.collectionToCommaDelimitedString(profiles)); @@ -394,13 +398,6 @@ public class ConfigFileApplicationListener removeUnprocessedDefaultProfiles(); } - void addProfiles(Set profiles) { - for (Profile profile : profiles) { - this.profiles.add(profile); - addProfileToEnvironment(profile.getName()); - } - } - private void removeUnprocessedDefaultProfiles() { this.profiles.removeIf( (profile) -> (profile != null && profile.isDefaultProfile())); @@ -526,7 +523,7 @@ public class ConfigFileApplicationListener for (Document document : documents) { if (filter.match(document)) { addActiveProfiles(document.getActiveProfiles()); - addProfiles(document.getIncludeProfiles()); + addIncludedProfiles(document.getIncludeProfiles()); loaded.add(document); } } @@ -542,6 +539,13 @@ public class ConfigFileApplicationListener } } + private void addIncludedProfiles(Set includeProfiles) { + LinkedList existingProfiles = new LinkedList<>(this.profiles); + this.profiles.clear(); + this.profiles.addAll(includeProfiles); + this.profiles.addAll(existingProfiles); + } + private List loadDocuments(PropertySourceLoader loader, String name, Resource resource) throws IOException { DocumentsCacheKey cacheKey = new DocumentsCacheKey(loader, resource); diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/config/ConfigFileApplicationListenerYamlProfileNegationTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/config/ConfigFileApplicationListenerYamlProfileNegationTests.java index 6150d5d926..74d1cdb676 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/config/ConfigFileApplicationListenerYamlProfileNegationTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/config/ConfigFileApplicationListenerYamlProfileNegationTests.java @@ -94,7 +94,7 @@ public class ConfigFileApplicationListenerYamlProfileNegationTests { application.setWebApplicationType(WebApplicationType.NONE); String configName = "--spring.config.name=cascadingprofiles"; this.context = application.run(configName); - assertVersionProperty(this.context, "E", "A", "B", "C", "E", "D"); + assertVersionProperty(this.context, "D", "A", "C", "E", "B", "D"); assertThat(this.context.getEnvironment().getProperty("not-a")).isNull(); assertThat(this.context.getEnvironment().getProperty("not-b")).isNull(); assertThat(this.context.getEnvironment().getProperty("not-c")).isNull(); diff --git a/spring-boot-project/spring-boot/src/test/resources/cascadingprofiles.yml b/spring-boot-project/spring-boot/src/test/resources/cascadingprofiles.yml index 43982554ed..084431b99d 100644 --- a/spring-boot-project/spring-boot/src/test/resources/cascadingprofiles.yml +++ b/spring-boot-project/spring-boot/src/test/resources/cascadingprofiles.yml @@ -12,6 +12,7 @@ spring: include: - C - E +version: A --- spring.profiles: B @@ -21,6 +22,19 @@ spring: include: - D - E +version: B + +--- +spring.profiles: C + +version: C + + +--- +spring.profiles: D + +version: D + --- spring.profiles: E From 67b548dafb5b6efe2cdafe06514e4a022ab60478 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Mon, 4 Jun 2018 14:59:29 -0700 Subject: [PATCH 2/2] Protect against infinite property include loop Update `ConfigFileApplicationListener` to ensure that a `spring.profiles.include` property that refers to an already processed profile doesn't cause an infinite loop. Closes gh-13361 --- .../context/config/ConfigFileApplicationListener.java | 1 + .../config/ConfigFileApplicationListenerTests.java | 11 +++++++++++ .../test/resources/applicationloop-loop.properties | 1 + .../src/test/resources/applicationloop.properties | 1 + 4 files changed, 14 insertions(+) create mode 100644 spring-boot-project/spring-boot/src/test/resources/applicationloop-loop.properties create mode 100644 spring-boot-project/spring-boot/src/test/resources/applicationloop.properties diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/config/ConfigFileApplicationListener.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/config/ConfigFileApplicationListener.java index 126d5e30d8..92584ec1d3 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/config/ConfigFileApplicationListener.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/config/ConfigFileApplicationListener.java @@ -543,6 +543,7 @@ public class ConfigFileApplicationListener LinkedList existingProfiles = new LinkedList<>(this.profiles); this.profiles.clear(); this.profiles.addAll(includeProfiles); + this.profiles.removeAll(this.processedProfiles); this.profiles.addAll(existingProfiles); } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/config/ConfigFileApplicationListenerTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/config/ConfigFileApplicationListenerTests.java index f08200a093..e0be4d1462 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/config/ConfigFileApplicationListenerTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/config/ConfigFileApplicationListenerTests.java @@ -892,6 +892,17 @@ public class ConfigFileApplicationListenerTests { assertThat(this.environment.getProperty("value")).isNull(); } + @Test + public void includeLoop() { + // gh-13361 + SpringApplication application = new SpringApplication(Config.class); + application.setWebApplicationType(WebApplicationType.NONE); + this.context = application.run("--spring.config.name=applicationloop"); + ConfigurableEnvironment environment = this.context.getEnvironment(); + assertThat(environment.acceptsProfiles("loop")).isTrue(); + + } + private Condition matchingPropertySource( final String sourceName) { return new Condition( diff --git a/spring-boot-project/spring-boot/src/test/resources/applicationloop-loop.properties b/spring-boot-project/spring-boot/src/test/resources/applicationloop-loop.properties new file mode 100644 index 0000000000..0602540b20 --- /dev/null +++ b/spring-boot-project/spring-boot/src/test/resources/applicationloop-loop.properties @@ -0,0 +1 @@ +spring.profiles.include=loop diff --git a/spring-boot-project/spring-boot/src/test/resources/applicationloop.properties b/spring-boot-project/spring-boot/src/test/resources/applicationloop.properties new file mode 100644 index 0000000000..0602540b20 --- /dev/null +++ b/spring-boot-project/spring-boot/src/test/resources/applicationloop.properties @@ -0,0 +1 @@ +spring.profiles.include=loop