From f481e2fc8a3b9a095e179fe4847f26f356cac9d9 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Mon, 20 Jul 2015 16:29:31 +0100 Subject: [PATCH] Fix request parameter handling in multipart http request snippet MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, request parameters for a POST were form encoded into the body, irrespective of the request’s type. This was incorrect for a multipart request where each request parameter should be included in a separate request part. This commit updates HttpDocumentation to treat request parameters differently when dealing with a multipart request. In such cases each request parameter is now included in the body of the request in a separate request part. Fixes gh-94 --- .../restdocs/http/HttpDocumentation.java | 32 +++++++---- .../restdocs/http/HttpDocumentationTests.java | 56 ++++++++++++++++--- 2 files changed, 69 insertions(+), 19 deletions(-) diff --git a/spring-restdocs/src/main/java/org/springframework/restdocs/http/HttpDocumentation.java b/spring-restdocs/src/main/java/org/springframework/restdocs/http/HttpDocumentation.java index 781024bd..025f17ed 100644 --- a/spring-restdocs/src/main/java/org/springframework/restdocs/http/HttpDocumentation.java +++ b/spring-restdocs/src/main/java/org/springframework/restdocs/http/HttpDocumentation.java @@ -125,13 +125,15 @@ public abstract class HttpDocumentation { this.writer.println(request.getContentAsString()); } else if (request.isPostRequest() || request.isPutRequest()) { - String queryString = request.getParameterMapAsQueryString(); - if (StringUtils.hasText(queryString)) { - this.writer.println(queryString); - } if (request.isMultipartRequest()) { writeParts(request); } + else { + String queryString = request.getParameterMapAsQueryString(); + if (StringUtils.hasText(queryString)) { + this.writer.println(queryString); + } + } } } @@ -152,6 +154,13 @@ public abstract class HttpDocumentation { private void writeParts(DocumentableHttpServletRequest request) throws IOException { + for (Entry parameter : request.getParameterMap().entrySet()) { + for (String value : parameter.getValue()) { + writePartBoundary(); + writePart(parameter.getKey(), value, null); + this.writer.println(); + } + } for (Entry> entry : request.getMultipartFiles() .entrySet()) { for (MultipartFile file : entry.getValue()) { @@ -167,14 +176,17 @@ public abstract class HttpDocumentation { this.writer.printf("--%s%n", MULTIPART_BOUNDARY); } - private void writePart(MultipartFile part) throws IOException { - this.writer.printf("Content-Disposition: form-data; name=%s%n", - part.getName()); - if (StringUtils.hasText(part.getContentType())) { - this.writer.printf("Content-Type: %s%n", part.getContentType()); + private void writePart(String name, String value, String contentType) { + this.writer.printf("Content-Disposition: form-data; name=%s%n", name); + if (StringUtils.hasText(contentType)) { + this.writer.printf("Content-Type: %s%n", contentType); } this.writer.println(); - this.writer.print(new String(part.getBytes())); + this.writer.print(value); + } + + private void writePart(MultipartFile part) throws IOException { + writePart(part.getName(), new String(part.getBytes()), part.getContentType()); } private void writeMultipartEnd() { diff --git a/spring-restdocs/src/test/java/org/springframework/restdocs/http/HttpDocumentationTests.java b/spring-restdocs/src/test/java/org/springframework/restdocs/http/HttpDocumentationTests.java index b55ddec8..b2486e32 100644 --- a/spring-restdocs/src/test/java/org/springframework/restdocs/http/HttpDocumentationTests.java +++ b/spring-restdocs/src/test/java/org/springframework/restdocs/http/HttpDocumentationTests.java @@ -50,6 +50,8 @@ import org.springframework.restdocs.test.ExpectedSnippet; */ public class HttpDocumentationTests { + private static final String BOUNDARY = "6o2knFse3p53ty9dmcQvWAIx1zInP11uCfbm"; + @Rule public final ExpectedSnippet snippet = new ExpectedSnippet(); @@ -163,14 +165,13 @@ public class HttpDocumentationTests { @Test public void multipartPost() throws IOException { - String boundary = "6o2knFse3p53ty9dmcQvWAIx1zInP11uCfbm"; - String expectedContent = String.format("--%s%nContent-Disposition: form-data; " - + "name=image%n%n<< data >>%n--%s--", boundary, boundary); + String expectedContent = createPart(String.format("Content-Disposition: " + + "form-data; " + "name=image%n%n<< data >>")); this.snippet.expectHttpRequest("multipart-post").withContents( httpRequest(POST, "/upload") .header(HttpHeaders.HOST, "localhost") .header("Content-Type", - "multipart/form-data; boundary=" + boundary) + "multipart/form-data; boundary=" + BOUNDARY) .content(expectedContent)); MockMultipartFile multipartFile = new MockMultipartFile("image", "documents/images/example.png", null, "<< data >>".getBytes()); @@ -178,17 +179,40 @@ public class HttpDocumentationTests { result(fileUpload("/upload").file(multipartFile))); } + @Test + public void multipartPostWithParameters() throws IOException { + String param1Part = createPart(String.format("Content-Disposition: form-data; " + + "name=a%n%napple"), false); + String param2Part = createPart(String.format("Content-Disposition: form-data; " + + "name=a%n%navocado"), false); + String param3Part = createPart(String.format("Content-Disposition: form-data; " + + "name=b%n%nbanana"), false); + String filePart = createPart(String.format("Content-Disposition: form-data; " + + "name=image%n%n<< data >>")); + String expectedContent = param1Part + param2Part + param3Part + filePart; + this.snippet.expectHttpRequest("multipart-post").withContents( + httpRequest(POST, "/upload") + .header(HttpHeaders.HOST, "localhost") + .header("Content-Type", + "multipart/form-data; boundary=" + BOUNDARY) + .content(expectedContent)); + MockMultipartFile multipartFile = new MockMultipartFile("image", + "documents/images/example.png", null, "<< data >>".getBytes()); + documentHttpRequest("multipart-post").handle( + result(fileUpload("/upload").file(multipartFile) + .param("a", "apple", "avocado").param("b", "banana"))); + } + @Test public void multipartPostWithContentType() throws IOException { - String boundary = "6o2knFse3p53ty9dmcQvWAIx1zInP11uCfbm"; - String expectedContent = String.format("--%s%nContent-Disposition: form-data; " - + "name=image%nContent-Type: image/png%n%n<< data >>%n--%s--", boundary, - boundary); + String expectedContent = createPart(String + .format("Content-Disposition: form-data; name=image%nContent-Type: " + + "image/png%n%n<< data >>")); this.snippet.expectHttpRequest("multipart-post-with-content-type").withContents( httpRequest(POST, "/upload") .header(HttpHeaders.HOST, "localhost") .header("Content-Type", - "multipart/form-data; boundary=" + boundary) + "multipart/form-data; boundary=" + BOUNDARY) .content(expectedContent)); MockMultipartFile multipartFile = new MockMultipartFile("image", "documents/images/example.png", MediaType.IMAGE_PNG_VALUE, @@ -216,4 +240,18 @@ public class HttpDocumentationTests { documentHttpRequest("get-request-custom-host").handle( result(get("/foo").header(HttpHeaders.HOST, "api.example.com"))); } + + private String createPart(String content) { + return this.createPart(content, true); + } + + private String createPart(String content, boolean last) { + String boundary = "6o2knFse3p53ty9dmcQvWAIx1zInP11uCfbm"; + StringBuilder part = new StringBuilder(); + part.append(String.format("--%s%n%s%n", boundary, content)); + if (last) { + part.append(String.format("--%s--", boundary)); + } + return part.toString(); + } }