From 1a3f810cd88ce9933fe862df1f2907a4dd12a4cf Mon Sep 17 00:00:00 2001 From: Scott Frederick Date: Fri, 23 Oct 2020 15:04:23 -0500 Subject: [PATCH] Prevent serialization exception from Env actuator When `EnvironmentEndpoint` is building a response to return to the web infrastructure, it creates a data structure containing all property values from all property sources. Prior to this commit, it was possible for the response data structure to contain property values that were not serializable to JSON by Jackson, which would cause an exception to be thrown by the web infrastructure. This commit ensures the data structure is serializable to JSON by ensuring property values are primitives or Strings, and returning a placeholder value if a property value is of any other type. Fixes gh-23805 --- .../boot/actuate/env/EnvironmentEndpoint.java | 13 ++++- .../actuate/env/EnvironmentEndpointTests.java | 51 +++++++++++++++++-- 2 files changed, 59 insertions(+), 5 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/env/EnvironmentEndpoint.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/env/EnvironmentEndpoint.java index 1e171545d99..4c1be68dad3 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/env/EnvironmentEndpoint.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/env/EnvironmentEndpoint.java @@ -57,6 +57,7 @@ import org.springframework.util.SystemPropertyUtils; * @author Christian Dupuis * @author Madhura Bhave * @author Stephane Nicoll + * @author Scott Frederick * @since 2.0.0 */ @Endpoint(id = "env") @@ -143,7 +144,7 @@ public class EnvironmentEndpoint { PlaceholdersResolver resolver) { Object resolved = resolver.resolvePlaceholders(source.getProperty(name)); Origin origin = ((source instanceof OriginLookup) ? ((OriginLookup) source).getOrigin(name) : null); - return new PropertyValueDescriptor(sanitize(name, resolved), origin); + return new PropertyValueDescriptor(stringifyIfNecessary(sanitize(name, resolved)), origin); } private PlaceholdersResolver getResolver() { @@ -182,6 +183,16 @@ public class EnvironmentEndpoint { return this.sanitizer.sanitize(name, object); } + protected Object stringifyIfNecessary(Object value) { + if (value == null || value.getClass().isPrimitive()) { + return value; + } + if (CharSequence.class.isAssignableFrom(value.getClass())) { + return value.toString(); + } + return "Complex property type " + value.getClass().getName(); + } + /** * {@link PropertySourcesPlaceholdersResolver} that sanitizes sensitive placeholders * if present. diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/env/EnvironmentEndpointTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/env/EnvironmentEndpointTests.java index dfe90081c4a..313a67f312a 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/env/EnvironmentEndpointTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/env/EnvironmentEndpointTests.java @@ -16,6 +16,9 @@ package org.springframework.boot.actuate.env; +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.InputStream; import java.util.Collections; import java.util.LinkedHashMap; import java.util.Map; @@ -39,6 +42,7 @@ import org.springframework.core.env.ConfigurableEnvironment; import org.springframework.core.env.Environment; import org.springframework.core.env.MapPropertySource; import org.springframework.core.env.StandardEnvironment; +import org.springframework.core.io.InputStreamSource; import org.springframework.mock.env.MockPropertySource; import static org.assertj.core.api.Assertions.assertThat; @@ -54,6 +58,7 @@ import static org.assertj.core.api.Assertions.assertThat; * @author Andy Wilkinson * @author HaiTao Zhang * @author Chris Bono + * @author Scott Frederick */ class EnvironmentEndpointTests { @@ -194,15 +199,22 @@ class EnvironmentEndpointTests { } @Test - @SuppressWarnings("unchecked") void propertyWithTypeOtherThanStringShouldNotFail() { ConfigurableEnvironment environment = emptyEnvironment(); environment.getPropertySources() .addFirst(singleKeyPropertySource("test", "foo", Collections.singletonMap("bar", "baz"))); EnvironmentDescriptor descriptor = new EnvironmentEndpoint(environment).environment(null); - Map foo = (Map) propertySources(descriptor).get("test").getProperties() - .get("foo").getValue(); - assertThat(foo.get("bar")).isEqualTo("baz"); + String value = (String) propertySources(descriptor).get("test").getProperties().get("foo").getValue(); + assertThat(value).isEqualTo("Complex property type java.util.Collections$SingletonMap"); + } + + @Test + void propertyWithCharSequenceTypeIsConvertedToString() throws Exception { + ConfigurableEnvironment environment = emptyEnvironment(); + environment.getPropertySources().addFirst(singleKeyPropertySource("test", "foo", new CharSequenceProperty())); + EnvironmentDescriptor descriptor = new EnvironmentEndpoint(environment).environment(null); + String value = (String) propertySources(descriptor).get("test").getProperties().get("foo").getValue(); + assertThat(value).isEqualTo("test value"); } @Test @@ -360,4 +372,35 @@ class EnvironmentEndpointTests { } + public static class CharSequenceProperty implements CharSequence, InputStreamSource { + + private final String value = "test value"; + + @Override + public int length() { + return this.value.length(); + } + + @Override + public char charAt(int index) { + return this.value.charAt(index); + } + + @Override + public CharSequence subSequence(int start, int end) { + return this.value.subSequence(start, end); + } + + @Override + public String toString() { + return this.value; + } + + @Override + public InputStream getInputStream() throws IOException { + return new ByteArrayInputStream(this.value.getBytes()); + } + + } + }