From 5de549d7d4324c7e30ac607e15ebac444f8c3f28 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Mon, 10 Aug 2020 16:05:18 +0200 Subject: [PATCH] Update contentType property via MockHttpServletResponse::setCharacterEncoding() Prior to this commit, MockHttpServletResponse's setCharacterEncoding() method did not update the contentType property, which violates the Servlet 2.4 Javadoc for getContentType() and setCharacterEncoding(). This commit addresses this issue; however, some existing tests may have to be updated as a result of this change. For example, note how some of the tests in this commit have been refactored to use MediaType##isCompatibleWith() instead of asserting exact matches for the value returned by MockHttpServletResponse's getContentType() method. Closes gh-25536 --- .../mock/web/MockHttpServletResponse.java | 7 ++-- .../web/MockHttpServletResponseTests.java | 6 +-- .../standalone/ViewResolutionTests.java | 5 +-- .../servlet/MockHttpServletResponse.java | 7 ++-- .../json/MappingJackson2JsonViewTests.java | 41 ++++++++----------- .../view/xml/MappingJackson2XmlViewTests.java | 38 +++++++---------- 6 files changed, 44 insertions(+), 60 deletions(-) diff --git a/spring-test/src/main/java/org/springframework/mock/web/MockHttpServletResponse.java b/spring-test/src/main/java/org/springframework/mock/web/MockHttpServletResponse.java index e51a8b0ad24..f0daaf90a62 100644 --- a/spring-test/src/main/java/org/springframework/mock/web/MockHttpServletResponse.java +++ b/spring-test/src/main/java/org/springframework/mock/web/MockHttpServletResponse.java @@ -169,14 +169,15 @@ public class MockHttpServletResponse implements HttpServletResponse { public void setCharacterEncoding(String characterEncoding) { this.characterEncoding = characterEncoding; this.charset = true; - updateContentTypeHeader(); + updateContentTypePropertyAndHeader(); } - private void updateContentTypeHeader() { + private void updateContentTypePropertyAndHeader() { if (this.contentType != null) { String value = this.contentType; if (this.charset && !this.contentType.toLowerCase().contains(CHARSET_PREFIX)) { value = value + ';' + CHARSET_PREFIX + this.characterEncoding; + this.contentType = value; } doAddHeaderValue(HttpHeaders.CONTENT_TYPE, value, true); } @@ -280,7 +281,7 @@ public class MockHttpServletResponse implements HttpServletResponse { this.charset = true; } } - updateContentTypeHeader(); + updateContentTypePropertyAndHeader(); } } diff --git a/spring-test/src/test/java/org/springframework/mock/web/MockHttpServletResponseTests.java b/spring-test/src/test/java/org/springframework/mock/web/MockHttpServletResponseTests.java index 37887d88c0f..b105ea7953a 100644 --- a/spring-test/src/test/java/org/springframework/mock/web/MockHttpServletResponseTests.java +++ b/spring-test/src/test/java/org/springframework/mock/web/MockHttpServletResponseTests.java @@ -144,7 +144,7 @@ class MockHttpServletResponseTests { void setContentTypeThenCharacterEncoding() { response.setContentType("test/plain"); response.setCharacterEncoding("UTF-8"); - assertThat(response.getContentType()).isEqualTo("test/plain"); + assertThat(response.getContentType()).isEqualTo("test/plain;charset=UTF-8"); assertThat(response.getHeader(CONTENT_TYPE)).isEqualTo("test/plain;charset=UTF-8"); assertThat(response.getCharacterEncoding()).isEqualTo("UTF-8"); } @@ -153,7 +153,7 @@ class MockHttpServletResponseTests { void setCharacterEncodingThenContentType() { response.setCharacterEncoding("UTF-8"); response.setContentType("test/plain"); - assertThat(response.getContentType()).isEqualTo("test/plain"); + assertThat(response.getContentType()).isEqualTo("test/plain;charset=UTF-8"); assertThat(response.getHeader(CONTENT_TYPE)).isEqualTo("test/plain;charset=UTF-8"); assertThat(response.getCharacterEncoding()).isEqualTo("UTF-8"); } @@ -488,7 +488,7 @@ class MockHttpServletResponseTests { assertThat(response.isCharset()).isTrue(); assertThat(response.getCharacterEncoding()).isEqualTo("UTF-8"); response.setContentType("text/plain"); - assertThat(response.getContentType()).isEqualTo("text/plain"); + assertThat(response.getContentType()).isEqualTo("text/plain;charset=UTF-8"); String contentTypeHeader = response.getHeader(CONTENT_TYPE); assertThat(contentTypeHeader).isEqualTo("text/plain;charset=UTF-8"); diff --git a/spring-test/src/test/java/org/springframework/test/web/servlet/samples/standalone/ViewResolutionTests.java b/spring-test/src/test/java/org/springframework/test/web/servlet/samples/standalone/ViewResolutionTests.java index 3f938afbf7c..7cfb36e874b 100644 --- a/spring-test/src/test/java/org/springframework/test/web/servlet/samples/standalone/ViewResolutionTests.java +++ b/spring-test/src/test/java/org/springframework/test/web/servlet/samples/standalone/ViewResolutionTests.java @@ -73,7 +73,7 @@ class ViewResolutionTests { standaloneSetup(new PersonController()).setSingleView(new MappingJackson2JsonView()).build() .perform(get("/person/Corea")) .andExpect(status().isOk()) - .andExpect(content().contentType(MediaType.APPLICATION_JSON)) + .andExpect(content().contentTypeCompatibleWith(MediaType.APPLICATION_JSON)) .andExpect(jsonPath("$.person.name").value("Corea")); } @@ -119,7 +119,7 @@ class ViewResolutionTests { mockMvc.perform(get("/person/Corea").accept(MediaType.APPLICATION_JSON)) .andExpect(status().isOk()) - .andExpect(content().contentType(MediaType.APPLICATION_JSON)) + .andExpect(content().contentTypeCompatibleWith(MediaType.APPLICATION_JSON)) .andExpect(jsonPath("$.person.name").value("Corea")); mockMvc.perform(get("/person/Corea").accept(MediaType.APPLICATION_XML)) @@ -150,4 +150,3 @@ class ViewResolutionTests { } } - diff --git a/spring-web/src/testFixtures/java/org/springframework/web/testfixture/servlet/MockHttpServletResponse.java b/spring-web/src/testFixtures/java/org/springframework/web/testfixture/servlet/MockHttpServletResponse.java index 94358fd786f..ed9bae93376 100644 --- a/spring-web/src/testFixtures/java/org/springframework/web/testfixture/servlet/MockHttpServletResponse.java +++ b/spring-web/src/testFixtures/java/org/springframework/web/testfixture/servlet/MockHttpServletResponse.java @@ -169,14 +169,15 @@ public class MockHttpServletResponse implements HttpServletResponse { public void setCharacterEncoding(String characterEncoding) { this.characterEncoding = characterEncoding; this.charset = true; - updateContentTypeHeader(); + updateContentTypePropertyAndHeader(); } - private void updateContentTypeHeader() { + private void updateContentTypePropertyAndHeader() { if (this.contentType != null) { String value = this.contentType; if (this.charset && !this.contentType.toLowerCase().contains(CHARSET_PREFIX)) { value = value + ';' + CHARSET_PREFIX + this.characterEncoding; + this.contentType = value; } doAddHeaderValue(HttpHeaders.CONTENT_TYPE, value, true); } @@ -280,7 +281,7 @@ public class MockHttpServletResponse implements HttpServletResponse { this.charset = true; } } - updateContentTypeHeader(); + updateContentTypePropertyAndHeader(); } } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/view/json/MappingJackson2JsonViewTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/view/json/MappingJackson2JsonViewTests.java index fa62c2a2223..d44dae46ac8 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/view/json/MappingJackson2JsonViewTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/view/json/MappingJackson2JsonViewTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -38,7 +38,6 @@ import com.fasterxml.jackson.databind.ser.FilterProvider; import com.fasterxml.jackson.databind.ser.SerializerFactory; import com.fasterxml.jackson.databind.ser.impl.SimpleBeanPropertyFilter; import com.fasterxml.jackson.databind.ser.impl.SimpleFilterProvider; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mozilla.javascript.Context; import org.mozilla.javascript.ContextFactory; @@ -60,30 +59,19 @@ import static org.mockito.Mockito.mock; * @author Arjen Poutsma * @author Rossen Stoyanchev * @author Sebastien Deleuze + * @author Sam Brannen */ public class MappingJackson2JsonViewTests { - private MappingJackson2JsonView view; + private MappingJackson2JsonView view = new MappingJackson2JsonView(); - private MockHttpServletRequest request; + private MockHttpServletRequest request = new MockHttpServletRequest(); - private MockHttpServletResponse response; + private MockHttpServletResponse response = new MockHttpServletResponse(); - private Context jsContext; + private Context jsContext = ContextFactory.getGlobal().enterContext(); - private ScriptableObject jsScope; - - - @BeforeEach - public void setUp() { - request = new MockHttpServletRequest(); - response = new MockHttpServletResponse(); - - jsContext = ContextFactory.getGlobal().enterContext(); - jsScope = jsContext.initStandardObjects(); - - view = new MappingJackson2JsonView(); - } + private ScriptableObject jsScope = jsContext.initStandardObjects(); @Test @@ -102,7 +90,8 @@ public class MappingJackson2JsonViewTests { assertThat(response.getHeader("Cache-Control")).isEqualTo("no-store"); - assertThat(response.getContentType()).isEqualTo(MappingJackson2JsonView.DEFAULT_CONTENT_TYPE); + MediaType mediaType = MediaType.parseMediaType(response.getContentType()); + assertThat(mediaType.isCompatibleWith(MediaType.parseMediaType(MappingJackson2JsonView.DEFAULT_CONTENT_TYPE))).isTrue(); String jsonResult = response.getContentAsString(); assertThat(jsonResult.length() > 0).isTrue(); @@ -117,12 +106,14 @@ public class MappingJackson2JsonViewTests { model.put("foo", "bar"); view.render(model, request, response); - assertThat(response.getContentType()).isEqualTo("application/json"); + MediaType mediaType = MediaType.parseMediaType(response.getContentType()); + assertThat(mediaType.isCompatibleWith(MediaType.APPLICATION_JSON)).isTrue(); request.setAttribute(View.SELECTED_CONTENT_TYPE, new MediaType("application", "vnd.example-v2+xml")); view.render(model, request, response); - assertThat(response.getContentType()).isEqualTo("application/vnd.example-v2+xml"); + mediaType = MediaType.parseMediaType(response.getContentType()); + assertThat(mediaType.isCompatibleWith(MediaType.parseMediaType("application/vnd.example-v2+xml"))).isTrue(); } @Test @@ -326,10 +317,10 @@ public class MappingJackson2JsonViewTests { if (jsonPrefix != null) { json = json.substring(5); } - Object jsResult = - jsContext.evaluateString(jsScope, "(" + json + ")", "JSON Stream", 1, null); + Object jsResult = jsContext.evaluateString(jsScope, "(" + json + ")", "JSON Stream", 1, null); assertThat(jsResult).as("Json Result did not eval as valid JavaScript").isNotNull(); - assertThat(response.getContentType()).isEqualTo("application/json"); + MediaType mediaType = MediaType.parseMediaType(response.getContentType()); + assertThat(mediaType.isCompatibleWith(MediaType.APPLICATION_JSON)).isTrue(); } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/view/xml/MappingJackson2XmlViewTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/view/xml/MappingJackson2XmlViewTests.java index c96121ea6ce..1ac1a785861 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/view/xml/MappingJackson2XmlViewTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/view/xml/MappingJackson2XmlViewTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -33,7 +33,6 @@ import com.fasterxml.jackson.databind.cfg.SerializerFactoryConfig; import com.fasterxml.jackson.databind.ser.BeanSerializerFactory; import com.fasterxml.jackson.databind.ser.SerializerFactory; import com.fasterxml.jackson.dataformat.xml.XmlMapper; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mozilla.javascript.Context; import org.mozilla.javascript.ContextFactory; @@ -51,30 +50,19 @@ import static org.mockito.Mockito.mock; /** * @author Sebastien Deleuze + * @author Sam Brannen */ public class MappingJackson2XmlViewTests { - private MappingJackson2XmlView view; + private MappingJackson2XmlView view = new MappingJackson2XmlView(); - private MockHttpServletRequest request; + private MockHttpServletRequest request = new MockHttpServletRequest(); - private MockHttpServletResponse response; + private MockHttpServletResponse response = new MockHttpServletResponse(); - private Context jsContext; + private Context jsContext = ContextFactory.getGlobal().enterContext(); - private ScriptableObject jsScope; - - - @BeforeEach - public void setUp() { - request = new MockHttpServletRequest(); - response = new MockHttpServletResponse(); - - jsContext = ContextFactory.getGlobal().enterContext(); - jsScope = jsContext.initStandardObjects(); - - view = new MappingJackson2XmlView(); - } + private ScriptableObject jsScope = jsContext.initStandardObjects(); @Test @@ -93,7 +81,8 @@ public class MappingJackson2XmlViewTests { assertThat(response.getHeader("Cache-Control")).isEqualTo("no-store"); - assertThat(response.getContentType()).isEqualTo(MappingJackson2XmlView.DEFAULT_CONTENT_TYPE); + MediaType mediaType = MediaType.parseMediaType(response.getContentType()); + assertThat(mediaType.isCompatibleWith(MediaType.parseMediaType(MappingJackson2XmlView.DEFAULT_CONTENT_TYPE))).isTrue(); String jsonResult = response.getContentAsString(); assertThat(jsonResult.length() > 0).isTrue(); @@ -108,12 +97,14 @@ public class MappingJackson2XmlViewTests { model.put("foo", "bar"); view.render(model, request, response); - assertThat(response.getContentType()).isEqualTo("application/xml"); + MediaType mediaType = MediaType.parseMediaType(response.getContentType()); + assertThat(mediaType.isCompatibleWith(MediaType.APPLICATION_XML)).isTrue(); request.setAttribute(View.SELECTED_CONTENT_TYPE, new MediaType("application", "vnd.example-v2+xml")); view.render(model, request, response); - assertThat(response.getContentType()).isEqualTo("application/vnd.example-v2+xml"); + mediaType = MediaType.parseMediaType(response.getContentType()); + assertThat(mediaType.isCompatibleWith(MediaType.parseMediaType("application/vnd.example-v2+xml"))).isTrue(); } @Test @@ -232,7 +223,8 @@ public class MappingJackson2XmlViewTests { Object xmlResult = jsContext.evaluateString(jsScope, "(" + response.getContentAsString() + ")", "XML Stream", 1, null); assertThat(xmlResult).as("XML Result did not eval as valid JavaScript").isNotNull(); - assertThat(response.getContentType()).isEqualTo("application/xml"); + MediaType mediaType = MediaType.parseMediaType(response.getContentType()); + assertThat(mediaType.isCompatibleWith(MediaType.APPLICATION_XML)).isTrue(); }