From da4f5d3454dd5190f730972223c068d9646dfee3 Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Fri, 13 Mar 2015 13:07:33 +0000 Subject: [PATCH] Fix issue with servlet request wrappers in Zuul Because of the way that a FormBodyServletRequestWrapper was implemented (extending the Zuul servlet 2.5 wrapper) it could barf at runtime if anyone called its servlet 3.0 methods. The fix for that was to extract our Servlet30RequestWrapper and extend that instead. Also tweaked the DebugFilter a bit so it doesn't try and display the whole payload. Probably speeds up file uploads a bit but the fact that we have to store the whole request body in memory is going to kill us eventually. See gh-254 --- .../zuul/filters/ProxyRequestHelper.java | 7 +- .../filters/pre/FormBodyWrapperFilter.java | 16 ++- .../filters/pre/Servlet30RequestWrapper.java | 116 ++++++++++++++++++ .../filters/pre/Servlet30WrapperFilter.java | 94 -------------- .../zuul/FormZuulProxyApplicationTests.java | 53 ++++++-- .../src/test/resources/static/index.html | 16 +++ 6 files changed, 190 insertions(+), 112 deletions(-) create mode 100644 spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/Servlet30RequestWrapper.java create mode 100644 spring-cloud-netflix-core/src/test/resources/static/index.html diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ProxyRequestHelper.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ProxyRequestHelper.java index 0b702a00..49b8ccef 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ProxyRequestHelper.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ProxyRequestHelper.java @@ -226,9 +226,14 @@ public class ProxyRequestHelper { private void debugRequestEntity(Map info, InputStream inputStream) throws IOException { + if (RequestContext.getCurrentContext().isChunkedRequestBody()) { + info.put("body", ""); + return; + } String entity = IOUtils.toString(inputStream); if (StringUtils.hasText(entity)) { - info.put("body", entity); + info.put("body", entity.length() <= 4096 ? entity : entity.substring(0, 4096) + + ""); } } 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 acebd4da..be238dff 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 @@ -108,7 +108,7 @@ public class FormBodyWrapperFilter extends ZuulFilter { return null; } - private class FormBodyRequestWrapper extends HttpServletRequestWrapper { + private class FormBodyRequestWrapper extends Servlet30RequestWrapper { private HttpServletRequest request; @@ -135,6 +135,9 @@ public class FormBodyWrapperFilter extends ZuulFilter { @Override public int getContentLength() { + if (super.getContentLength() <= 0) { + return super.getContentLength(); + } if (this.contentData == null) { buildContentData(); } @@ -143,15 +146,10 @@ public class FormBodyWrapperFilter extends ZuulFilter { @Override public ServletInputStream getInputStream() throws IOException { - if (RequestContext.getCurrentContext().isChunkedRequestBody()) { - return this.request.getInputStream(); - } - else { - if (this.contentData == null) { - buildContentData(); - } - return new ServletInputStreamWrapper(this.contentData); + if (this.contentData == null) { + buildContentData(); } + return new ServletInputStreamWrapper(this.contentData); } private synchronized void buildContentData() { diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/Servlet30RequestWrapper.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/Servlet30RequestWrapper.java new file mode 100644 index 00000000..3c7a2944 --- /dev/null +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/Servlet30RequestWrapper.java @@ -0,0 +1,116 @@ +/* + * Copyright 2013-2015 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.netflix.zuul.filters.pre; + +import java.io.IOException; +import java.util.Collection; + +import javax.servlet.AsyncContext; +import javax.servlet.DispatcherType; +import javax.servlet.ServletContext; +import javax.servlet.ServletException; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import javax.servlet.http.Part; + +import com.netflix.zuul.http.HttpServletRequestWrapper; + +class Servlet30RequestWrapper extends HttpServletRequestWrapper { + private HttpServletRequest request; + + Servlet30RequestWrapper(HttpServletRequest request) { + super(request); + this.request = request; + } + + @Override + public boolean authenticate(HttpServletResponse response) throws IOException, + ServletException { + return this.request.authenticate(response); + } + + @Override + public void login(String username, String password) throws ServletException { + this.request.login(username, password); + } + + @Override + public void logout() throws ServletException { + this.request.logout(); + } + + @Override + public Collection getParts() throws IOException, IllegalStateException, + ServletException { + return this.request.getParts(); + } + + @Override + public Part getPart(String name) throws IOException, IllegalStateException, + ServletException { + return this.request.getPart(name); + } + + @Override + public ServletContext getServletContext() { + return this.request.getServletContext(); + } + + @Override + public AsyncContext startAsync() { + return this.request.startAsync(); + } + + @Override + public AsyncContext startAsync(ServletRequest servletRequest, + ServletResponse servletResponse) { + return this.request.startAsync(servletRequest, servletResponse); + } + + @Override + public boolean isAsyncStarted() { + try { + return this.request.isAsyncStarted(); + } + catch (Throwable e) { + return false; + } + } + + @Override + public boolean isAsyncSupported() { + try { + return this.request.isAsyncSupported(); + } + catch (Throwable e) { + return false; + } + } + + @Override + public AsyncContext getAsyncContext() { + return this.request.getAsyncContext(); + } + + @Override + public DispatcherType getDispatcherType() { + return this.request.getDispatcherType(); + } + +} \ No newline at end of file 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 564a5f3d..32556258 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 @@ -16,19 +16,9 @@ package org.springframework.cloud.netflix.zuul.filters.pre; -import java.io.IOException; import java.lang.reflect.Field; -import java.util.Collection; -import javax.servlet.AsyncContext; -import javax.servlet.DispatcherType; -import javax.servlet.ServletContext; -import javax.servlet.ServletException; -import javax.servlet.ServletRequest; -import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; -import javax.servlet.http.Part; import org.springframework.util.Assert; import org.springframework.util.ReflectionUtils; @@ -82,88 +72,4 @@ public class Servlet30WrapperFilter extends ZuulFilter { return null; } - private class Servlet30RequestWrapper extends HttpServletRequestWrapper { - private HttpServletRequest request; - - Servlet30RequestWrapper(HttpServletRequest request) { - super(request); - this.request = request; - } - - @Override - public boolean authenticate(HttpServletResponse response) throws IOException, - ServletException { - return this.request.authenticate(response); - } - - @Override - public void login(String username, String password) throws ServletException { - this.request.login(username, password); - } - - @Override - public void logout() throws ServletException { - this.request.logout(); - } - - @Override - public Collection getParts() throws IOException, IllegalStateException, - ServletException { - return this.request.getParts(); - } - - @Override - public Part getPart(String name) throws IOException, IllegalStateException, - ServletException { - return this.request.getPart(name); - } - - @Override - public ServletContext getServletContext() { - return this.request.getServletContext(); - } - - @Override - public AsyncContext startAsync() { - return this.request.startAsync(); - } - - @Override - public AsyncContext startAsync(ServletRequest servletRequest, - ServletResponse servletResponse) { - return this.request.startAsync(servletRequest, servletResponse); - } - - @Override - public boolean isAsyncStarted() { - try { - return this.request.isAsyncStarted(); - } - catch (Throwable e) { - return false; - } - } - - @Override - public boolean isAsyncSupported() { - try { - return this.request.isAsyncSupported(); - } - catch (Throwable e) { - return false; - } - } - - @Override - public AsyncContext getAsyncContext() { - return this.request.getAsyncContext(); - } - - @Override - public DispatcherType getDispatcherType() { - return this.request.getDispatcherType(); - } - - } - } 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 0ed59e82..7cab1ed8 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 @@ -17,13 +17,19 @@ package org.springframework.cloud.netflix.zuul; import java.io.IOException; +import java.io.InputStream; import java.util.Arrays; +import java.util.Map; + +import lombok.extern.slf4j.Slf4j; import org.junit.Test; import org.junit.runner.RunWith; import org.springframework.beans.factory.annotation.Value; -import org.springframework.boot.SpringApplication; +import org.springframework.boot.actuate.trace.InMemoryTraceRepository; +import org.springframework.boot.actuate.trace.TraceRepository; import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.boot.builder.SpringApplicationBuilder; import org.springframework.boot.test.IntegrationTest; import org.springframework.boot.test.SpringApplicationConfiguration; import org.springframework.boot.test.TestRestTemplate; @@ -58,7 +64,7 @@ import static org.junit.Assert.assertEquals; @RunWith(SpringJUnit4ClassRunner.class) @SpringApplicationConfiguration(classes = FormZuulProxyApplication.class) @WebAppConfiguration -@IntegrationTest({ "server.port: 0", "zuul.routes.simple: /simple/**" }) +@IntegrationTest({ "server.port:0", "zuul.routes.simple:/simple/**" }) @DirtiesContext public class FormZuulProxyApplicationTests { @@ -72,7 +78,7 @@ public class FormZuulProxyApplicationTests { HttpHeaders headers = new HttpHeaders(); headers.setContentType(MediaType.APPLICATION_FORM_URLENCODED); ResponseEntity result = new TestRestTemplate().exchange( - "http://localhost:" + this.port + "/simple", HttpMethod.POST, + "http://localhost:" + this.port + "/simple/form", HttpMethod.POST, new HttpEntity>(form, headers), String.class); assertEquals(HttpStatus.OK, result.getStatusCode()); @@ -86,7 +92,7 @@ public class FormZuulProxyApplicationTests { HttpHeaders headers = new HttpHeaders(); headers.setContentType(MediaType.MULTIPART_FORM_DATA); ResponseEntity result = new TestRestTemplate().exchange( - "http://localhost:" + this.port + "/simple", HttpMethod.POST, + "http://localhost:" + this.port + "/simple/form", HttpMethod.POST, new HttpEntity>(form, headers), String.class); assertEquals(HttpStatus.OK, result.getStatusCode()); @@ -118,7 +124,7 @@ public class FormZuulProxyApplicationTests { headers.setContentType(MediaType .valueOf(MediaType.APPLICATION_FORM_URLENCODED_VALUE + "; charset=UTF-8")); ResponseEntity result = new TestRestTemplate().exchange( - "http://localhost:" + this.port + "/simple", HttpMethod.POST, + "http://localhost:" + this.port + "/simple/form", HttpMethod.POST, new HttpEntity>(form, headers), String.class); assertEquals(HttpStatus.OK, result.getStatusCode()); @@ -132,9 +138,10 @@ public class FormZuulProxyApplicationTests { @RestController @EnableZuulProxy @RibbonClient(name = "simple", configuration = FormRibbonClientConfiguration.class) +@Slf4j class FormZuulProxyApplication { - @RequestMapping(value = "/", method = RequestMethod.POST) + @RequestMapping(value = "/form", method = RequestMethod.POST) public String accept(@RequestParam MultiValueMap form) throws IOException { return "Posted! " + form; @@ -144,7 +151,22 @@ class FormZuulProxyApplication { @RequestMapping(value = "/file", method = RequestMethod.POST) public String file(@RequestParam(required = false) MultipartFile file) throws IOException { - return "Posted! " + (file == null ? "" : new String(file.getBytes())); + byte[] bytes = new byte[0]; + if (file != null) { + if (file.getSize() > 1024) { + bytes = new byte[1024]; + InputStream inputStream = file.getInputStream(); + inputStream.read(bytes); + byte[] buffer = new byte[1024 * 1024 * 10]; + while (inputStream.read(buffer) >= 0) { + log.info("Read more bytes"); + } + } + else { + bytes = file.getBytes(); + } + } + return "Posted! " + new String(bytes); } @Bean @@ -174,8 +196,23 @@ class FormZuulProxyApplication { }; } + @Bean + public TraceRepository traceRepository() { + return new InMemoryTraceRepository() { + @Override + public void add(Map map) { + if (map.containsKey("body")) { + map.get("body"); + } + super.add(map); + } + }; + } + public static void main(String[] args) { - SpringApplication.run(FormZuulProxyApplication.class, args); + new SpringApplicationBuilder(FormZuulProxyApplication.class).properties( + "zuul.routes.simple:/simple/**", "multipart.maxFileSize:4096MB", + "multipart.maxRequestSize:4096MB").run(args); } } diff --git a/spring-cloud-netflix-core/src/test/resources/static/index.html b/spring-cloud-netflix-core/src/test/resources/static/index.html new file mode 100644 index 00000000..c7dbaf3c --- /dev/null +++ b/spring-cloud-netflix-core/src/test/resources/static/index.html @@ -0,0 +1,16 @@ + + +
+ File to upload:
Name:

Press here to upload the file via proxy! +
+
+ File to upload:
Name:

Press here to upload the file directly! +
+ + \ No newline at end of file