MetricsInterceptorConfiguration modifies potenially unmodifiable list.

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<ClientHttpRequestInterceptor> interceptors = new ArrayList<>();
+					interceptors.add(interceptor);
+					interceptors.addAll(restTemplate.getInterceptors());
+					restTemplate.setInterceptors(interceptors);
 				}
 				return bean;
 			}

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