From 5a798e93f17f4e73982b2624671f786fbfebfc7e Mon Sep 17 00:00:00 2001 From: Josh Cummings Date: Mon, 3 Jun 2024 13:33:21 -0600 Subject: [PATCH 1/2] Polish MVC Tests Issue gh-14418 --- .../annotation/web/configurers/AuthorizeRequestsTests.java | 4 ++-- .../web/configurers/HttpSecuritySecurityMatchersTests.java | 4 ++-- .../web/configurers/UrlAuthorizationConfigurerTests.java | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/AuthorizeRequestsTests.java b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/AuthorizeRequestsTests.java index 926daa2907..e1339af459 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/AuthorizeRequestsTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/AuthorizeRequestsTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 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. @@ -76,7 +76,7 @@ public class AuthorizeRequestsTests { @BeforeEach public void setup() { this.servletContext = spy(MockServletContext.mvc()); - this.request = new MockHttpServletRequest("GET", ""); + this.request = new MockHttpServletRequest(this.servletContext, "GET", ""); this.request.setMethod("GET"); this.response = new MockHttpServletResponse(); this.chain = new MockFilterChain(); diff --git a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/HttpSecuritySecurityMatchersTests.java b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/HttpSecuritySecurityMatchersTests.java index 3bb278ede0..6360865b5f 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/HttpSecuritySecurityMatchersTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/HttpSecuritySecurityMatchersTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 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. @@ -75,7 +75,7 @@ public class HttpSecuritySecurityMatchersTests { @BeforeEach public void setup() throws Exception { - this.request = new MockHttpServletRequest("GET", ""); + this.request = new MockHttpServletRequest(MockServletContext.mvc(), "GET", ""); this.request.setMethod("GET"); this.response = new MockHttpServletResponse(); this.chain = new MockFilterChain(); diff --git a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/UrlAuthorizationConfigurerTests.java b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/UrlAuthorizationConfigurerTests.java index 4f02aa624b..24668c29cf 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/UrlAuthorizationConfigurerTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/UrlAuthorizationConfigurerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 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. @@ -72,7 +72,7 @@ public class UrlAuthorizationConfigurerTests { @BeforeEach public void setup() { - this.request = new MockHttpServletRequest("GET", ""); + this.request = new MockHttpServletRequest(MockServletContext.mvc(), "GET", ""); this.request.setMethod("GET"); this.response = new MockHttpServletResponse(); this.chain = new MockFilterChain(); From cdd626644e2316084f756686e248bf47f9ca0647 Mon Sep 17 00:00:00 2001 From: Josh Cummings Date: Fri, 31 May 2024 17:36:59 -0600 Subject: [PATCH 2/2] 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 | 51 +++++-------------- .../AbstractRequestMatcherRegistryTests.java | 10 ++-- 2 files changed, 19 insertions(+), 42 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 996a6741ee..8dc4cbae0b 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.Collection; 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 abstract class AbstractRequestMatcherRegistry { 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 @@ public abstract class AbstractRequestMatcherRegistry { static class DeferredRequestMatcher implements RequestMatcher { - final Function requestMatcherFactory; + final Function requestMatcherFactory; final AtomicReference description = new AtomicReference<>(); - volatile RequestMatcher requestMatcher; + final Map requestMatchers = new ConcurrentHashMap<>(); - 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; - }; + 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 0ebbdb5ba7..e5f96703ba 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 @@ public class AbstractRequestMatcherRegistryTests { 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);