From 5ef0ee0ed4a607b983f213d841be3aa211767391 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Tue, 2 May 2023 13:19:41 -0700 Subject: [PATCH] Prevent early initialization of SessionRepository beans Replace `SessionRepositoryFilterConfiguration` filter registration bean with a `DelegatingFilterProxyRegistrationBean` so that `SessionRepository` beans are not initialized early. Fixes gh-35240 --- .../SessionRepositoryFilterConfiguration.java | 13 ++-- ...onEarlyInitializationIntegrationTests.java | 66 +++++++++++++++++++ .../SessionAutoConfigurationJdbcTests.java | 4 +- .../SessionAutoConfigurationTests.java | 21 ++++-- .../FilterOrderingIntegrationTests.java | 29 ++++---- 5 files changed, 106 insertions(+), 27 deletions(-) create mode 100644 spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/session/SessionAutoConfigurationEarlyInitializationIntegrationTests.java diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/session/SessionRepositoryFilterConfiguration.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/session/SessionRepositoryFilterConfiguration.java index 3338603aeb..6b8785d074 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/session/SessionRepositoryFilterConfiguration.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/session/SessionRepositoryFilterConfiguration.java @@ -21,12 +21,14 @@ import java.util.stream.Collectors; import javax.servlet.DispatcherType; +import org.springframework.beans.factory.ListableBeanFactory; import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; import org.springframework.boot.context.properties.EnableConfigurationProperties; -import org.springframework.boot.web.servlet.FilterRegistrationBean; +import org.springframework.boot.web.servlet.DelegatingFilterProxyRegistrationBean; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.session.web.http.SessionRepositoryFilter; +import org.springframework.util.Assert; /** * Configuration for customizing the registration of the {@link SessionRepositoryFilter}. @@ -39,9 +41,12 @@ import org.springframework.session.web.http.SessionRepositoryFilter; class SessionRepositoryFilterConfiguration { @Bean - FilterRegistrationBean> sessionRepositoryFilterRegistration( - SessionProperties sessionProperties, SessionRepositoryFilter filter) { - FilterRegistrationBean> registration = new FilterRegistrationBean<>(filter); + DelegatingFilterProxyRegistrationBean sessionRepositoryFilterRegistration(SessionProperties sessionProperties, + ListableBeanFactory beanFactory) { + String[] targetBeanNames = beanFactory.getBeanNamesForType(SessionRepositoryFilter.class, false, false); + Assert.state(targetBeanNames.length == 1, "Expected single SessionRepositoryFilter bean"); + DelegatingFilterProxyRegistrationBean registration = new DelegatingFilterProxyRegistrationBean( + targetBeanNames[0]); registration.setDispatcherTypes(getDispatcherTypes(sessionProperties)); registration.setOrder(sessionProperties.getServlet().getFilterOrder()); return registration; diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/session/SessionAutoConfigurationEarlyInitializationIntegrationTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/session/SessionAutoConfigurationEarlyInitializationIntegrationTests.java new file mode 100644 index 0000000000..6d449f83a4 --- /dev/null +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/session/SessionAutoConfigurationEarlyInitializationIntegrationTests.java @@ -0,0 +1,66 @@ +/* + * Copyright 2012-2023 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 + * + * https://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.session; + +import java.util.LinkedHashMap; + +import org.junit.jupiter.api.Test; + +import org.springframework.boot.autoconfigure.ImportAutoConfiguration; +import org.springframework.boot.autoconfigure.web.servlet.ServletWebServerFactoryAutoConfiguration; +import org.springframework.boot.test.context.runner.WebApplicationContextRunner; +import org.springframework.boot.web.servlet.context.AnnotationConfigServletWebServerApplicationContext; +import org.springframework.context.ConfigurableApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.session.MapSessionRepository; +import org.springframework.session.config.annotation.web.http.EnableSpringHttpSession; +import org.springframework.util.Assert; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Integration tests to ensure {@link SessionAutoConfiguration} and + * {@link SessionRepositoryFilterConfiguration} does not cause early initialization. + * + * @author Phillip Webb + */ +public class SessionAutoConfigurationEarlyInitializationIntegrationTests { + + @Test + void configurationIsFrozenWhenSessionRepositoryAccessed() { + new WebApplicationContextRunner(AnnotationConfigServletWebServerApplicationContext::new) + .withSystemProperties("spring.jndi.ignore=true") + .withPropertyValues("server.port=0") + .withUserConfiguration(TestConfiguration.class) + .run((context) -> assertThat(context).hasSingleBean(MapSessionRepository.class)); + } + + @Configuration(proxyBeanMethods = false) + @EnableSpringHttpSession + @ImportAutoConfiguration({ ServletWebServerFactoryAutoConfiguration.class, SessionAutoConfiguration.class }) + static class TestConfiguration { + + @Bean + MapSessionRepository mapSessionRepository(ConfigurableApplicationContext context) { + Assert.isTrue(context.getBeanFactory().isConfigurationFrozen(), "Context should be frozen"); + return new MapSessionRepository(new LinkedHashMap<>()); + } + + } + +} diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/session/SessionAutoConfigurationJdbcTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/session/SessionAutoConfigurationJdbcTests.java index 508e3ad3bf..3643eff117 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/session/SessionAutoConfigurationJdbcTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/session/SessionAutoConfigurationJdbcTests.java @@ -36,7 +36,7 @@ import org.springframework.boot.sql.init.DatabaseInitializationSettings; import org.springframework.boot.test.context.FilteredClassLoader; import org.springframework.boot.test.context.assertj.AssertableWebApplicationContext; import org.springframework.boot.test.context.runner.WebApplicationContextRunner; -import org.springframework.boot.web.servlet.FilterRegistrationBean; +import org.springframework.boot.web.servlet.AbstractFilterRegistrationBean; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Primary; @@ -100,7 +100,7 @@ class SessionAutoConfigurationJdbcTests extends AbstractSessionAutoConfiguration this.contextRunner .withPropertyValues("spring.session.store-type=jdbc", "spring.session.servlet.filter-order=123") .run((context) -> { - FilterRegistrationBean registration = context.getBean(FilterRegistrationBean.class); + AbstractFilterRegistrationBean registration = context.getBean(AbstractFilterRegistrationBean.class); assertThat(registration.getOrder()).isEqualTo(123); }); } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/session/SessionAutoConfigurationTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/session/SessionAutoConfigurationTests.java index 80e8b85db7..ec4a96f0e1 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/session/SessionAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/session/SessionAutoConfigurationTests.java @@ -28,7 +28,7 @@ import org.springframework.boot.autoconfigure.AutoConfigurations; import org.springframework.boot.autoconfigure.web.ServerProperties; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.boot.test.context.runner.WebApplicationContextRunner; -import org.springframework.boot.web.servlet.FilterRegistrationBean; +import org.springframework.boot.web.servlet.AbstractFilterRegistrationBean; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.core.annotation.Order; @@ -41,6 +41,7 @@ import org.springframework.session.web.http.DefaultCookieSerializer; import org.springframework.session.web.http.HeaderHttpSessionIdResolver; import org.springframework.session.web.http.HttpSessionIdResolver; import org.springframework.session.web.http.SessionRepositoryFilter; +import org.springframework.web.filter.DelegatingFilterProxy; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; @@ -99,8 +100,16 @@ class SessionAutoConfigurationTests extends AbstractSessionAutoConfigurationTest @Test void filterIsRegisteredWithAsyncErrorAndRequestDispatcherTypes() { this.contextRunner.withUserConfiguration(SessionRepositoryConfiguration.class).run((context) -> { - FilterRegistrationBean registration = context.getBean(FilterRegistrationBean.class); - assertThat(registration.getFilter()).isSameAs(context.getBean(SessionRepositoryFilter.class)); + AbstractFilterRegistrationBean registration = context.getBean(AbstractFilterRegistrationBean.class); + DelegatingFilterProxy delegatingFilterProxy = (DelegatingFilterProxy) registration.getFilter(); + try { + // Trigger actual initialization + delegatingFilterProxy.doFilter(null, null, null); + } + catch (Exception ex) { + } + assertThat(delegatingFilterProxy).extracting("delegate") + .isSameAs(context.getBean(SessionRepositoryFilter.class)); assertThat(registration) .extracting("dispatcherTypes", InstanceOfAssertFactories.iterable(DispatcherType.class)) .containsOnly(DispatcherType.ASYNC, DispatcherType.ERROR, DispatcherType.REQUEST); @@ -112,7 +121,7 @@ class SessionAutoConfigurationTests extends AbstractSessionAutoConfigurationTest this.contextRunner.withUserConfiguration(SessionRepositoryConfiguration.class) .withPropertyValues("spring.session.servlet.filter-order=123") .run((context) -> { - FilterRegistrationBean registration = context.getBean(FilterRegistrationBean.class); + AbstractFilterRegistrationBean registration = context.getBean(AbstractFilterRegistrationBean.class); assertThat(registration.getOrder()).isEqualTo(123); }); } @@ -122,7 +131,7 @@ class SessionAutoConfigurationTests extends AbstractSessionAutoConfigurationTest this.contextRunner.withUserConfiguration(SessionRepositoryConfiguration.class) .withPropertyValues("spring.session.servlet.filter-dispatcher-types=error, request") .run((context) -> { - FilterRegistrationBean registration = context.getBean(FilterRegistrationBean.class); + AbstractFilterRegistrationBean registration = context.getBean(AbstractFilterRegistrationBean.class); assertThat(registration) .extracting("dispatcherTypes", InstanceOfAssertFactories.iterable(DispatcherType.class)) .containsOnly(DispatcherType.ERROR, DispatcherType.REQUEST); @@ -134,7 +143,7 @@ class SessionAutoConfigurationTests extends AbstractSessionAutoConfigurationTest this.contextRunner.withUserConfiguration(SessionRepositoryConfiguration.class) .withPropertyValues("spring.session.servlet.filter-dispatcher-types=") .run((context) -> { - FilterRegistrationBean registration = context.getBean(FilterRegistrationBean.class); + AbstractFilterRegistrationBean registration = context.getBean(AbstractFilterRegistrationBean.class); assertThat(registration) .extracting("dispatcherTypes", InstanceOfAssertFactories.iterable(DispatcherType.class)) .isEmpty(); diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/FilterOrderingIntegrationTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/FilterOrderingIntegrationTests.java index e13e55b4a0..1a62aa822a 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/FilterOrderingIntegrationTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/FilterOrderingIntegrationTests.java @@ -16,8 +16,6 @@ package org.springframework.boot.autoconfigure.web.servlet; -import java.util.ArrayList; -import java.util.Iterator; import java.util.List; import java.util.concurrent.ConcurrentHashMap; @@ -44,7 +42,7 @@ import org.springframework.data.redis.connection.RedisConnectionFactory; import org.springframework.security.web.FilterChainProxy; import org.springframework.session.MapSessionRepository; import org.springframework.session.config.annotation.web.http.EnableSpringHttpSession; -import org.springframework.session.web.http.SessionRepositoryFilter; +import org.springframework.web.filter.DelegatingFilterProxy; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.BDDMockito.given; @@ -73,18 +71,19 @@ class FilterOrderingIntegrationTests { List registeredFilters = this.context.getBean(MockServletWebServerFactory.class) .getWebServer() .getRegisteredFilters(); - List filters = new ArrayList<>(registeredFilters.size()); - for (RegisteredFilter registeredFilter : registeredFilters) { - filters.add(registeredFilter.getFilter()); - } - Iterator iterator = filters.iterator(); - assertThat(iterator.next()).isInstanceOf(OrderedCharacterEncodingFilter.class); - assertThat(iterator.next()).isInstanceOf(SessionRepositoryFilter.class); - assertThat(iterator.next()).isInstanceOf(Filter.class); - assertThat(iterator.next()).isInstanceOf(Filter.class); - assertThat(iterator.next()).isInstanceOf(OrderedRequestContextFilter.class); - assertThat(iterator.next()).isInstanceOf(ErrorPageSecurityFilter.class); - assertThat(iterator.next()).isInstanceOf(FilterChainProxy.class); + assertThat(registeredFilters.get(0).getFilter()).isInstanceOf(OrderedCharacterEncodingFilter.class); + assertThat(registeredFilters.get(1).getFilter()).isInstanceOf(DelegatingFilterProxy.class) + .extracting("targetBeanName") + .isEqualTo("springSessionRepositoryFilter"); + assertThat(registeredFilters.get(2).getFilter()).isInstanceOf(Filter.class) + .extracting("beanName") + .isEqualTo("hiddenHttpMethodFilter"); + assertThat(registeredFilters.get(3).getFilter()).isInstanceOf(Filter.class) + .extracting("beanName") + .isEqualTo("formContentFilter"); + assertThat(registeredFilters.get(4).getFilter()).isInstanceOf(OrderedRequestContextFilter.class); + assertThat(registeredFilters.get(5).getFilter()).isInstanceOf(ErrorPageSecurityFilter.class); + assertThat(registeredFilters.get(6).getFilter()).isInstanceOf(FilterChainProxy.class); } private void load() {