Browse Source

Polish ServletRegistration API Deferral

Tomcat uses different ServletContext instances from startup- and request-time.
This commit ensures that if the programmatic API isn't available at startup-time,
then use the ServletContext attached to the HttpServletRequest at runtime.

Issue gh-13794
Josh Cummings 1 year ago
parent
commit
a98baa7522

+ 19 - 12
config/src/main/java/org/springframework/security/config/annotation/web/AbstractRequestMatcherRegistry.java

@@ -23,7 +23,7 @@ import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.atomic.AtomicReference;
-import java.util.function.Supplier;
+import java.util.function.Function;
 
 import javax.servlet.DispatcherType;
 import javax.servlet.ServletContext;
@@ -44,7 +44,6 @@ import org.springframework.security.web.util.matcher.RegexRequestMatcher;
 import org.springframework.security.web.util.matcher.RequestMatcher;
 import org.springframework.util.Assert;
 import org.springframework.util.ClassUtils;
-import org.springframework.util.function.SingletonSupplier;
 import org.springframework.web.context.WebApplicationContext;
 import org.springframework.web.servlet.handler.HandlerMappingIntrospector;
 
@@ -327,7 +326,8 @@ public abstract class AbstractRequestMatcherRegistry<C> {
 				matchers.add(resolve(ant, mvc, servletContext));
 			}
 			else {
-				matchers.add(new DeferredRequestMatcher(() -> resolve(ant, mvc, servletContext), mvc, ant));
+				matchers.add(new DeferredRequestMatcher((request) -> resolve(ant, mvc, request.getServletContext()),
+						mvc, ant));
 			}
 		}
 		return requestMatchers(matchers.toArray(new RequestMatcher[0]));
@@ -584,27 +584,34 @@ public abstract class AbstractRequestMatcherRegistry<C> {
 
 	static class DeferredRequestMatcher implements RequestMatcher {
 
-		final Supplier<RequestMatcher> requestMatcher;
+		final Function<HttpServletRequest, RequestMatcher> requestMatcherFactory;
 
 		final AtomicReference<String> description = new AtomicReference<>();
 
-		DeferredRequestMatcher(Supplier<RequestMatcher> resolver, RequestMatcher... candidates) {
-			this.requestMatcher = SingletonSupplier.of(() -> {
-				RequestMatcher matcher = resolver.get();
-				this.description.set(matcher.toString());
-				return matcher;
-			});
+		volatile RequestMatcher requestMatcher;
+
+		DeferredRequestMatcher(Function<HttpServletRequest, RequestMatcher> resolver, RequestMatcher... candidates) {
+			this.requestMatcherFactory = (request) -> {
+				if (this.requestMatcher == null) {
+					synchronized (this) {
+						if (this.requestMatcher == null) {
+							this.requestMatcher = resolver.apply(request);
+						}
+					}
+				}
+				return this.requestMatcher;
+			};
 			this.description.set("Deferred " + Arrays.toString(candidates));
 		}
 
 		@Override
 		public boolean matches(HttpServletRequest request) {
-			return this.requestMatcher.get().matches(request);
+			return this.requestMatcherFactory.apply(request).matches(request);
 		}
 
 		@Override
 		public MatchResult matcher(HttpServletRequest request) {
-			return this.requestMatcher.get().matcher(request);
+			return this.requestMatcherFactory.apply(request).matcher(request);
 		}
 
 		@Override

+ 1 - 1
config/src/test/java/org/springframework/security/config/annotation/web/AbstractRequestMatcherRegistryTests.java

@@ -385,7 +385,7 @@ public class AbstractRequestMatcherRegistryTests {
 			List<RequestMatcher> requestMatchers = new ArrayList<>();
 			for (RequestMatcher requestMatcher : wrappedMatchers) {
 				if (requestMatcher instanceof AbstractRequestMatcherRegistry.DeferredRequestMatcher) {
-					requestMatchers.add(((DeferredRequestMatcher) requestMatcher).requestMatcher.get());
+					requestMatchers.add(((DeferredRequestMatcher) requestMatcher).requestMatcher);
 				}
 				else {
 					requestMatchers.add(requestMatcher);