From 4aa99b95310e5b6a6215715d5c3f825b64b53d83 Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Wed, 22 Feb 2017 13:48:43 +0100 Subject: [PATCH] Avoid exposing several javax.validaton.Validator beans This commit makes sure that the Spring `Validator` used by the MVC layer doesn't expose a JSR-303 contract, if any. The default implementation of the `mvcValidator` is `LocalValidatorFactoryBean`. While this object is exposed as a Spring `Validator` only, its runtime capabilities expose that contract as well as the standard `Validator` and `ValidatorFactory` ones. Concretely, if an auto-configuration is checking if a `javax.validation.Validator` bean is missing, the condition will match since we only know about "advertized types": beans haven't been created yet so we can't inspect their runtime capabilities. Since the condition match, we will auto-configure a bean. At runtime though, we're no longer ale to inject a `javax.validation.Validator` by type since two candidates are available. This commit introduces `SpringValidatorAdapterWrapper`, a wrapper class on any `SpringValidatorAdapter` (`LocalValidatorFactoryBean` being one of the available implementations) that only exposes the Spring contract. Also, if a `javax.validation.Validator` bean is available, we will use it for the MVC layer, rather than creating a new one. Closes gh-8223 --- .../web/SpringValidatorAdapterWrapper.java | 83 ++++++++++ .../web/WebMvcAutoConfiguration.java | 91 ++++++++++- .../SpringValidatorAdapterWrapperTests.java | 152 ++++++++++++++++++ .../web/WebMvcAutoConfigurationTests.java | 86 ++++++++++ 4 files changed, 407 insertions(+), 5 deletions(-) create mode 100644 spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/SpringValidatorAdapterWrapper.java create mode 100644 spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/SpringValidatorAdapterWrapperTests.java diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/SpringValidatorAdapterWrapper.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/SpringValidatorAdapterWrapper.java new file mode 100644 index 00000000000..1fc4c430908 --- /dev/null +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/SpringValidatorAdapterWrapper.java @@ -0,0 +1,83 @@ +/* + * Copyright 2012-2017 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.boot.autoconfigure.web; + +import org.springframework.beans.BeansException; +import org.springframework.beans.factory.DisposableBean; +import org.springframework.beans.factory.InitializingBean; +import org.springframework.context.ApplicationContext; +import org.springframework.context.ApplicationContextAware; +import org.springframework.validation.Errors; +import org.springframework.validation.Validator; +import org.springframework.validation.beanvalidation.SpringValidatorAdapter; + +/** + * Wraps a {@link SpringValidatorAdapter} so that only the Spring's {@link Validator} + * type is exposed. This prevents such a bean to expose both the Spring and JSR-303 + * validator contract at the same time. + * + * @author Stephane Nicoll + */ +class SpringValidatorAdapterWrapper + implements Validator, ApplicationContextAware, InitializingBean, DisposableBean { + + private final SpringValidatorAdapter target; + private final boolean managed; + + SpringValidatorAdapterWrapper(SpringValidatorAdapter target, boolean managed) { + this.target = target; + this.managed = managed; + } + + public SpringValidatorAdapter getTarget() { + return this.target; + } + + @Override + public boolean supports(Class clazz) { + return this.target.supports(clazz); + } + + @Override + public void validate(Object target, Errors errors) { + this.target.validate(target, errors); + } + + @Override + public void setApplicationContext(ApplicationContext applicationContext) + throws BeansException { + if (!this.managed && this.target instanceof ApplicationContextAware) { + ((ApplicationContextAware) this.target).setApplicationContext( + applicationContext); + } + } + + @Override + public void afterPropertiesSet() throws Exception { + if (!this.managed && this.target instanceof InitializingBean) { + ((InitializingBean) this.target).afterPropertiesSet(); + } + } + + @Override + public void destroy() throws Exception { + if (!this.managed && this.target instanceof DisposableBean) { + ((DisposableBean) this.target).destroy(); + } + } + +} diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/WebMvcAutoConfiguration.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/WebMvcAutoConfiguration.java index fe03fd17618..4feacc1ec72 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/WebMvcAutoConfiguration.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/WebMvcAutoConfiguration.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2016 the original author or authors. + * Copyright 2012-2017 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. @@ -42,11 +42,14 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication; +import org.springframework.boot.autoconfigure.validation.ValidationAutoConfiguration; import org.springframework.boot.autoconfigure.web.ResourceProperties.Strategy; import org.springframework.boot.context.properties.EnableConfigurationProperties; +import org.springframework.boot.validation.MessageInterpolatorFactory; import org.springframework.boot.web.filter.OrderedHiddenHttpMethodFilter; import org.springframework.boot.web.filter.OrderedHttpPutFormContentFilter; import org.springframework.boot.web.filter.OrderedRequestContextFilter; +import org.springframework.context.ApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; @@ -61,9 +64,14 @@ import org.springframework.format.datetime.DateFormatter; import org.springframework.http.HttpHeaders; import org.springframework.http.MediaType; import org.springframework.http.converter.HttpMessageConverter; +import org.springframework.util.ClassUtils; import org.springframework.util.StringUtils; import org.springframework.validation.DefaultMessageCodesResolver; import org.springframework.validation.MessageCodesResolver; +import org.springframework.validation.Validator; +import org.springframework.validation.beanvalidation.LocalValidatorFactoryBean; +import org.springframework.validation.beanvalidation.OptionalValidatorFactoryBean; +import org.springframework.validation.beanvalidation.SpringValidatorAdapter; import org.springframework.web.accept.ContentNegotiationManager; import org.springframework.web.bind.support.ConfigurableWebBindingInitializer; import org.springframework.web.context.request.RequestContextListener; @@ -83,6 +91,7 @@ import org.springframework.web.servlet.config.annotation.ResourceChainRegistrati import org.springframework.web.servlet.config.annotation.ResourceHandlerRegistration; import org.springframework.web.servlet.config.annotation.ResourceHandlerRegistry; import org.springframework.web.servlet.config.annotation.WebMvcConfigurationSupport; +import org.springframework.web.servlet.config.annotation.WebMvcConfigurer; import org.springframework.web.servlet.config.annotation.WebMvcConfigurerAdapter; import org.springframework.web.servlet.handler.AbstractHandlerExceptionResolver; import org.springframework.web.servlet.handler.AbstractUrlHandlerMapping; @@ -118,13 +127,16 @@ import org.springframework.web.servlet.view.InternalResourceViewResolver; WebMvcConfigurerAdapter.class }) @ConditionalOnMissingBean(WebMvcConfigurationSupport.class) @AutoConfigureOrder(Ordered.HIGHEST_PRECEDENCE + 10) -@AutoConfigureAfter(DispatcherServletAutoConfiguration.class) +@AutoConfigureAfter({ DispatcherServletAutoConfiguration.class, + ValidationAutoConfiguration.class }) public class WebMvcAutoConfiguration { public static String DEFAULT_PREFIX = ""; public static String DEFAULT_SUFFIX = ""; + private static final String JSR303_VALIDATOR_CLASS = "javax.validation.Validator"; + @Bean @ConditionalOnMissingBean(HiddenHttpMethodFilter.class) public OrderedHiddenHttpMethodFilter hiddenHttpMethodFilter() { @@ -148,6 +160,8 @@ public class WebMvcAutoConfiguration { private static final Log logger = LogFactory .getLog(WebMvcConfigurerAdapter.class); + private final ApplicationContext applicationContext; + private final ResourceProperties resourceProperties; private final WebMvcProperties mvcProperties; @@ -156,20 +170,39 @@ public class WebMvcAutoConfiguration { private final HttpMessageConverters messageConverters; + private final Validator userDefinedValidator; + final ResourceHandlerRegistrationCustomizer resourceHandlerRegistrationCustomizer; - public WebMvcAutoConfigurationAdapter(ResourceProperties resourceProperties, - WebMvcProperties mvcProperties, ListableBeanFactory beanFactory, - HttpMessageConverters messageConverters, + public WebMvcAutoConfigurationAdapter(ApplicationContext applicationContext, + ResourceProperties resourceProperties, WebMvcProperties mvcProperties, + ListableBeanFactory beanFactory, HttpMessageConverters messageConverters, + ObjectProvider> webMvcConfigurers, ObjectProvider resourceHandlerRegistrationCustomizerProvider) { + this.applicationContext = applicationContext; this.resourceProperties = resourceProperties; this.mvcProperties = mvcProperties; this.beanFactory = beanFactory; this.messageConverters = messageConverters; + this.userDefinedValidator = findUserDefinedValidator( + webMvcConfigurers.getIfAvailable()); this.resourceHandlerRegistrationCustomizer = resourceHandlerRegistrationCustomizerProvider .getIfAvailable(); } + private static Validator findUserDefinedValidator( + List webMvcConfigurers) { + if (webMvcConfigurers != null) { + for (WebMvcConfigurer webMvcConfigurer : webMvcConfigurers) { + Validator validator = webMvcConfigurer.getValidator(); + if (validator != null) { + return validator; + } + } + } + return null; + } + @Override public void configureMessageConverters(List> converters) { converters.addAll(this.messageConverters.getConverters()); @@ -265,6 +298,23 @@ public class WebMvcAutoConfiguration { } } + @Override + public Validator getValidator() { + // We want to make sure that the exposed 'mvcValidator' bean isn't going to + // expose the standard JSR-303 type + if (isJsr303Present() && this.userDefinedValidator == null) { + return new Jsr303ValidatorHandler(this.applicationContext) + .wrapJsr303Validator(); + + } + return null; // Keep default or user defined, if any + } + + private boolean isJsr303Present() { + return ClassUtils.isPresent(JSR303_VALIDATOR_CLASS, + this.applicationContext.getClassLoader()); + } + private Collection getBeansOfType(Class type) { return this.beanFactory.getBeansOfType(type).values(); } @@ -533,4 +583,35 @@ public class WebMvcAutoConfiguration { } + static class Jsr303ValidatorHandler { + + private final ApplicationContext applicationContext; + + Jsr303ValidatorHandler(ApplicationContext applicationContext) { + this.applicationContext = applicationContext; + } + + public Validator wrapJsr303Validator() { + try { + javax.validation.Validator validator = this.applicationContext + .getBean(javax.validation.Validator.class); + if (validator instanceof LocalValidatorFactoryBean) { + return new SpringValidatorAdapterWrapper( + (LocalValidatorFactoryBean) validator, true); + } + else { + return new SpringValidatorAdapterWrapper( + new SpringValidatorAdapter(validator), false); + } + } + catch (NoSuchBeanDefinitionException ex) { + OptionalValidatorFactoryBean factory = new OptionalValidatorFactoryBean(); + MessageInterpolatorFactory interpolatorFactory = new MessageInterpolatorFactory(); + factory.setMessageInterpolator(interpolatorFactory.getObject()); + return new SpringValidatorAdapterWrapper(factory, false); + } + + } + } + } diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/SpringValidatorAdapterWrapperTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/SpringValidatorAdapterWrapperTests.java new file mode 100644 index 00000000000..ff1b9525a92 --- /dev/null +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/SpringValidatorAdapterWrapperTests.java @@ -0,0 +1,152 @@ +/* + * Copyright 2012-2017 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.boot.autoconfigure.web; + +import java.util.HashMap; + +import javax.validation.constraints.Min; + +import org.junit.After; +import org.junit.Test; + +import org.springframework.context.ApplicationContext; +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.validation.MapBindingResult; +import org.springframework.validation.beanvalidation.LocalValidatorFactoryBean; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + +/** + * Tests for {@link SpringValidatorAdapterWrapper}. + * + * @author Stephane Nicoll + */ +public class SpringValidatorAdapterWrapperTests { + + private AnnotationConfigApplicationContext context; + + @After + public void close() { + if (this.context != null) { + this.context.close(); + } + } + + @Test + public void wrapLocalValidatorFactoryBean() { + SpringValidatorAdapterWrapper wrapper = load( + LocalValidatorFactoryBeanConfig.class); + assertThat(wrapper.supports(SampleData.class)).isTrue(); + MapBindingResult errors = new MapBindingResult( + new HashMap(), "test"); + wrapper.validate(new SampleData(40), errors); + assertThat(errors.getErrorCount()).isEqualTo(1); + } + + @Test + public void wrapperInvokesCallbackOnNonManagedBean() { + load(NonManagedBeanConfig.class); + LocalValidatorFactoryBean validator = this.context.getBean( + NonManagedBeanConfig.class).validator; + verify(validator, times(1)).setApplicationContext(any(ApplicationContext.class)); + verify(validator, times(1)).afterPropertiesSet(); + verify(validator, times(0)).destroy(); + this.context.close(); + this.context = null; + verify(validator, times(1)).destroy(); + } + + @Test + public void wrapperDoesNotInvokeCallbackOnManagedBean() { + load(ManagedBeanConfig.class); + LocalValidatorFactoryBean validator = this.context.getBean( + ManagedBeanConfig.class).validator; + verify(validator, times(0)).setApplicationContext(any(ApplicationContext.class)); + verify(validator, times(0)).afterPropertiesSet(); + verify(validator, times(0)).destroy(); + this.context.close(); + this.context = null; + verify(validator, times(0)).destroy(); + } + + private SpringValidatorAdapterWrapper load(Class config) { + AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(); + ctx.register(config); + ctx.refresh(); + this.context = ctx; + return this.context.getBean(SpringValidatorAdapterWrapper.class); + } + + @Configuration + static class LocalValidatorFactoryBeanConfig { + + @Bean + public LocalValidatorFactoryBean validator() { + return new LocalValidatorFactoryBean(); + } + + @Bean + public SpringValidatorAdapterWrapper wrapper() { + return new SpringValidatorAdapterWrapper(validator(), true); + } + + } + + @Configuration + static class NonManagedBeanConfig { + + private final LocalValidatorFactoryBean validator + = mock(LocalValidatorFactoryBean.class); + + @Bean + public SpringValidatorAdapterWrapper wrapper() { + return new SpringValidatorAdapterWrapper(this.validator, false); + } + + } + + @Configuration + static class ManagedBeanConfig { + + private final LocalValidatorFactoryBean validator + = mock(LocalValidatorFactoryBean.class); + + @Bean + public SpringValidatorAdapterWrapper wrapper() { + return new SpringValidatorAdapterWrapper(this.validator, true); + } + + } + + static class SampleData { + + @Min(42) + private int counter; + + SampleData(int counter) { + this.counter = counter; + } + + } + +} diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/WebMvcAutoConfigurationTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/WebMvcAutoConfigurationTests.java index dc713a38f44..3d6b27aa10f 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/WebMvcAutoConfigurationTests.java +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/WebMvcAutoConfigurationTests.java @@ -27,6 +27,7 @@ import java.util.Map; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import javax.validation.ValidatorFactory; import org.assertj.core.api.Condition; import org.joda.time.DateTime; @@ -60,6 +61,9 @@ import org.springframework.test.web.servlet.MockMvc; import org.springframework.test.web.servlet.setup.MockMvcBuilders; import org.springframework.util.ReflectionUtils; import org.springframework.util.StringUtils; +import org.springframework.validation.Validator; +import org.springframework.validation.beanvalidation.LocalValidatorFactoryBean; +import org.springframework.validation.beanvalidation.SpringValidatorAdapter; import org.springframework.web.accept.ContentNegotiationManager; import org.springframework.web.bind.support.ConfigurableWebBindingInitializer; import org.springframework.web.filter.HttpPutFormContentFilter; @@ -94,6 +98,7 @@ import org.springframework.web.servlet.view.AbstractView; import org.springframework.web.servlet.view.ContentNegotiatingViewResolver; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.forwardedUrl; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @@ -640,6 +645,55 @@ public class WebMvcAutoConfigurationTests { } } + @Test + public void validationNoJsr303ValidatorExposedByDefault() { + load(); + assertThat(this.context.getBeansOfType(ValidatorFactory.class)).isEmpty(); + assertThat(this.context.getBeansOfType(javax.validation.Validator.class)) + .isEmpty(); + assertThat(this.context.getBeansOfType(Validator.class)).hasSize(1); + } + + @Test + public void validationCustomConfigurerTakesPrecedence() { + load(MvcValidator.class); + assertThat(this.context.getBeansOfType(ValidatorFactory.class)).isEmpty(); + assertThat(this.context.getBeansOfType(javax.validation.Validator.class)) + .isEmpty(); + assertThat(this.context.getBeansOfType(Validator.class)).hasSize(1); + Validator validator = this.context.getBean(Validator.class); + assertThat(validator).isSameAs(this.context.getBean(MvcValidator.class) + .validator); + } + + @Test + public void validationJsr303CustomValidatorReusedAsSpringValidator() { + load(CustomValidator.class); + assertThat(this.context.getBeansOfType(ValidatorFactory.class)).hasSize(1); + assertThat(this.context.getBeansOfType(javax.validation.Validator.class)) + .hasSize(1); + assertThat(this.context.getBeansOfType(Validator.class)).hasSize(2); + Validator validator = this.context.getBean("mvcValidator", Validator.class); + assertThat(validator).isInstanceOf(SpringValidatorAdapterWrapper.class); + assertThat(((SpringValidatorAdapterWrapper) validator).getTarget()) + .isSameAs(this.context.getBean(javax.validation.Validator.class)); + } + + @Test + public void validationJsr303ValidatorExposedAsSpringValidator() { + load(Jsr303Validator.class); + assertThat(this.context.getBeansOfType(ValidatorFactory.class)).isEmpty(); + assertThat(this.context.getBeansOfType(javax.validation.Validator.class)) + .hasSize(1); + assertThat(this.context.getBeansOfType(Validator.class)).hasSize(1); + Validator validator = this.context.getBean(Validator.class); + assertThat(validator).isInstanceOf(SpringValidatorAdapterWrapper.class); + SpringValidatorAdapter target = ((SpringValidatorAdapterWrapper) validator) + .getTarget(); + assertThat(new DirectFieldAccessor(target).getPropertyValue("targetValidator")) + .isSameAs(this.context.getBean(javax.validation.Validator.class)); + } + private void load(Class config, String... environment) { this.context = new AnnotationConfigEmbeddedWebApplicationContext(); EnvironmentTestUtils.addEnvironment(this.context, environment); @@ -818,4 +872,36 @@ public class WebMvcAutoConfigurationTests { } + @Configuration + protected static class MvcValidator extends WebMvcConfigurerAdapter { + + private final Validator validator = mock(Validator.class); + + @Override + public Validator getValidator() { + return this.validator; + } + + } + + @Configuration + static class Jsr303Validator { + + @Bean + public javax.validation.Validator jsr303Validator() { + return mock(javax.validation.Validator.class); + } + + } + + @Configuration + static class CustomValidator { + + @Bean + public Validator customValidator() { + return new LocalValidatorFactoryBean(); + } + + } + }