From 519be4db06329658633477c4fb046895ca177c30 Mon Sep 17 00:00:00 2001 From: libetl Date: Wed, 10 Aug 2016 07:54:06 +0200 Subject: [PATCH] MetricsInterceptorConfiguration modifies potenially unmodifiable list. fixes gh-1240 --- .../MetricsInterceptorConfiguration.java | 14 +++++-- ...ricsClientHttpRequestInterceptorTests.java | 37 +++++++++++++++++++ 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/metrics/MetricsInterceptorConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/metrics/MetricsInterceptorConfiguration.java index 8b316bba..e48f68b3 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/metrics/MetricsInterceptorConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/metrics/MetricsInterceptorConfiguration.java @@ -13,6 +13,8 @@ package org.springframework.cloud.netflix.metrics; +import java.util.ArrayList; + import org.aspectj.lang.JoinPoint; import org.springframework.beans.BeansException; import org.springframework.beans.factory.config.BeanPostProcessor; @@ -25,6 +27,7 @@ import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.http.client.ClientHttpRequestInterceptor; import org.springframework.web.client.RestTemplate; import org.springframework.web.servlet.config.annotation.InterceptorRegistry; import org.springframework.web.servlet.config.annotation.WebMvcConfigurerAdapter; @@ -80,8 +83,8 @@ public class MetricsInterceptorConfiguration { return new MetricsInterceptorPostProcessor(); } - private static class MetricsInterceptorPostProcessor implements - BeanPostProcessor, ApplicationContextAware { + private static class MetricsInterceptorPostProcessor + implements BeanPostProcessor, ApplicationContextAware { private ApplicationContext context; private MetricsClientHttpRequestInterceptor interceptor; @@ -97,7 +100,12 @@ public class MetricsInterceptorConfiguration { this.interceptor = this.context .getBean(MetricsClientHttpRequestInterceptor.class); } - ((RestTemplate) bean).getInterceptors().add(interceptor); + RestTemplate restTemplate = (RestTemplate) bean; + // create a new list as the old one may be unmodifiable (ie Arrays.asList()) + ArrayList interceptors = new ArrayList<>(); + interceptors.add(interceptor); + interceptors.addAll(restTemplate.getInterceptors()); + restTemplate.setInterceptors(interceptors); } return bean; } diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/metrics/MetricsClientHttpRequestInterceptorTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/metrics/MetricsClientHttpRequestInterceptorTests.java index 816b1cac..db5ac7b1 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/metrics/MetricsClientHttpRequestInterceptorTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/metrics/MetricsClientHttpRequestInterceptorTests.java @@ -15,8 +15,11 @@ package org.springframework.cloud.netflix.metrics; import static org.junit.Assert.assertEquals; +import java.util.Arrays; + import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.Mockito; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.autoconfigure.ImportAutoConfiguration; import org.springframework.boot.autoconfigure.PropertyPlaceholderAutoConfiguration; @@ -28,6 +31,7 @@ import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Primary; import org.springframework.http.HttpMethod; import org.springframework.http.MediaType; +import org.springframework.http.client.ClientHttpRequestInterceptor; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.TestPropertySource; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; @@ -58,6 +62,9 @@ public class MetricsClientHttpRequestInterceptorTests { @Autowired RestTemplate restTemplate; + @Autowired + RestTemplate restTemplateWithFakeInterceptorsList; + @Test public void metricsGatheredWhenSuccessful() { MockRestServiceServer mockServer = MockRestServiceServer.createServer(restTemplate); @@ -79,6 +86,28 @@ public class MetricsClientHttpRequestInterceptorTests { mockServer.verify(); } + + @Test + public void restTemplateWithFakeInterceptorList() { + MockRestServiceServer mockServer = MockRestServiceServer.createServer(restTemplateWithFakeInterceptorsList); + mockServer.expect(MockRestRequestMatchers.requestTo("/test/123")) + .andExpect(MockRestRequestMatchers.method(HttpMethod.GET)) + .andRespond(MockRestResponseCreators.withSuccess("OK", MediaType.APPLICATION_JSON)); + String s = restTemplate.getForObject("/test/{id}", String.class, 123); + + MonitorConfig.Builder builder = new MonitorConfig.Builder("metricName") + .withTag("method", "GET") + .withTag("uri", "_test_-id-") + .withTag("status", "200") + .withTag("clientName", "none"); + + BasicTimer timer = servoMonitorCache.getTimer(builder.build()); + + assertEquals(2L, (long) timer.getCount()); + assertEquals("OK", s); + + mockServer.verify(); + } } @Configuration @@ -95,7 +124,15 @@ class MetricsRestTemplateTestConfig { @Configuration class MetricsRestTemplateRestTemplateConfig { @Bean + @Primary RestTemplate restTemplate() { return new RestTemplate(); } + + @Bean(name="restTemplateWithFakeInterceptorsList") + RestTemplate restTemplateWithFakeInterceptorsList() { + RestTemplate restTemplate = new RestTemplate(); + restTemplate.setInterceptors(Arrays.asList(Mockito.mock(ClientHttpRequestInterceptor.class))); + return restTemplate; + } } \ No newline at end of file