From dfc362b8f8b09ed3801917abe96bba094ed12edb Mon Sep 17 00:00:00 2001 From: Moritz Halbritter Date: Mon, 14 Apr 2025 14:46:15 +0200 Subject: [PATCH] Align FilterRegistrationBean empty dispatcher type logic with FilterRegistration annotation Closes gh-45130 --- .../AbstractFilterRegistrationBean.java | 3 +- .../AbstractFilterRegistrationBeanTests.java | 64 +++++++++++++++++++ ...oSpringWebFilterRegistrationBeanTests.java | 7 +- 3 files changed, 72 insertions(+), 2 deletions(-) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/AbstractFilterRegistrationBean.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/AbstractFilterRegistrationBean.java index 0225c574ef8..2ca818bdfb1 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/AbstractFilterRegistrationBean.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/AbstractFilterRegistrationBean.java @@ -31,6 +31,7 @@ import jakarta.servlet.ServletContext; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; +import org.springframework.util.CollectionUtils; import org.springframework.util.StringUtils; import org.springframework.web.filter.OncePerRequestFilter; @@ -167,7 +168,7 @@ public abstract class AbstractFilterRegistrationBean extends D * @since 3.2.0 */ public EnumSet determineDispatcherTypes() { - if (this.dispatcherTypes == null) { + if (CollectionUtils.isEmpty(this.dispatcherTypes)) { T filter = getFilter(); if (ClassUtils.isPresent("org.springframework.web.filter.OncePerRequestFilter", filter.getClass().getClassLoader()) && filter instanceof OncePerRequestFilter) { diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/AbstractFilterRegistrationBeanTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/AbstractFilterRegistrationBeanTests.java index 6c6a2411af8..478da2a1dea 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/AbstractFilterRegistrationBeanTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/AbstractFilterRegistrationBeanTests.java @@ -25,14 +25,22 @@ import java.util.Map; import jakarta.servlet.DispatcherType; import jakarta.servlet.Filter; +import jakarta.servlet.FilterChain; import jakarta.servlet.FilterRegistration; import jakarta.servlet.ServletContext; import jakarta.servlet.ServletException; +import jakarta.servlet.ServletRequest; +import jakarta.servlet.ServletResponse; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.web.filter.OncePerRequestFilter; + +import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatCode; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.assertj.core.api.Assertions.assertThatIllegalStateException; @@ -47,6 +55,7 @@ import static org.mockito.Mockito.never; * Abstract base for {@link AbstractFilterRegistrationBean} tests. * * @author Phillip Webb + * @author Moritz Halbritter */ @ExtendWith(MockitoExtension.class) abstract class AbstractFilterRegistrationBeanTests { @@ -227,6 +236,42 @@ abstract class AbstractFilterRegistrationBeanTests { }).doesNotThrowAnyException(); } + @Test + void shouldDetermineDispatcherTypesIfNotSet() { + AbstractFilterRegistrationBean simpleFilter = new AbstractFilterRegistrationBean<>() { + @Override + public SimpleFilter getFilter() { + return new SimpleFilter(); + } + }; + assertThat(simpleFilter.determineDispatcherTypes()).containsExactly(DispatcherType.REQUEST); + } + + @Test + void shouldDetermineDispatcherTypesForOncePerRequestFilters() { + AbstractFilterRegistrationBean simpleFilter = new AbstractFilterRegistrationBean<>() { + @Override + public SimpleOncePerRequestFilter getFilter() { + return new SimpleOncePerRequestFilter(); + } + }; + assertThat(simpleFilter.determineDispatcherTypes()) + .containsExactlyInAnyOrderElementsOf(EnumSet.allOf(DispatcherType.class)); + } + + @Test + void shouldDetermineDispatcherTypesForSetDispatcherTypes() { + AbstractFilterRegistrationBean simpleFilter = new AbstractFilterRegistrationBean<>() { + @Override + public SimpleFilter getFilter() { + return new SimpleFilter(); + } + }; + simpleFilter.setDispatcherTypes(DispatcherType.INCLUDE, DispatcherType.FORWARD); + assertThat(simpleFilter.determineDispatcherTypes()).containsExactlyInAnyOrder(DispatcherType.INCLUDE, + DispatcherType.FORWARD); + } + protected abstract Filter getExpectedFilter(); protected abstract AbstractFilterRegistrationBean createFilterRegistrationBean( @@ -238,4 +283,23 @@ abstract class AbstractFilterRegistrationBeanTests { return bean; } + private static final class SimpleFilter implements Filter { + + @Override + public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) { + + } + + } + + private static final class SimpleOncePerRequestFilter extends OncePerRequestFilter { + + @Override + protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, + FilterChain filterChain) { + + } + + } + } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/NoSpringWebFilterRegistrationBeanTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/NoSpringWebFilterRegistrationBeanTests.java index 7aa6bbd21c1..d611ee652da 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/NoSpringWebFilterRegistrationBeanTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/NoSpringWebFilterRegistrationBeanTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2021 the original author or authors. + * Copyright 2012-2025 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. @@ -45,4 +45,9 @@ class NoSpringWebFilterRegistrationBeanTests extends AbstractFilterRegistrationB return eq(this.filter); } + @Override + void shouldDetermineDispatcherTypesForOncePerRequestFilters() { + // Disabled because OncePerRequestFilter isn't on the classpath + } + }