NPE in FilterChainProxy.getFilters(String)

[lrozenblyum]

Describe the bug org.springframework.security.web.FilterChainProxy.getFilters(String) throws NPE in Spring Security 6.5.6. It used to work in Spring Security 5.8.12.

java.lang.NullPointerException
	at java.base/java.util.concurrent.ConcurrentHashMap.computeIfAbsent(ConcurrentHashMap.java:1693)
	at org.springframework.security.config.annotation.web.AbstractRequestMatcherRegistry$DeferredRequestMatcher.lambda$new$0(AbstractRequestMatcherRegistry.java:420)
	at org.springframework.security.config.annotation.web.AbstractRequestMatcherRegistry$DeferredRequestMatcher.matches(AbstractRequestMatcherRegistry.java:430)
	at org.springframework.security.web.DefaultSecurityFilterChain.matches(DefaultSecurityFilterChain.java:89)
	at org.springframework.security.web.FilterChainProxy.getFilters(FilterChainProxy.java:248)
	at org.springframework.security.web.FilterChainProxy.getFilters(FilterChainProxy.java:261)
	at org.springframework.security.config.annotation.web.configuration.WebSecurityConfiguration$CompositeFilterChainProxy.getFilters(WebSecurityConfiguration.java:324)
	at org.springframework.security.config.annotation.web.configuration.WebMvcSecurityConfiguration$CompositeFilterChainProxy.getFilters(WebMvcSecurityConfiguration.java:245)

To Reproduce Execute the method above with some argument like `/'

Expected behavior List of filters available should be loaded instead of NPE.

Sample

@Controller
public class MainController {

	@Autowired
	private FilterChainProxy filterChainProxy;
	@GetMapping("/")
	public void mainPage() {
		List<Filter> filters = filterChainProxy.getFilters("/"); // NPE here
	}

Same issue is easily reproduced in a @SpringBootTest.

[ngocnhan-tran1996]

I'm using Spring Boot 3.5.7 and Spring Security 6.5.6, it doesn't throw NPE. May I miss something?

spring-security-gh-18157.zip

[lrozenblyum]

@ngocnhan-tran1996 I see. In my environment 1 similar test is fine, while another one throws the NPE mentioned above. I'll try to prepare a minimal reproducible example

[lrozenblyum]

@ngocnhan-tran1996 Here it is

import static org.assertj.core.api.Assertions.assertThat;

import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.context.annotation.Bean;
import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
import org.springframework.security.config.annotation.web.configuration.WebSecurityCustomizer;
import org.springframework.security.web.FilterChainProxy;
import org.springframework.web.servlet.config.annotation.EnableWebMvc;

@SpringBootTest(classes = SpringSecurityGH18157Test.Config.class)
@EnableWebMvc
@EnableWebSecurity
class SpringSecurityGH18157Test {
	@Autowired
	private FilterChainProxy filterChainProxy;

	private static class Config {
		@Bean
		WebSecurityCustomizer configureWebSecurity() {
			return customizer -> customizer.ignoring().requestMatchers("/test**");
		}
	}

	@Test
	void canLoadFilters() {
		assertThat(filterChainProxy.getFilters("/")).isNotEmpty();
	}
}

Thanks to WebSecurityCustomizer a new instance of DefaultSecurityFilterChain is created, when it tries to match the request with NPE due to missing ServletContext in the HttpServletRequest (which is in turn caused by constructors of org.springframework.security.web.FilterInvocation.FilterInvocation(String, String) which pass null as ServletContext

[jzheaux]

Hi, @lrozenblyum, thanks for the reproducer.

I think that this method is a bit smelly since it claims that one can identify the filter chain only with the URI. In 7, we should deprecate it and anything that internally uses DummyRequest.

As for 6.x, let's update the JavaDoc to clarify the limitations of this method. Something like the following might work:

/**
 * Attempt to find the matching filter chain based on the given {@code url}.
 * Note that the URI is often not enough information and this method should
 * be used with caution. Instead, consider using Spring Security's testing support
 * that mocks a full HTTP request.
 * @param url the URL
 * @return matching filter list
 */

Also in this specific case, I'd recommend favoring permitAll in authorizeHttpRequests over ignoring in WebSecurity as detailed in the warning message that is logged whenever ignoring is used.

[lrozenblyum]

Thanks for the explanation @jzheaux. A couple of questions:

1. Should this method get protection against NPE's or so far so good?
2. By this sentence Instead, consider using Spring Security's testing support that mocks a full HTTP request. do you mean to actually execute the given request via sth like mockMvc and observe the results of filters execution? Or will it allow to get the actually filters list like the original method allowed?