Skip to content

Commit 942dba2

Browse files
impl: HttpHeaders.backedBy and HttpHeaders.copyOf to reduce ambiguity on the constructor.
Fixes spring-projects#34340 chore: Update constructor references with backedBy method. Signed-off-by: Bryce J. Fisher <bryce.fisher@gmail.com>
1 parent d4030a8 commit 942dba2

14 files changed

+118
-22
lines changed

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

+47
Original file line numberDiff line numberDiff line change
@@ -452,7 +452,9 @@ public HttpHeaders() {
452452
* headers map structures, primarily for internal use within the framework.
453453
* @param headers the headers map (expected to operate with case-insensitive keys)
454454
* @since 5.1
455+
* @deprecated Will be made default visibility in favor of {@link #backedBy(MultiValueMap)} in a future release.
455456
*/
457+
@Deprecated
456458
public HttpHeaders(MultiValueMap<String, String> headers) {
457459
Assert.notNull(headers, "MultiValueMap must not be null");
458460
if (headers == EMPTY) {
@@ -477,7 +479,9 @@ else if (headers instanceof HttpHeaders httpHeaders) {
477479
* likely to be out of sync and should be discarded.
478480
* @param httpHeaders the headers to expose
479481
* @since 7.0
482+
* @deprecated Will be made private in favor of {@link #backedBy(HttpHeaders)} in a future release.
480483
*/
484+
@Deprecated
481485
public HttpHeaders(HttpHeaders httpHeaders) {
482486
Assert.notNull(httpHeaders, "HttpHeaders must not be null");
483487
if (httpHeaders == EMPTY) {
@@ -491,6 +495,49 @@ public HttpHeaders(HttpHeaders httpHeaders) {
491495
}
492496
}
493497

498+
/**
499+
* Construct a new {@code HttpHeaders} instance backed by an existing map.
500+
* <p>This constructor is available as an optimization for adapting to existing
501+
* headers map structures, primarily for internal use within the framework.
502+
* @param headers the headers map (expected to operate with case-insensitive keys)
503+
*/
504+
@SuppressWarnings("deprecation") // @Deprecated wll be removed when visibility is changed.
505+
public static HttpHeaders backedBy(MultiValueMap<String, String> headers) {
506+
return new HttpHeaders(headers);
507+
}
508+
509+
/**
510+
* Construct a new {@code HttpHeaders} instance by removing any read-only
511+
* wrapper that may have been previously applied around the given
512+
* {@code HttpHeaders} via {@link #readOnlyHttpHeaders(HttpHeaders)}.
513+
* <p>Once the writable instance is mutated, the read-only instance is
514+
* likely to be out of sync and should be discarded.
515+
* @param httpHeaders the headers to expose
516+
*/
517+
@SuppressWarnings("deprecation") // @Deprecated wll be removed when visibility is changed.
518+
public static HttpHeaders backedBy(HttpHeaders httpHeaders) {
519+
return new HttpHeaders(httpHeaders);
520+
}
521+
522+
/**
523+
* Constructs a new {@code HttpHeaders} instance with the given headers.
524+
* Changes made to this new instance will NOT be reflected in the original headers.
525+
* @param headers The headers to copy
526+
*/
527+
public static HttpHeaders copyOf(MultiValueMap<String, String> headers) {
528+
HttpHeaders httpHeadersCopy = new HttpHeaders();
529+
headers.forEach((key, values) -> httpHeadersCopy.put(key, new ArrayList<>(values)));
530+
return httpHeadersCopy;
531+
}
532+
533+
/**
534+
* Constructs a new {@code HttpHeaders} instance with the given headers.
535+
* Changes made to this new instance will NOT be reflected in the original headers.
536+
* @param httpHeaders The headers to copy
537+
*/
538+
public static HttpHeaders copyOf(HttpHeaders httpHeaders) {
539+
return copyOf(httpHeaders.headers);
540+
}
494541

495542
/**
496543
* Get the list of header values for the given header name, if any.

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

+2-1
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
* <p>This caches the parsed representations of the "Accept" and "Content-Type" headers
3737
* and will get out of sync with the backing map it is mutated at runtime.
3838
*
39+
* @implNote This does NOT make the underlying MultiValueMap readonly.
3940
* @author Brian Clozel
4041
* @author Sam Brannen
4142
* @since 5.1.1
@@ -49,7 +50,7 @@ class ReadOnlyHttpHeaders extends HttpHeaders {
4950
@SuppressWarnings("serial")
5051
private @Nullable List<MediaType> cachedAccept;
5152

52-
53+
@SuppressWarnings("deprecation") // @Deprecated wll be removed when visibility is changed.
5354
ReadOnlyHttpHeaders(MultiValueMap<String, String> headers) {
5455
super(headers);
5556
}

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ public Mono<Void> write(Publisher<? extends Part> parts,
9191
}
9292

9393
private <T> Flux<DataBuffer> encodePart(byte[] boundary, Part part, DataBufferFactory bufferFactory) {
94-
HttpHeaders headers = new HttpHeaders(part.headers());
94+
HttpHeaders headers = HttpHeaders.backedBy(part.headers());
9595

9696
String name = part.name();
9797
if (!headers.containsHeader(HttpHeaders.CONTENT_DISPOSITION)) {

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ public JettyCoreServerHttpRequest(Request request, JettyDataBufferFactory dataBu
5656
super(HttpMethod.valueOf(request.getMethod()),
5757
request.getHttpURI().toURI(),
5858
request.getContext().getContextPath(),
59-
new HttpHeaders(new JettyHeadersAdapter(request.getHeaders())));
59+
HttpHeaders.backedBy(new JettyHeadersAdapter(request.getHeaders())));
6060
this.dataBufferFactory = dataBufferFactory;
6161
this.request = request;
6262
}

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ class JettyCoreServerHttpResponse extends AbstractServerHttpResponse implements
5656

5757

5858
public JettyCoreServerHttpResponse(Response response, JettyDataBufferFactory dataBufferFactory) {
59-
super(dataBufferFactory, new HttpHeaders(new JettyHeadersAdapter(response.getHeaders())));
59+
super(dataBufferFactory, HttpHeaders.backedBy(new JettyHeadersAdapter(response.getHeaders())));
6060
this.response = response;
6161

6262
// 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

+1-1
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ public ReactorNetty2ServerHttpRequest(HttpServerRequest request, Netty5DataBuffe
7171
throws URISyntaxException {
7272

7373
super(HttpMethod.valueOf(request.method().name()), initUri(request), "",
74-
new HttpHeaders(new Netty5HeadersAdapter(request.requestHeaders())));
74+
HttpHeaders.backedBy(new Netty5HeadersAdapter(request.requestHeaders())));
7575
Assert.notNull(bufferFactory, "DataBufferFactory must not be null");
7676
this.request = request;
7777
this.bufferFactory = bufferFactory;

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ class ReactorNetty2ServerHttpResponse extends AbstractServerHttpResponse impleme
5858

5959

6060
public ReactorNetty2ServerHttpResponse(HttpServerResponse response, DataBufferFactory bufferFactory) {
61-
super(bufferFactory, new HttpHeaders(new Netty5HeadersAdapter(response.responseHeaders())));
61+
super(bufferFactory, HttpHeaders.backedBy(new Netty5HeadersAdapter(response.responseHeaders())));
6262
Assert.notNull(response, "HttpServerResponse must not be null");
6363
this.response = response;
6464
}

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ public ReactorServerHttpRequest(HttpServerRequest request, NettyDataBufferFactor
6868

6969
super(HttpMethod.valueOf(request.method().name()),
7070
ReactorUriHelper.createUri(request), request.forwardedPrefix(),
71-
new HttpHeaders(new Netty4HeadersAdapter(request.requestHeaders())));
71+
HttpHeaders.backedBy(new Netty4HeadersAdapter(request.requestHeaders())));
7272
Assert.notNull(bufferFactory, "DataBufferFactory must not be null");
7373
this.request = request;
7474
this.bufferFactory = bufferFactory;

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ class ReactorServerHttpResponse extends AbstractServerHttpResponse implements Ze
5757

5858

5959
public ReactorServerHttpResponse(HttpServerResponse response, DataBufferFactory bufferFactory) {
60-
super(bufferFactory, new HttpHeaders(new Netty4HeadersAdapter(Objects.requireNonNull(response,
60+
super(bufferFactory, HttpHeaders.backedBy(new Netty4HeadersAdapter(Objects.requireNonNull(response,
6161
"HttpServerResponse must not be null").responseHeaders())));
6262
this.response = response;
6363
}

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ private static HttpHeaders initHeaders(HttpHeaders headerValues, HttpServletRequ
168168
String requestContentType = request.getContentType();
169169
if (StringUtils.hasLength(requestContentType)) {
170170
contentType = MediaType.parseMediaType(requestContentType);
171-
headers = new HttpHeaders(headerValues);
171+
headers = HttpHeaders.backedBy(headerValues);
172172
headers.setContentType(contentType);
173173
}
174174
}
@@ -184,7 +184,7 @@ private static HttpHeaders initHeaders(HttpHeaders headerValues, HttpServletRequ
184184
if (headerValues.getFirst(HttpHeaders.CONTENT_TYPE) == null) {
185185
int contentLength = request.getContentLength();
186186
if (contentLength != -1) {
187-
headers = (headers != null ? headers : new HttpHeaders(headerValues));
187+
headers = (headers != null ? headers : HttpHeaders.backedBy(headerValues));
188188
headers.setContentLength(contentLength);
189189
}
190190
}

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ private static HttpHeaders createTomcatHttpHeaders(HttpServletRequest request) {
9696
ReflectionUtils.getField(COYOTE_REQUEST_FIELD, requestFacade);
9797
Assert.state(connectorRequest != null, "No Tomcat connector request");
9898
Request tomcatRequest = connectorRequest.getCoyoteRequest();
99-
return new HttpHeaders(new TomcatHeadersAdapter(tomcatRequest.getMimeHeaders()));
99+
return HttpHeaders.backedBy(new TomcatHeadersAdapter(tomcatRequest.getMimeHeaders()));
100100
}
101101

102102
private static RequestFacade getRequestFacade(HttpServletRequest request) {
@@ -140,7 +140,7 @@ private static HttpHeaders createTomcatHttpHeaders(HttpServletResponse response)
140140
Assert.state(connectorResponse != null, "No Tomcat connector response");
141141
Response tomcatResponse = connectorResponse.getCoyoteResponse();
142142
TomcatHeadersAdapter headers = new TomcatHeadersAdapter(tomcatResponse.getMimeHeaders());
143-
return new HttpHeaders(headers);
143+
return HttpHeaders.backedBy(headers);
144144
}
145145

146146
private static ResponseFacade getResponseFacade(HttpServletResponse response) {

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ public UndertowServerHttpRequest(HttpServerExchange exchange, DataBufferFactory
6666
throws URISyntaxException {
6767

6868
super(HttpMethod.valueOf(exchange.getRequestMethod().toString()), initUri(exchange), "",
69-
new HttpHeaders(new UndertowHeadersAdapter(exchange.getRequestHeaders())));
69+
HttpHeaders.backedBy(new UndertowHeadersAdapter(exchange.getRequestHeaders())));
7070
this.exchange = exchange;
7171
this.body = new RequestBodyPublisher(exchange, bufferFactory);
7272
this.body.registerListeners(exchange);

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ class UndertowServerHttpResponse extends AbstractListenerServerHttpResponse impl
7070
private static HttpHeaders createHeaders(HttpServerExchange exchange) {
7171
Assert.notNull(exchange, "HttpServerExchange must not be null");
7272
UndertowHeadersAdapter headersMap = new UndertowHeadersAdapter(exchange.getResponseHeaders());
73-
return new HttpHeaders(headersMap);
73+
return HttpHeaders.backedBy(headersMap);
7474
}
7575

7676

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

+56-8
Original file line numberDiff line numberDiff line change
@@ -58,23 +58,71 @@ class HttpHeadersTests {
5858
final HttpHeaders headers = new HttpHeaders();
5959

6060
@Test
61-
void constructorUnwrapsReadonly() {
61+
void backedByFactoryUnwrapsReadonly() {
6262
headers.setContentType(MediaType.APPLICATION_JSON);
6363
HttpHeaders readOnly = HttpHeaders.readOnlyHttpHeaders(headers);
6464
assertThat(readOnly.getContentType()).isEqualTo(MediaType.APPLICATION_JSON);
65-
HttpHeaders writable = new HttpHeaders(readOnly);
65+
HttpHeaders writable = HttpHeaders.backedBy(readOnly);
6666
writable.setContentType(MediaType.TEXT_PLAIN);
67+
6768
// content-type value is cached by ReadOnlyHttpHeaders
69+
assertThat(readOnly.getContentType())
70+
.describedAs("readOnly HttpHeaders should NOT have updated content-type value")
71+
.isEqualTo(MediaType.APPLICATION_JSON);
72+
assertThat(writable.getContentType())
73+
.describedAs("writable HttpHeaders should have updated content-type value")
74+
.isEqualTo(MediaType.TEXT_PLAIN);
75+
assertThat(headers.getContentType())
76+
.describedAs("initial HttpHeaders should have updated content-type value")
77+
.isEqualTo(MediaType.TEXT_PLAIN);
78+
}
79+
80+
@Test
81+
void backedByFactoryUnwrapsMultipleHeaders() {
82+
headers.setContentType(MediaType.APPLICATION_JSON);
83+
HttpHeaders readonlyOriginalExchangeHeaders = HttpHeaders.readOnlyHttpHeaders(headers);
84+
HttpHeaders firewallHeaders = HttpHeaders.backedBy(readonlyOriginalExchangeHeaders);
85+
HttpHeaders writeable = HttpHeaders.backedBy(firewallHeaders);
86+
writeable.setContentType(MediaType.TEXT_PLAIN);
87+
88+
// If readonly headers are unwrapped multiple times, the content-type value should be updated
89+
assertThat(writeable.getContentType()).isEqualTo(MediaType.TEXT_PLAIN);
90+
assertThat(firewallHeaders.getContentType()).isEqualTo(MediaType.TEXT_PLAIN);
91+
assertThat(readonlyOriginalExchangeHeaders.getContentType()).isEqualTo(MediaType.TEXT_PLAIN);
92+
assertThat(headers.getContentType()).isEqualTo(MediaType.TEXT_PLAIN);
93+
}
94+
95+
@Test
96+
void copyOfFactoryUnwrapsReadonly() {
97+
headers.setContentType(MediaType.APPLICATION_JSON);
98+
HttpHeaders readOnly = HttpHeaders.readOnlyHttpHeaders(headers);
6899
assertThat(readOnly.getContentType()).isEqualTo(MediaType.APPLICATION_JSON);
69-
assertThat(writable.getContentType()).isEqualTo(MediaType.TEXT_PLAIN);
100+
HttpHeaders writable = HttpHeaders.copyOf(readOnly);
101+
writable.setContentType(MediaType.TEXT_PLAIN);
102+
103+
assertThat(readOnly.getContentType())
104+
.describedAs("readOnly HttpHeaders should NOT have updated content-type value")
105+
.isEqualTo(MediaType.APPLICATION_JSON);
106+
assertThat(writable.getContentType())
107+
.describedAs("writable HttpHeaders should have updated content-type value")
108+
.isEqualTo(MediaType.TEXT_PLAIN);
109+
assertThat(headers.getContentType())
110+
.describedAs("initial HttpHeaders should have updated content-type value")
111+
.isEqualTo(MediaType.APPLICATION_JSON);
70112
}
71113

72114
@Test
73-
void writableHttpHeadersUnwrapsMultiple() {
74-
HttpHeaders originalExchangeHeaders = HttpHeaders.readOnlyHttpHeaders(new HttpHeaders());
75-
HttpHeaders firewallHeaders = new HttpHeaders(originalExchangeHeaders);
76-
HttpHeaders writeable = new HttpHeaders(firewallHeaders);
77-
writeable.setContentType(MediaType.APPLICATION_JSON);
115+
void copyOfFactoryUnwrapsMultipleHeaders() {
116+
headers.setContentType(MediaType.APPLICATION_JSON);
117+
HttpHeaders readonlyOriginalExchangeHeaders = HttpHeaders.readOnlyHttpHeaders(headers);
118+
HttpHeaders firewallHeaders = HttpHeaders.copyOf(readonlyOriginalExchangeHeaders);
119+
HttpHeaders writeable = HttpHeaders.copyOf(firewallHeaders);
120+
writeable.setContentType(MediaType.TEXT_PLAIN);
121+
122+
assertThat(writeable.getContentType()).isEqualTo(MediaType.TEXT_PLAIN);
123+
assertThat(firewallHeaders.getContentType()).isEqualTo(MediaType.APPLICATION_JSON);
124+
assertThat(readonlyOriginalExchangeHeaders.getContentType()).isEqualTo(MediaType.APPLICATION_JSON);
125+
assertThat(headers.getContentType()).isEqualTo(MediaType.APPLICATION_JSON);
78126
}
79127

80128
@Test

0 commit comments

Comments
 (0)