Client/ServerRequest defensively accesses internal attribute map

Issue: SPR-17486

Author: jhoeller

spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ClientRequest.java

@@ -76,13 +76,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));
 	}
 
 	/**
@@ -91,8 +85,7 @@ default Optional<Object> attribute(String name) {
 	Map<String, Object> attributes();
 
 	/**
-	 * Writes this request to the given {@link ClientHttpRequest}.
-	 *
+	 * Write this request to the given {@link ClientHttpRequest}.
 	 * @param request the client http request to write to
 	 * @param strategies the strategies to use when writing
 	 * @return {@code Mono<Void>} to indicate when writing is complete

spring-webflux/src/main/java/org/springframework/web/reactive/function/server/ServerRequest.java

@@ -50,7 +50,8 @@
 
 /**
  * Represents a server-side HTTP request, as handled by a {@code HandlerFunction}.
- * Access to headers and body is offered by {@link Headers} and
+ *
+ * <p>Access to headers and body is offered by {@link Headers} and
  * {@link #body(BodyExtractor)}, respectively.
  *
  * @author Arjen Poutsma
@@ -60,7 +61,7 @@
 public interface ServerRequest {
 
 	/**
-	 * Return the HTTP method.
+	 * Get the HTTP method.
 	 * @return the HTTP method as an HttpMethod enum value, or {@code null}
 	 * if not resolvable (e.g. in case of a non-standard HTTP method)
 	 */
@@ -70,18 +71,18 @@ default HttpMethod method() {
 	}
 
 	/**
-	 * Return the name of the HTTP method.
+	 * Get the name of the HTTP method.
 	 * @return the HTTP method as a String
 	 */
 	String methodName();
 
 	/**
-	 * Return the request URI.
+	 * Get the request URI.
 	 */
 	URI uri();
 
 	/**
-	 * Return a {@code UriBuilderComponents}  from the URI associated with this
+	 * Get a {@code UriBuilderComponents}  from the URI associated with this
 	 * {@code ServerRequest}, while also overlaying with values from the headers
 	 * "Forwarded" (<a href="http://tools.ietf.org/html/rfc7239">RFC 7239</a>),
 	 * or "X-Forwarded-Host", "X-Forwarded-Port", and "X-Forwarded-Proto" if
@@ -91,26 +92,26 @@ default HttpMethod method() {
 	UriBuilder uriBuilder();
 
 	/**
-	 * Return the request path.
+	 * Get the request path.
 	 */
 	default String path() {
 		return uri().getRawPath();
 	}
 
 	/**
-	 * Return the request path as {@code PathContainer}.
+	 * Get the request path as a {@code PathContainer}.
 	 */
 	default PathContainer pathContainer() {
 		return PathContainer.parsePath(path());
 	}
 
 	/**
-	 * Return the headers of this request.
+	 * Get the headers of this request.
 	 */
 	Headers headers();
 
 	/**
-	 * Return the cookies of this request.
+	 * Get the cookies of this request.
 	 */
 	MultiValueMap<String, HttpCookie> cookies();
 
@@ -166,28 +167,22 @@ default PathContainer pathContainer() {
 	<T> Flux<T> bodyToFlux(ParameterizedTypeReference<T> typeReference);
 
 	/**
-	 * Return the request attribute value if present.
+	 * Get the request attribute value if present.
 	 * @param name the attribute name
 	 * @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));
 	}
 
 	/**
-	 * Return a mutable map of request attributes.
+	 * Get a mutable map of request attributes.
 	 * @return the request attributes
 	 */
 	Map<String, Object> attributes();
 
 	/**
-	 * Return the first query parameter with the given name, if present.
+	 * Get the first query parameter with the given name, if present.
 	 * @param name the parameter name
 	 * @return the parameter value
 	 */
@@ -206,12 +201,12 @@ default Optional<String> queryParam(String name) {
 	}
 
 	/**
-	 * Return all query parameters for this request.
+	 * Get all query parameters for this request.
 	 */
 	MultiValueMap<String, String> queryParams();
 
 	/**
-	 * Return the path variable with the given name, if present.
+	 * Get the path variable with the given name, if present.
 	 * @param name the variable name
 	 * @return the variable value
 	 * @throws IllegalArgumentException if there is no path variable with the given name
@@ -227,38 +222,38 @@ default String pathVariable(String name) {
 	}
 
 	/**
-	 * Return all path variables for this request.
+	 * Get all path variables for this request.
 	 */
 	Map<String, String> pathVariables();
 
 	/**
-	 * Return the web session for this request. Always guaranteed to
-	 * return an instance either matching to the session id requested by the
-	 * client, or with a new session id either because the client did not
-	 * specify one or because the underlying session had expired. Use of this
-	 * method does not automatically create a session.
+	 * Get the web session for this request.
+	 * <p>Always guaranteed to return an instance either matching the session id
+	 * requested by the client, or with a new session id either because the client
+	 * did not specify one or because the underlying session had expired.
+	 * <p>Use of this method does not automatically create a session.
 	 */
 	Mono<WebSession> session();
 
 	/**
-	 * Return the authenticated user for the request, if any.
+	 * Get the authenticated user for the request, if any.
 	 */
 	Mono<? extends Principal> principal();
 
 	/**
-	 * Return the form data from the body of the request if the Content-Type is
+	 * Get the form data from the body of the request if the Content-Type is
 	 * {@code "application/x-www-form-urlencoded"} or an empty map otherwise.
 	 * <p><strong>Note:</strong> calling this method causes the request body to
-	 * be read and parsed in full and the resulting {@code MultiValueMap} is
+	 * be read and parsed in full, and the resulting {@code MultiValueMap} is
 	 * cached so that this method is safe to call more than once.
 	 */
 	Mono<MultiValueMap<String, String>> formData();
 
 	/**
-	 * Return the parts of a multipart request if the Content-Type is
+	 * Get the parts of a multipart request if the Content-Type is
 	 * {@code "multipart/form-data"} or an empty map otherwise.
 	 * <p><strong>Note:</strong> calling this method causes the request body to
-	 * be read and parsed in full and the resulting {@code MultiValueMap} is
+	 * be read and parsed in full, and the resulting {@code MultiValueMap} is
 	 * cached so that this method is safe to call more than once.
 	 */
 	Mono<MultiValueMap<String, Part>> multipartData();
@@ -285,59 +280,60 @@ static ServerRequest create(ServerWebExchange exchange, List<HttpMessageReader<?
 	interface Headers {
 
 		/**
-		 * Return the list of acceptable {@code MediaType media types},
-		 * as specified by the {@code Accept} header.
-		 * <p>Returns an empty list when the acceptable media types are unspecified.
+		 * Get the list of acceptable media types, as specified by the {@code Accept}
+		 * header.
+		 * <p>Returns an empty list if the acceptable media types are unspecified.
 		 */
 		List<MediaType> accept();
 
 		/**
-		 * Return the list of acceptable {@code Charset charsets},
-		 * as specified by the {@code Accept-Charset} header.
+		 * Get the list of acceptable charsets, as specified by the
+		 * {@code Accept-Charset} header.
 		 */
 		List<Charset> acceptCharset();
 
 		/**
-		 * Return the list of acceptable {@code Locale.LanguageRange languages},
-		 * as specified by the {@code Accept-Language} header.
+		 * Get the list of acceptable languages, as specified by the
+		 * {@code Accept-Language} header.
 		 */
 		List<Locale.LanguageRange> acceptLanguage();
 
 		/**
-		 * Return the length of the body in bytes, as specified by the
+		 * Get the length of the body in bytes, as specified by the
 		 * {@code Content-Length} header.
 		 */
 		OptionalLong contentLength();
 
 		/**
-		 * Return the {@code MediaType media type} of the body, as specified
-		 * by the {@code Content-Type} header.
+		 * Get the media type of the body, as specified by the
+		 * {@code Content-Type} header.
 		 */
 		Optional<MediaType> contentType();
 
 		/**
-		 * Return the value of the required {@code Host} header.
-		 * <p>If the header value does not contain a port, the returned
-		 * {@linkplain InetSocketAddress#getPort() port} will be {@code 0}.
+		 * Get the value of the {@code Host} header, if available.
+		 * <p>If the header value does not contain a port, the
+		 * {@linkplain InetSocketAddress#getPort() port} in the returned address will
+		 * be {@code 0}.
 		 */
 		@Nullable
 		InetSocketAddress host();
 
 		/**
-		 * Return the value of the {@code Range} header.
+		 * Get the value of the {@code Range} header.
 		 * <p>Returns an empty list when the range is unknown.
 		 */
 		List<HttpRange> range();
 
 		/**
-		 * Return the header value(s), if any, for the header of the given name.
-		 * <p>Return an empty list if no header values are found.
+		 * Get the header value(s), if any, for the header of the given name.
+		 * <p>Returns an empty list if no header values are found.
 		 * @param headerName the header name
 		 */
 		List<String> header(String headerName);
 
 		/**
-		 * Return the headers as a {@link HttpHeaders} instance.
+		 * Get the headers as an instance of {@link HttpHeaders}.
 		 */
 		HttpHeaders asHttpHeaders();
 	}