From 6353603d63418a46094bbabb1070af970b23e7cd Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Tue, 3 Oct 2023 16:13:24 +0100 Subject: [PATCH] Avoid exposing Jetty's WebSocketUpgradeFilter as a bean When the filter is exposed as a bean (directly or through a registration bean), it's picked up by the auto-configuration of MockMvc. This causes a problem as MockMvc does not call init on a filter before it's used and WebSocketUpgradeFilter fails with a NullPointerException if its doFilter method is called when its init method has not been called. This commit reworks the WebSocket auto-configuration to use a ServletContextInitalizer to register WebSocketUpgradeFilter rather than a FilterRegistrationBean. This ensure that the filter is still registered at the required position in the chain (last filter before the servlet) while also preventing it from being registered with the auto-configured MockMvc in tests. Closes gh-37660 --- .../WebSocketServletAutoConfiguration.java | 28 ++++---- ...ebSocketServletAutoConfigurationTests.java | 65 ++++++++++--------- 2 files changed, 49 insertions(+), 44 deletions(-) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/websocket/servlet/WebSocketServletAutoConfiguration.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/websocket/servlet/WebSocketServletAutoConfiguration.java index 48d3379ef3f..1a3f0458638 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/websocket/servlet/WebSocketServletAutoConfiguration.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/websocket/servlet/WebSocketServletAutoConfiguration.java @@ -16,9 +16,10 @@ package org.springframework.boot.autoconfigure.websocket.servlet; -import java.util.List; +import java.util.EnumSet; import jakarta.servlet.DispatcherType; +import jakarta.servlet.FilterRegistration.Dynamic; import jakarta.servlet.Servlet; import jakarta.websocket.server.ServerContainer; import org.apache.catalina.startup.Tomcat; @@ -29,13 +30,15 @@ import org.eclipse.jetty.websocket.servlet.WebSocketUpgradeFilter; import org.springframework.boot.autoconfigure.AutoConfiguration; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; +import org.springframework.boot.autoconfigure.condition.ConditionalOnNotWarDeployment; import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication; import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication.Type; import org.springframework.boot.autoconfigure.web.servlet.ServletWebServerFactoryAutoConfiguration; -import org.springframework.boot.web.servlet.FilterRegistrationBean; +import org.springframework.boot.web.servlet.ServletContextInitializer; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.core.Ordered; +import org.springframework.core.annotation.Order; /** * Auto configuration for WebSocket servlet server in embedded Tomcat, Jetty or Undertow. @@ -86,17 +89,16 @@ public class WebSocketServletAutoConfiguration { } @Bean - @ConditionalOnMissingBean(value = WebSocketUpgradeFilter.class, - parameterizedContainer = FilterRegistrationBean.class) - FilterRegistrationBean webSocketUpgradeFilter() { - WebSocketUpgradeFilter websocketFilter = new WebSocketUpgradeFilter(); - FilterRegistrationBean registration = new FilterRegistrationBean<>(websocketFilter); - registration.setAsyncSupported(true); - registration.setDispatcherTypes(DispatcherType.REQUEST); - registration.setName(WebSocketUpgradeFilter.class.getName()); - registration.setOrder(Ordered.LOWEST_PRECEDENCE); - registration.setUrlPatterns(List.of("/*")); - return registration; + @ConditionalOnNotWarDeployment + @Order(Ordered.LOWEST_PRECEDENCE) + @ConditionalOnMissingBean(name = "websocketUpgradeFilterServletContextInitializer") + ServletContextInitializer websocketUpgradeFilterServletContextInitializer() { + return (servletContext) -> { + Dynamic registration = servletContext.addFilter(WebSocketUpgradeFilter.class.getName(), + new WebSocketUpgradeFilter()); + registration.setAsyncSupported(true); + registration.addMappingForUrlPatterns(EnumSet.of(DispatcherType.REQUEST), false, "/*"); + }; } } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/websocket/servlet/WebSocketServletAutoConfigurationTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/websocket/servlet/WebSocketServletAutoConfigurationTests.java index 423c89df1d5..f38bbc9e8bb 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/websocket/servlet/WebSocketServletAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/websocket/servlet/WebSocketServletAutoConfigurationTests.java @@ -36,6 +36,7 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; +import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.boot.autoconfigure.AutoConfigurations; import org.springframework.boot.autoconfigure.web.servlet.DispatcherServletAutoConfiguration; import org.springframework.boot.test.context.runner.WebApplicationContextRunner; @@ -47,7 +48,9 @@ import org.springframework.boot.web.embedded.jetty.JettyServletWebServerFactory; import org.springframework.boot.web.embedded.tomcat.TomcatServletWebServerFactory; import org.springframework.boot.web.server.WebServer; import org.springframework.boot.web.server.WebServerFactoryCustomizerBeanPostProcessor; +import org.springframework.boot.web.servlet.AbstractFilterRegistrationBean; import org.springframework.boot.web.servlet.FilterRegistrationBean; +import org.springframework.boot.web.servlet.ServletContextInitializer; import org.springframework.boot.web.servlet.context.AnnotationConfigServletWebServerApplicationContext; import org.springframework.boot.web.servlet.server.ServletWebServerFactory; import org.springframework.context.annotation.Bean; @@ -104,34 +107,44 @@ class WebSocketServletAutoConfigurationTests { } @Test - @SuppressWarnings("rawtypes") - void whenCustomUpgradeFilterRegistrationIsDefinedAutoConfiguredRegistrationOfJettyUpgradeFilterBacksOff() { - new WebApplicationContextRunner() - .withConfiguration(AutoConfigurations.of(JettyConfiguration.class, - WebSocketServletAutoConfiguration.JettyWebSocketConfiguration.class)) - .withUserConfiguration(CustomUpgradeFilterRegistrationConfiguration.class) - .run((context) -> { - Map filterRegistrations = context - .getBeansOfType(FilterRegistrationBean.class); - assertThat(filterRegistrations).containsOnlyKeys("unauthorizedFilter", - "customUpgradeFilterRegistration"); - }); + @Servlet5ClassPathOverrides + void jettyWebSocketUpgradeFilterIsAddedToServletContext() { + try (AnnotationConfigServletWebServerApplicationContext context = new AnnotationConfigServletWebServerApplicationContext( + JettyConfiguration.class, WebSocketServletAutoConfiguration.JettyWebSocketConfiguration.class)) { + assertThat(context.getServletContext().getFilterRegistration(WebSocketUpgradeFilter.class.getName())) + .isNotNull(); + } } @Test @SuppressWarnings("rawtypes") - void whenCustomUpgradeFilterIsDefinedAutoConfiguredRegistrationOfJettyUpgradeFilterBacksOff() { + void jettyWebSocketUpgradeFilterIsNotExposedAsABean() { new WebApplicationContextRunner() .withConfiguration(AutoConfigurations.of(JettyConfiguration.class, WebSocketServletAutoConfiguration.JettyWebSocketConfiguration.class)) - .withUserConfiguration(CustomUpgradeFilterConfiguration.class) .run((context) -> { - Map filterRegistrations = context - .getBeansOfType(FilterRegistrationBean.class); - assertThat(filterRegistrations).containsOnlyKeys("unauthorizedFilter"); + Map filters = context.getBeansOfType(Filter.class); + assertThat(filters.values()).noneMatch(WebSocketUpgradeFilter.class::isInstance); + Map filterRegistrations = context + .getBeansOfType(AbstractFilterRegistrationBean.class); + assertThat(filterRegistrations.values()).extracting(AbstractFilterRegistrationBean::getFilter) + .noneMatch(WebSocketUpgradeFilter.class::isInstance); }); } + @Test + @Servlet5ClassPathOverrides + void jettyWebSocketUpgradeFilterServletContextInitializerBacksOffWhenBeanWithSameNameIsDefined() { + try (AnnotationConfigServletWebServerApplicationContext context = new AnnotationConfigServletWebServerApplicationContext( + JettyConfiguration.class, CustomWebSocketUpgradeFilterServletContextInitializerConfiguration.class, + WebSocketServletAutoConfiguration.JettyWebSocketConfiguration.class)) { + BeanDefinition definition = context.getBeanFactory() + .getBeanDefinition("websocketUpgradeFilterServletContextInitializer"); + assertThat(definition.getFactoryBeanName()) + .contains("CustomWebSocketUpgradeFilterServletContextInitializerConfiguration"); + } + } + static Stream testConfiguration() { String response = "Tomcat"; return Stream.of( @@ -196,23 +209,13 @@ class WebSocketServletAutoConfigurationTests { } @Configuration(proxyBeanMethods = false) - static class CustomUpgradeFilterRegistrationConfiguration { + static class CustomWebSocketUpgradeFilterServletContextInitializerConfiguration { @Bean - FilterRegistrationBean customUpgradeFilterRegistration() { - FilterRegistrationBean registration = new FilterRegistrationBean<>( - new WebSocketUpgradeFilter()); - return registration; - } + ServletContextInitializer websocketUpgradeFilterServletContextInitializer() { + return (servletContext) -> { - } - - @Configuration(proxyBeanMethods = false) - static class CustomUpgradeFilterConfiguration { - - @Bean - WebSocketUpgradeFilter customUpgradeFilter() { - return new WebSocketUpgradeFilter(); + }; } }