Spring-framework: In contrast to the Javadoc, ServerHttpRequest.Builder implementation does not override headers

[juanmbellini]

Javadoc of ServerHttpRequest.Builder clearly says:
https://github.com/spring-projects/spring-framework/blob/5190eaf503bf1c280a9b9671252f15b77d72defc/spring-web/src/main/java/org/springframework/http/server/reactive/ServerHttpRequest.java#L140

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:

https://github.com/spring-projects/spring-framework/blob/5190eaf503bf1c280a9b9671252f15b77d72defc/spring-web/src/main/java/org/springframework/http/server/reactive/DefaultServerHttpRequestBuilder.java#L48

https://github.com/spring-projects/spring-framework/blob/5190eaf503bf1c280a9b9671252f15b77d72defc/spring-web/src/main/java/org/springframework/http/server/reactive/DefaultServerHttpRequestBuilder.java#L115

https://github.com/spring-projects/spring-framework/blob/5190eaf503bf1c280a9b9671252f15b77d72defc/spring-web/src/main/java/org/springframework/http/HttpHeaders.java#L417

https://github.com/spring-projects/spring-framework/blob/5190eaf503bf1c280a9b9671252f15b77d72defc/spring-web/src/main/java/org/springframework/http/HttpHeaders.java#L1622

[sbrannen]

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

[sbrannen]

Related commit: a1ae9ac1bdd6aaaceb42c2a65b37ed92450334cc

[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.