From 7405efa326773a69bfc71a06a31d51a9781eb98e Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Fri, 21 Jun 2019 17:52:32 +0100 Subject: [PATCH] Ensure that each Tomcat customizer is only called once Fixes gh-17264 --- ...ebServerFactoryAutoConfigurationTests.java | 148 ++++++++++++++++-- ...ebServerFactoryAutoConfigurationTests.java | 136 ++++++++++++++-- .../TomcatReactiveWebServerFactory.java | 7 +- .../tomcat/TomcatServletWebServerFactory.java | 6 +- 4 files changed, 264 insertions(+), 33 deletions(-) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/reactive/ReactiveWebServerFactoryAutoConfigurationTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/reactive/ReactiveWebServerFactoryAutoConfigurationTests.java index aa22a05e6ff..e6624746d78 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/reactive/ReactiveWebServerFactoryAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/reactive/ReactiveWebServerFactoryAutoConfigurationTests.java @@ -16,6 +16,8 @@ package org.springframework.boot.autoconfigure.web.reactive; +import org.apache.catalina.Context; +import org.apache.catalina.connector.Connector; import org.apache.catalina.startup.Tomcat; import org.eclipse.jetty.server.Server; import org.junit.jupiter.api.Test; @@ -46,6 +48,10 @@ import org.springframework.http.server.reactive.HttpHandler; import org.springframework.web.server.adapter.ForwardedHeaderTransformer; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; /** * Tests for {@link ReactiveWebServerFactoryAutoConfiguration}. @@ -108,36 +114,100 @@ class ReactiveWebServerFactoryAutoConfigurationTests { @Test void tomcatConnectorCustomizerBeanIsAddedToFactory() { ReactiveWebApplicationContextRunner runner = new ReactiveWebApplicationContextRunner( - AnnotationConfigReactiveWebApplicationContext::new) + AnnotationConfigReactiveWebServerApplicationContext::new) .withConfiguration(AutoConfigurations.of(ReactiveWebServerFactoryAutoConfiguration.class)) - .withUserConfiguration(TomcatConnectorCustomizerConfiguration.class); + .withUserConfiguration(HttpHandlerConfiguration.class, + TomcatConnectorCustomizerConfiguration.class) + .withPropertyValues("server.port: 0"); runner.run((context) -> { TomcatReactiveWebServerFactory factory = context.getBean(TomcatReactiveWebServerFactory.class); - assertThat(factory.getTomcatConnectorCustomizers()).hasSize(1); + TomcatConnectorCustomizer customizer = context.getBean("connectorCustomizer", + TomcatConnectorCustomizer.class); + assertThat(factory.getTomcatConnectorCustomizers()).contains(customizer); + verify(customizer, times(1)).customize(any(Connector.class)); + }); + } + + @Test + void tomcatConnectorCustomizerRegisteredAsBeanAndViaFactoryIsOnlyCalledOnce() { + ReactiveWebApplicationContextRunner runner = new ReactiveWebApplicationContextRunner( + AnnotationConfigReactiveWebServerApplicationContext::new) + .withConfiguration(AutoConfigurations.of(ReactiveWebServerFactoryAutoConfiguration.class)) + .withUserConfiguration(HttpHandlerConfiguration.class, + DoubleRegistrationTomcatConnectorCustomizerConfiguration.class) + .withPropertyValues("server.port: 0"); + runner.run((context) -> { + TomcatReactiveWebServerFactory factory = context.getBean(TomcatReactiveWebServerFactory.class); + TomcatConnectorCustomizer customizer = context.getBean("connectorCustomizer", + TomcatConnectorCustomizer.class); + assertThat(factory.getTomcatConnectorCustomizers()).contains(customizer); + verify(customizer, times(1)).customize(any(Connector.class)); }); } @Test void tomcatContextCustomizerBeanIsAddedToFactory() { ReactiveWebApplicationContextRunner runner = new ReactiveWebApplicationContextRunner( - AnnotationConfigReactiveWebApplicationContext::new) + AnnotationConfigReactiveWebServerApplicationContext::new) .withConfiguration(AutoConfigurations.of(ReactiveWebServerFactoryAutoConfiguration.class)) - .withUserConfiguration(TomcatContextCustomizerConfiguration.class); + .withUserConfiguration(HttpHandlerConfiguration.class, + TomcatContextCustomizerConfiguration.class) + .withPropertyValues("server.port: 0"); runner.run((context) -> { TomcatReactiveWebServerFactory factory = context.getBean(TomcatReactiveWebServerFactory.class); - assertThat(factory.getTomcatContextCustomizers()).hasSize(1); + TomcatContextCustomizer customizer = context.getBean("contextCustomizer", TomcatContextCustomizer.class); + assertThat(factory.getTomcatContextCustomizers()).contains(customizer); + verify(customizer, times(1)).customize(any(Context.class)); + }); + } + + @Test + void tomcatContextCustomizerRegisteredAsBeanAndViaFactoryIsOnlyCalledOnce() { + ReactiveWebApplicationContextRunner runner = new ReactiveWebApplicationContextRunner( + AnnotationConfigReactiveWebServerApplicationContext::new) + .withConfiguration(AutoConfigurations.of(ReactiveWebServerFactoryAutoConfiguration.class)) + .withUserConfiguration(HttpHandlerConfiguration.class, + DoubleRegistrationTomcatContextCustomizerConfiguration.class) + .withPropertyValues("server.port: 0"); + runner.run((context) -> { + TomcatReactiveWebServerFactory factory = context.getBean(TomcatReactiveWebServerFactory.class); + TomcatContextCustomizer customizer = context.getBean("contextCustomizer", TomcatContextCustomizer.class); + assertThat(factory.getTomcatContextCustomizers()).contains(customizer); + verify(customizer, times(1)).customize(any(Context.class)); }); } @Test void tomcatProtocolHandlerCustomizerBeanIsAddedToFactory() { ReactiveWebApplicationContextRunner runner = new ReactiveWebApplicationContextRunner( - AnnotationConfigReactiveWebApplicationContext::new) + AnnotationConfigReactiveWebServerApplicationContext::new) .withConfiguration(AutoConfigurations.of(ReactiveWebServerFactoryAutoConfiguration.class)) - .withUserConfiguration(TomcatProtocolHandlerCustomizerConfiguration.class); + .withUserConfiguration(HttpHandlerConfiguration.class, + TomcatProtocolHandlerCustomizerConfiguration.class) + .withPropertyValues("server.port: 0"); runner.run((context) -> { TomcatReactiveWebServerFactory factory = context.getBean(TomcatReactiveWebServerFactory.class); - assertThat(factory.getTomcatProtocolHandlerCustomizers()).hasSize(1); + TomcatProtocolHandlerCustomizer customizer = context.getBean("protocolHandlerCustomizer", + TomcatProtocolHandlerCustomizer.class); + assertThat(factory.getTomcatProtocolHandlerCustomizers()).contains(customizer); + verify(customizer, times(1)).customize(any()); + }); + } + + @Test + void tomcatProtocolHandlerCustomizerRegisteredAsBeanAndViaFactoryIsOnlyCalledOnce() { + ReactiveWebApplicationContextRunner runner = new ReactiveWebApplicationContextRunner( + AnnotationConfigReactiveWebServerApplicationContext::new) + .withConfiguration(AutoConfigurations.of(ReactiveWebServerFactoryAutoConfiguration.class)) + .withUserConfiguration(HttpHandlerConfiguration.class, + DoubleRegistrationTomcatProtocolHandlerCustomizerConfiguration.class) + .withPropertyValues("server.port: 0"); + runner.run((context) -> { + TomcatReactiveWebServerFactory factory = context.getBean(TomcatReactiveWebServerFactory.class); + TomcatProtocolHandlerCustomizer customizer = context.getBean("protocolHandlerCustomizer", + TomcatProtocolHandlerCustomizer.class); + assertThat(factory.getTomcatProtocolHandlerCustomizers()).contains(customizer); + verify(customizer, times(1)).customize(any()); }); } @@ -245,8 +315,24 @@ class ReactiveWebServerFactoryAutoConfigurationTests { @Bean public TomcatConnectorCustomizer connectorCustomizer() { - return (connector) -> { - }; + return mock(TomcatConnectorCustomizer.class); + } + + } + + @Configuration(proxyBeanMethods = false) + static class DoubleRegistrationTomcatConnectorCustomizerConfiguration { + + private final TomcatConnectorCustomizer customizer = mock(TomcatConnectorCustomizer.class); + + @Bean + public TomcatConnectorCustomizer connectorCustomizer() { + return this.customizer; + } + + @Bean + public WebServerFactoryCustomizer tomcatCustomizer() { + return (tomcat) -> tomcat.addConnectorCustomizers(this.customizer); } } @@ -256,8 +342,24 @@ class ReactiveWebServerFactoryAutoConfigurationTests { @Bean public TomcatContextCustomizer contextCustomizer() { - return (context) -> { - }; + return mock(TomcatContextCustomizer.class); + } + + } + + @Configuration(proxyBeanMethods = false) + static class DoubleRegistrationTomcatContextCustomizerConfiguration { + + private final TomcatContextCustomizer customizer = mock(TomcatContextCustomizer.class); + + @Bean + public TomcatContextCustomizer contextCustomizer() { + return this.customizer; + } + + @Bean + public WebServerFactoryCustomizer tomcatCustomizer() { + return (tomcat) -> tomcat.addContextCustomizers(this.customizer); } } @@ -267,8 +369,24 @@ class ReactiveWebServerFactoryAutoConfigurationTests { @Bean public TomcatProtocolHandlerCustomizer protocolHandlerCustomizer() { - return (protocolHandler) -> { - }; + return mock(TomcatProtocolHandlerCustomizer.class); + } + + } + + @Configuration(proxyBeanMethods = false) + static class DoubleRegistrationTomcatProtocolHandlerCustomizerConfiguration { + + private final TomcatProtocolHandlerCustomizer customizer = mock(TomcatProtocolHandlerCustomizer.class); + + @Bean + public TomcatProtocolHandlerCustomizer protocolHandlerCustomizer() { + return this.customizer; + } + + @Bean + public WebServerFactoryCustomizer tomcatCustomizer() { + return (tomcat) -> tomcat.addProtocolHandlerCustomizers(this.customizer); } } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/ServletWebServerFactoryAutoConfigurationTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/ServletWebServerFactoryAutoConfigurationTests.java index 01b8142f54d..473a0effa57 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/ServletWebServerFactoryAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/ServletWebServerFactoryAutoConfigurationTests.java @@ -22,6 +22,8 @@ import javax.servlet.ServletContext; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.apache.catalina.Context; +import org.apache.catalina.connector.Connector; import org.apache.catalina.startup.Tomcat; import org.eclipse.jetty.server.Server; import org.junit.jupiter.api.Test; @@ -59,6 +61,9 @@ import org.springframework.web.servlet.DispatcherServlet; import org.springframework.web.servlet.FrameworkServlet; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; /** @@ -188,10 +193,30 @@ class ServletWebServerFactoryAutoConfigurationTests { WebApplicationContextRunner runner = new WebApplicationContextRunner( AnnotationConfigServletWebServerApplicationContext::new) .withConfiguration(AutoConfigurations.of(ServletWebServerFactoryAutoConfiguration.class)) - .withUserConfiguration(TomcatConnectorCustomizerConfiguration.class); + .withUserConfiguration(TomcatConnectorCustomizerConfiguration.class) + .withPropertyValues("server.port: 0"); runner.run((context) -> { TomcatServletWebServerFactory factory = context.getBean(TomcatServletWebServerFactory.class); - assertThat(factory.getTomcatConnectorCustomizers()).hasSize(1); + TomcatConnectorCustomizer customizer = context.getBean("connectorCustomizer", + TomcatConnectorCustomizer.class); + assertThat(factory.getTomcatConnectorCustomizers()).contains(customizer); + verify(customizer, times(1)).customize(any(Connector.class)); + }); + } + + @Test + void tomcatConnectorCustomizerRegisteredAsBeanAndViaFactoryIsOnlyCalledOnce() { + WebApplicationContextRunner runner = new WebApplicationContextRunner( + AnnotationConfigServletWebServerApplicationContext::new) + .withConfiguration(AutoConfigurations.of(ServletWebServerFactoryAutoConfiguration.class)) + .withUserConfiguration(DoubleRegistrationTomcatConnectorCustomizerConfiguration.class) + .withPropertyValues("server.port: 0"); + runner.run((context) -> { + TomcatServletWebServerFactory factory = context.getBean(TomcatServletWebServerFactory.class); + TomcatConnectorCustomizer customizer = context.getBean("connectorCustomizer", + TomcatConnectorCustomizer.class); + assertThat(factory.getTomcatConnectorCustomizers()).contains(customizer); + verify(customizer, times(1)).customize(any(Connector.class)); }); } @@ -199,10 +224,29 @@ class ServletWebServerFactoryAutoConfigurationTests { void tomcatContextCustomizerBeanIsAddedToFactory() { WebApplicationContextRunner runner = new WebApplicationContextRunner( AnnotationConfigServletWebServerApplicationContext::new) - .withConfiguration(AutoConfigurations.of(ServletWebServerFactoryAutoConfiguration.class)); + .withConfiguration(AutoConfigurations.of(ServletWebServerFactoryAutoConfiguration.class)) + .withUserConfiguration(TomcatContextCustomizerConfiguration.class) + .withPropertyValues("server.port: 0"); runner.run((context) -> { TomcatServletWebServerFactory factory = context.getBean(TomcatServletWebServerFactory.class); - assertThat(factory.getTomcatContextCustomizers()).hasSize(1); + TomcatContextCustomizer customizer = context.getBean("contextCustomizer", TomcatContextCustomizer.class); + assertThat(factory.getTomcatContextCustomizers()).contains(customizer); + verify(customizer, times(1)).customize(any(Context.class)); + }); + } + + @Test + void tomcatContextCustomizerRegisteredAsBeanAndViaFactoryIsOnlyCalledOnce() { + WebApplicationContextRunner runner = new WebApplicationContextRunner( + AnnotationConfigServletWebServerApplicationContext::new) + .withConfiguration(AutoConfigurations.of(ServletWebServerFactoryAutoConfiguration.class)) + .withUserConfiguration(DoubleRegistrationTomcatContextCustomizerConfiguration.class) + .withPropertyValues("server.port: 0"); + runner.run((context) -> { + TomcatServletWebServerFactory factory = context.getBean(TomcatServletWebServerFactory.class); + TomcatContextCustomizer customizer = context.getBean("contextCustomizer", TomcatContextCustomizer.class); + assertThat(factory.getTomcatContextCustomizers()).contains(customizer); + verify(customizer, times(1)).customize(any(Context.class)); }); } @@ -211,10 +255,30 @@ class ServletWebServerFactoryAutoConfigurationTests { WebApplicationContextRunner runner = new WebApplicationContextRunner( AnnotationConfigServletWebServerApplicationContext::new) .withConfiguration(AutoConfigurations.of(ServletWebServerFactoryAutoConfiguration.class)) - .withUserConfiguration(TomcatProtocolHandlerCustomizerConfiguration.class); + .withUserConfiguration(TomcatProtocolHandlerCustomizerConfiguration.class) + .withPropertyValues("server.port: 0"); runner.run((context) -> { TomcatServletWebServerFactory factory = context.getBean(TomcatServletWebServerFactory.class); - assertThat(factory.getTomcatProtocolHandlerCustomizers()).hasSize(1); + TomcatProtocolHandlerCustomizer customizer = context.getBean("protocolHandlerCustomizer", + TomcatProtocolHandlerCustomizer.class); + assertThat(factory.getTomcatProtocolHandlerCustomizers()).contains(customizer); + verify(customizer, times(1)).customize(any()); + }); + } + + @Test + void tomcatProtocolHandlerCustomizerRegisteredAsBeanAndViaFactoryIsOnlyCalledOnce() { + WebApplicationContextRunner runner = new WebApplicationContextRunner( + AnnotationConfigServletWebServerApplicationContext::new) + .withConfiguration(AutoConfigurations.of(ServletWebServerFactoryAutoConfiguration.class)) + .withUserConfiguration(DoubleRegistrationTomcatProtocolHandlerCustomizerConfiguration.class) + .withPropertyValues("server.port: 0"); + runner.run((context) -> { + TomcatServletWebServerFactory factory = context.getBean(TomcatServletWebServerFactory.class); + TomcatProtocolHandlerCustomizer customizer = context.getBean("protocolHandlerCustomizer", + TomcatProtocolHandlerCustomizer.class); + assertThat(factory.getTomcatProtocolHandlerCustomizers()).contains(customizer); + verify(customizer, times(1)).customize(any()); }); } @@ -356,8 +420,24 @@ class ServletWebServerFactoryAutoConfigurationTests { @Bean public TomcatConnectorCustomizer connectorCustomizer() { - return (connector) -> { - }; + return mock(TomcatConnectorCustomizer.class); + } + + } + + @Configuration(proxyBeanMethods = false) + static class DoubleRegistrationTomcatConnectorCustomizerConfiguration { + + private final TomcatConnectorCustomizer customizer = mock(TomcatConnectorCustomizer.class); + + @Bean + public TomcatConnectorCustomizer connectorCustomizer() { + return this.customizer; + } + + @Bean + public WebServerFactoryCustomizer tomcatCustomizer() { + return (tomcat) -> tomcat.addConnectorCustomizers(this.customizer); } } @@ -367,8 +447,24 @@ class ServletWebServerFactoryAutoConfigurationTests { @Bean public TomcatContextCustomizer contextCustomizer() { - return (context) -> { - }; + return mock(TomcatContextCustomizer.class); + } + + } + + @Configuration(proxyBeanMethods = false) + static class DoubleRegistrationTomcatContextCustomizerConfiguration { + + private final TomcatContextCustomizer customizer = mock(TomcatContextCustomizer.class); + + @Bean + public TomcatContextCustomizer contextCustomizer() { + return this.customizer; + } + + @Bean + public WebServerFactoryCustomizer tomcatCustomizer() { + return (tomcat) -> tomcat.addContextCustomizers(this.customizer); } } @@ -378,8 +474,24 @@ class ServletWebServerFactoryAutoConfigurationTests { @Bean public TomcatProtocolHandlerCustomizer protocolHandlerCustomizer() { - return (protocolHandler) -> { - }; + return mock(TomcatProtocolHandlerCustomizer.class); + } + + } + + @Configuration(proxyBeanMethods = false) + static class DoubleRegistrationTomcatProtocolHandlerCustomizerConfiguration { + + private final TomcatProtocolHandlerCustomizer customizer = mock(TomcatProtocolHandlerCustomizer.class); + + @Bean + public TomcatProtocolHandlerCustomizer protocolHandlerCustomizer() { + return this.customizer; + } + + @Bean + public WebServerFactoryCustomizer tomcatCustomizer() { + return (tomcat) -> tomcat.addProtocolHandlerCustomizers(this.customizer); } } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatReactiveWebServerFactory.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatReactiveWebServerFactory.java index 6318105b876..db6ac05e244 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatReactiveWebServerFactory.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatReactiveWebServerFactory.java @@ -22,6 +22,7 @@ import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.LinkedHashSet; import java.util.List; import org.apache.catalina.Context; @@ -71,11 +72,11 @@ public class TomcatReactiveWebServerFactory extends AbstractReactiveWebServerFac private List contextLifecycleListeners = getDefaultLifecycleListeners(); - private List tomcatContextCustomizers = new ArrayList<>(); + private Collection tomcatContextCustomizers = new LinkedHashSet<>(); - private List tomcatConnectorCustomizers = new ArrayList<>(); + private Collection tomcatConnectorCustomizers = new LinkedHashSet<>(); - private List> tomcatProtocolHandlerCustomizers = new ArrayList<>(); + private Collection> tomcatProtocolHandlerCustomizers = new LinkedHashSet<>(); private String protocol = DEFAULT_PROTOCOL; diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatServletWebServerFactory.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatServletWebServerFactory.java index 8bd3dc7e285..131ff8d265c 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatServletWebServerFactory.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatServletWebServerFactory.java @@ -116,11 +116,11 @@ public class TomcatServletWebServerFactory extends AbstractServletWebServerFacto private List contextLifecycleListeners = getDefaultLifecycleListeners(); - private List tomcatContextCustomizers = new ArrayList<>(); + private Collection tomcatContextCustomizers = new LinkedHashSet<>(); - private List tomcatConnectorCustomizers = new ArrayList<>(); + private Collection tomcatConnectorCustomizers = new LinkedHashSet<>(); - private List> tomcatProtocolHandlerCustomizers = new ArrayList<>(); + private Collection> tomcatProtocolHandlerCustomizers = new LinkedHashSet<>(); private final List additionalTomcatConnectors = new ArrayList<>();