From 043732aef3ef79206a53ddf692f8f268d423e020 Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Thu, 12 Feb 2015 18:37:54 +0000 Subject: [PATCH] More careful handling of multipart data in proxy To write file data the message converter needs the file content to be provided in the form of an HttpEntity per file. Fixes gh-197 again --- .../filters/pre/FormBodyWrapperFilter.java | 42 +++++++++++++------ .../filters/pre/Servlet30WrapperFilter.java | 14 ++++++- .../zuul/FormZuulProxyApplicationTests.java | 42 ++++++++++++++----- 3 files changed, 73 insertions(+), 25 deletions(-) diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/FormBodyWrapperFilter.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/FormBodyWrapperFilter.java index 96816a91..acebd4da 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/FormBodyWrapperFilter.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/FormBodyWrapperFilter.java @@ -25,6 +25,7 @@ import java.util.Map.Entry; import javax.servlet.ServletInputStream; import javax.servlet.http.HttpServletRequest; +import org.springframework.http.HttpEntity; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpOutputMessage; import org.springframework.http.InvalidMediaTypeException; @@ -34,6 +35,8 @@ import org.springframework.util.Assert; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; import org.springframework.util.ReflectionUtils; +import org.springframework.web.multipart.MultipartFile; +import org.springframework.web.multipart.MultipartRequest; import com.netflix.zuul.ZuulFilter; import com.netflix.zuul.context.RequestContext; @@ -152,22 +155,36 @@ public class FormBodyWrapperFilter extends ZuulFilter { } private synchronized void buildContentData() { - MultiValueMap builder = new LinkedMultiValueMap(); - for (Entry entry : this.request.getParameterMap() - .entrySet()) { - for (String value : entry.getValue()) { - builder.add(entry.getKey(), value); - } - } - FormHttpOutputMessage data = new FormHttpOutputMessage(); - data.getHeaders().setContentType( - MediaType.valueOf(this.request.getContentType())); try { + MultiValueMap builder = new LinkedMultiValueMap(); + for (Entry entry : this.request.getParameterMap() + .entrySet()) { + for (String value : entry.getValue()) { + builder.add(entry.getKey(), value); + } + } + if (this.request instanceof MultipartRequest) { + MultipartRequest multi = (MultipartRequest) this.request; + for (Entry part : multi.getFileMap() + .entrySet()) { + MultipartFile file = part.getValue(); + HttpHeaders headers = new HttpHeaders(); + headers.setContentDispositionFormData(file.getName(), + file.getOriginalFilename()); + headers.setContentType(MediaType.valueOf(file.getContentType())); + HttpEntity entity = new HttpEntity( + file.getBytes(), headers); + builder.set(part.getKey(), entity); + } + } + FormHttpOutputMessage data = new FormHttpOutputMessage(); + this.contentType = MediaType.valueOf(this.request.getContentType()); + data.getHeaders().setContentType(this.contentType); this.converter.write(builder, this.contentType, data); + // copy new content type including multipart boundary this.contentType = data.getHeaders().getContentType(); - this.contentLength = new Long(data.getHeaders().getContentLength()) - .intValue(); this.contentData = data.getInput(); + this.contentLength = this.contentData.length; } catch (Exception e) { throw new IllegalStateException("Cannot convert form data", e); @@ -190,6 +207,7 @@ public class FormBodyWrapperFilter extends ZuulFilter { } public byte[] getInput() throws IOException { + this.output.flush(); return this.output.toByteArray(); } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/Servlet30WrapperFilter.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/Servlet30WrapperFilter.java index 69abe82b..564a5f3d 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/Servlet30WrapperFilter.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/Servlet30WrapperFilter.java @@ -136,12 +136,22 @@ public class Servlet30WrapperFilter extends ZuulFilter { @Override public boolean isAsyncStarted() { - return this.request.isAsyncStarted(); + try { + return this.request.isAsyncStarted(); + } + catch (Throwable e) { + return false; + } } @Override public boolean isAsyncSupported() { - return this.request.isAsyncSupported(); + try { + return this.request.isAsyncSupported(); + } + catch (Throwable e) { + return false; + } } @Override diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/FormZuulProxyApplicationTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/FormZuulProxyApplicationTests.java index 1d5a4875..0ed59e82 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/FormZuulProxyApplicationTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/FormZuulProxyApplicationTests.java @@ -16,11 +16,11 @@ package org.springframework.cloud.netflix.zuul; +import java.io.IOException; import java.util.Arrays; import org.junit.Test; import org.junit.runner.RunWith; -import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; import org.springframework.boot.SpringApplication; import org.springframework.boot.autoconfigure.EnableAutoConfiguration; @@ -28,7 +28,6 @@ import org.springframework.boot.test.IntegrationTest; import org.springframework.boot.test.SpringApplicationConfiguration; import org.springframework.boot.test.TestRestTemplate; import org.springframework.cloud.netflix.ribbon.RibbonClient; -import org.springframework.cloud.netflix.zuul.filters.ProxyRouteLocator; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.http.HttpEntity; @@ -42,10 +41,11 @@ import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; import org.springframework.test.context.web.WebAppConfiguration; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; -import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestMethod; +import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.bind.annotation.RestController; +import org.springframework.web.multipart.MultipartFile; import com.netflix.appinfo.EurekaInstanceConfig; import com.netflix.loadbalancer.BaseLoadBalancer; @@ -65,12 +65,6 @@ public class FormZuulProxyApplicationTests { @Value("${local.server.port}") private int port; - @Autowired - private ProxyRouteLocator routes; - - @Autowired - private RoutesEndpoint endpoint; - @Test public void postWithForm() { MultiValueMap form = new LinkedMultiValueMap(); @@ -99,6 +93,23 @@ public class FormZuulProxyApplicationTests { assertEquals("Posted! {foo=[bar]}", result.getBody()); } + @Test + public void postWithMultipartFile() { + MultiValueMap form = new LinkedMultiValueMap(); + HttpHeaders part = new HttpHeaders(); + part.setContentType(MediaType.TEXT_PLAIN); + part.setContentDispositionFormData("file", "foo.txt"); + form.set("foo", new HttpEntity("bar".getBytes(), part)); + HttpHeaders headers = new HttpHeaders(); + headers.setContentType(MediaType.MULTIPART_FORM_DATA); + ResponseEntity result = new TestRestTemplate().exchange( + "http://localhost:" + this.port + "/simple/file", HttpMethod.POST, + new HttpEntity>(form, headers), + String.class); + assertEquals(HttpStatus.OK, result.getStatusCode()); + assertEquals("Posted! bar", result.getBody()); + } + @Test public void postWithUTF8Form() { MultiValueMap form = new LinkedMultiValueMap(); @@ -124,10 +135,18 @@ public class FormZuulProxyApplicationTests { class FormZuulProxyApplication { @RequestMapping(value = "/", method = RequestMethod.POST) - public String delete(@RequestBody MultiValueMap form) { + public String accept(@RequestParam MultiValueMap form) + throws IOException { return "Posted! " + form; } + // TODO: Why does this not work if you add @RequestParam as above? + @RequestMapping(value = "/file", method = RequestMethod.POST) + public String file(@RequestParam(required = false) MultipartFile file) + throws IOException { + return "Posted! " + (file == null ? "" : new String(file.getBytes())); + } + @Bean public ZuulFilter sampleFilter() { return new ZuulFilter() { @@ -156,7 +175,7 @@ class FormZuulProxyApplication { } public static void main(String[] args) { - SpringApplication.run(SampleZuulProxyApplication.class, args); + SpringApplication.run(FormZuulProxyApplication.class, args); } } @@ -170,6 +189,7 @@ class FormRibbonClientConfiguration { BaseLoadBalancer balancer = new BaseLoadBalancer(); balancer.setServersList(Arrays.asList(new Server("localhost", instance .getNonSecurePort()))); + // balancer.setServersList(Arrays.asList(new Server("localhost", 8000))); return balancer; }