Skip to content

Commit b6cb9c0

Browse files
committed
Detect bad properties in profile specific files
Throw an `InvalidConfigDataPropertyException` if bad properties are detected in profile specific files. The following properties will now trigger an exception if used in a profile specific file: `spring.profiles.include` `spring.profiles.active` `spring.profiles.default` `spring.config.activate.on-profile` `spring.profiles` Prior to this commit, profile based properties in a profile specific file would be silently ignored, making them hard to find. Fixes gh-24733
1 parent 5ed2b11 commit b6cb9c0

14 files changed

+165
-84
lines changed

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/config/ConfigDataEnvironment.java

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@
3434
import org.springframework.boot.context.properties.bind.Bindable;
3535
import org.springframework.boot.context.properties.bind.Binder;
3636
import org.springframework.boot.context.properties.bind.PlaceholdersResolver;
37-
import org.springframework.boot.context.properties.source.ConfigurationPropertyName;
3837
import org.springframework.boot.context.properties.source.ConfigurationPropertySource;
3938
import org.springframework.boot.logging.DeferredLogFactory;
4039
import org.springframework.core.env.ConfigurableEnvironment;
@@ -98,9 +97,6 @@ class ConfigDataEnvironment {
9897

9998
private static final ConfigDataLocation[] EMPTY_LOCATIONS = new ConfigDataLocation[0];
10099

101-
private static final ConfigurationPropertyName INCLUDE_PROFILES = ConfigurationPropertyName
102-
.of(Profiles.INCLUDE_PROFILES_PROPERTY_NAME);
103-
104100
private static final Bindable<ConfigDataLocation[]> CONFIG_DATA_LOCATION_ARRAY = Bindable
105101
.of(ConfigDataLocation[].class);
106102

@@ -293,9 +289,9 @@ private Collection<? extends String> getIncludedProfiles(ConfigDataEnvironmentCo
293289
continue;
294290
}
295291
Binder binder = new Binder(Collections.singleton(source), placeholdersResolver);
296-
binder.bind(INCLUDE_PROFILES, STRING_LIST).ifBound((includes) -> {
292+
binder.bind(Profiles.INCLUDE_PROFILES, STRING_LIST).ifBound((includes) -> {
297293
if (!contributor.isActive(activationContext)) {
298-
InactiveConfigDataAccessException.throwIfPropertyFound(contributor, INCLUDE_PROFILES);
294+
InactiveConfigDataAccessException.throwIfPropertyFound(contributor, Profiles.INCLUDE_PROFILES);
299295
}
300296
result.addAll(includes);
301297
});

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/config/ConfigDataEnvironmentContributor.java

Lines changed: 36 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2020 the original author or authors.
2+
* Copyright 2012-2021 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -55,6 +55,8 @@ class ConfigDataEnvironmentContributor implements Iterable<ConfigDataEnvironment
5555

5656
private final ConfigDataResource resource;
5757

58+
private final boolean profileSpecific;
59+
5860
private final PropertySource<?> propertySource;
5961

6062
private final ConfigurationPropertySource configurationPropertySource;
@@ -72,6 +74,7 @@ class ConfigDataEnvironmentContributor implements Iterable<ConfigDataEnvironment
7274
* @param kind the contributor kind
7375
* @param location the location of this contributor
7476
* @param resource the resource that contributed the data or {@code null}
77+
* @param profileSpecific if the contributor is from a profile specific import
7578
* @param propertySource the property source for the data or {@code null}
7679
* @param configurationPropertySource the configuration property source for the data
7780
* or {@code null}
@@ -80,12 +83,13 @@ class ConfigDataEnvironmentContributor implements Iterable<ConfigDataEnvironment
8083
* @param children the children of this contributor at each {@link ImportPhase}
8184
*/
8285
ConfigDataEnvironmentContributor(Kind kind, ConfigDataLocation location, ConfigDataResource resource,
83-
PropertySource<?> propertySource, ConfigurationPropertySource configurationPropertySource,
84-
ConfigDataProperties properties, boolean ignoreImports,
85-
Map<ImportPhase, List<ConfigDataEnvironmentContributor>> children) {
86+
boolean profileSpecific, PropertySource<?> propertySource,
87+
ConfigurationPropertySource configurationPropertySource, ConfigDataProperties properties,
88+
boolean ignoreImports, Map<ImportPhase, List<ConfigDataEnvironmentContributor>> children) {
8689
this.kind = kind;
8790
this.location = location;
8891
this.resource = resource;
92+
this.profileSpecific = profileSpecific;
8993
this.properties = properties;
9094
this.propertySource = propertySource;
9195
this.configurationPropertySource = configurationPropertySource;
@@ -122,6 +126,14 @@ ConfigDataResource getResource() {
122126
return this.resource;
123127
}
124128

129+
/**
130+
* Return if the contributor is from a profile specific import.
131+
* @return if the contributor is profile specific
132+
*/
133+
boolean isProfileSpecific() {
134+
return this.profileSpecific;
135+
}
136+
125137
/**
126138
* Return the property source for this contributor.
127139
* @return the property source or {@code null}
@@ -201,7 +213,8 @@ ConfigDataEnvironmentContributor withBoundProperties(Binder binder) {
201213
properties = properties.withoutImports();
202214
}
203215
return new ConfigDataEnvironmentContributor(Kind.BOUND_IMPORT, this.location, this.resource,
204-
this.propertySource, this.configurationPropertySource, properties, this.ignoreImports, null);
216+
this.profileSpecific, this.propertySource, this.configurationPropertySource, properties,
217+
this.ignoreImports, null);
205218
}
206219

207220
/**
@@ -215,8 +228,9 @@ ConfigDataEnvironmentContributor withChildren(ImportPhase importPhase,
215228
List<ConfigDataEnvironmentContributor> children) {
216229
Map<ImportPhase, List<ConfigDataEnvironmentContributor>> updatedChildren = new LinkedHashMap<>(this.children);
217230
updatedChildren.put(importPhase, children);
218-
return new ConfigDataEnvironmentContributor(this.kind, this.location, this.resource, this.propertySource,
219-
this.configurationPropertySource, this.properties, this.ignoreImports, updatedChildren);
231+
return new ConfigDataEnvironmentContributor(this.kind, this.location, this.resource, this.profileSpecific,
232+
this.propertySource, this.configurationPropertySource, this.properties, this.ignoreImports,
233+
updatedChildren);
220234
}
221235

222236
/**
@@ -240,8 +254,9 @@ ConfigDataEnvironmentContributor withReplacement(ConfigDataEnvironmentContributo
240254
}
241255
updatedChildren.put(importPhase, Collections.unmodifiableList(updatedContributors));
242256
});
243-
return new ConfigDataEnvironmentContributor(this.kind, this.location, this.resource, this.propertySource,
244-
this.configurationPropertySource, this.properties, this.ignoreImports, updatedChildren);
257+
return new ConfigDataEnvironmentContributor(this.kind, this.location, this.resource, this.profileSpecific,
258+
this.propertySource, this.configurationPropertySource, this.properties, this.ignoreImports,
259+
updatedChildren);
245260
}
246261

247262
/**
@@ -252,7 +267,7 @@ ConfigDataEnvironmentContributor withReplacement(ConfigDataEnvironmentContributo
252267
static ConfigDataEnvironmentContributor of(List<ConfigDataEnvironmentContributor> contributors) {
253268
Map<ImportPhase, List<ConfigDataEnvironmentContributor>> children = new LinkedHashMap<>();
254269
children.put(ImportPhase.BEFORE_PROFILE_ACTIVATION, Collections.unmodifiableList(contributors));
255-
return new ConfigDataEnvironmentContributor(Kind.ROOT, null, null, null, null, null, false, children);
270+
return new ConfigDataEnvironmentContributor(Kind.ROOT, null, null, false, null, null, null, false, children);
256271
}
257272

258273
/**
@@ -265,8 +280,8 @@ static ConfigDataEnvironmentContributor of(List<ConfigDataEnvironmentContributor
265280
static ConfigDataEnvironmentContributor ofInitialImport(ConfigDataLocation initialImport) {
266281
List<ConfigDataLocation> imports = Collections.singletonList(initialImport);
267282
ConfigDataProperties properties = new ConfigDataProperties(imports, null);
268-
return new ConfigDataEnvironmentContributor(Kind.INITIAL_IMPORT, null, null, null, null, properties, false,
269-
null);
283+
return new ConfigDataEnvironmentContributor(Kind.INITIAL_IMPORT, null, null, false, null, null, properties,
284+
false, null);
270285
}
271286

272287
/**
@@ -277,7 +292,7 @@ static ConfigDataEnvironmentContributor ofInitialImport(ConfigDataLocation initi
277292
* @return a new {@link ConfigDataEnvironmentContributor} instance
278293
*/
279294
static ConfigDataEnvironmentContributor ofExisting(PropertySource<?> propertySource) {
280-
return new ConfigDataEnvironmentContributor(Kind.EXISTING, null, null, propertySource,
295+
return new ConfigDataEnvironmentContributor(Kind.EXISTING, null, null, false, propertySource,
281296
ConfigurationPropertySource.from(propertySource), null, false, null);
282297
}
283298

@@ -287,26 +302,29 @@ static ConfigDataEnvironmentContributor ofExisting(PropertySource<?> propertySou
287302
* import further contributors later.
288303
* @param location the location of this contributor
289304
* @param resource the config data resource
305+
* @param profileSpecific if the contributor is from a profile specific import
290306
* @param configData the config data
291307
* @param propertySourceIndex the index of the property source that should be used
292308
* @return a new {@link ConfigDataEnvironmentContributor} instance
293309
*/
294310
static ConfigDataEnvironmentContributor ofUnboundImport(ConfigDataLocation location, ConfigDataResource resource,
295-
ConfigData configData, int propertySourceIndex) {
311+
boolean profileSpecific, ConfigData configData, int propertySourceIndex) {
296312
PropertySource<?> propertySource = configData.getPropertySources().get(propertySourceIndex);
297313
ConfigurationPropertySource configurationPropertySource = ConfigurationPropertySource.from(propertySource);
298314
boolean ignoreImports = configData.getOptions().contains(ConfigData.Option.IGNORE_IMPORTS);
299-
return new ConfigDataEnvironmentContributor(Kind.UNBOUND_IMPORT, location, resource, propertySource,
300-
configurationPropertySource, null, ignoreImports, null);
315+
return new ConfigDataEnvironmentContributor(Kind.UNBOUND_IMPORT, location, resource, profileSpecific,
316+
propertySource, configurationPropertySource, null, ignoreImports, null);
301317
}
302318

303319
/**
304320
* Factory method to create an {@link Kind#EMPTY_LOCATION empty location} contributor.
305321
* @param location the location of this contributor
322+
* @param profileSpecific if the contributor is from a profile specific import
306323
* @return a new {@link ConfigDataEnvironmentContributor} instance
307324
*/
308-
static ConfigDataEnvironmentContributor ofEmptyLocation(ConfigDataLocation location) {
309-
return new ConfigDataEnvironmentContributor(Kind.EMPTY_LOCATION, location, null, null, null, null, true, null);
325+
static ConfigDataEnvironmentContributor ofEmptyLocation(ConfigDataLocation location, boolean profileSpecific) {
326+
return new ConfigDataEnvironmentContributor(Kind.EMPTY_LOCATION, location, null, profileSpecific, null, null,
327+
null, true, null);
310328
}
311329

312330
/**

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/config/ConfigDataEnvironmentContributors.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2020 the original author or authors.
2+
* Copyright 2012-2021 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -163,12 +163,14 @@ private List<ConfigDataEnvironmentContributor> asContributors(
163163
imported.forEach((resolutionResult, data) -> {
164164
ConfigDataLocation location = resolutionResult.getLocation();
165165
ConfigDataResource resource = resolutionResult.getResource();
166+
boolean profileSpecific = resolutionResult.isProfileSpecific();
166167
if (data.getPropertySources().isEmpty()) {
167-
contributors.add(ConfigDataEnvironmentContributor.ofEmptyLocation(location));
168+
contributors.add(ConfigDataEnvironmentContributor.ofEmptyLocation(location, profileSpecific));
168169
}
169170
else {
170171
for (int i = data.getPropertySources().size() - 1; i >= 0; i--) {
171-
contributors.add(ConfigDataEnvironmentContributor.ofUnboundImport(location, resource, data, i));
172+
contributors.add(ConfigDataEnvironmentContributor.ofUnboundImport(location, resource,
173+
profileSpecific, data, i));
172174
}
173175
}
174176
});

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/config/ConfigDataLocationResolvers.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2020 the original author or authors.
2+
* Copyright 2012-2021 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -111,21 +111,21 @@ List<ConfigDataResolutionResult> resolve(ConfigDataLocationResolverContext conte
111111

112112
private List<ConfigDataResolutionResult> resolve(ConfigDataLocationResolver<?> resolver,
113113
ConfigDataLocationResolverContext context, ConfigDataLocation location, Profiles profiles) {
114-
List<ConfigDataResolutionResult> resolved = resolve(location, () -> resolver.resolve(context, location));
114+
List<ConfigDataResolutionResult> resolved = resolve(location, false, () -> resolver.resolve(context, location));
115115
if (profiles == null) {
116116
return resolved;
117117
}
118-
List<ConfigDataResolutionResult> profileSpecific = resolve(location,
118+
List<ConfigDataResolutionResult> profileSpecific = resolve(location, true,
119119
() -> resolver.resolveProfileSpecific(context, location, profiles));
120120
return merge(resolved, profileSpecific);
121121
}
122122

123-
private List<ConfigDataResolutionResult> resolve(ConfigDataLocation location,
123+
private List<ConfigDataResolutionResult> resolve(ConfigDataLocation location, boolean profileSpecific,
124124
Supplier<List<? extends ConfigDataResource>> resolveAction) {
125125
List<ConfigDataResource> resources = nonNullList(resolveAction.get());
126126
List<ConfigDataResolutionResult> resolved = new ArrayList<>(resources.size());
127127
for (ConfigDataResource resource : resources) {
128-
resolved.add(new ConfigDataResolutionResult(location, resource));
128+
resolved.add(new ConfigDataResolutionResult(location, resource, profileSpecific));
129129
}
130130
return resolved;
131131
}

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/config/ConfigDataProperties.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2020 the original author or authors.
2+
* Copyright 2012-2021 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -94,7 +94,7 @@ ConfigDataProperties withoutImports() {
9494

9595
ConfigDataProperties withLegacyProfiles(String[] legacyProfiles, ConfigurationProperty property) {
9696
if (this.activate != null && !ObjectUtils.isEmpty(this.activate.onProfile)) {
97-
throw new InvalidConfigDataPropertyException(property, NAME.append("activate.on-profile"), null);
97+
throw new InvalidConfigDataPropertyException(property, false, NAME.append("activate.on-profile"), null);
9898
}
9999
return new ConfigDataProperties(this.imports, new Activate(this.activate.onCloudPlatform, legacyProfiles));
100100
}

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/config/ConfigDataResolutionResult.java

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2020 the original author or authors.
2+
* Copyright 2012-2021 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -28,9 +28,12 @@ class ConfigDataResolutionResult {
2828

2929
private final ConfigDataResource resource;
3030

31-
ConfigDataResolutionResult(ConfigDataLocation location, ConfigDataResource resource) {
31+
private final boolean profileSpecific;
32+
33+
ConfigDataResolutionResult(ConfigDataLocation location, ConfigDataResource resource, boolean profileSpecific) {
3234
this.location = location;
3335
this.resource = resource;
36+
this.profileSpecific = profileSpecific;
3437
}
3538

3639
ConfigDataLocation getLocation() {
@@ -41,4 +44,8 @@ ConfigDataResource getResource() {
4144
return this.resource;
4245
}
4346

47+
boolean isProfileSpecific() {
48+
return this.profileSpecific;
49+
}
50+
4451
}

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/config/InvalidConfigDataPropertyException.java

Lines changed: 38 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2020 the original author or authors.
2+
* Copyright 2012-2021 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -18,13 +18,16 @@
1818

1919
import java.util.Collections;
2020
import java.util.LinkedHashMap;
21+
import java.util.LinkedHashSet;
2122
import java.util.Map;
23+
import java.util.Set;
2224

2325
import org.apache.commons.logging.Log;
2426

2527
import org.springframework.boot.context.properties.source.ConfigurationProperty;
2628
import org.springframework.boot.context.properties.source.ConfigurationPropertyName;
2729
import org.springframework.boot.context.properties.source.ConfigurationPropertySource;
30+
import org.springframework.core.env.AbstractEnvironment;
2831

2932
/**
3033
* Exception thrown if an invalid property is found when processing config data.
@@ -35,12 +38,23 @@
3538
*/
3639
public class InvalidConfigDataPropertyException extends ConfigDataException {
3740

38-
private static final Map<ConfigurationPropertyName, ConfigurationPropertyName> WARNING;
41+
private static final Map<ConfigurationPropertyName, ConfigurationPropertyName> WARNINGS;
3942
static {
40-
Map<ConfigurationPropertyName, ConfigurationPropertyName> warning = new LinkedHashMap<>();
41-
warning.put(ConfigurationPropertyName.of("spring.profiles"),
43+
Map<ConfigurationPropertyName, ConfigurationPropertyName> warnings = new LinkedHashMap<>();
44+
warnings.put(ConfigurationPropertyName.of("spring.profiles"),
4245
ConfigurationPropertyName.of("spring.config.activate.on-profile"));
43-
WARNING = Collections.unmodifiableMap(warning);
46+
WARNINGS = Collections.unmodifiableMap(warnings);
47+
}
48+
49+
private static final Set<ConfigurationPropertyName> PROFILE_SPECIFIC_ERRORS;
50+
static {
51+
Set<ConfigurationPropertyName> errors = new LinkedHashSet<>();
52+
errors.add(Profiles.INCLUDE_PROFILES);
53+
errors.add(ConfigurationPropertyName.of(AbstractEnvironment.ACTIVE_PROFILES_PROPERTY_NAME));
54+
errors.add(ConfigurationPropertyName.of(AbstractEnvironment.DEFAULT_PROFILES_PROPERTY_NAME));
55+
errors.add(ConfigurationPropertyName.of("spring.config.activate.on-profile"));
56+
errors.add(ConfigurationPropertyName.of("spring.profiles"));
57+
PROFILE_SPECIFIC_ERRORS = Collections.unmodifiableSet(errors);
4458
}
4559

4660
private final ConfigurationProperty property;
@@ -49,9 +63,9 @@ public class InvalidConfigDataPropertyException extends ConfigDataException {
4963

5064
private final ConfigDataResource location;
5165

52-
InvalidConfigDataPropertyException(ConfigurationProperty property, ConfigurationPropertyName replacement,
53-
ConfigDataResource location) {
54-
super(getMessage(property, replacement, location), null);
66+
InvalidConfigDataPropertyException(ConfigurationProperty property, boolean profileSpecific,
67+
ConfigurationPropertyName replacement, ConfigDataResource location) {
68+
super(getMessage(property, profileSpecific, replacement, location), null);
5569
this.property = property;
5670
this.replacement = replacement;
5771
this.location = location;
@@ -94,24 +108,35 @@ public ConfigurationPropertyName getReplacement() {
94108
static void throwOrWarn(Log logger, ConfigDataEnvironmentContributor contributor) {
95109
ConfigurationPropertySource propertySource = contributor.getConfigurationPropertySource();
96110
if (propertySource != null) {
97-
WARNING.forEach((invalid, replacement) -> {
98-
ConfigurationProperty property = propertySource.getConfigurationProperty(invalid);
111+
WARNINGS.forEach((name, replacement) -> {
112+
ConfigurationProperty property = propertySource.getConfigurationProperty(name);
99113
if (property != null) {
100-
logger.warn(getMessage(property, replacement, contributor.getResource()));
114+
logger.warn(getMessage(property, false, replacement, contributor.getResource()));
101115
}
102116
});
117+
if (contributor.isProfileSpecific()) {
118+
PROFILE_SPECIFIC_ERRORS.forEach((name) -> {
119+
ConfigurationProperty property = propertySource.getConfigurationProperty(name);
120+
if (property != null) {
121+
throw new InvalidConfigDataPropertyException(property, true, null, contributor.getResource());
122+
}
123+
});
124+
}
103125
}
104126
}
105127

106-
private static String getMessage(ConfigurationProperty property, ConfigurationPropertyName replacement,
107-
ConfigDataResource location) {
128+
private static String getMessage(ConfigurationProperty property, boolean profileSpecific,
129+
ConfigurationPropertyName replacement, ConfigDataResource location) {
108130
StringBuilder message = new StringBuilder("Property '");
109131
message.append(property.getName());
110132
if (location != null) {
111133
message.append("' imported from location '");
112134
message.append(location);
113135
}
114136
message.append("' is invalid");
137+
if (profileSpecific) {
138+
message.append(" in a profile specific resource");
139+
}
115140
if (replacement != null) {
116141
message.append(" and should be replaced with '");
117142
message.append(replacement);

0 commit comments

Comments
 (0)