Skip to content

Commit 142b970

Browse files
craftmaster2190craftmaster2190
committed
impl: HttpHeaders.backedBy and HttpHeaders.copyOf to reduce ambiguity on the constructor.
Fixes #34340
1 parent d4030a8 commit 142b970

File tree

3 files changed

+102
-8
lines changed

3 files changed

+102
-8
lines changed

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

+45
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 private 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,47 @@ 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+
public static HttpHeaders backedBy(MultiValueMap<String, String> headers) {
505+
return new HttpHeaders(headers);
506+
}
507+
508+
/**
509+
* Construct a new {@code HttpHeaders} instance by removing any read-only
510+
* wrapper that may have been previously applied around the given
511+
* {@code HttpHeaders} via {@link #readOnlyHttpHeaders(HttpHeaders)}.
512+
* <p>Once the writable instance is mutated, the read-only instance is
513+
* likely to be out of sync and should be discarded.
514+
* @param httpHeaders the headers to expose
515+
*/
516+
public static HttpHeaders backedBy(HttpHeaders httpHeaders) {
517+
return backedBy(httpHeaders.headers);
518+
}
519+
520+
/**
521+
* Constructs a new {@code HttpHeaders} instance with the given headers.
522+
* Changes made to this new instance will NOT be reflected in the original headers.
523+
* @param headers The headers to copy
524+
*/
525+
public static HttpHeaders copyOf(MultiValueMap<String, String> headers) {
526+
HttpHeaders httpHeadersCopy = new HttpHeaders();
527+
headers.forEach((key, values) -> httpHeadersCopy.put(key, new ArrayList<>(values)));
528+
return httpHeadersCopy;
529+
}
530+
531+
/**
532+
* Constructs a new {@code HttpHeaders} instance with the given headers.
533+
* Changes made to this new instance will NOT be reflected in the original headers.
534+
* @param httpHeaders The headers to copy
535+
*/
536+
public static HttpHeaders copyOf(HttpHeaders httpHeaders) {
537+
return copyOf(httpHeaders.headers);
538+
}
494539

495540
/**
496541
* Get the list of header values for the given header name, if any.

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

+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

‎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)