From ba30ee03dfe9c92184b25b2a38a8d7a55cf47c72 Mon Sep 17 00:00:00 2001
From: Phillip Webb <pwebb@pivotal.io>
Date: Wed, 23 Oct 2019 13:05:01 -0700
Subject: [PATCH] Improve health contributor null support

Update `NamedContributorsMapAdapter` to check for `null` keys or values
during construction. Also update `HealthEndpointSupport` to allow
null component entries.

See gh-18687
---
 .../actuate/health/HealthEndpointSupport.java    |  8 ++++++--
 .../health/NamedContributorsMapAdapter.java      |  3 +++
 .../health/NamedContributorsMapAdapterTests.java | 16 ++++++++++++++++
 3 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/HealthEndpointSupport.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/HealthEndpointSupport.java
index 848f1786c1..932bfdbe45 100644
--- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/HealthEndpointSupport.java
+++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/HealthEndpointSupport.java
@@ -134,8 +134,8 @@ abstract class HealthEndpointSupport<C, T> {
 
 	protected final CompositeHealth getCompositeHealth(ApiVersion apiVersion, Map<String, HealthComponent> components,
 			StatusAggregator statusAggregator, boolean showComponents, Set<String> groupNames) {
-		Status status = statusAggregator.getAggregateStatus(
-				components.values().stream().map(HealthComponent::getStatus).collect(Collectors.toSet()));
+		Status status = statusAggregator
+				.getAggregateStatus(components.values().stream().map(this::getStatus).collect(Collectors.toSet()));
 		Map<String, HealthComponent> instances = showComponents ? components : null;
 		if (groupNames != null) {
 			return new SystemHealth(apiVersion, status, instances, groupNames);
@@ -143,6 +143,10 @@ abstract class HealthEndpointSupport<C, T> {
 		return new CompositeHealth(apiVersion, status, instances);
 	}
 
+	private Status getStatus(HealthComponent component) {
+		return (component != null) ? component.getStatus() : Status.UNKNOWN;
+	}
+
 	/**
 	 * A health result containing health and the group that created it.
 	 *
diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/NamedContributorsMapAdapter.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/NamedContributorsMapAdapter.java
index 7a35c3eed3..82ffcd46b5 100644
--- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/NamedContributorsMapAdapter.java
+++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/NamedContributorsMapAdapter.java
@@ -43,6 +43,9 @@ abstract class NamedContributorsMapAdapter<V, C> implements NamedContributors<C>
 	NamedContributorsMapAdapter(Map<String, V> map, Function<V, ? extends C> valueAdapter) {
 		Assert.notNull(map, "Map must not be null");
 		Assert.notNull(valueAdapter, "ValueAdapter must not be null");
+		map.keySet().stream().forEach((key) -> Assert.notNull(key, "Map must not contain null keys"));
+		map.values().stream().map(valueAdapter)
+				.forEach((value) -> Assert.notNull(value, "Map must not contain null values"));
 		this.map = Collections.unmodifiableMap(new LinkedHashMap<>(map));
 		this.valueAdapter = valueAdapter;
 	}
diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/health/NamedContributorsMapAdapterTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/health/NamedContributorsMapAdapterTests.java
index 6befd8cf98..9160356992 100644
--- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/health/NamedContributorsMapAdapterTests.java
+++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/health/NamedContributorsMapAdapterTests.java
@@ -48,6 +48,22 @@ class NamedContributorsMapAdapterTests {
 				.withMessage("ValueAdapter must not be null");
 	}
 
+	@Test
+	void createWhenMapContainsNullValueThrowsException() {
+		assertThatIllegalArgumentException()
+				.isThrownBy(() -> new TestNamedContributorsMapAdapter<>(Collections.singletonMap("test", null),
+						Function.identity()))
+				.withMessage("Map must not contain null values");
+	}
+
+	@Test
+	void createWhenMapContainsNullKeyThrowsException() {
+		assertThatIllegalArgumentException()
+				.isThrownBy(() -> new TestNamedContributorsMapAdapter<>(Collections.singletonMap(null, "test"),
+						Function.identity()))
+				.withMessage("Map must not contain null keys");
+	}
+
 	@Test
 	void iterateReturnsAdaptedEntries() {
 		TestNamedContributorsMapAdapter<String> adapter = createAdapter();