Make serialization of @ConfigurationProperties beans more defensive

Previously, serialization of a @ConfigurationProperties bean to JSON
would fail if:

- A property on the bean returned the bean (the bean was
  self-referential)
- An exception was thrown when attempting to retrieve a property's
  value.

This commit makes the serialization more defensive by skipping any
property that is affected by either of the problems described above.
Debug logging has been added to aid diagnosis of missing properties.

Closes gh-10846

spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/ConfigurationPropertiesReportEndpoint.java

@@ -23,10 +23,12 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
+import com.fasterxml.jackson.core.JsonGenerator;
 import com.fasterxml.jackson.databind.BeanDescription;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.SerializationConfig;
 import com.fasterxml.jackson.databind.SerializationFeature;
+import com.fasterxml.jackson.databind.SerializerProvider;
 import com.fasterxml.jackson.databind.introspect.Annotated;
 import com.fasterxml.jackson.databind.introspect.AnnotatedMethod;
 import com.fasterxml.jackson.databind.introspect.JacksonAnnotationIntrospector;
@@ -37,6 +39,8 @@ import com.fasterxml.jackson.databind.ser.PropertyWriter;
 import com.fasterxml.jackson.databind.ser.SerializerFactory;
 import com.fasterxml.jackson.databind.ser.impl.SimpleBeanPropertyFilter;
 import com.fasterxml.jackson.databind.ser.impl.SimpleFilterProvider;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
 
 import org.springframework.beans.BeansException;
 import org.springframework.boot.context.properties.ConfigurationBeanFactoryMetaData;
@@ -64,7 +68,7 @@ import org.springframework.util.StringUtils;
 public class ConfigurationPropertiesReportEndpoint
 		extends AbstractEndpoint<Map<String, Object>> implements ApplicationContextAware {
 
-	private static final String CGLIB_FILTER_ID = "cglibFilter";
+	private static final String CONFIGURATION_PROPERTIES_FILTER_ID = "configurationPropertiesFilter";
 
 	private final Sanitizer sanitizer = new Sanitizer();
 
@@ -174,7 +178,7 @@ public class ConfigurationPropertiesReportEndpoint
 	protected void configureObjectMapper(ObjectMapper mapper) {
 		mapper.configure(SerializationFeature.FAIL_ON_EMPTY_BEANS, false);
 		mapper.configure(SerializationFeature.WRITE_NULL_MAP_VALUES, false);
-		applyCglibFilters(mapper);
+		applyConfigurationPropertiesFilter(mapper);
 		applySerializationModifier(mapper);
 	}
 
@@ -188,15 +192,11 @@ public class ConfigurationPropertiesReportEndpoint
 		mapper.setSerializerFactory(factory);
 	}
 
-	/**
-	 * Configure PropertyFilter to make sure Jackson doesn't process CGLIB generated bean
-	 * properties.
-	 * @param mapper the object mapper
-	 */
-	private void applyCglibFilters(ObjectMapper mapper) {
-		mapper.setAnnotationIntrospector(new CglibAnnotationIntrospector());
-		mapper.setFilterProvider(new SimpleFilterProvider().addFilter(CGLIB_FILTER_ID,
-				new CglibBeanPropertyFilter()));
+	private void applyConfigurationPropertiesFilter(ObjectMapper mapper) {
+		mapper.setAnnotationIntrospector(
+				new ConfigurationPropertiesAnnotationIntrospector());
+		mapper.setFilterProvider(new SimpleFilterProvider()
+				.setDefaultFilter(new ConfigurationPropertiesPropertyFilter()));
 	}
 
 	/**
@@ -275,14 +275,14 @@ public class ConfigurationPropertiesReportEndpoint
 	 * properties.
 	 */
 	@SuppressWarnings("serial")
-	private static class CglibAnnotationIntrospector
+	private static class ConfigurationPropertiesAnnotationIntrospector
 			extends JacksonAnnotationIntrospector {
 
 		@Override
 		public Object findFilterId(Annotated a) {
 			Object id = super.findFilterId(a);
 			if (id == null) {
-				id = CGLIB_FILTER_ID;
+				id = CONFIGURATION_PROPERTIES_FILTER_ID;
 			}
 			return id;
 		}
@@ -290,10 +290,20 @@ public class ConfigurationPropertiesReportEndpoint
 	}
 
 	/**
-	 * {@link SimpleBeanPropertyFilter} to filter out all bean properties whose names
-	 * start with '$$'.
+	 * {@link SimpleBeanPropertyFilter} for serialization of
+	 * {@link ConfigurationProperties} beans. The filter hides:
+	 *
+	 * <ul>
+	 * <li>Properties that have a name starting with '$$'.
+	 * <li>Properties that are self-referential.
+	 * <li>Properties that throw an exception when retrieving their value.
+	 * </ul>
 	 */
-	private static class CglibBeanPropertyFilter extends SimpleBeanPropertyFilter {
+	private static class ConfigurationPropertiesPropertyFilter
+			extends SimpleBeanPropertyFilter {
+
+		private static final Log logger = LogFactory
+				.getLog(ConfigurationPropertiesPropertyFilter.class);
 
 		@Override
 		protected boolean include(BeanPropertyWriter writer) {
@@ -309,6 +319,31 @@ public class ConfigurationPropertiesReportEndpoint
 			return !name.startsWith("$$");
 		}
 
+		@Override
+		public void serializeAsField(Object pojo, JsonGenerator jgen,
+				SerializerProvider provider, PropertyWriter writer) throws Exception {
+			if (writer instanceof BeanPropertyWriter) {
+				try {
+					if (pojo == ((BeanPropertyWriter) writer).get(pojo)) {
+						if (logger.isDebugEnabled()) {
+							logger.debug("Skipping '" + writer.getFullName() + "' on '"
+									+ pojo.getClass().getName()
+									+ "' as it is self-referential");
+						}
+						return;
+					}
+				}
+				catch (Exception ex) {
+					if (logger.isDebugEnabled()) {
+						logger.debug("Skipping '" + writer.getFullName() + "' on '"
+								+ pojo.getClass().getName() + "' as an exception "
+								+ "was thrown when retrieving its value", ex);
+					}
+					return;
+				}
+			}
+			super.serializeAsField(pojo, jgen, provider, writer);
+		}
 	}
 
 	/**

spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/ConfigurationPropertiesReportEndpointSerializationTests.java

@@ -22,6 +22,7 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
+import com.zaxxer.hikari.HikariDataSource;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
@@ -100,8 +101,8 @@ public class ConfigurationPropertiesReportEndpointSerializationTests {
 
 	@Test
 	@SuppressWarnings("unchecked")
-	public void testCycle() throws Exception {
-		this.context.register(CycleConfig.class);
+	public void testSelfReferentialProperty() throws Exception {
+		this.context.register(SelfReferentialConfig.class);
 		EnvironmentTestUtils.addEnvironment(this.context, "foo.name:foo");
 		this.context.refresh();
 		ConfigurationPropertiesReportEndpoint report = this.context
@@ -114,8 +115,30 @@ public class ConfigurationPropertiesReportEndpointSerializationTests {
 		Map<String, Object> map = (Map<String, Object>) nestedProperties
 				.get("properties");
 		assertThat(map).isNotNull();
-		assertThat(map).hasSize(1);
-		assertThat(map.get("error")).isEqualTo("Cannot serialize 'foo'");
+		assertThat(map).containsOnlyKeys("bar", "name");
+		assertThat(map).containsEntry("name", "foo");
+		Map<String, Object> bar = (Map<String, Object>) map.get("bar");
+		assertThat(bar).containsOnlyKeys("name");
+		assertThat(bar).containsEntry("name", "123456");
+	}
+
+	@Test
+	@SuppressWarnings("unchecked")
+	public void testCycle() {
+		this.context.register(CycleConfig.class);
+		this.context.refresh();
+		ConfigurationPropertiesReportEndpoint report = this.context
+				.getBean(ConfigurationPropertiesReportEndpoint.class);
+		Map<String, Object> properties = report.invoke();
+		Map<String, Object> nestedProperties = (Map<String, Object>) properties
+				.get("cycle");
+		assertThat(nestedProperties).isNotNull();
+		assertThat(nestedProperties.get("prefix")).isEqualTo("cycle");
+		Map<String, Object> map = (Map<String, Object>) nestedProperties
+				.get("properties");
+		assertThat(map).isNotNull();
+		assertThat(map).containsOnlyKeys("error");
+		assertThat(map).containsEntry("error", "Cannot serialize 'cycle'");
 	}
 
 	@Test
@@ -149,7 +172,6 @@ public class ConfigurationPropertiesReportEndpointSerializationTests {
 		Map<String, Object> nestedProperties = (Map<String, Object>) properties
 				.get("foo");
 		assertThat(nestedProperties).isNotNull();
-		System.err.println(nestedProperties);
 		assertThat(nestedProperties.get("prefix")).isEqualTo("foo");
 		Map<String, Object> map = (Map<String, Object>) nestedProperties
 				.get("properties");
@@ -190,7 +212,6 @@ public class ConfigurationPropertiesReportEndpointSerializationTests {
 		Map<String, Object> nestedProperties = (Map<String, Object>) properties
 				.get("foo");
 		assertThat(nestedProperties).isNotNull();
-		System.err.println(nestedProperties);
 		assertThat(nestedProperties.get("prefix")).isEqualTo("foo");
 		Map<String, Object> map = (Map<String, Object>) nestedProperties
 				.get("properties");
@@ -223,6 +244,20 @@ public class ConfigurationPropertiesReportEndpointSerializationTests {
 		assertThat(list).containsExactly("abc");
 	}
 
+	@Test
+	@SuppressWarnings("unchecked")
+	public void hikariDataSourceConfigurationPropertiesBeanCanBeSerialized() {
+		this.context = new AnnotationConfigApplicationContext();
+		this.context.register(HikariDataSourceConfig.class);
+		this.context.refresh();
+		ConfigurationPropertiesReportEndpoint endpoint = this.context
+				.getBean(ConfigurationPropertiesReportEndpoint.class);
+		Map<String, Object> properties = endpoint.invoke();
+		Map<String, Object> nestedProperties = (Map<String, Object>) ((Map<String, Object>) properties
+				.get("hikariDataSource")).get("properties");
+		assertThat(nestedProperties).doesNotContainKey("error");
+	}
+
 	@Configuration
 	@EnableConfigurationProperties
 	public static class Base {
@@ -248,24 +283,12 @@ public class ConfigurationPropertiesReportEndpointSerializationTests {
 
 	@Configuration
 	@Import(Base.class)
-	public static class CycleConfig {
+	public static class SelfReferentialConfig {
 
 		@Bean
 		@ConfigurationProperties(prefix = "foo")
-		public Cycle foo() {
-			return new Cycle();
-		}
-
-	}
-
-	@Configuration
-	@Import(Base.class)
-	public static class MetadataCycleConfig {
-
-		@Bean
-		@ConfigurationProperties(prefix = "bar")
-		public Cycle foo() {
-			return new Cycle();
+		public SelfReferential foo() {
+			return new SelfReferential();
 		}
 
 	}
@@ -373,11 +396,11 @@ public class ConfigurationPropertiesReportEndpointSerializationTests {
 
 	}
 
-	public static class Cycle extends Foo {
+	public static class SelfReferential extends Foo {
 
 		private Foo self;
 
-		public Cycle() {
+		public SelfReferential() {
 			this.self = this;
 		}
 
@@ -449,4 +472,57 @@ public class ConfigurationPropertiesReportEndpointSerializationTests {
 
 	}
 
+	static class Cycle {
+
+		private final Alpha alpha = new Alpha(this);
+
+		public Alpha getAlpha() {
+			return this.alpha;
+		}
+
+		static class Alpha {
+
+			private final Cycle cycle;
+
+			Alpha(Cycle cycle) {
+				this.cycle = cycle;
+			}
+
+			public Cycle getCycle() {
+				return this.cycle;
+			}
+
+		}
+
+	}
+
+	@Configuration
+	@Import(Base.class)
+	static class CycleConfig {
+
+		@Bean
+		@ConfigurationProperties(prefix = "cycle")
+		public Cycle cycle() {
+			return new Cycle();
+		}
+
+	}
+
+	@Configuration
+	@EnableConfigurationProperties
+	static class HikariDataSourceConfig {
+
+		@Bean
+		public ConfigurationPropertiesReportEndpoint endpoint() {
+			return new ConfigurationPropertiesReportEndpoint();
+		}
+
+		@Bean
+		@ConfigurationProperties(prefix = "test.datasource")
+		public HikariDataSource hikariDataSource() {
+			return new HikariDataSource();
+		}
+
+	}
+
 }