From 422bc42cf651e234abecdd48ed5f7209b4287ecd Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Tue, 29 Sep 2015 09:43:16 +0100 Subject: [PATCH] Be less greedy with logging request bodies in Zuul traces Most clients send "Transfer-Encoding: chunked" so this isn't often an issue, but if they don't (e.g. if you use "-H Expect:" with curl) then it's not a great idea to slurp it all into memory. Fixes gh-563 --- .../zuul/filters/ProxyRequestHelper.java | 24 ++++++--- .../zuul/filters/ProxyRequestHelperTests.java | 54 +++++++++++++++---- 2 files changed, 59 insertions(+), 19 deletions(-) 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 b70ccbe7..48c0ec63 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 @@ -18,6 +18,8 @@ package org.springframework.cloud.netflix.zuul.filters; import java.io.IOException; import java.io.InputStream; +import java.io.InputStreamReader; +import java.nio.charset.Charset; import java.util.Collection; import java.util.Enumeration; import java.util.HashSet; @@ -29,8 +31,6 @@ import java.util.Set; import javax.servlet.http.HttpServletRequest; -import lombok.extern.apachecommons.CommonsLog; - import org.apache.commons.io.IOUtils; import org.springframework.boot.actuate.trace.TraceRepository; import org.springframework.http.HttpHeaders; @@ -43,6 +43,8 @@ import org.springframework.web.util.WebUtils; import com.netflix.zuul.context.RequestContext; import com.netflix.zuul.util.HTTPRequestUtils; +import lombok.extern.apachecommons.CommonsLog; + /** * @author Dave Syer */ @@ -69,9 +71,13 @@ public class ProxyRequestHelper { String contextURI = (String) context.get("requestURI"); if (contextURI != null) { try { - uri = UriUtils.encodePath(contextURI, WebUtils.DEFAULT_CHARACTER_ENCODING); - } catch (Exception e) { - log.debug("unable to encode uri path from context, falling back to uri from request", e); + uri = UriUtils.encodePath(contextURI, + WebUtils.DEFAULT_CHARACTER_ENCODING); + } + catch (Exception e) { + log.debug( + "unable to encode uri path from context, falling back to uri from request", + e); } } return uri; @@ -253,10 +259,12 @@ public class ProxyRequestHelper { info.put("body", ""); return; } - String entity = IOUtils.toString(inputStream); + char[] buffer = new char[4096]; + int count = IOUtils.read(new InputStreamReader(inputStream, Charset.forName("UTF-8")), + buffer); + String entity = new String(buffer).substring(0, count); if (StringUtils.hasText(entity)) { - info.put("body", entity.length() <= 4096 ? entity : entity.substring(0, 4096) - + ""); + info.put("body", entity.length() < 4096 ? entity : entity + ""); } } diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/ProxyRequestHelperTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/ProxyRequestHelperTests.java index 1c03ee9f..fda67fa9 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/ProxyRequestHelperTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/ProxyRequestHelperTests.java @@ -16,18 +16,28 @@ package org.springframework.cloud.netflix.zuul.filters; -import org.junit.Before; -import org.junit.Test; -import org.mockito.Mock; -import org.springframework.boot.actuate.trace.TraceRepository; -import org.springframework.mock.web.MockHttpServletRequest; -import org.springframework.util.MultiValueMap; +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasSize; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.notNullValue; +import static org.hamcrest.Matchers.nullValue; +import static org.junit.Assert.assertThat; +import static org.mockito.MockitoAnnotations.initMocks; import java.util.List; -import static org.hamcrest.Matchers.*; -import static org.junit.Assert.assertThat; -import static org.mockito.MockitoAnnotations.initMocks; +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mock; +import org.springframework.boot.actuate.trace.InMemoryTraceRepository; +import org.springframework.boot.actuate.trace.Trace; +import org.springframework.boot.actuate.trace.TraceRepository; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.util.LinkedMultiValueMap; +import org.springframework.util.MultiValueMap; + +import com.netflix.zuul.context.RequestContext; /** * @author Spencer Gibb @@ -42,6 +52,29 @@ public class ProxyRequestHelperTests { initMocks(this); } + @Test + public void debug() throws Exception { + MockHttpServletRequest request = new MockHttpServletRequest("POST", "/"); + request.setContent("{}".getBytes()); + request.addHeader("singleName", "singleValue"); + request.addHeader("multiName", "multiValue1"); + request.addHeader("multiName", "multiValue2"); + RequestContext.getCurrentContext().setRequest(request); + + ProxyRequestHelper helper = new ProxyRequestHelper(); + this.traceRepository = new InMemoryTraceRepository(); + helper.setTraces(this.traceRepository); + + MultiValueMap headers = helper.buildZuulRequestHeaders(request); + + helper.debug("POST", "http://example.com", headers, + new LinkedMultiValueMap(), request.getInputStream()); + Trace actual = this.traceRepository.findAll().get(0); + System.err.println(actual.getInfo()); + assertThat((String)actual.getInfo().get("body"), equalTo("{}")); + + } + @Test public void buildZuulRequestHeadersWork() { MockHttpServletRequest request = new MockHttpServletRequest("GET", "/"); @@ -50,7 +83,7 @@ public class ProxyRequestHelperTests { request.addHeader("multiName", "multiValue2"); ProxyRequestHelper helper = new ProxyRequestHelper(); - helper.setTraces(traceRepository); + helper.setTraces(this.traceRepository); MultiValueMap headers = helper.buildZuulRequestHeaders(request); List singleName = headers.get("singleName"); @@ -79,5 +112,4 @@ public class ProxyRequestHelperTests { assertThat(acceptEncodings, contains("gzip")); } - }