Skip to content

Commit 5b57c5f

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

File tree

3 files changed

+101
-8
lines changed

3 files changed

+101
-8
lines changed

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

Lines changed: 44 additions & 0 deletions
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,6 +479,7 @@ 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
*/
481484
public HttpHeaders(HttpHeaders httpHeaders) {
482485
Assert.notNull(httpHeaders, "HttpHeaders must not be null");
@@ -491,6 +494,47 @@ public HttpHeaders(HttpHeaders httpHeaders) {
491494
}
492495
}
493496

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

495539
/**
496540
* Get the list of header values for the given header name, if any.

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

Lines changed: 1 addition & 0 deletions
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‎

Lines changed: 56 additions & 8 deletions
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)