Spring MockHttpServletRequest.addHeader duplicates "Content-Type" header

I first encountered Stack Overflow question no. 79616625, reporting duplicate Content-Type headers in REST docs, specifically in request docs.

A reproducer can be found here at https://github.com/Jeffrey-Oh/restassured-restdocs-issue.

The culprit seems to be method MockHttpServletRequest::addHeader. Its current implementation has a problem:

public void addHeader(String name, Object value) {
  if (HttpHeaders.CONTENT_TYPE.equalsIgnoreCase(name) &&
    !this.headers.containsKey(HttpHeaders.CONTENT_TYPE)) {
    setContentType(value.toString());
  }
  else if (HttpHeaders.ACCEPT_LANGUAGE.equalsIgnoreCase(name) &&
    !this.headers.containsKey(HttpHeaders.ACCEPT_LANGUAGE)) {
    try {
      HttpHeaders headers = new HttpHeaders();
      headers.add(HttpHeaders.ACCEPT_LANGUAGE, value.toString());
      List<Locale> locales = headers.getAcceptLanguageAsLocales();
      this.locales.clear();
      this.locales.addAll(locales);
      if (this.locales.isEmpty()) {
        this.locales.add(Locale.ENGLISH);
      }
    }
    catch (IllegalArgumentException ex) {
      // Invalid Accept-Language format -> just store plain header
    }
    doAddHeaderValue(name, value, true);
  }
  else {
    doAddHeaderValue(name, value, false);
  }
}

Please note, how a Content-Type header is only set if it does not exist yet. If it already exists and is set again, at the end of the method we fall through to doAddHeaderValue(name, value, false), adding a second value to the already existing header. Technically, the underlying data structure is a map with the header name being the key and a HeaderValueHolder being able to hold multiple values. For Content-Type, however, only a single value is allowed, and there also should not be multiple instances of this header. By the way, the value false in doAddHeaderValue(name, value, false) means to not replace but amend the value list.

I have not checked the logic for the similar case Accept-Language, but AFAIK there multiple values are allowed - not sure about multiple occurrences of the header itself though.

Please, fix the logic for Content-Type.

CC @Jeffrey-Oh

Comment From: kriegaex

A simple change would be to always set (i.e. overwrite, not add) the Content-Type header (untested change):

  if (HttpHeaders.CONTENT_TYPE.equalsIgnoreCase(name)) {
    setContentType(value.toString());
  }

I also forgot to link to some REST docs with duplicate headers:

• https://github.com/Jeffrey-Oh/restassured-restdocs-issue/blob/master/build/generated-snippets/user/logout/http-request.adoc
• https://github.com/Jeffrey-Oh/restassured-restdocs-issue/blob/master/build/generated-snippets/user/logout/httpie-request.adoc
• https://github.com/Jeffrey-Oh/restassured-restdocs-issue/blob/master/build/generated-snippets/user/logout/curl-request.adoc

Comment From: kriegaex

Thanks for the swift reaction, @bclozel. 🙂🙏

Comment From: bclozel

Thanks for the detailed report @kriegaex

Comment From: erizzo-cfa

Sorry to comment on a closed issue, but the change made to "fix" this resulted in what I think is an unintended consequence (regression?): a Content-Type header will always override a value that was set with setContentType(). It's not obvious that an explicit call to setContentType() will be overridden by a Content-Type header that's been set in the builder. In fact, I find it surprising that the header replaces the explicit value - I doubt I'm the only person who'd be surprised by that. Doesn't it seem that this part of the behavior change was unintentional and unnecessary to address the original issue (duplicate Content-Type header values)?

Comment From: erizzo-cfa

In fact, it seems to break MockMultipartHttpServletRequestBuilder which has an explicit call to contentType(MediaType.MULTIPART_FORM_DATA) in its constructor. Multipart requests that have a content type header other than multipart/form-data typically don't work with "upload" type endpoints (which is how I discovered this issue, when some of our tests started failing after the update to Spring). If a MockMvc is set up in a typical fashion with some default headers (application/json is often used for REST endpoint tests), then code that uses MockMvcRequestBuilders.multipart(String, Object...) is now sending the wrong content-type, which it wasn't doing before.

Comment From: bclozel

Please create a new issue for this and share a code snippet that demonstrates the problem