In contrast to the Javadoc, ServerHttpRequest.Builder implementation does not override headers

[juanmbellini]

Javadoc of ServerHttpRequest.Builder clearly says:

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

Line 140 in 5190eaf

* Set or override the specified header.

which is part of the header(String, String) method (signature can be seen below).

/**
 * Set or override the specified header.
 */
Builder header(String key, String value);

but the default implementation (DefaultServerHttpRequestBuilder) uses an instance of HttpHeaders, which internally uses a MultiValueMap<String, String> to store headers. So, calling the said method does not override the header, but adds it in the MultiValueMap.

Check the following links:

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

Line 48 in 5190eaf

private HttpHeaders httpHeaders;

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

Line 115 in 5190eaf

this.httpHeaders.add(key, value);

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

Line 417 in 5190eaf

final MultiValueMap<String, String> headers;

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

Line 1622 in 5190eaf

this.headers.add(headerName, headerValue);

[sbrannen]

Thanks for pointing out that inconsistency. We'll look into it.

[sbrannen]

Related commit: a1ae9ac

[bclozel]

We should fix the javadoc since this method is only about adding a header value.
Developers can mutate existing headers with headers(Consumer<HttpHeaders> headersConsumer).

[rstoyanchev]

It seems unlikely for code to depend on adding a header when mutating a server request. That probably explains the original Javadoc. So I think we should fix the behavior. However to make it a little less disruptive we could add a new method in 5.1.x:

/**
 * Set or override the specified header value(s).
 */
Builder header(String key, String... value);

Then deprecate the existing one in 5.1.x and remove it in 5.2 since having the two side by side is inconsistent and limiting (one adds, the other sets, no way to set with one, etc.), so we don't want a prolonged period of keeping them side by side.