From d96a7b4bfc50b40723b3edeba793243af542e027 Mon Sep 17 00:00:00 2001
From: Juergen Hoeller <jhoeller@pivotal.io>
Date: Mon, 12 Nov 2018 11:52:21 +0100
Subject: [PATCH] ClientRequest.attribute(name) defensively handles null value

Issue: SPR-17486
---
 .../function/client/ClientRequest.java        |  8 +----
 .../client/DefaultWebClientTests.java         | 34 +++++++++++++------
 2 files changed, 24 insertions(+), 18 deletions(-)

diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ClientRequest.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ClientRequest.java
index 04e5edb1de..7130039994 100644
--- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ClientRequest.java
+++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ClientRequest.java
@@ -86,13 +86,7 @@ public interface ClientRequest {
 	 * @return the attribute value
 	 */
 	default Optional<Object> attribute(String name) {
-		Map<String, Object> attributes = attributes();
-		if (attributes.containsKey(name)) {
-			return Optional.of(attributes.get(name));
-		}
-		else {
-			return Optional.empty();
-		}
+		return Optional.ofNullable(attributes().get(name));
 	}
 
 	/**
diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultWebClientTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultWebClientTests.java
index ab8997f04a..b7d73c26a7 100644
--- a/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultWebClientTests.java
+++ b/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultWebClientTests.java
@@ -62,7 +62,6 @@ public class DefaultWebClientTests {
 
 	@Test
 	public void basic() {
-
 		this.builder.build().get().uri("/path").exchange();
 
 		ClientRequest request = verifyAndGetRequest();
@@ -73,7 +72,6 @@ public class DefaultWebClientTests {
 
 	@Test
 	public void uriBuilder() {
-
 		this.builder.build().get()
 				.uri(builder -> builder.path("/path").queryParam("q", "12").build())
 				.exchange();
@@ -85,7 +83,6 @@ public class DefaultWebClientTests {
 
 	@Test
 	public void uriBuilderWithPathOverride() {
-
 		this.builder.build().get()
 				.uri(builder -> builder.replacePath("/path").build())
 				.exchange();
@@ -97,7 +94,6 @@ public class DefaultWebClientTests {
 
 	@Test
 	public void requestHeaderAndCookie() {
-
 		this.builder.build().get().uri("/path").accept(MediaType.APPLICATION_JSON)
 				.cookies(cookies -> cookies.add("id", "123"))	// SPR-16178
 				.exchange();
@@ -110,7 +106,6 @@ public class DefaultWebClientTests {
 
 	@Test
 	public void defaultHeaderAndCookie() {
-
 		WebClient client = this.builder
 				.defaultHeader("Accept", "application/json").defaultCookie("id", "123")
 				.build();
@@ -125,7 +120,6 @@ public class DefaultWebClientTests {
 
 	@Test
 	public void defaultHeaderAndCookieOverrides() {
-
 		WebClient client = this.builder
 				.defaultHeader("Accept", "application/json")
 				.defaultCookie("id", "123")
@@ -141,7 +135,6 @@ public class DefaultWebClientTests {
 
 	@Test
 	public void defaultRequest() {
-
 		ThreadLocal<String> context = new NamedThreadLocal<>("foo");
 
 		Map<String, Object> actual = new HashMap<>();
@@ -176,7 +169,6 @@ public class DefaultWebClientTests {
 
 	@Test
 	public void mutateDoesCopy() {
-
 		// First, build the clients
 
 		WebClient.Builder builder = WebClient.builder()
@@ -216,8 +208,7 @@ public class DefaultWebClientTests {
 	}
 
 	@Test
-	public void attributes() {
-
+	public void withStringAttribute() {
 		Map<String, Object> actual = new HashMap<>();
 		ExchangeFilterFunction filter = (request, next) -> {
 			actual.putAll(request.attributes());
@@ -230,11 +221,32 @@ public class DefaultWebClientTests {
 				.exchange();
 
 		assertEquals("bar", actual.get("foo"));
+
+		ClientRequest request = verifyAndGetRequest();
+		assertEquals("bar", request.attribute("foo").get());
+	}
+
+	@Test
+	public void withNullAttribute() {
+		Map<String, Object> actual = new HashMap<>();
+		ExchangeFilterFunction filter = (request, next) -> {
+			actual.putAll(request.attributes());
+			return next.exchange(request);
+		};
+
+		this.builder.filter(filter).build()
+				.get().uri("/path")
+				.attribute("foo", null)
+				.exchange();
+
+		assertNull(actual.get("foo"));
+
+		ClientRequest request = verifyAndGetRequest();
+		assertFalse(request.attribute("foo").isPresent());
 	}
 
 	@Test
 	public void apply() {
-
 		WebClient client = this.builder
 				.apply(builder -> builder
 						.defaultHeader("Accept", "application/json")