Merge pull request #1004 from jebeaudet/fix-IgnoredHeadersCasingNotIgnored
* fix-IgnoredHeadersCasingNotIgnored: Sensitive headers set in PreDecorationFilter no longer override previously set ignored headers. Removed the case sensitiveness when the sensitive headers are set. Fixes https://github.com/spring-cloud/spring-cloud-netflix/issues/1003 Test for default ribbon client configuration Tidy up more compiler warnings for generics Add note to clarify that zuul starter does not include discovery
This commit is contained in:
@@ -1060,13 +1060,17 @@ independently for all the backends.
|
||||
|
||||
To enable it, annotate a Spring Boot main class with
|
||||
`@EnableZuulProxy`, and this forwards local calls to the appropriate
|
||||
service. By convention, a service with the Eureka ID "users", will
|
||||
service. By convention, a service with the ID "users", will
|
||||
receive requests from the proxy located at `/users` (with the prefix
|
||||
stripped). The proxy uses Ribbon to locate an instance to forward to
|
||||
via Eureka, and all requests are executed in a hystrix command, so
|
||||
via discovery, and all requests are executed in a hystrix command, so
|
||||
failures will show up in Hystrix metrics, and once the circuit is open
|
||||
the proxy will not try to contact the service.
|
||||
|
||||
NOTE: the Zuul starter does not include a discovery client, so for
|
||||
routes based on service IDs you need to provide one of those
|
||||
on the classpath as well (e.g. Eureka is one choice).
|
||||
|
||||
To skip having a service automatically added, set
|
||||
`zuul.ignored-services` to a list of service id patterns. If a service
|
||||
matches a pattern that is ignored, but also included in the explicitly
|
||||
|
||||
@@ -18,6 +18,7 @@ package org.springframework.cloud.netflix.rx;
|
||||
|
||||
import org.springframework.http.MediaType;
|
||||
import org.springframework.web.servlet.mvc.method.annotation.SseEmitter;
|
||||
|
||||
import rx.Observable;
|
||||
|
||||
/**
|
||||
@@ -28,18 +29,17 @@ import rx.Observable;
|
||||
*/
|
||||
class ObservableSseEmitter<T> extends SseEmitter {
|
||||
|
||||
private final ResponseBodyEmitterSubscriber<T> subscriber;
|
||||
public ObservableSseEmitter(Observable<T> observable) {
|
||||
this(null, observable);
|
||||
}
|
||||
|
||||
public ObservableSseEmitter(Observable<T> observable) {
|
||||
this(null, observable);
|
||||
}
|
||||
public ObservableSseEmitter(MediaType mediaType, Observable<T> observable) {
|
||||
this(null, mediaType, observable);
|
||||
}
|
||||
|
||||
public ObservableSseEmitter(MediaType mediaType, Observable<T> observable) {
|
||||
this(null, mediaType, observable);
|
||||
}
|
||||
|
||||
public ObservableSseEmitter(Long timeout, MediaType mediaType, Observable<T> observable) {
|
||||
super(timeout);
|
||||
this.subscriber = new ResponseBodyEmitterSubscriber<>(mediaType, observable, this);
|
||||
}
|
||||
public ObservableSseEmitter(Long timeout, MediaType mediaType,
|
||||
Observable<T> observable) {
|
||||
super(timeout);
|
||||
new ResponseBodyEmitterSubscriber<>(mediaType, observable, this);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -29,22 +29,19 @@ import rx.Single;
|
||||
*/
|
||||
class SingleDeferredResult<T> extends DeferredResult<T> {
|
||||
|
||||
private static final Object EMPTY_RESULT = new Object();
|
||||
private static final Object EMPTY_RESULT = new Object();
|
||||
|
||||
private final DeferredResultSubscriber<T> subscriber;
|
||||
public SingleDeferredResult(Single<T> single) {
|
||||
this(null, EMPTY_RESULT, single);
|
||||
}
|
||||
|
||||
public SingleDeferredResult(Single<T> single) {
|
||||
this(null, EMPTY_RESULT, single);
|
||||
}
|
||||
public SingleDeferredResult(long timeout, Single<T> single) {
|
||||
this(timeout, EMPTY_RESULT, single);
|
||||
}
|
||||
|
||||
public SingleDeferredResult(long timeout, Single<T> single) {
|
||||
this(timeout, EMPTY_RESULT, single);
|
||||
}
|
||||
|
||||
public SingleDeferredResult(Long timeout, Object timeoutResult, Single<T> single) {
|
||||
super(timeout, timeoutResult);
|
||||
Assert.notNull(single, "single can not be null");
|
||||
|
||||
subscriber = new DeferredResultSubscriber<>(single.toObservable(), this);
|
||||
}
|
||||
public SingleDeferredResult(Long timeout, Object timeoutResult, Single<T> single) {
|
||||
super(timeout, timeoutResult);
|
||||
Assert.notNull(single, "single can not be null");
|
||||
new DeferredResultSubscriber<>(single.toObservable(), this);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -31,42 +31,46 @@ import rx.Single;
|
||||
import rx.functions.Func1;
|
||||
|
||||
/**
|
||||
* A specialized {@link AsyncHandlerMethodReturnValueHandler} that handles {@link Single} return types.
|
||||
* A specialized {@link AsyncHandlerMethodReturnValueHandler} that handles {@link Single}
|
||||
* return types.
|
||||
*
|
||||
* @author Spencer Gibb
|
||||
* @author Jakub Narloch
|
||||
*/
|
||||
public class SingleReturnValueHandler implements AsyncHandlerMethodReturnValueHandler {
|
||||
|
||||
@Override
|
||||
public boolean isAsyncReturnValue(Object returnValue, MethodParameter returnType) {
|
||||
return returnValue != null && supportsReturnType(returnType);
|
||||
}
|
||||
@Override
|
||||
public boolean isAsyncReturnValue(Object returnValue, MethodParameter returnType) {
|
||||
return returnValue != null && supportsReturnType(returnType);
|
||||
}
|
||||
|
||||
@Override
|
||||
public boolean supportsReturnType(MethodParameter returnType) {
|
||||
return Single.class.isAssignableFrom(returnType.getParameterType()) || isResponseEntity(returnType);
|
||||
}
|
||||
@Override
|
||||
public boolean supportsReturnType(MethodParameter returnType) {
|
||||
return Single.class.isAssignableFrom(returnType.getParameterType())
|
||||
|| isResponseEntity(returnType);
|
||||
}
|
||||
|
||||
private boolean isResponseEntity(MethodParameter returnType) {
|
||||
if(ResponseEntity.class.isAssignableFrom(returnType.getParameterType())) {
|
||||
Class<?> bodyType = ResolvableType.forMethodParameter(returnType).getGeneric(0).resolve();
|
||||
if (ResponseEntity.class.isAssignableFrom(returnType.getParameterType())) {
|
||||
Class<?> bodyType = ResolvableType.forMethodParameter(returnType)
|
||||
.getGeneric(0).resolve();
|
||||
return bodyType != null && Single.class.isAssignableFrom(bodyType);
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
@SuppressWarnings("unchecked")
|
||||
@Override
|
||||
public void handleReturnValue(Object returnValue, MethodParameter returnType, ModelAndViewContainer mavContainer, NativeWebRequest webRequest) throws Exception {
|
||||
@Override
|
||||
public void handleReturnValue(Object returnValue, MethodParameter returnType,
|
||||
ModelAndViewContainer mavContainer, NativeWebRequest webRequest)
|
||||
throws Exception {
|
||||
|
||||
if (returnValue == null) {
|
||||
mavContainer.setRequestHandled(true);
|
||||
return;
|
||||
}
|
||||
if (returnValue == null) {
|
||||
mavContainer.setRequestHandled(true);
|
||||
return;
|
||||
}
|
||||
|
||||
ResponseEntity<Single<?>> responseEntity = getResponseEntity(returnValue);
|
||||
if(responseEntity != null) {
|
||||
if (responseEntity != null) {
|
||||
returnValue = responseEntity.getBody();
|
||||
if (returnValue == null) {
|
||||
mavContainer.setRequestHandled(true);
|
||||
@@ -74,10 +78,10 @@ public class SingleReturnValueHandler implements AsyncHandlerMethodReturnValueHa
|
||||
}
|
||||
}
|
||||
|
||||
final Single<?> single = Single.class.cast(returnValue);
|
||||
WebAsyncUtils.getAsyncManager(webRequest)
|
||||
.startDeferredResultProcessing(convertToDeferredResult(responseEntity, single), mavContainer);
|
||||
}
|
||||
final Single<?> single = Single.class.cast(returnValue);
|
||||
WebAsyncUtils.getAsyncManager(webRequest).startDeferredResultProcessing(
|
||||
convertToDeferredResult(responseEntity, single), mavContainer);
|
||||
}
|
||||
|
||||
@SuppressWarnings("unchecked")
|
||||
private ResponseEntity<Single<?>> getResponseEntity(Object returnValue) {
|
||||
@@ -88,28 +92,32 @@ public class SingleReturnValueHandler implements AsyncHandlerMethodReturnValueHa
|
||||
return null;
|
||||
}
|
||||
|
||||
protected DeferredResult<?> convertToDeferredResult(final ResponseEntity<Single<?>> responseEntity, Single<?> single) {
|
||||
protected DeferredResult<?> convertToDeferredResult(
|
||||
final ResponseEntity<Single<?>> responseEntity, Single<?> single) {
|
||||
|
||||
//TODO: fix when java8 :-)
|
||||
Single<ResponseEntity> singleResponse = single.map(new Func1<Object, ResponseEntity>() {
|
||||
@Override
|
||||
public ResponseEntity call(Object object) {
|
||||
return new ResponseEntity<>(object, getHttpHeaders(responseEntity), getHttpStatus(responseEntity));
|
||||
}
|
||||
});
|
||||
// TODO: use lambda when java8 :-)
|
||||
Single<ResponseEntity<?>> singleResponse = single
|
||||
.map(new Func1<Object, ResponseEntity<?>>() {
|
||||
@Override
|
||||
public ResponseEntity<?> call(Object object) {
|
||||
return new ResponseEntity<Object>(object,
|
||||
getHttpHeaders(responseEntity),
|
||||
getHttpStatus(responseEntity));
|
||||
}
|
||||
});
|
||||
|
||||
return new SingleDeferredResult<>(singleResponse);
|
||||
}
|
||||
|
||||
private HttpStatus getHttpStatus(ResponseEntity<?> responseEntity) {
|
||||
if(responseEntity == null) {
|
||||
if (responseEntity == null) {
|
||||
return HttpStatus.OK;
|
||||
}
|
||||
return responseEntity.getStatusCode();
|
||||
}
|
||||
|
||||
private HttpHeaders getHttpHeaders(ResponseEntity<?> responseEntity) {
|
||||
if(responseEntity == null) {
|
||||
if (responseEntity == null) {
|
||||
return new HttpHeaders();
|
||||
}
|
||||
return responseEntity.getHeaders();
|
||||
|
||||
@@ -85,10 +85,12 @@ public class ZuulProxyConfiguration extends ZuulConfiguration {
|
||||
|
||||
// pre filters
|
||||
@Bean
|
||||
public PreDecorationFilter preDecorationFilter(RouteLocator routeLocator) {
|
||||
public PreDecorationFilter preDecorationFilter(RouteLocator routeLocator,
|
||||
ProxyRequestHelper proxyRequestHelper) {
|
||||
return new PreDecorationFilter(routeLocator,
|
||||
this.server.getServletPrefix(),
|
||||
this.zuulProperties);
|
||||
this.zuulProperties,
|
||||
proxyRequestHelper);
|
||||
}
|
||||
|
||||
// route filters
|
||||
|
||||
@@ -43,13 +43,17 @@ public class PreDecorationFilter extends ZuulFilter {
|
||||
private ZuulProperties properties;
|
||||
|
||||
private UrlPathHelper urlPathHelper = new UrlPathHelper();
|
||||
|
||||
private ProxyRequestHelper proxyRequestHelper;
|
||||
|
||||
public PreDecorationFilter(RouteLocator routeLocator,
|
||||
String dispatcherServletPath, ZuulProperties properties) {
|
||||
String dispatcherServletPath, ZuulProperties properties,
|
||||
ProxyRequestHelper proxyRequestHelper) {
|
||||
this.routeLocator = routeLocator;
|
||||
this.properties = properties;
|
||||
this.urlPathHelper.setRemoveSemicolonContent(properties.isRemoveSemicolonContent());
|
||||
this.dispatcherServletPath = dispatcherServletPath;
|
||||
this.proxyRequestHelper = proxyRequestHelper;
|
||||
}
|
||||
|
||||
@Override
|
||||
@@ -81,9 +85,9 @@ public class PreDecorationFilter extends ZuulFilter {
|
||||
ctx.put("requestURI", route.getPath());
|
||||
ctx.put("proxy", route.getId());
|
||||
if (route.getSensitiveHeaders().isEmpty()) {
|
||||
ctx.put(ProxyRequestHelper.IGNORED_HEADERS, this.properties.getSensitiveHeaders());
|
||||
proxyRequestHelper.addIgnoredHeaders(this.properties.getSensitiveHeaders().toArray(new String[0]));
|
||||
} else {
|
||||
ctx.put(ProxyRequestHelper.IGNORED_HEADERS, route.getSensitiveHeaders());
|
||||
proxyRequestHelper.addIgnoredHeaders(route.getSensitiveHeaders().toArray(new String[0]));
|
||||
}
|
||||
|
||||
if (route.getRetryable() != null) {
|
||||
|
||||
@@ -0,0 +1,63 @@
|
||||
/*
|
||||
* Copyright 2013-2014 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.ribbon;
|
||||
|
||||
import org.junit.Test;
|
||||
import org.junit.runner.RunWith;
|
||||
import org.springframework.beans.factory.annotation.Autowired;
|
||||
import org.springframework.boot.autoconfigure.PropertyPlaceholderAutoConfiguration;
|
||||
import org.springframework.boot.test.IntegrationTest;
|
||||
import org.springframework.boot.test.SpringApplicationConfiguration;
|
||||
import org.springframework.cloud.netflix.archaius.ArchaiusAutoConfiguration;
|
||||
import org.springframework.cloud.netflix.ribbon.RibbonAutoConfigurationIntegrationTests.TestConfiguration;
|
||||
import org.springframework.context.annotation.Configuration;
|
||||
import org.springframework.context.annotation.Import;
|
||||
import org.springframework.test.annotation.DirtiesContext;
|
||||
import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
|
||||
|
||||
import com.netflix.client.config.CommonClientConfigKey;
|
||||
import com.netflix.client.config.IClientConfig;
|
||||
|
||||
import static org.junit.Assert.assertEquals;
|
||||
|
||||
/**
|
||||
* @author Dave Syer
|
||||
*/
|
||||
@RunWith(SpringJUnit4ClassRunner.class)
|
||||
@SpringApplicationConfiguration(classes = TestConfiguration.class)
|
||||
@DirtiesContext
|
||||
@IntegrationTest("ribbon.ConnectTimeout=25000")
|
||||
public class RibbonAutoConfigurationIntegrationTests {
|
||||
|
||||
@Autowired
|
||||
private SpringClientFactory factory;
|
||||
|
||||
@Test
|
||||
public void serverListIsConfigured() throws Exception {
|
||||
IClientConfig config = this.factory.getClientConfig("client");
|
||||
assertEquals(25000,
|
||||
config.getPropertyAsInteger(CommonClientConfigKey.ConnectTimeout, 3000));
|
||||
}
|
||||
|
||||
@Configuration
|
||||
@RibbonClient("client")
|
||||
@Import({ PropertyPlaceholderAutoConfiguration.class, ArchaiusAutoConfiguration.class,
|
||||
RibbonAutoConfiguration.class })
|
||||
protected static class TestConfiguration {
|
||||
}
|
||||
|
||||
}
|
||||
@@ -16,7 +16,9 @@
|
||||
|
||||
package org.springframework.cloud.netflix.zuul.filters.pre;
|
||||
|
||||
import java.util.Arrays;
|
||||
import java.util.Collections;
|
||||
import java.util.HashSet;
|
||||
import java.util.List;
|
||||
import java.util.Set;
|
||||
|
||||
@@ -53,6 +55,8 @@ public class PreDecorationFilterTests {
|
||||
private DiscoveryClientRouteLocator routeLocator;
|
||||
|
||||
private MockHttpServletRequest request = new MockHttpServletRequest();
|
||||
|
||||
private ProxyRequestHelper proxyRequestHelper = new ProxyRequestHelper();
|
||||
|
||||
@Before
|
||||
public void init() {
|
||||
@@ -60,7 +64,7 @@ public class PreDecorationFilterTests {
|
||||
this.properties = new ZuulProperties();
|
||||
this.routeLocator = new DiscoveryClientRouteLocator("/", this.discovery,
|
||||
this.properties);
|
||||
this.filter = new PreDecorationFilter(this.routeLocator, "/", this.properties);
|
||||
this.filter = new PreDecorationFilter(this.routeLocator, "/", this.properties, proxyRequestHelper);
|
||||
RequestContext ctx = RequestContext.getCurrentContext();
|
||||
ctx.clear();
|
||||
ctx.setRequest(this.request);
|
||||
@@ -81,7 +85,7 @@ public class PreDecorationFilterTests {
|
||||
|
||||
@Test
|
||||
public void skippedIfForwardToSet() throws Exception {
|
||||
RequestContext.getCurrentContext().set("forward.to", "mycontext");
|
||||
RequestContext.getCurrentContext().set("forward.to", "myconteext");
|
||||
assertEquals(false, this.filter.shouldFilter());
|
||||
}
|
||||
|
||||
@@ -189,7 +193,7 @@ public class PreDecorationFilterTests {
|
||||
new ZuulRoute("foo", "/foo/**", null, "forward:/foo", true, null, null));
|
||||
|
||||
this.filter = new PreDecorationFilter(this.routeLocator,
|
||||
"/special", this.properties);
|
||||
"/special", this.properties, proxyRequestHelper);
|
||||
|
||||
this.request.setRequestURI("/api/bar/1");
|
||||
|
||||
@@ -233,7 +237,7 @@ public class PreDecorationFilterTests {
|
||||
this.routeLocator.addRoute(
|
||||
new ZuulRoute("foo", "/foo/**", null, "forward:/foo", true, null, null));
|
||||
this.filter = new PreDecorationFilter(this.routeLocator,
|
||||
"/special", this.properties);
|
||||
"/special", this.properties, proxyRequestHelper);
|
||||
|
||||
|
||||
this.filter.run();
|
||||
@@ -258,7 +262,7 @@ public class PreDecorationFilterTests {
|
||||
new ZuulRoute("foo", "/foo/**", null, "forward:/foo", true, null, null));
|
||||
|
||||
this.filter = new PreDecorationFilter(this.routeLocator,
|
||||
"/special", this.properties);
|
||||
"/special", this.properties, proxyRequestHelper);
|
||||
|
||||
this.filter.run();
|
||||
|
||||
@@ -296,6 +300,51 @@ public class PreDecorationFilterTests {
|
||||
assertTrue("sensitiveHeaders is wrong", sensitiveHeaders.containsAll(Collections.singletonList("x-bar")));
|
||||
assertFalse("sensitiveHeaders is wrong", sensitiveHeaders.contains("Cookie"));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void sensitiveHeadersCaseInsensitive() throws Exception {
|
||||
this.properties.setPrefix("/api");
|
||||
this.properties.setStripPrefix(true);
|
||||
this.properties.setSensitiveHeaders(Collections.singleton("X-bAr"));
|
||||
this.request.setRequestURI("/api/foo/1");
|
||||
this.routeLocator.addRoute("/foo/**", "foo");
|
||||
this.filter.run();
|
||||
RequestContext ctx = RequestContext.getCurrentContext();
|
||||
@SuppressWarnings("unchecked")
|
||||
Set<String> sensitiveHeaders = (Set<String>) ctx.get(ProxyRequestHelper.IGNORED_HEADERS);
|
||||
assertTrue("sensitiveHeaders is wrong", sensitiveHeaders.containsAll(Collections.singletonList("x-bar")));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void sensitiveHeadersOverrideCaseInsensitive() throws Exception {
|
||||
this.properties.setPrefix("/api");
|
||||
this.properties.setStripPrefix(true);
|
||||
this.properties.setSensitiveHeaders(Collections.singleton("X-bAr"));
|
||||
this.request.setRequestURI("/api/foo/1");
|
||||
ZuulRoute route = new ZuulRoute("/foo/**", "foo");
|
||||
route.setSensitiveHeaders(Collections.singleton("X-Foo"));
|
||||
this.routeLocator.addRoute(route);
|
||||
this.filter.run();
|
||||
RequestContext ctx = RequestContext.getCurrentContext();
|
||||
@SuppressWarnings("unchecked")
|
||||
Set<String> sensitiveHeaders = (Set<String>) ctx.get(ProxyRequestHelper.IGNORED_HEADERS);
|
||||
assertTrue("sensitiveHeaders is wrong", sensitiveHeaders.containsAll(Collections.singletonList("x-foo")));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void ignoredHeadersAlreadySetInRequestContextDontGetOverridden() throws Exception {
|
||||
this.properties.setPrefix("/api");
|
||||
this.properties.setStripPrefix(true);
|
||||
this.properties.setSensitiveHeaders(Collections.singleton("x-bar"));
|
||||
this.request.setRequestURI("/api/foo/1");
|
||||
this.routeLocator.addRoute("/foo/**", "foo");
|
||||
RequestContext ctx = RequestContext.getCurrentContext();
|
||||
ctx.set(ProxyRequestHelper.IGNORED_HEADERS, new HashSet<>(Arrays.asList("x-foo")));
|
||||
this.filter.run();
|
||||
@SuppressWarnings("unchecked")
|
||||
Set<String> sensitiveHeaders = (Set<String>) ctx.get(ProxyRequestHelper.IGNORED_HEADERS);
|
||||
assertTrue("sensitiveHeaders is wrong", sensitiveHeaders.containsAll(Arrays.asList("x-bar","x-foo")));
|
||||
}
|
||||
|
||||
private Object getHeader(List<Pair<String, String>> headers, String key) {
|
||||
String value = null;
|
||||
|
||||
Reference in New Issue
Block a user