Skip to content

MockMvc request builders should expose HttpHeaders #35588

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+136 −22
Open

MockMvc request builders should expose HttpHeaders #35588

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
23d7684
Add method to provide headers via consumer
mieseprem Oct 8, 2025
671565a
Re-add LinkedHashMap for headers
mieseprem Oct 10, 2025
1530549
Change internal headers object from MultiValueMap to HttpHeaders
mieseprem Oct 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 23 additions & 9 deletions ...a/org/springframework/test/web/servlet/request/AbstractMockHttpServletRequestBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.function.Consumer;

import jakarta.servlet.ServletContext;
import jakarta.servlet.ServletRequest;
Expand Down Expand Up @@ -109,7 +110,7 @@ public abstract class AbstractMockHttpServletRequestBuilder<B extends AbstractMo

private @Nullable String contentType;

private final MultiValueMap<String, Object> headers = new LinkedMultiValueMap<>();
private final HttpHeaders headers = new HttpHeaders();

private final MultiValueMap<String, String> parameters = new LinkedMultiValueMap<>();

Expand Down Expand Up @@ -342,7 +343,7 @@ public B accept(String... mediaTypes) {
* @param values one or more header values
*/
public B header(String name, Object... values) {
this.headers.addAll(name, Arrays.asList(values));
this.headers.addAll(name, Arrays.stream(values).map(String::valueOf).toList());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The MultiValueMap<String, Object> matches to how MockHttpServletRequest manages header values, allowing Collections, arrays, as well as date and number values, which it then formats when headers are obtained. So this change is unlikely to go without causing regressions.

The code in this method could match the behavior of getDateHeader and getIntHeader of MockHttpServletRequest, in reverse by formatting to String values in the same way, to ensure backwards compatibility.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To avoid heading in the wrong direction, I’ll briefly confirm what I’ve understood:

Working internally with HttpHeaders isn’t the best idea because of potential regressions. It’s better to continue using MultiValueMap internally.

So, if we still want to offer a method headers(Consumer<HttpHeaders>), we should consume the HttpHeaders and then call the existing headers(HttpHeaders) method. I can change it in that direction.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, if we still want to offer a method headers(Consumer<HttpHeaders>), we should consume the HttpHeaders and then call the existing headers(HttpHeaders) method.

That unfortunately does not work.

If you create a new instance of HttpHeaders and provide that to the Consumer<HttpHeaders>, the consumer can only add new headers. It cannot remove or process existing headers.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you create a new instance of HttpHeaders and provide that to the Consumer<HttpHeaders>, the consumer can only add new headers. It cannot remove or process existing headers

Ah, I think I’ve got it now. Yes — my approach indeed won’t work.

The challenge is "extracting" the HttpHeaders from the Consumer without first converting the existing MultiValueMap to HttpHeaders and then converting back after consumption. That seems cumbersome. I’ll think about possible solutions.

Copy link
Author

@mieseprem mieseprem Oct 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think there’s any sensible way to implement this? As long as the internal headers map is a MultiValueMap<String, Object>, I can’t see how to make it compatible with the MultiValueMap<String, String> used by HttpHeaders. I can’t simply change the value type of objects already placed into the builder’s headers map into their string representations — that would almost certainly cause regressions.

For a moment I considered an implementation along these lines, but that was based on the incorrect assumption that the Consumer would only contain HttpHeaders instructions for purely String‑based header values (to be clear, this code produces the wrong result):

public B headers(Consumer<HttpHeaders> headersConsumer){
	HttpHeaders intermediateHeaders = new HttpHeaders();
	
	var stringHeaders = this.headers.entrySet().stream().
		filter(entry -> entry.getValue().stream().allMatch(obj -> obj instanceof String))
		.peek(entry -> intermediateHeaders.addAll(entry.getKey(), entry.getValue().stream().map(String.class::cast).toList()))
		.map(Map.Entry::getKey)
		.toList();
	stringHeaders.forEach(this.headers::remove);

	headersConsumer.accept(intermediateHeaders);
	return headers(intermediateHeaders);
}

If I'm not (again) totally wrong, this should be a test that need to pass:

@Test
void headersConsumer() {
	this.builder.header("X-Foo-String", "bar");
	this.builder.header("X-Foo-Date", LocalDate.now());
	this.builder.header("X-Foo-List-Int", List.of(1, 2, 3));

	this.builder.headers(httpHeaders -> {
		httpHeaders.put("X-Baz", Arrays.asList("qux", "quux"));
		httpHeaders.remove("X-Foo-Date");
	});

	MockHttpServletRequest request = this.builder.buildRequest(this.servletContext);
	List<String> headerNames = Collections.list(request.getHeaderNames());

	assertThat(headerNames).containsExactly("X-Foo-String", "X-Foo-List-Int", "X-Baz");
}

I think for this to work at all, roughly the following would need to happen:

  • create from each MultiValueMap<String, Object> entry a MultiValueMap<String, String> one (this step alone seems very complex — I’m thinking of your point about getDateHeader(String) and getIntHeader(String))
  • it’s easy to create an HttpHeaders object from a MultiValueMap<String, String> (call that X)
  • consume the Consumer with the created object X
  • afterwards, add, remove or modify entries in the original headers map (we must not copy back untouched entries that we converted to String in X)

Either I’m overthinking this, I’ve gone completely off track, or I’m perhaps not the right person to implement this task 🤷

Copy link
Contributor

@rstoyanchev rstoyanchev Oct 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry if this wasn't clear, but my suggestion was not to change to MultiValueMap. Keep HttpHeaders internally, as you have done, and improve the way values are formatted. So instead of a simple String#valueOf, we would need to match the formatting that would be done otherwise in MockHttpServletRequest.

For this you can trace the code to add and obtain headers in MockHttpServletRequest. You will see that doAddHeaderValue recognizes collections and arrays. Also getDateHeader and getIntHeader recognize various additional value types.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, got it. So I will give it another try 👍

return self();
}

Expand All @@ -355,6 +356,20 @@ public B headers(HttpHeaders httpHeaders) {
return self();
}

/**
* Manipulate this builder's headers with the given consumer. The
* headers provided to the consumer are "live", so that the consumer can be used to
* {@linkplain HttpHeaders#set(String, String) overwrite} existing header values,
* {@linkplain HttpHeaders#remove(String) remove} values, or use any of the other
* {@link HttpHeaders} methods.
* @param headersConsumer a function that consumes the {@code HttpHeaders}
* @return this builder
*/
public B headers(Consumer<HttpHeaders> headersConsumer){
headersConsumer.accept(this.headers);
return self();
}

/**
* Add a request parameter to {@link MockHttpServletRequest#getParameterMap()}.
* <p>In the Servlet API, a request parameter may be parsed from the query
Expand Down Expand Up @@ -665,12 +680,11 @@ public Object merge(@Nullable Object parent) {
this.contentType = parentBuilder.contentType;
}

for (Map.Entry<String, List<Object>> entry : parentBuilder.headers.entrySet()) {
String headerName = entry.getKey();
if (!this.headers.containsKey(headerName)) {
this.headers.put(headerName, entry.getValue());
parentBuilder.headers.forEach((name, values) -> {
if (!this.headers.containsHeader(name)) {
this.headers.put(name, values);
}
}
});
for (Map.Entry<String, List<String>> entry : parentBuilder.parameters.entrySet()) {
String paramName = entry.getKey();
if (!this.parameters.containsKey(paramName)) {
Expand Down Expand Up @@ -808,8 +822,8 @@ public final MockHttpServletRequest buildRequest(ServletContext servletContext)
});

if (!ObjectUtils.isEmpty(this.content) &&
!this.headers.containsKey(HttpHeaders.CONTENT_LENGTH) &&
!this.headers.containsKey(HttpHeaders.TRANSFER_ENCODING)) {
!this.headers.containsHeader(HttpHeaders.CONTENT_LENGTH) &&
!this.headers.containsHeader(HttpHeaders.TRANSFER_ENCODING)) {

request.addHeader(HttpHeaders.CONTENT_LENGTH, this.content.length);
}
Expand Down
16 changes: 16 additions & 0 deletions ...java/org/springframework/test/web/servlet/request/MockHttpServletRequestBuilderTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.function.Consumer;

import jakarta.servlet.ServletContext;
import jakarta.servlet.http.Cookie;
Expand Down Expand Up @@ -543,6 +544,21 @@ void headers() {
assertThat(request.getHeader("Content-Type")).isEqualTo(MediaType.APPLICATION_JSON.toString());
}

@Test
void headersConsumer() {
Consumer<HttpHeaders> httpHeadersConsumer = httpHeaders -> {
httpHeaders.setContentType(MediaType.APPLICATION_JSON);
httpHeaders.put("foo", Arrays.asList("bar", "baz"));
};
this.builder.headers(httpHeadersConsumer);

MockHttpServletRequest request = this.builder.buildRequest(this.servletContext);
List<String> headers = Collections.list(request.getHeaders("foo"));

assertThat(headers).containsExactly("bar", "baz");
assertThat(request.getHeader("Content-Type")).isEqualTo(MediaType.APPLICATION_JSON.toString());
}

@Test
void cookie() {
Cookie cookie1 = new Cookie("foo", "bar");
Expand Down