diff --git a/spring-web/src/main/java/org/springframework/http/HttpHeaders.java b/spring-web/src/main/java/org/springframework/http/HttpHeaders.java index f0432d50502a..86f2158b905f 100644 --- a/spring-web/src/main/java/org/springframework/http/HttpHeaders.java +++ b/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. + *

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 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)}. + *

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 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. diff --git a/spring-web/src/main/java/org/springframework/http/ReadOnlyHttpHeaders.java b/spring-web/src/main/java/org/springframework/http/ReadOnlyHttpHeaders.java index eb5fafdc41d2..99a5df9afdcb 100644 --- a/spring-web/src/main/java/org/springframework/http/ReadOnlyHttpHeaders.java +++ b/spring-web/src/main/java/org/springframework/http/ReadOnlyHttpHeaders.java @@ -36,6 +36,7 @@ *

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 cachedAccept; - ReadOnlyHttpHeaders(MultiValueMap headers) { super(headers); } diff --git a/spring-web/src/main/java/org/springframework/http/codec/multipart/PartHttpMessageWriter.java b/spring-web/src/main/java/org/springframework/http/codec/multipart/PartHttpMessageWriter.java index fe33e3b2114c..103838ca4cab 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/multipart/PartHttpMessageWriter.java +++ b/spring-web/src/main/java/org/springframework/http/codec/multipart/PartHttpMessageWriter.java @@ -91,7 +91,7 @@ public Mono write(Publisher parts, } private Flux 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)) { diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/JettyCoreServerHttpRequest.java b/spring-web/src/main/java/org/springframework/http/server/reactive/JettyCoreServerHttpRequest.java index a43730b8b445..8ddfe704e3af 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/JettyCoreServerHttpRequest.java +++ b/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; } diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/JettyCoreServerHttpResponse.java b/spring-web/src/main/java/org/springframework/http/server/reactive/JettyCoreServerHttpResponse.java index dc319c186201..33974018300a 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/JettyCoreServerHttpResponse.java +++ b/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 diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/ReactorNetty2ServerHttpRequest.java b/spring-web/src/main/java/org/springframework/http/server/reactive/ReactorNetty2ServerHttpRequest.java index 833e67aaa0e4..5e4d4af696cb 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/ReactorNetty2ServerHttpRequest.java +++ b/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; diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/ReactorNetty2ServerHttpResponse.java b/spring-web/src/main/java/org/springframework/http/server/reactive/ReactorNetty2ServerHttpResponse.java index 3d89eaad7a44..45789db8e8b8 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/ReactorNetty2ServerHttpResponse.java +++ b/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; } diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/ReactorServerHttpRequest.java b/spring-web/src/main/java/org/springframework/http/server/reactive/ReactorServerHttpRequest.java index 53715fb2489d..2a42c6f4cefe 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/ReactorServerHttpRequest.java +++ b/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; diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/ReactorServerHttpResponse.java b/spring-web/src/main/java/org/springframework/http/server/reactive/ReactorServerHttpResponse.java index d0f8cea29ef9..f7aed1d21148 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/ReactorServerHttpResponse.java +++ b/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; } diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/ServletServerHttpRequest.java b/spring-web/src/main/java/org/springframework/http/server/reactive/ServletServerHttpRequest.java index b4d59361895f..011430fd8ab3 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/ServletServerHttpRequest.java +++ b/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); } } diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/TomcatHttpHandlerAdapter.java b/spring-web/src/main/java/org/springframework/http/server/reactive/TomcatHttpHandlerAdapter.java index 6fd3af0148d5..c5c018e11667 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/TomcatHttpHandlerAdapter.java +++ b/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) { diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/UndertowServerHttpRequest.java b/spring-web/src/main/java/org/springframework/http/server/reactive/UndertowServerHttpRequest.java index e1c6494fe670..b3f96a533d86 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/UndertowServerHttpRequest.java +++ b/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); diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/UndertowServerHttpResponse.java b/spring-web/src/main/java/org/springframework/http/server/reactive/UndertowServerHttpResponse.java index 1a3f0f9ca448..1db025f339ee 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/UndertowServerHttpResponse.java +++ b/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); } diff --git a/spring-web/src/test/java/org/springframework/http/HttpHeadersTests.java b/spring-web/src/test/java/org/springframework/http/HttpHeadersTests.java index 9045bcdfefbc..166309e690a3 100644 --- a/spring-web/src/test/java/org/springframework/http/HttpHeadersTests.java +++ b/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