Use UTF-8 for application/json in MockHttpServletResponse

[izeye]

This PR changes to use UTF-8 for application/json in MockHttpServletResponse.

[sbrannen]

Hi @izeye,

Is there an existing issue related to this change? If not, can you please briefly explain the rationale for the change (even if it seems self explanatory to some)?

Also, please note that you only changed the code in the test fixture. Thus, you would also need to update org.springframework.mock.web.MockHttpServletResponse, MockHttpServletResponseTests, etc.

[izeye]

@sbrannen Thanks for the feedback!

Is there an existing issue related to this change? If not, can you please briefly explain the rationale for the change (even if it seems self explanatory to some)?

I noticed that MockMvcResultHandlers.print() with UTF-8 doesn't work with the MediaType.APPLICATION_JSON (application/json) while it works with the deprecated MediaType.APPLICATION_JSON_UTF8 (application/json;charset=UTF-8). See https://github.com/izeye/spring-boot-throwaway-branches/blob/minimal-rest/src/test/java/com/izeye/throwaway/HomeControllerTests.java#L67-L70 for an example.

Also, please note that you only changed the code in the test fixture. Thus, you would also need to update org.springframework.mock.web.MockHttpServletResponse, MockHttpServletResponseTests, etc.

Sorry. I should have looked into it closely, but it seems that I just searched and picked a wrong one. Thanks for guiding me to the right direction. I updated accordingly. Please let me know if there's anything missing or worng.

[sbrannen]

Sorry. I should have looked into it closely, but it seems that I just searched and picked a wrong one.

No problem. Even core maintainers (like me) often forget to update both versions of those mocks.

Thanks for guiding me to the right direction.

You're welcome.

I updated accordingly. Please let me know if there's anything missing or worng.

I think that looks pretty good now; however, having looked at the issue in more detail I'm starting to wonder if that is the correct "fix".

As far as I know, we don't set the character encoding for the response to UTF-8 in such scenarios in production code (in spring-webmvc). Instead, we let the client correctly interpret the response as UTF-8.

For the particular test scenario you've described, I see the print() functionality as the "client" in Mock MVC.

In other words, I don't think we should make this change to MockHttpServletResponse.setContentType(String). Instead, I think we should make a change to org.springframework.test.web.servlet.result.PrintingResultHandler.printResponse(MockHttpServletResponse) to infer UTF-8 if the content type in the response is application/json.

@rstoyanchev and @sdeleuze, what are your thoughts on the matter?

[rstoyanchev]

There is some history. The corresponding MediaType was deprecated in #22788 and removed from use throughout because the charset is no longer defined for application/json. Subsequently, a similar change was considered for MockMvc in #23219 but resulted in a more limited change. I do not think we should revive the charset parameter on application/json anywhere but if there is a specific problem to solve somewhere we can try to be more lenient where it is present or needs to be implied.

[sbrannen]

Hi @izeye,

Thanks again for the PR.

In the end, we decided to address this differently (see comments above).

I am therefore closing this as superseded by #27926.