From f4509d6e3a1cf70a561b990ac699962d0d6d7594 Mon Sep 17 00:00:00 2001 From: Dzmitry Kabysh Date: Mon, 7 Jan 2019 19:19:36 -0800 Subject: [PATCH 1/2] Allow any Accept and Content-Type raw values See gh-2079 --- .../MockHttpServletRequestBuilder.java | 33 +++++++++++---- .../MockHttpServletRequestBuilderTests.java | 40 +++++++++++++++++++ 2 files changed, 66 insertions(+), 7 deletions(-) diff --git a/spring-test/src/main/java/org/springframework/test/web/servlet/request/MockHttpServletRequestBuilder.java b/spring-test/src/main/java/org/springframework/test/web/servlet/request/MockHttpServletRequestBuilder.java index c5b527b4adab..51be1286e5d7 100644 --- a/spring-test/src/main/java/org/springframework/test/web/servlet/request/MockHttpServletRequestBuilder.java +++ b/spring-test/src/main/java/org/springframework/test/web/servlet/request/MockHttpServletRequestBuilder.java @@ -294,7 +294,8 @@ public MockHttpServletRequestBuilder contentType(MediaType contentType) { * @since 4.1.2 */ public MockHttpServletRequestBuilder contentType(String contentType) { - this.contentType = MediaType.parseMediaType(contentType).toString(); + Assert.notNull(contentType, "'contentType' must not be null"); + this.contentType = contentType; return this; } @@ -314,11 +315,9 @@ public MockHttpServletRequestBuilder accept(MediaType... mediaTypes) { */ public MockHttpServletRequestBuilder accept(String... mediaTypes) { Assert.notEmpty(mediaTypes, "'mediaTypes' must not be empty"); - List result = new ArrayList<>(mediaTypes.length); - for (String mediaType : mediaTypes) { - result.add(MediaType.parseMediaType(mediaType)); - } - this.headers.set("Accept", MediaType.toString(result)); + List result = new ArrayList<>(mediaTypes.length); + result.addAll(Arrays.asList(mediaTypes)); + this.headers.set("Accept", String.join(", ", result)); return this; } @@ -712,7 +711,7 @@ public final MockHttpServletRequest buildRequest(ServletContext servletContext) if (this.content != null && this.content.length > 0) { String requestContentType = request.getContentType(); - if (requestContentType != null) { + if (requestContentType != null && isValidContentType(requestContentType)) { MediaType mediaType = MediaType.parseMediaType(requestContentType); if (MediaType.APPLICATION_FORM_URLENCODED.includes(mediaType)) { addRequestParams(request, parseFormData(mediaType)); @@ -742,6 +741,26 @@ public final MockHttpServletRequest buildRequest(ServletContext servletContext) return request; } + /** + * Some validation checks to find out if processing of a string value make sense. + */ + private boolean isValidContentType(String mimeType) { + if (!StringUtils.hasLength(mimeType)) { + return false; + } + + int index = mimeType.indexOf(';'); + String fullType = (index >= 0 ? mimeType.substring(0, index) : mimeType).trim(); + if (fullType.isEmpty()) { + return false; + } + int subIndex = fullType.indexOf('/'); + if (subIndex == -1) { + return false; + } + return subIndex != fullType.length() - 1; + } + /** * Create a new {@link MockHttpServletRequest} based on the supplied * {@code ServletContext}. diff --git a/spring-test/src/test/java/org/springframework/test/web/servlet/request/MockHttpServletRequestBuilderTests.java b/spring-test/src/test/java/org/springframework/test/web/servlet/request/MockHttpServletRequestBuilderTests.java index ae111d7877b6..3479d385e622 100644 --- a/spring-test/src/test/java/org/springframework/test/web/servlet/request/MockHttpServletRequestBuilderTests.java +++ b/spring-test/src/test/java/org/springframework/test/web/servlet/request/MockHttpServletRequestBuilderTests.java @@ -29,6 +29,8 @@ import java.util.Locale; import java.util.Map; +import java.util.stream.Collectors; +import java.util.stream.Stream; import javax.servlet.ServletContext; import javax.servlet.http.Cookie; @@ -337,6 +339,21 @@ public void acceptHeader() { assertThat(result.get(1).toString()).isEqualTo("application/xml"); } + @Test + public void anyAcceptMultipleContentTypeViaStringArray() { + this.builder.accept("any", "any2"); + + MockHttpServletRequest request = this.builder.buildRequest(this.servletContext); + List accept = Collections.list(request.getHeaders("Accept")); + List result = Stream.of(accept.get(0).split(",")) + .map(String::trim) + .collect(Collectors.toList()); + + assertThat(result.get(0)).isEqualTo("any"); + assertThat(result).hasSize(2); + assertThat(result.get(1)).isEqualTo("any2"); + } + @Test public void contentType() { this.builder.contentType(MediaType.TEXT_HTML); @@ -363,6 +380,19 @@ public void contentTypeViaString() { assertThat(contentTypes.get(0)).isEqualTo("text/html"); } + @Test + public void anyContentTypeViaString() { + this.builder.contentType("any"); + + MockHttpServletRequest request = this.builder.buildRequest(this.servletContext); + String contentType = request.getContentType(); + List contentTypes = Collections.list(request.getHeaders("Content-Type")); + + assertThat(contentType).isEqualTo("any"); + assertThat(contentTypes).hasSize(1); + assertThat(contentTypes.get(0)).isEqualTo("any"); + } + @Test // SPR-11308 public void contentTypeViaHeader() { this.builder.header("Content-Type", MediaType.TEXT_HTML_VALUE); @@ -372,6 +402,16 @@ public void contentTypeViaHeader() { assertThat(contentType).isEqualTo("text/html"); } + @Test // SPR-17643 + public void invalidContentTypeViaHeader() { + this.builder.header("Content-Type", "yaml"); + this.builder.content("some content"); + MockHttpServletRequest request = this.builder.buildRequest(this.servletContext); + String contentType = request.getContentType(); + + assertThat(contentType).isEqualTo("yaml"); + } + @Test // SPR-11308 public void contentTypeViaMultipleHeaderValues() { this.builder.header("Content-Type", MediaType.TEXT_HTML_VALUE, MediaType.ALL_VALUE); From 542297b30d1dbe36171f76d2a6d6d41f731683f6 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Fri, 13 Dec 2019 16:54:53 +0000 Subject: [PATCH 2/2] Polishing of contribution See gh-2079 --- .../MockHttpServletRequestBuilder.java | 46 +++++++------------ .../MockHttpServletRequestBuilderTests.java | 37 ++++----------- 2 files changed, 25 insertions(+), 58 deletions(-) diff --git a/spring-test/src/main/java/org/springframework/test/web/servlet/request/MockHttpServletRequestBuilder.java b/spring-test/src/main/java/org/springframework/test/web/servlet/request/MockHttpServletRequestBuilder.java index 51be1286e5d7..ef178c394bb6 100644 --- a/spring-test/src/main/java/org/springframework/test/web/servlet/request/MockHttpServletRequestBuilder.java +++ b/spring-test/src/main/java/org/springframework/test/web/servlet/request/MockHttpServletRequestBuilder.java @@ -289,7 +289,8 @@ public MockHttpServletRequestBuilder contentType(MediaType contentType) { } /** - * Set the 'Content-Type' header of the request. + * Set the 'Content-Type' header of the request as a raw String value, + * possibly not even well formed (for testing purposes). * @param contentType the content type * @since 4.1.2 */ @@ -310,14 +311,14 @@ public MockHttpServletRequestBuilder accept(MediaType... mediaTypes) { } /** - * Set the 'Accept' header to the given media type(s). - * @param mediaTypes one or more media types + * Set the 'Accept' header using raw String values, possibly not even well + * formed (for testing purposes). + * @param mediaTypes one or more media types; internally joined as + * comma-separated String */ public MockHttpServletRequestBuilder accept(String... mediaTypes) { Assert.notEmpty(mediaTypes, "'mediaTypes' must not be empty"); - List result = new ArrayList<>(mediaTypes.length); - result.addAll(Arrays.asList(mediaTypes)); - this.headers.set("Accept", String.join(", ", result)); + this.headers.set("Accept", String.join(", ", mediaTypes)); return this; } @@ -711,10 +712,15 @@ public final MockHttpServletRequest buildRequest(ServletContext servletContext) if (this.content != null && this.content.length > 0) { String requestContentType = request.getContentType(); - if (requestContentType != null && isValidContentType(requestContentType)) { - MediaType mediaType = MediaType.parseMediaType(requestContentType); - if (MediaType.APPLICATION_FORM_URLENCODED.includes(mediaType)) { - addRequestParams(request, parseFormData(mediaType)); + if (requestContentType != null) { + try { + MediaType mediaType = MediaType.parseMediaType(requestContentType); + if (MediaType.APPLICATION_FORM_URLENCODED.includes(mediaType)) { + addRequestParams(request, parseFormData(mediaType)); + } + } + catch (Exception ex) { + // Must be invalid, ignore.. } } } @@ -741,26 +747,6 @@ public final MockHttpServletRequest buildRequest(ServletContext servletContext) return request; } - /** - * Some validation checks to find out if processing of a string value make sense. - */ - private boolean isValidContentType(String mimeType) { - if (!StringUtils.hasLength(mimeType)) { - return false; - } - - int index = mimeType.indexOf(';'); - String fullType = (index >= 0 ? mimeType.substring(0, index) : mimeType).trim(); - if (fullType.isEmpty()) { - return false; - } - int subIndex = fullType.indexOf('/'); - if (subIndex == -1) { - return false; - } - return subIndex != fullType.length() - 1; - } - /** * Create a new {@link MockHttpServletRequest} based on the supplied * {@code ServletContext}. diff --git a/spring-test/src/test/java/org/springframework/test/web/servlet/request/MockHttpServletRequestBuilderTests.java b/spring-test/src/test/java/org/springframework/test/web/servlet/request/MockHttpServletRequestBuilderTests.java index 3479d385e622..17fb123acec3 100644 --- a/spring-test/src/test/java/org/springframework/test/web/servlet/request/MockHttpServletRequestBuilderTests.java +++ b/spring-test/src/test/java/org/springframework/test/web/servlet/request/MockHttpServletRequestBuilderTests.java @@ -29,8 +29,6 @@ import java.util.Locale; import java.util.Map; -import java.util.stream.Collectors; -import java.util.stream.Stream; import javax.servlet.ServletContext; import javax.servlet.http.Cookie; @@ -339,19 +337,11 @@ public void acceptHeader() { assertThat(result.get(1).toString()).isEqualTo("application/xml"); } - @Test - public void anyAcceptMultipleContentTypeViaStringArray() { + @Test // gh-2079 + public void acceptHeaderWithInvalidValues() { this.builder.accept("any", "any2"); - MockHttpServletRequest request = this.builder.buildRequest(this.servletContext); - List accept = Collections.list(request.getHeaders("Accept")); - List result = Stream.of(accept.get(0).split(",")) - .map(String::trim) - .collect(Collectors.toList()); - - assertThat(result.get(0)).isEqualTo("any"); - assertThat(result).hasSize(2); - assertThat(result.get(1)).isEqualTo("any2"); + assertThat(request.getHeader("Accept")).isEqualTo("any, any2"); } @Test @@ -380,17 +370,11 @@ public void contentTypeViaString() { assertThat(contentTypes.get(0)).isEqualTo("text/html"); } - @Test - public void anyContentTypeViaString() { + @Test // gh-2079 + public void contentTypeWithInvalidValue() { this.builder.contentType("any"); - MockHttpServletRequest request = this.builder.buildRequest(this.servletContext); - String contentType = request.getContentType(); - List contentTypes = Collections.list(request.getHeaders("Content-Type")); - - assertThat(contentType).isEqualTo("any"); - assertThat(contentTypes).hasSize(1); - assertThat(contentTypes.get(0)).isEqualTo("any"); + assertThat(request.getContentType()).isEqualTo("any"); } @Test // SPR-11308 @@ -402,14 +386,11 @@ public void contentTypeViaHeader() { assertThat(contentType).isEqualTo("text/html"); } - @Test // SPR-17643 - public void invalidContentTypeViaHeader() { + @Test // gh-2079 + public void contentTypeViaHeaderWithInvalidValue() { this.builder.header("Content-Type", "yaml"); - this.builder.content("some content"); MockHttpServletRequest request = this.builder.buildRequest(this.servletContext); - String contentType = request.getContentType(); - - assertThat(contentType).isEqualTo("yaml"); + assertThat(request.getContentType()).isEqualTo("yaml"); } @Test // SPR-11308