impl: HttpHeaders.backedBy and HttpHeaders.copyOf to reduce ambiguity…

… on the constructor.

chore: Update constructor references with backedBy method.
chore: Remove `@Deprecated` annotations.

Fixes #34340

Signed-off-by: Bryce J. Fisher <[email protected]>

spring-web/src/main/java/org/springframework/http/HttpHeaders.java

@@ -491,6 +491,49 @@ public HttpHeaders(HttpHeaders httpHeaders) {
 		}
 	}
 
+	/**
+	 * Construct a new {@code HttpHeaders} instance backed by an existing map.
+	 * <p>This factory method is available as an optimization for adapting to existing
+	 * headers map structures, primarily for internal use within the framework.
+	 * @param headers the headers map (expected to operate with case-insensitive keys)
+	 * @see #HttpHeaders(MultiValueMap)
+	 */
+	public static HttpHeaders backedBy(MultiValueMap<String, String> headers) {
+		return new HttpHeaders(headers);
+	}
+
+	/**
+	 * Construct a new {@code HttpHeaders} instance by removing any read-only
+	 * wrapper that may have been previously applied around the given
+	 * {@code HttpHeaders} via {@link #readOnlyHttpHeaders(HttpHeaders)}.
+	 * <p>Once the writable instance is mutated, the read-only instance is
+	 * likely to be out of sync and should be discarded.
+	 * @param httpHeaders the headers to expose
+	 * @see #HttpHeaders(HttpHeaders)
+	 */
+	public static HttpHeaders backedBy(HttpHeaders httpHeaders) {
+		return new HttpHeaders(httpHeaders);
+	}
+
+	/**
+	 * Constructs a new {@code HttpHeaders} instance with the given headers with a new underlying map.
+	 * Changes made to this new instance will NOT be reflected in the original headers.
+	 * @param headers The headers to copy
+	 */
+	public static HttpHeaders copyOf(MultiValueMap<String, String> headers) {
+		HttpHeaders httpHeadersCopy = new HttpHeaders();
+		headers.forEach((key, values) -> httpHeadersCopy.put(key, new ArrayList<>(values)));
+		return httpHeadersCopy;
+	}
+
+	/**
+	 * Constructs a new {@code HttpHeaders} instance with the given headers with a new underlying map.
+	 * Changes made to this new instance will NOT be reflected in the original headers.
+	 * @param httpHeaders The headers to copy
+	 */
+	public static HttpHeaders copyOf(HttpHeaders httpHeaders) {
+		return copyOf(httpHeaders.headers);
+	}
 
 	/**
 	 * Get the list of header values for the given header name, if any.

spring-web/src/main/java/org/springframework/http/ReadOnlyHttpHeaders.java

@@ -36,6 +36,7 @@
  * <p>This caches the parsed representations of the "Accept" and "Content-Type" headers
  * and will get out of sync with the backing map it is mutated at runtime.
  *
+ * @implNote This does NOT make the underlying MultiValueMap readonly.
  * @author Brian Clozel
  * @author Sam Brannen
  * @since 5.1.1
@@ -49,7 +50,6 @@ class ReadOnlyHttpHeaders extends HttpHeaders {
 	@SuppressWarnings("serial")
 	private @Nullable List<MediaType> cachedAccept;
 
-
 	ReadOnlyHttpHeaders(MultiValueMap<String, String> headers) {
 		super(headers);
 	}

spring-web/src/main/java/org/springframework/http/codec/multipart/PartHttpMessageWriter.java

@@ -91,7 +91,7 @@ public Mono<Void> write(Publisher<? extends Part> parts,
 	}
 
 	private <T> Flux<DataBuffer> encodePart(byte[] boundary, Part part, DataBufferFactory bufferFactory) {
-		HttpHeaders headers = new HttpHeaders(part.headers());
+		HttpHeaders headers = HttpHeaders.backedBy(part.headers());
 
 		String name = part.name();
 		if (!headers.containsHeader(HttpHeaders.CONTENT_DISPOSITION)) {

spring-web/src/main/java/org/springframework/http/server/reactive/JettyCoreServerHttpRequest.java

@@ -56,7 +56,7 @@ public JettyCoreServerHttpRequest(Request request, JettyDataBufferFactory dataBu
 		super(HttpMethod.valueOf(request.getMethod()),
 				request.getHttpURI().toURI(),
 				request.getContext().getContextPath(),
-				new HttpHeaders(new JettyHeadersAdapter(request.getHeaders())));
+				HttpHeaders.backedBy(new JettyHeadersAdapter(request.getHeaders())));
 		this.dataBufferFactory = dataBufferFactory;
 		this.request = request;
 	}

spring-web/src/main/java/org/springframework/http/server/reactive/JettyCoreServerHttpResponse.java

@@ -56,7 +56,7 @@ class JettyCoreServerHttpResponse extends AbstractServerHttpResponse implements
 
 
 	public JettyCoreServerHttpResponse(Response response, JettyDataBufferFactory dataBufferFactory) {
-		super(dataBufferFactory, new HttpHeaders(new JettyHeadersAdapter(response.getHeaders())));
+		super(dataBufferFactory, HttpHeaders.backedBy(new JettyHeadersAdapter(response.getHeaders())));
 		this.response = response;
 
 		// remove all existing cookies from the response and add them to the cookie map, to be added back later

spring-web/src/main/java/org/springframework/http/server/reactive/ReactorNetty2ServerHttpRequest.java

@@ -71,7 +71,7 @@ public ReactorNetty2ServerHttpRequest(HttpServerRequest request, Netty5DataBuffe
 			throws URISyntaxException {
 
 		super(HttpMethod.valueOf(request.method().name()), initUri(request), "",
-				new HttpHeaders(new Netty5HeadersAdapter(request.requestHeaders())));
+				HttpHeaders.backedBy(new Netty5HeadersAdapter(request.requestHeaders())));
 		Assert.notNull(bufferFactory, "DataBufferFactory must not be null");
 		this.request = request;
 		this.bufferFactory = bufferFactory;

spring-web/src/main/java/org/springframework/http/server/reactive/ReactorNetty2ServerHttpResponse.java

@@ -58,7 +58,7 @@ class ReactorNetty2ServerHttpResponse extends AbstractServerHttpResponse impleme
 
 
 	public ReactorNetty2ServerHttpResponse(HttpServerResponse response, DataBufferFactory bufferFactory) {
-		super(bufferFactory, new HttpHeaders(new Netty5HeadersAdapter(response.responseHeaders())));
+		super(bufferFactory, HttpHeaders.backedBy(new Netty5HeadersAdapter(response.responseHeaders())));
 		Assert.notNull(response, "HttpServerResponse must not be null");
 		this.response = response;
 	}

spring-web/src/main/java/org/springframework/http/server/reactive/ReactorServerHttpRequest.java

@@ -68,7 +68,7 @@ public ReactorServerHttpRequest(HttpServerRequest request, NettyDataBufferFactor
 
 		super(HttpMethod.valueOf(request.method().name()),
 				ReactorUriHelper.createUri(request), request.forwardedPrefix(),
-				new HttpHeaders(new Netty4HeadersAdapter(request.requestHeaders())));
+				HttpHeaders.backedBy(new Netty4HeadersAdapter(request.requestHeaders())));
 		Assert.notNull(bufferFactory, "DataBufferFactory must not be null");
 		this.request = request;
 		this.bufferFactory = bufferFactory;

spring-web/src/main/java/org/springframework/http/server/reactive/ReactorServerHttpResponse.java

@@ -57,7 +57,7 @@ class ReactorServerHttpResponse extends AbstractServerHttpResponse implements Ze
 
 
 	public ReactorServerHttpResponse(HttpServerResponse response, DataBufferFactory bufferFactory) {
-		super(bufferFactory, new HttpHeaders(new Netty4HeadersAdapter(Objects.requireNonNull(response,
+		super(bufferFactory, HttpHeaders.backedBy(new Netty4HeadersAdapter(Objects.requireNonNull(response,
 				"HttpServerResponse must not be null").responseHeaders())));
 		this.response = response;
 	}

spring-web/src/main/java/org/springframework/http/server/reactive/ServletServerHttpRequest.java

@@ -168,7 +168,7 @@ private static HttpHeaders initHeaders(HttpHeaders headerValues, HttpServletRequ
 			String requestContentType = request.getContentType();
 			if (StringUtils.hasLength(requestContentType)) {
 				contentType = MediaType.parseMediaType(requestContentType);
-				headers = new HttpHeaders(headerValues);
+				headers = HttpHeaders.backedBy(headerValues);
 				headers.setContentType(contentType);
 			}
 		}
@@ -184,7 +184,7 @@ private static HttpHeaders initHeaders(HttpHeaders headerValues, HttpServletRequ
 		if (headerValues.getFirst(HttpHeaders.CONTENT_TYPE) == null) {
 			int contentLength = request.getContentLength();
 			if (contentLength != -1) {
-				headers = (headers != null ? headers : new HttpHeaders(headerValues));
+				headers = (headers != null ? headers : HttpHeaders.backedBy(headerValues));
 				headers.setContentLength(contentLength);
 			}
 		}

spring-web/src/main/java/org/springframework/http/server/reactive/TomcatHttpHandlerAdapter.java

@@ -96,7 +96,7 @@ private static HttpHeaders createTomcatHttpHeaders(HttpServletRequest request) {
 					ReflectionUtils.getField(COYOTE_REQUEST_FIELD, requestFacade);
 			Assert.state(connectorRequest != null, "No Tomcat connector request");
 			Request tomcatRequest = connectorRequest.getCoyoteRequest();
-			return new HttpHeaders(new TomcatHeadersAdapter(tomcatRequest.getMimeHeaders()));
+			return HttpHeaders.backedBy(new TomcatHeadersAdapter(tomcatRequest.getMimeHeaders()));
 		}
 
 		private static RequestFacade getRequestFacade(HttpServletRequest request) {
@@ -140,7 +140,7 @@ private static HttpHeaders createTomcatHttpHeaders(HttpServletResponse response)
 			Assert.state(connectorResponse != null, "No Tomcat connector response");
 			Response tomcatResponse = connectorResponse.getCoyoteResponse();
 			TomcatHeadersAdapter headers = new TomcatHeadersAdapter(tomcatResponse.getMimeHeaders());
-			return new HttpHeaders(headers);
+			return HttpHeaders.backedBy(headers);
 		}
 
 		private static ResponseFacade getResponseFacade(HttpServletResponse response) {

spring-web/src/main/java/org/springframework/http/server/reactive/UndertowServerHttpRequest.java

@@ -66,7 +66,7 @@ public UndertowServerHttpRequest(HttpServerExchange exchange, DataBufferFactory
 			throws URISyntaxException {
 
 		super(HttpMethod.valueOf(exchange.getRequestMethod().toString()), initUri(exchange), "",
-				new HttpHeaders(new UndertowHeadersAdapter(exchange.getRequestHeaders())));
+				HttpHeaders.backedBy(new UndertowHeadersAdapter(exchange.getRequestHeaders())));
 		this.exchange = exchange;
 		this.body = new RequestBodyPublisher(exchange, bufferFactory);
 		this.body.registerListeners(exchange);

spring-web/src/main/java/org/springframework/http/server/reactive/UndertowServerHttpResponse.java

@@ -70,7 +70,7 @@ class UndertowServerHttpResponse extends AbstractListenerServerHttpResponse impl
 	private static HttpHeaders createHeaders(HttpServerExchange exchange) {
 		Assert.notNull(exchange, "HttpServerExchange must not be null");
 		UndertowHeadersAdapter headersMap = new UndertowHeadersAdapter(exchange.getResponseHeaders());
-		return new HttpHeaders(headersMap);
+		return HttpHeaders.backedBy(headersMap);
 	}
 
 

spring-web/src/test/java/org/springframework/http/HttpHeadersTests.java

@@ -57,24 +57,79 @@ class HttpHeadersTests {
 
 	final HttpHeaders headers = new HttpHeaders();
 
+	/**
+	 * Not the different behaviors found in {@link HttpHeaders#backedBy(HttpHeaders)) {@link HttpHeaders#HttpHeaders(HttpHeaders)}
+	 * versus the {@link HttpHeaders#copyOf(HttpHeaders)} methods.
+	 */
 	@Test
-	void constructorUnwrapsReadonly() {
+	void backedByFactoryUnwrapsReadonly() {
 		headers.setContentType(MediaType.APPLICATION_JSON);
 		HttpHeaders readOnly = HttpHeaders.readOnlyHttpHeaders(headers);
+
 		assertThat(readOnly.getContentType()).isEqualTo(MediaType.APPLICATION_JSON);
-		HttpHeaders writable = new HttpHeaders(readOnly);
+		HttpHeaders writable = HttpHeaders.backedBy(readOnly);
 		writable.setContentType(MediaType.TEXT_PLAIN);
+
 		// content-type value is cached by ReadOnlyHttpHeaders
+		assertThat(readOnly.getContentType())
+				.describedAs("readOnly HttpHeaders should NOT have updated content-type value")
+				.isEqualTo(MediaType.APPLICATION_JSON); // Note that the content type was cached by the previous assertion
+		assertThat(writable.getContentType())
+				.describedAs("writable HttpHeaders should have updated content-type value")
+				.isEqualTo(MediaType.TEXT_PLAIN);
+		assertThat(headers.getContentType())
+				.describedAs("initial HttpHeaders should have updated content-type value")
+				.isEqualTo(MediaType.TEXT_PLAIN); // Note that both writable and the original changed because the backing map is shared
+	}
+
+	@Test
+	void backedByFactoryUnwrapsMultipleHeaders() {
+		headers.setContentType(MediaType.APPLICATION_JSON);
+		HttpHeaders readonlyOriginalExchangeHeaders = HttpHeaders.readOnlyHttpHeaders(headers);
+		HttpHeaders firewallHeaders = HttpHeaders.backedBy(readonlyOriginalExchangeHeaders);
+		HttpHeaders writeable = HttpHeaders.backedBy(firewallHeaders);
+		writeable.setContentType(MediaType.TEXT_PLAIN);
+
+		// If readonly headers are unwrapped multiple times, the content-type value should be updated
+		assertThat(writeable.getContentType()).isEqualTo(MediaType.TEXT_PLAIN); // Note that all changed because the backing map is shared
+		assertThat(firewallHeaders.getContentType()).isEqualTo(MediaType.TEXT_PLAIN);
+		assertThat(readonlyOriginalExchangeHeaders.getContentType()).isEqualTo(MediaType.TEXT_PLAIN); // Note that the content type was not cached because there was no previous call to getContentType
+		assertThat(headers.getContentType()).isEqualTo(MediaType.TEXT_PLAIN);
+	}
+
+	@Test
+	void copyOfFactoryUnwrapsReadonly() {
+		headers.setContentType(MediaType.APPLICATION_JSON);
+		HttpHeaders readOnly = HttpHeaders.readOnlyHttpHeaders(headers);
+
 		assertThat(readOnly.getContentType()).isEqualTo(MediaType.APPLICATION_JSON);
-		assertThat(writable.getContentType()).isEqualTo(MediaType.TEXT_PLAIN);
+		HttpHeaders writable = HttpHeaders.copyOf(readOnly);
+		writable.setContentType(MediaType.TEXT_PLAIN);
+
+		// content-type value is cached by ReadOnlyHttpHeaders
+		assertThat(readOnly.getContentType())
+				.describedAs("readOnly HttpHeaders should NOT have updated content-type value")
+				.isEqualTo(MediaType.APPLICATION_JSON); // Note that the content type was cached by the previous assertion
+		assertThat(writable.getContentType())
+				.describedAs("writable HttpHeaders should have updated content-type value")
+				.isEqualTo(MediaType.TEXT_PLAIN); // Note that only the copy is changed, because the backing map is a new instance
+		assertThat(headers.getContentType())
+				.describedAs("initial HttpHeaders should have updated content-type value")
+				.isEqualTo(MediaType.APPLICATION_JSON);
 	}
 
 	@Test
-	void writableHttpHeadersUnwrapsMultiple() {
-		HttpHeaders originalExchangeHeaders = HttpHeaders.readOnlyHttpHeaders(new HttpHeaders());
-		HttpHeaders firewallHeaders = new HttpHeaders(originalExchangeHeaders);
-		HttpHeaders writeable = new HttpHeaders(firewallHeaders);
-		writeable.setContentType(MediaType.APPLICATION_JSON);
+	void copyOfFactoryUnwrapsMultipleHeaders() {
+		headers.setContentType(MediaType.APPLICATION_JSON);
+		HttpHeaders readonlyOriginalExchangeHeaders = HttpHeaders.readOnlyHttpHeaders(headers);
+		HttpHeaders firewallHeaders = HttpHeaders.copyOf(readonlyOriginalExchangeHeaders);
+		HttpHeaders writeable = HttpHeaders.copyOf(firewallHeaders);
+		writeable.setContentType(MediaType.TEXT_PLAIN);
+
+		assertThat(writeable.getContentType()).isEqualTo(MediaType.TEXT_PLAIN); // Note that only the copy is changed, because the backing map is a new instance
+		assertThat(firewallHeaders.getContentType()).isEqualTo(MediaType.APPLICATION_JSON);
+		assertThat(readonlyOriginalExchangeHeaders.getContentType()).isEqualTo(MediaType.APPLICATION_JSON);
+		assertThat(headers.getContentType()).isEqualTo(MediaType.APPLICATION_JSON);
 	}
 
 	@Test