Merge remote-tracking branch 'origin/1.4.x'

docs/src/main/asciidoc/spring-cloud-netflix.adoc

@@ -157,9 +157,9 @@ In general, additional metadata does not change the behavior of the client, unle
 There are a couple of special cases, described later in this document, where Spring Cloud already assigns meaning to the metadata map.
 // TODO Add links from here to the relevant places in the document
 
-==== Using Eureka on Cloudfoundry
+==== Using Eureka on Cloud Foundry
 
-Cloudfoundry has a global router so that all instances of the same app have the same hostname (other PaaS solutions with a similar architecture have the same arrangement).
+Cloud Foundry has a global router so that all instances of the same app have the same hostname (other PaaS solutions with a similar architecture have the same arrangement).
 This is not necessarily a barrier to using Eureka.
 However, if you use the router (recommended or even mandatory, depending on the way your platform was set up), you need to explicitly set the hostname and port numbers (secure or non-secure) so that they use the router.
 You might also want to use instance metadata so that you can distinguish between the instances on the client (for example, in a custom load balancer).
@@ -173,7 +173,7 @@ eureka:
     nonSecurePort: 80
 ----
 
-Depending on the way the security rules are set up in your Cloudfoundry instance, you might be able to register and use the IP address of the host VM for direct service-to-service calls.
+Depending on the way the security rules are set up in your Cloud Foundry instance, you might be able to register and use the IP address of the host VM for direct service-to-service calls.
 This feature is not yet available on Pivotal Web Services (https://run.pivotal.io[PWS]).
 
 ==== Using Eureka on AWS
@@ -211,7 +211,7 @@ eureka:
 ----
 
 With the metadata shown in the preceding example and multiple service instances deployed on localhost, the random value is inserted there to make the instance unique.
-In Cloudfoundry, the `vcap.application.instance_id` is populated automatically in a Spring Boot application, so the random value is not needed.
+In Cloud Foundry, the `vcap.application.instance_id` is populated automatically in a Spring Boot application, so the random value is not needed.
 
 === Using the EurekaClient
 
@@ -1079,7 +1079,7 @@ hystrix:
 myusers-service:
   ribbon:
     NIWSServerListClassName: com.netflix.loadbalancer.ConfigurationBasedServerList
-    ListOfServers: http://example1.com,http://example2.com
+    listOfServers: http://example1.com,http://example2.com
     ConnectTimeout: 1000
     ReadTimeout: 3000
     MaxTotalHttpConnections: 500

spring-cloud-netflix-ribbon/src/main/java/org/springframework/cloud/netflix/ribbon/okhttp/OkHttpRibbonConfiguration.java

@@ -94,7 +94,7 @@ public class OkHttpRibbonConfiguration {
 	@Bean
 	@ConditionalOnMissingBean(AbstractLoadBalancerAwareClient.class)
 	@ConditionalOnClass(name = "org.springframework.retry.support.RetryTemplate")
-	public RetryableOkHttpLoadBalancingClient okHttpLoadBalancingClient(
+	public RetryableOkHttpLoadBalancingClient retryableOkHttpLoadBalancingClient(
 		IClientConfig config,
 		ServerIntrospector serverIntrospector,
 		ILoadBalancer loadBalancer,
@@ -112,7 +112,7 @@ public class OkHttpRibbonConfiguration {
 	@Bean
 	@ConditionalOnMissingBean(AbstractLoadBalancerAwareClient.class)
 	@ConditionalOnMissingClass(value = "org.springframework.retry.support.RetryTemplate")
-	public OkHttpLoadBalancingClient retryableOkHttpLoadBalancingClient(
+	public OkHttpLoadBalancingClient okHttpLoadBalancingClient(
 		IClientConfig config,
 		ServerIntrospector serverIntrospector, ILoadBalancer loadBalancer,
 		RetryHandler retryHandler, OkHttpClient delegate) {

spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilter.java

@@ -35,6 +35,7 @@ import org.apache.http.Header;
 import org.apache.http.HttpHost;
 import org.apache.http.HttpRequest;
 import org.apache.http.HttpResponse;
+import org.apache.http.client.HttpClient;
 import org.apache.http.client.config.CookieSpecs;
 import org.apache.http.client.config.RequestConfig;
 import org.apache.http.client.methods.CloseableHttpResponse;
@@ -56,6 +57,7 @@ import org.springframework.cloud.netflix.zuul.filters.ZuulProperties;
 import org.springframework.cloud.netflix.zuul.filters.ZuulProperties.Host;
 import org.springframework.cloud.netflix.zuul.util.ZuulRuntimeException;
 import org.springframework.context.event.EventListener;
+import org.springframework.http.HttpHeaders;
 import org.springframework.util.LinkedMultiValueMap;
 import org.springframework.util.MultiValueMap;
 import org.springframework.util.StringUtils;
@@ -93,6 +95,7 @@ public class SimpleHostRoutingFilter extends ZuulFilter {
 	private HttpClientConnectionManager connectionManager;
 	private CloseableHttpClient httpClient;
 	private boolean customHttpClient = false;
+	private boolean useServlet31 = true;
 
 	@EventListener
 	public void onPropertyChange(EnvironmentChangeEvent event) {
@@ -127,6 +130,7 @@ public class SimpleHostRoutingFilter extends ZuulFilter {
 				.isForceOriginalQueryStringEncoding();
 		this.connectionManagerFactory = connectionManagerFactory;
 		this.httpClientFactory = httpClientFactory;
+		checkServletVersion();
 	}
 
 	public SimpleHostRoutingFilter(ProxyRequestHelper helper, ZuulProperties properties,
@@ -138,6 +142,7 @@ public class SimpleHostRoutingFilter extends ZuulFilter {
 				.isForceOriginalQueryStringEncoding();
 		this.httpClient = httpClient;
 		this.customHttpClient = true;
+		checkServletVersion();
 	}
 
 	@PostConstruct
@@ -193,7 +198,7 @@ public class SimpleHostRoutingFilter extends ZuulFilter {
 				.buildZuulRequestQueryParams(request);
 		String verb = getVerb(request);
 		InputStream requestEntity = getRequestBody(request);
-		if (request.getContentLength() < 0) {
+		if (getContentLength(request) < 0) {
 			context.setChunkedRequestBody();
 		}
 
@@ -211,6 +216,21 @@ public class SimpleHostRoutingFilter extends ZuulFilter {
 		return null;
 	}
 
+	protected void checkServletVersion() {
+		// To support Servlet API 3.1 we need to check if getContentLengthLong exists
+		// Spring 5 minimum support is 3.0, so this stays
+		try {
+			HttpServletRequest.class.getMethod("getContentLengthLong");
+			useServlet31 = true;
+		} catch(NoSuchMethodException e) {
+			useServlet31 = false;
+		}
+	}
+
+	protected void setUseServlet31(boolean useServlet31) {
+		this.useServlet31 = useServlet31;
+	}
+
 	protected HttpClientConnectionManager getConnectionManager() {
 		return connectionManager;
 	}
@@ -235,7 +255,7 @@ public class SimpleHostRoutingFilter extends ZuulFilter {
 		URL host = RequestContext.getCurrentContext().getRouteHost();
 		HttpHost httpHost = getHttpHost(host);
 		uri = StringUtils.cleanPath((host.getPath() + uri).replaceAll("/{2,}", "/"));
-		int contentLength = request.getContentLength();
+		long contentLength = getContentLength(request);
 
 		ContentType contentType = null;
 
@@ -382,4 +402,19 @@ public class SimpleHostRoutingFilter extends ZuulFilter {
 	boolean isSslHostnameValidationEnabled() {
 		return this.sslHostnameValidationEnabled;
 	}
-}
+
+	// Get the header value as a long in order to more correctly proxy very large requests
+	protected long getContentLength(HttpServletRequest request) {
+		if(useServlet31){
+			return request.getContentLengthLong();
+		}
+		String contentLengthHeader = request.getHeader(HttpHeaders.CONTENT_LENGTH);
+		if (contentLengthHeader != null) {
+			try {
+				return Long.parseLong(contentLengthHeader);
+			}
+			catch (NumberFormatException e){}
+		}
+		return request.getContentLength();
+	}
+}

spring-cloud-netflix-zuul/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilterTests.java

@@ -26,13 +26,21 @@ import java.net.URL;
 import java.nio.charset.Charset;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
 import java.util.concurrent.TimeUnit;
+import java.util.zip.GZIPInputStream;
 import java.util.zip.GZIPOutputStream;
 
+import javax.servlet.MultipartConfigElement;
+import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
 import com.netflix.zuul.context.RequestContext;
 import com.netflix.zuul.monitoring.CounterFactory;
+
+import org.apache.commons.io.IOUtils;
+import org.apache.commons.lang3.ArrayUtils;
 import org.apache.http.HttpEntityEnclosingRequest;
 import org.apache.http.HttpHost;
 import org.apache.http.HttpRequest;
@@ -67,19 +75,27 @@ import org.springframework.cloud.netflix.zuul.util.ZuulRuntimeException;
 import org.springframework.context.annotation.AnnotationConfigApplicationContext;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
+import org.springframework.http.HttpHeaders;
 import org.springframework.mock.web.MockHttpServletRequest;
+import org.springframework.mock.web.MockHttpServletResponse;
+import org.springframework.mock.web.MockMultipartFile;
+import org.springframework.mock.web.MockMultipartHttpServletRequest;
 import org.springframework.test.annotation.DirtiesContext;
 import org.springframework.test.context.junit4.SpringRunner;
 import org.springframework.test.util.ReflectionTestUtils;
 import org.springframework.util.LinkedMultiValueMap;
 import org.springframework.util.ReflectionUtils;
 import org.springframework.web.bind.annotation.PathVariable;
+import org.springframework.web.bind.annotation.RequestHeader;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RequestMethod;
 import org.springframework.web.bind.annotation.RestController;
 
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.not;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 import static org.springframework.boot.test.context.SpringBootTest.WebEnvironment.RANDOM_PORT;
 import static org.springframework.boot.test.util.EnvironmentTestUtils.addEnvironment;
@@ -106,6 +122,7 @@ public class SimpleHostRoutingFilterTests {
 	@Before
 	public void setup() {
 		CounterFactory.initialize(new EmptyCounterFactory());
+		RequestContext.testSetCurrentContext(new RequestContext());
 	}
 
 	@After
@@ -114,6 +131,9 @@ public class SimpleHostRoutingFilterTests {
 			this.context.close();
 		}
 		CounterFactory.initialize(null);
+
+		RequestContext.testSetCurrentContext(null);
+		RequestContext.getCurrentContext().clear();
 	}
 
 	@Test
@@ -228,6 +248,283 @@ public class SimpleHostRoutingFilterTests {
 		assertTrue(response.getLastHeader("Location").getValue().contains("/app/get/5"));
 	}
 
+	@Test
+	public void contentLengthNegativeTest() throws IOException {
+		contentLengthTest(-1000L);
+	}
+
+	@Test
+	public void contentLengthNegativeOneTest() throws IOException {
+		contentLengthTest(-1L);
+	}
+
+	@Test
+	public void contentLengthZeroTest() throws IOException {
+		contentLengthTest(0L);
+	}
+
+	@Test
+	public void contentLengthOneTest() throws IOException {
+		contentLengthTest(1L);
+	}
+
+	@Test
+	public void contentLength1KbTest() throws IOException {
+		contentLengthTest(1000L);
+	}
+
+	@Test
+	public void contentLength1MbTest() throws IOException {
+		contentLengthTest(1000000L);
+	}
+
+	@Test
+	public void contentLength1GbTest() throws IOException {
+		contentLengthTest(1000000000L);
+	}
+
+	@Test
+	public void contentLength2GbTest() throws IOException {
+		contentLengthTest(2000000000L);
+	}
+
+	@Test
+	public void contentLength3GbTest() throws IOException {
+		contentLengthTest(3000000000L);
+	}
+
+	@Test
+	public void contentLength4GbTest() throws IOException {
+		contentLengthTest(4000000000L);
+	}
+
+	@Test
+	public void contentLength5GbTest() throws IOException {
+		contentLengthTest(5000000000L);
+	}
+
+	@Test
+	public void contentLength6GbTest() throws IOException {
+		contentLengthTest(6000000000L);
+	}
+
+	@Test
+	public void contentLengthServlet30WithHeaderNegativeTest() throws IOException {
+		contentLengthServlet30WithHeaderTest(-1000L);
+	}
+
+	@Test
+	public void contentLengthServlet30WithHeaderNegativeOneTest() throws IOException {
+		contentLengthServlet30WithHeaderTest(-1L);
+	}
+
+	@Test
+	public void contentLengthServlet30WithHeaderZeroTest() throws IOException {
+		contentLengthServlet30WithHeaderTest(0L);
+	}
+
+	@Test
+	public void contentLengthServlet30WithHeaderOneTest() throws IOException {
+		contentLengthServlet30WithHeaderTest(1L);
+	}
+
+	@Test
+	public void contentLengthServlet30WithHeader1KbTest() throws IOException {
+		contentLengthServlet30WithHeaderTest(1000L);
+	}
+
+	@Test
+	public void contentLengthServlet30WithHeader1MbTest() throws IOException {
+		contentLengthServlet30WithHeaderTest(1000000L);
+	}
+
+	@Test
+	public void contentLengthServlet30WithHeader1GbTest() throws IOException {
+		contentLengthServlet30WithHeaderTest(1000000000L);
+	}
+
+	@Test
+	public void contentLengthServlet30WithHeader2GbTest() throws IOException {
+		contentLengthServlet30WithHeaderTest(2000000000L);
+	}
+
+	@Test
+	public void contentLengthServlet30WithHeader3GbTest() throws IOException {
+		contentLengthServlet30WithHeaderTest(3000000000L);
+	}
+
+	@Test
+	public void contentLengthServlet30WithHeader4GbTest() throws IOException {
+		contentLengthServlet30WithHeaderTest(4000000000L);
+	}
+
+	@Test
+	public void contentLengthServlet30WithHeader5GbTest() throws IOException {
+		contentLengthServlet30WithHeaderTest(5000000000L);
+	}
+
+	@Test
+	public void contentLengthServlet30WithHeader6GbTest() throws IOException {
+		contentLengthServlet30WithHeaderTest(6000000000L);
+	}
+
+	@Test
+	public void contentLengthServlet30WithInvalidLongHeaderTest() throws IOException {
+		setupContext();
+		MockMultipartHttpServletRequest request = getMockedReqest(-1L);
+		request.addHeader(HttpHeaders.CONTENT_LENGTH, "InvalidLong");
+		contentLengthTest(-1L, getServlet30Filter(), request);
+	}
+
+	@Test
+	public void contentLengthServlet30WithoutHeaderNegativeTest() throws IOException {
+		contentLengthServlet30WithoutHeaderTest(-1000L);
+	}
+
+	@Test
+	public void contentLengthServlet30WithoutHeaderNegativeOneTest() throws IOException {
+		contentLengthServlet30WithoutHeaderTest(-1L);
+	}
+
+	@Test
+	public void contentLengthServlet30WithoutHeaderZeroTest() throws IOException {
+		contentLengthServlet30WithoutHeaderTest(0L);
+	}
+
+	@Test
+	public void contentLengthServlet30WithoutHeaderOneTest() throws IOException {
+		contentLengthServlet30WithoutHeaderTest(1L);
+	}
+
+	@Test
+	public void contentLengthServlet30WithoutHeader1KbTest() throws IOException {
+		contentLengthServlet30WithoutHeaderTest(1000L);
+	}
+
+	@Test
+	public void contentLengthServlet30WithoutHeader1MbTest() throws IOException {
+		contentLengthServlet30WithoutHeaderTest(1000000L);
+	}
+
+	@Test
+	public void contentLengthServlet30WithoutHeader1GbTest() throws IOException {
+		contentLengthServlet30WithoutHeaderTest(1000000000L);
+	}
+
+	@Test
+	public void contentLengthServlet30WithoutHeader2GbTest() throws IOException {
+		contentLengthServlet30WithoutHeaderTest(2000000000L);
+	}
+
+	@Test
+	public void contentLengthServlet30WithoutHeader3GbTest() throws IOException {
+		contentLengthServlet30WithoutHeaderTest(3000000000L);
+	}
+
+	@Test
+	public void contentLengthServlet30WithoutHeader4GbTest() throws IOException {
+		contentLengthServlet30WithoutHeaderTest(4000000000L);
+	}
+
+	@Test
+	public void contentLengthServlet30WithoutHeader5GbTest() throws IOException {
+		contentLengthServlet30WithoutHeaderTest(5000000000L);
+	}
+
+	@Test
+	public void contentLengthServlet30WithoutHeader6GbTest() throws IOException {
+		contentLengthServlet30WithoutHeaderTest(6000000000L);
+	}
+
+	public void contentLengthTest(Long contentLength) throws IOException {
+		setupContext();
+
+		contentLengthTest(contentLength, getFilter(), getMockedReqest(contentLength));
+	}
+
+	public void contentLengthServlet30WithHeaderTest(Long contentLength) throws IOException {
+		setupContext();
+		MockMultipartHttpServletRequest request = getMockedReqest(contentLength);
+		request.addHeader(HttpHeaders.CONTENT_LENGTH, contentLength);
+		contentLengthTest(contentLength, getServlet30Filter(), request);
+	}
+
+	public void contentLengthServlet30WithoutHeaderTest(Long contentLength) throws IOException {
+		setupContext();
+
+		//Although contentLength.intValue is not always equals to contentLength, that's the expected result when calling
+		// request.getContentLength() from servlet 3.0 implementation.
+		contentLengthTest(Long.parseLong("" + contentLength.intValue()), getServlet30Filter(), getMockedReqest(contentLength));
+	}
+
+	public MockMultipartHttpServletRequest getMockedReqest(final Long contentLength) throws IOException {
+
+		MockMultipartHttpServletRequest request = new MockMultipartHttpServletRequest() {
+			@Override
+			public int getContentLength() {
+				return contentLength.intValue();
+			}
+
+			@Override
+			public long getContentLengthLong() {
+				return contentLength;
+			}
+		};
+
+		return request;
+	}
+
+	public void contentLengthTest(Long expectedContentLength, SimpleHostRoutingFilter filter, MockMultipartHttpServletRequest request) throws IOException {
+		byte[] data = "poprqwueproqiwuerpoqweiurpo".getBytes();
+		MockMultipartFile file = new MockMultipartFile("test.zip", "test.zip",
+				"application/zip", data);
+		String boundary = "q1w2e3r4t5y6u7i8o9";
+		request.setContentType("multipart/form-data; boundary=" + boundary);
+		request.setContent(
+				createFileContent(data, boundary, "application/zip", "test.zip"));
+		request.addFile(file);
+		request.setMethod("POST");
+		request.setParameter("variant", "php");
+		request.setParameter("os", "mac");
+		request.setParameter("version", "3.4");
+		request.setRequestURI("/app/echo");
+
+		MockHttpServletResponse response = new MockHttpServletResponse();
+		RequestContext.getCurrentContext().setRequest(request);
+		RequestContext.getCurrentContext().setResponse(new MockHttpServletResponse());
+		URL url = new URL("http://localhost:" + this.port);
+		RequestContext.getCurrentContext().set("routeHost", url);
+		filter.run();
+
+		String responseString = IOUtils.toString(new GZIPInputStream(
+				((CloseableHttpResponse) RequestContext.getCurrentContext()
+						.get("zuulResponse")).getEntity().getContent()));
+		assertTrue(!responseString.isEmpty());
+		if (expectedContentLength < 0) {
+			assertThat(responseString, containsString("\""
+					+ HttpHeaders.TRANSFER_ENCODING.toLowerCase() + "\":\"chunked\""));
+			assertThat(responseString,
+					not(containsString(HttpHeaders.CONTENT_LENGTH.toLowerCase())));
+		}
+		else {
+			assertThat(responseString,
+					containsString("\"" + HttpHeaders.CONTENT_LENGTH.toLowerCase()
+							+ "\":\"" + expectedContentLength + "\""));
+		}
+	}
+
+	public byte[] createFileContent(byte[] data, String boundary, String contentType,
+			String fileName) {
+		String start = "--" + boundary
+				+ "\r\n Content-Disposition: form-data; name=\"file\"; filename=\""
+				+ fileName + "\"\r\n" + "Content-type: " + contentType + "\r\n\r\n";
+		;
+
+		String end = "\r\n--" + boundary + "--"; // correction suggested @butfly
+		return ArrayUtils.addAll(start.getBytes(),
+				ArrayUtils.addAll(data, end.getBytes()));
+	}
+
 	@Test
 	public void zuulHostKeysUpdateHttpClient() {
 		setupContext();
@@ -331,6 +628,12 @@ public class SimpleHostRoutingFilterTests {
 		return this.context.getBean(SimpleHostRoutingFilter.class);
 	}
 
+	private SimpleHostRoutingFilter getServlet30Filter() {
+		SimpleHostRoutingFilter filter = getFilter();
+		filter.setUseServlet31(false);
+		return filter;
+	}
+
 	@Configuration
 	@EnableConfigurationProperties
 	protected static class TestConfiguration {
@@ -374,6 +677,20 @@ public class SimpleHostRoutingFilterTests {
             response.sendRedirect("/app/get/5");
             return null;
         }
+
+		@RequestMapping(value = "/echo")
+		public Map<String, Object> echoRequestAttributes(@RequestHeader HttpHeaders httpHeaders, HttpServletRequest request) throws IOException {
+			Map<String, Object> result = new HashMap<>();
+			result.put("headers", httpHeaders.toSingleValueMap());
+
+			return result;
+		}
+
+		@Bean
+		MultipartConfigElement multipartConfigElement() {
+			long maxSize = 10l * 1024 * 1024 * 1024;
+			return new MultipartConfigElement("", maxSize, maxSize, 0);
+		}
     }
 
     static class GZIPCompression {
@@ -389,4 +706,18 @@ public class SimpleHostRoutingFilterTests {
             return obj.toByteArray();
         }
     }
+
+//class GZIPCompression {
+//
+//	public static byte[] compress(final String str) throws IOException {
+//		if ((str == null) || (str.length() == 0)) {
+//			return null;
+//		}
+//		ByteArrayOutputStream obj = new ByteArrayOutputStream();
+//		GZIPOutputStream gzip = new GZIPOutputStream(obj);
+//		gzip.write(str.getBytes("UTF-8"));
+//		gzip.close();
+//		return obj.toByteArray();
+//	}
+//>>>>>>> origin/1.4.x:spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilterTests.java
 }