From 8cf19331483e09264452890e73434c06093e3b8e Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Wed, 12 Oct 2016 21:32:37 +0200 Subject: [PATCH] Only print request/response body if char enc is present in MVC Test Commit e65a1a4372 introduced support in PrintingResultHandler for only printing the request or response body in the Spring MVC Test framework if the content type is known to be text-based (e.g., plain text, HTML, XHTML, XML, JSON, etc.). For unknown content types the body is assumed to be text-based and is therefore always printed. The latter behavior, however, is undesirable since the content may in fact not be text-based. This commit addresses this issue by making the printing of the request or response body an opt-in feature. Specifically, if a character encoding has been set, the request or response body will be printed by the PrintingResultHandler. Note, however, that the character encoding is set to ISO-8859-1 in MockHttpServletResponse by default. In addition, MockHttpServletRequest's getContentAsString() method now throws an IllegalStateException if the character encoding has not been set. Issue: SPR-14776 --- .../mock/web/MockHttpServletRequest.java | 22 +++++--- .../servlet/result/PrintingResultHandler.java | 36 ++----------- .../mock/web/MockHttpServletRequestTests.java | 19 +++---- .../result/PrintingResultHandlerTests.java | 51 +++++++++---------- .../mock/web/test/MockHttpServletRequest.java | 22 +++++--- 5 files changed, 71 insertions(+), 79 deletions(-) diff --git a/spring-test/src/main/java/org/springframework/mock/web/MockHttpServletRequest.java b/spring-test/src/main/java/org/springframework/mock/web/MockHttpServletRequest.java index 9117ead65de..f900e865491 100644 --- a/spring-test/src/main/java/org/springframework/mock/web/MockHttpServletRequest.java +++ b/spring-test/src/main/java/org/springframework/mock/web/MockHttpServletRequest.java @@ -373,6 +373,10 @@ public class MockHttpServletRequest implements HttpServletRequest { /** * Set the content of the request body as a byte array. + *

If the supplied byte array represents text such as XML or JSON, the + * {@link #setCharacterEncoding character encoding} should typically be + * set as well. + * @see #setCharacterEncoding(String) * @see #getContentAsByteArray() * @see #getContentAsString() */ @@ -382,6 +386,7 @@ public class MockHttpServletRequest implements HttpServletRequest { /** * Get the content of the request body as a byte array. + * @return the content as a byte array, potentially {@code null} * @since 5.0 * @see #setContent(byte[]) * @see #getContentAsString() @@ -392,19 +397,24 @@ public class MockHttpServletRequest implements HttpServletRequest { /** * Get the content of the request body as a {@code String}, using the configured - * {@linkplain #getCharacterEncoding character encoding} if present. + * {@linkplain #getCharacterEncoding character encoding}. + * @return the content as a {@code String}, potentially {@code null} + * @throws IllegalStateException if the character encoding has not been set + * @throws UnsupportedEncodingException if the character encoding is not supported * @since 5.0 * @see #setContent(byte[]) - * @see #getContentAsByteArray() * @see #setCharacterEncoding(String) + * @see #getContentAsByteArray() */ - public String getContentAsString() throws UnsupportedEncodingException { + public String getContentAsString() throws IllegalStateException, UnsupportedEncodingException { + Assert.state(this.characterEncoding != null, + "Cannot get content as a String for a null character encoding. " + + "Consider setting the characterEncoding in the request."); + if (this.content == null) { return null; } - - return (this.characterEncoding != null ? - new String(this.content, this.characterEncoding) : new String(this.content)); + return new String(this.content, this.characterEncoding); } @Override diff --git a/spring-test/src/main/java/org/springframework/test/web/servlet/result/PrintingResultHandler.java b/spring-test/src/main/java/org/springframework/test/web/servlet/result/PrintingResultHandler.java index 89a4ce4c7a3..c7a370fcaad 100644 --- a/spring-test/src/main/java/org/springframework/test/web/servlet/result/PrintingResultHandler.java +++ b/spring-test/src/main/java/org/springframework/test/web/servlet/result/PrintingResultHandler.java @@ -16,9 +16,7 @@ package org.springframework.test.web.servlet.result; -import java.util.Arrays; import java.util.Enumeration; -import java.util.List; import java.util.Map; import javax.servlet.http.Cookie; @@ -31,8 +29,6 @@ import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.test.web.servlet.MvcResult; import org.springframework.test.web.servlet.ResultHandler; import org.springframework.util.LinkedMultiValueMap; -import org.springframework.util.MimeType; -import org.springframework.util.MimeTypeUtils; import org.springframework.util.MultiValueMap; import org.springframework.util.ObjectUtils; import org.springframework.validation.BindingResult; @@ -58,12 +54,7 @@ import org.springframework.web.servlet.support.RequestContextUtils; */ public class PrintingResultHandler implements ResultHandler { - private static final String NOT_PRINTABLE = ""; - - private static final List printableMimeTypes = Arrays.asList( - MimeTypeUtils.APPLICATION_JSON, MimeTypeUtils.APPLICATION_XML, - new MimeType("text", "*"), new MimeType("application", "*+json"), - new MimeType("application", "*+xml")); + private static final String MISSING_CHARACTER_ENCODING = ""; private final ResultValuePrinter printer; @@ -115,8 +106,8 @@ public class PrintingResultHandler implements ResultHandler { * Print the request. */ protected void printRequest(MockHttpServletRequest request) throws Exception { - String body = (isPrintableContentType(request.getContentType()) ? - request.getContentAsString() : NOT_PRINTABLE); + String body = (request.getCharacterEncoding() != null ? + request.getContentAsString() : MISSING_CHARACTER_ENCODING); this.printer.printValue("HTTP Method", request.getMethod()); this.printer.printValue("Request URI", request.getRequestURI()); @@ -238,8 +229,8 @@ public class PrintingResultHandler implements ResultHandler { * Print the response. */ protected void printResponse(MockHttpServletResponse response) throws Exception { - String body = (isPrintableContentType(response.getContentType()) ? - response.getContentAsString() : NOT_PRINTABLE); + String body = (response.getCharacterEncoding() != null ? + response.getContentAsString() : MISSING_CHARACTER_ENCODING); this.printer.printValue("Status", response.getStatus()); this.printer.printValue("Error message", response.getErrorMessage()); @@ -284,23 +275,6 @@ public class PrintingResultHandler implements ResultHandler { } - /** - * Determine if the supplied content type is printable (i.e., text-based). - *

If the supplied content type is {@code null} (i.e., unknown), this method - * assumes that the content is printable by default and returns {@code true}. - * @param contentType the content type to check; {@code null} if unknown - * @return {@code true} if the content type is known to be or assumed to be printable - * @since 5.0 - */ - private static boolean isPrintableContentType(String contentType) { - if (contentType == null) { - return true; - } - MimeType mimeType = MimeType.valueOf(contentType); - return printableMimeTypes.stream().anyMatch(printable -> printable.includes(mimeType)); - } - - /** * A contract for how to actually write result information. */ diff --git a/spring-test/src/test/java/org/springframework/mock/web/MockHttpServletRequestTests.java b/spring-test/src/test/java/org/springframework/mock/web/MockHttpServletRequestTests.java index 6ba9394b0a9..55d6fb74d1e 100644 --- a/spring-test/src/test/java/org/springframework/mock/web/MockHttpServletRequestTests.java +++ b/spring-test/src/test/java/org/springframework/mock/web/MockHttpServletRequestTests.java @@ -30,7 +30,9 @@ import java.util.Map; import javax.servlet.http.Cookie; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import org.springframework.util.StreamUtils; @@ -54,7 +56,10 @@ public class MockHttpServletRequestTests { private static final String IF_MODIFIED_SINCE = "If-Modified-Since"; - private MockHttpServletRequest request = new MockHttpServletRequest(); + private final MockHttpServletRequest request = new MockHttpServletRequest(); + + @Rule + public final ExpectedException exception = ExpectedException.none(); @Test @@ -76,13 +81,10 @@ public class MockHttpServletRequestTests { } @Test - public void setContentAndGetContentAsStringWithDefaultCharacterEncoding() throws IOException { - String palindrome = "ablE was I ere I saw Elba"; - byte[] bytes = palindrome.getBytes(); - request.setContent(bytes); - assertEquals(bytes.length, request.getContentLength()); - assertNotNull(request.getContentAsString()); - assertEquals(palindrome, request.getContentAsString()); + public void getContentAsStringWithoutSettingCharacterEncoding() throws IOException { + exception.expect(IllegalStateException.class); + exception.expectMessage("Cannot get content as a String for a null character encoding"); + request.getContentAsString(); } @Test @@ -102,7 +104,6 @@ public class MockHttpServletRequestTests { assertNotNull(request.getInputStream()); assertEquals(-1, request.getInputStream().read()); assertNull(request.getContentAsByteArray()); - assertNull(request.getContentAsString()); } @Test diff --git a/spring-test/src/test/java/org/springframework/test/web/servlet/result/PrintingResultHandlerTests.java b/spring-test/src/test/java/org/springframework/test/web/servlet/result/PrintingResultHandlerTests.java index fcbb4129e3f..0ac5176b1a7 100644 --- a/spring-test/src/test/java/org/springframework/test/web/servlet/result/PrintingResultHandlerTests.java +++ b/spring-test/src/test/java/org/springframework/test/web/servlet/result/PrintingResultHandlerTests.java @@ -17,9 +17,7 @@ package org.springframework.test.web.servlet.result; import java.net.URI; -import java.util.Arrays; import java.util.HashMap; -import java.util.List; import java.util.Map; import javax.servlet.http.Cookie; @@ -33,7 +31,6 @@ import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.test.web.servlet.StubMvcResult; import org.springframework.util.Assert; import org.springframework.util.LinkedMultiValueMap; -import org.springframework.util.MimeTypeUtils; import org.springframework.util.MultiValueMap; import org.springframework.validation.BindException; import org.springframework.validation.BindingResult; @@ -53,11 +50,6 @@ import static org.junit.Assert.*; */ public class PrintingResultHandlerTests { - private static final List textContentTypes = Arrays.asList(MimeTypeUtils.APPLICATION_JSON_VALUE, - MimeTypeUtils.APPLICATION_XML_VALUE, MimeTypeUtils.APPLICATION_XHTML_XML_VALUE, - MimeTypeUtils.TEXT_HTML_VALUE, MimeTypeUtils.TEXT_PLAIN_VALUE); - - private final TestPrintingResultHandler handler = new TestPrintingResultHandler(); private final MockHttpServletRequest request = new MockHttpServletRequest("GET", "/") { @@ -147,43 +139,48 @@ public class PrintingResultHandlerTests { } @Test - public void printRequestWithTextContentTypes() throws Exception { - this.request.setContent("text".getBytes()); + public void printRequestWithCharacterEncoding() throws Exception { + this.request.setCharacterEncoding("UTF-8"); + this.request.setContent("text".getBytes("UTF-8")); - for (String contentType: textContentTypes) { - this.request.setContentType(contentType); - this.handler.handle(this.mvcResult); - assertValue("MockHttpServletRequest", "Body", "text"); - } + this.handler.handle(this.mvcResult); + + assertValue("MockHttpServletRequest", "Body", "text"); } @Test - public void printResponseWithTextContentTypes() throws Exception { + public void printRequestWithoutCharacterEncoding() throws Exception { + this.handler.handle(this.mvcResult); + + assertValue("MockHttpServletRequest", "Body", ""); + } + + @Test + public void printResponseWithCharacterEncoding() throws Exception { + this.response.setCharacterEncoding("UTF-8"); this.response.getWriter().print("text"); - for (String contentType: textContentTypes) { - this.response.setContentType(contentType); - this.handler.handle(this.mvcResult); - assertValue("MockHttpServletResponse", "Body", "text"); - } + this.handler.handle(this.mvcResult); + assertValue("MockHttpServletResponse", "Body", "text"); } @Test - public void printRequestWithBinaryContentType() throws Exception { - this.request.setContentType(MimeTypeUtils.IMAGE_JPEG_VALUE); + public void printResponseWithDefaultCharacterEncoding() throws Exception { + this.response.getWriter().print("text"); this.handler.handle(this.mvcResult); - assertValue("MockHttpServletRequest", "Body", ""); + assertValue("MockHttpServletResponse", "Body", "text"); } @Test - public void printResponseWithBinaryContentType() throws Exception { - this.response.setContentType(MimeTypeUtils.IMAGE_JPEG_VALUE); + public void printResponseWithoutCharacterEncoding() throws Exception { + this.response.setCharacterEncoding(null); + this.response.getWriter().print("text"); this.handler.handle(this.mvcResult); - assertValue("MockHttpServletResponse", "Body", ""); + assertValue("MockHttpServletResponse", "Body", ""); } @Test diff --git a/spring-web/src/test/java/org/springframework/mock/web/test/MockHttpServletRequest.java b/spring-web/src/test/java/org/springframework/mock/web/test/MockHttpServletRequest.java index 527c1a704c1..7c75496b2a9 100644 --- a/spring-web/src/test/java/org/springframework/mock/web/test/MockHttpServletRequest.java +++ b/spring-web/src/test/java/org/springframework/mock/web/test/MockHttpServletRequest.java @@ -373,6 +373,10 @@ public class MockHttpServletRequest implements HttpServletRequest { /** * Set the content of the request body as a byte array. + *

If the supplied byte array represents text such as XML or JSON, the + * {@link #setCharacterEncoding character encoding} should typically be + * set as well. + * @see #setCharacterEncoding(String) * @see #getContentAsByteArray() * @see #getContentAsString() */ @@ -382,6 +386,7 @@ public class MockHttpServletRequest implements HttpServletRequest { /** * Get the content of the request body as a byte array. + * @return the content as a byte array, potentially {@code null} * @since 5.0 * @see #setContent(byte[]) * @see #getContentAsString() @@ -392,19 +397,24 @@ public class MockHttpServletRequest implements HttpServletRequest { /** * Get the content of the request body as a {@code String}, using the configured - * {@linkplain #getCharacterEncoding character encoding} if present. + * {@linkplain #getCharacterEncoding character encoding}. + * @return the content as a {@code String}, potentially {@code null} + * @throws IllegalStateException if the character encoding has not been set + * @throws UnsupportedEncodingException if the character encoding is not supported * @since 5.0 * @see #setContent(byte[]) - * @see #getContentAsByteArray() * @see #setCharacterEncoding(String) + * @see #getContentAsByteArray() */ - public String getContentAsString() throws UnsupportedEncodingException { + public String getContentAsString() throws IllegalStateException, UnsupportedEncodingException { + Assert.state(this.characterEncoding != null, + "Cannot get content as a String for a null character encoding. " + + "Consider setting the characterEncoding in the request."); + if (this.content == null) { return null; } - - return (this.characterEncoding != null ? - new String(this.content, this.characterEncoding) : new String(this.content)); + return new String(this.content, this.characterEncoding); } @Override