From 53e049a8123a78b6bb2ef1ec550e216829bc1436 Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Tue, 20 Sep 2016 10:55:15 +0100 Subject: [PATCH] Ensure EnvironmentController does not leak system properties In the JSON and YAML endpoints system properties and env vars could leak if the config contains placeholders with default values. This change explicitly switches off that replacement (making the JSON and JAML consistent with the properties endpoint). Fixes gh-480, closes gh-492 --- pom.xml | 2 +- .../environment/EnvironmentController.java | 9 +++++--- .../EnvironmentControllerTests.java | 22 ++++++++++++++++--- 3 files changed, 26 insertions(+), 7 deletions(-) diff --git a/pom.xml b/pom.xml index 40c4a907..ea51ae27 100644 --- a/pom.xml +++ b/pom.xml @@ -11,7 +11,7 @@ org.springframework.cloud spring-cloud-build - 1.2.0.RELEASE + 1.2.1.BUILD-SNAPSHOT diff --git a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/EnvironmentController.java b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/EnvironmentController.java index 1f86c61b..2e27743c 100644 --- a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/EnvironmentController.java +++ b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/EnvironmentController.java @@ -152,7 +152,7 @@ public class EnvironmentController { throws Exception { validateProfiles(profiles); Environment environment = labelled(name, profiles, label); - Map properties = convertToMap(environment); + Map properties = convertToMap(environment, resolvePlaceholders); String json = this.objectMapper.writeValueAsString(properties); if (resolvePlaceholders) { json = resolvePlaceholders(prepareEnvironment(environment), json); @@ -188,7 +188,7 @@ public class EnvironmentController { throws Exception { validateProfiles(profiles); Environment environment = labelled(name, profiles, label); - Map result = convertToMap(environment); + Map result = convertToMap(environment, resolvePlaceholders); if (this.stripDocument && result.size() == 1 && result.keySet().iterator().next().equals("document")) { Object value = result.get("document"); @@ -208,10 +208,13 @@ public class EnvironmentController { return getSuccess(yaml); } - private Map convertToMap(Environment input) throws BindException { + private Map convertToMap(Environment input, boolean resolvePlaceholders) throws BindException { Map target = new LinkedHashMap<>(); PropertiesConfigurationFactory> factory = new PropertiesConfigurationFactory<>( target); + if (!resolvePlaceholders) { + factory.setResolvePlaceholders(false); + } Map data = convertToProperties(input); LinkedHashMap properties = new LinkedHashMap<>(); for (String key : data.keySet()) { diff --git a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/EnvironmentControllerTests.java b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/EnvironmentControllerTests.java index d429e7c9..bed6062b 100644 --- a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/EnvironmentControllerTests.java +++ b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/EnvironmentControllerTests.java @@ -138,7 +138,15 @@ public class EnvironmentControllerTests { public void placeholdersNotResolvedInYamlFromSystemPropertiesWhenNotFlaggedWithDefault() throws Exception { whenPlaceholdersSystemPropsWithDefault(); String yaml = this.controller.yaml("foo", "bar", false).getBody(); - // If there is a default value we can't prevent the placeholder being resolved + // If there is a default value we prevent the placeholder being resolved + assertEquals("a:\n b:\n c: ${foo:spam}\n", yaml); + } + + @Test + public void placeholdersResolvedInYamlFromSystemPropertiesWhenFlagged() throws Exception { + whenPlaceholdersSystemPropsWithDefault(); + String yaml = this.controller.yaml("foo", "bar", true).getBody(); + // If there is a default value we do not prevent the placeholder being resolved assertEquals("a:\n b:\n c: spam\n", yaml); } @@ -335,10 +343,18 @@ public class EnvironmentControllerTests { } @Test - public void placeholdersResolvedInJsonFromSystemPropertiesWhenNotFlaggedWithDefault() throws Exception { + public void placeholdersNotResolvedInJsonFromSystemPropertiesWhenNotFlaggedWithDefault() throws Exception { whenPlaceholdersSystemPropsWithDefault(); String json = this.controller.jsonProperties("foo", "bar", false).getBody(); - // If there is a default value we can't prevent the placeholder being resolved + // If there is a default value we do not prevent the placeholder being resolved + assertEquals("{\"a\":{\"b\":{\"c\":\"${foo:spam}\"}}}", json); + } + + @Test + public void placeholdersResolvedInJsonFromSystemPropertiesWhenFlagged() throws Exception { + whenPlaceholdersSystemPropsWithDefault(); + String json = this.controller.jsonProperties("foo", "bar", true).getBody(); + // If there is a default value we prevent the placeholder being resolved assertEquals("{\"a\":{\"b\":{\"c\":\"spam\"}}}", json); }