From cdd626644e2316084f756686e248bf47f9ca0647 Mon Sep 17 00:00:00 2001 From: Josh Cummings Date: Fri, 31 May 2024 17:36:59 -0600 Subject: [PATCH] Use Request-Level Servlet Context Spring Security cannot use the ServletContext attached to the ApplicationContext since there may be child ApplicationContext's with their own ServletContext. Because of that, it is necessary to always use the ServletContext attached to the request. Closes gh-14418 --- .../web/AbstractRequestMatcherRegistry.java | 53 +++++-------------- .../AbstractRequestMatcherRegistryTests.java | 10 ++-- 2 files changed, 20 insertions(+), 43 deletions(-) diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/AbstractRequestMatcherRegistry.java b/config/src/main/java/org/springframework/security/config/annotation/web/AbstractRequestMatcherRegistry.java index 996a6741ee6..8dc4cbae0bd 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/AbstractRequestMatcherRegistry.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/AbstractRequestMatcherRegistry.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2023 the original author or authors. + * Copyright 2002-2024 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. @@ -22,6 +22,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Function; @@ -322,36 +323,15 @@ public C requestMatchers(HttpMethod method, String... patterns) { if (servletContext == null) { return requestMatchers(RequestMatchers.antMatchersAsArray(method, patterns)); } - boolean isProgrammaticApiAvailable = isProgrammaticApiAvailable(servletContext); List matchers = new ArrayList<>(); for (String pattern : patterns) { AntPathRequestMatcher ant = new AntPathRequestMatcher(pattern, (method != null) ? method.name() : null); MvcRequestMatcher mvc = createMvcMatchers(method, pattern).get(0); - if (isProgrammaticApiAvailable) { - matchers.add(resolve(ant, mvc, servletContext)); - } - else { - this.logger - .warn("The ServletRegistration API was not available at startup time. This may be due to a misconfiguration; " - + "if you are using AbstractSecurityWebApplicationInitializer, please double-check the recommendations outlined in " - + "https://docs.spring.io/spring-security/reference/servlet/configuration/java.html#abstractsecuritywebapplicationinitializer-with-spring-mvc"); - matchers.add(new DeferredRequestMatcher((request) -> resolve(ant, mvc, request.getServletContext()), - mvc, ant)); - } + matchers.add(new DeferredRequestMatcher((c) -> resolve(ant, mvc, c), mvc, ant)); } return requestMatchers(matchers.toArray(new RequestMatcher[0])); } - private static boolean isProgrammaticApiAvailable(ServletContext servletContext) { - try { - servletContext.getServletRegistrations(); - return true; - } - catch (UnsupportedOperationException ex) { - return false; - } - } - private RequestMatcher resolve(AntPathRequestMatcher ant, MvcRequestMatcher mvc, ServletContext servletContext) { Map registrations = mappableServletRegistrations(servletContext); if (registrations.isEmpty()) { @@ -593,34 +573,29 @@ static List regexMatchers(String... regexPatterns) { static class DeferredRequestMatcher implements RequestMatcher { - final Function requestMatcherFactory; + final Function requestMatcherFactory; final AtomicReference description = new AtomicReference<>(); - volatile RequestMatcher requestMatcher; - - DeferredRequestMatcher(Function resolver, RequestMatcher... candidates) { - this.requestMatcherFactory = (request) -> { - if (this.requestMatcher == null) { - synchronized (this) { - if (this.requestMatcher == null) { - this.requestMatcher = resolver.apply(request); - } - } - } - return this.requestMatcher; - }; + final Map requestMatchers = new ConcurrentHashMap<>(); + + DeferredRequestMatcher(Function resolver, RequestMatcher... candidates) { + this.requestMatcherFactory = (sc) -> this.requestMatchers.computeIfAbsent(sc, resolver); this.description.set("Deferred " + Arrays.toString(candidates)); } + RequestMatcher requestMatcher(ServletContext servletContext) { + return this.requestMatcherFactory.apply(servletContext); + } + @Override public boolean matches(HttpServletRequest request) { - return this.requestMatcherFactory.apply(request).matches(request); + return this.requestMatcherFactory.apply(request.getServletContext()).matches(request); } @Override public MatchResult matcher(HttpServletRequest request) { - return this.requestMatcherFactory.apply(request).matcher(request); + return this.requestMatcherFactory.apply(request.getServletContext()).matcher(request); } @Override diff --git a/config/src/test/java/org/springframework/security/config/annotation/web/AbstractRequestMatcherRegistryTests.java b/config/src/test/java/org/springframework/security/config/annotation/web/AbstractRequestMatcherRegistryTests.java index 0ebbdb5ba7f..e5f96703ba7 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/web/AbstractRequestMatcherRegistryTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/web/AbstractRequestMatcherRegistryTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2023 the original author or authors. + * Copyright 2002-2024 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. @@ -381,11 +381,13 @@ protected List chainRequestMatchers(List request return requestMatchers; } - private static List unwrap(List wrappedMatchers) { + private List unwrap(List wrappedMatchers) { List requestMatchers = new ArrayList<>(); for (RequestMatcher requestMatcher : wrappedMatchers) { - if (requestMatcher instanceof AbstractRequestMatcherRegistry.DeferredRequestMatcher) { - requestMatchers.add(((DeferredRequestMatcher) requestMatcher).requestMatcher); + if (requestMatcher instanceof DeferredRequestMatcher) { + DeferredRequestMatcher deferred = (DeferredRequestMatcher) requestMatcher; + WebApplicationContext web = (WebApplicationContext) getApplicationContext(); + requestMatchers.add(deferred.requestMatcher(web.getServletContext())); } else { requestMatchers.add(requestMatcher);