Przeglądaj źródła

Cache RequestPath

In this way PathPatternRequestMatcher won't need to reparse for each
request matcher.

Issue gh-16771
Josh Cummings 5 miesięcy temu
rodzic
commit
3d96878d43

+ 32 - 1
web/src/main/java/org/springframework/security/web/access/RequestMatcherDelegatingWebInvocationPrivilegeEvaluator.java

@@ -17,16 +17,20 @@
 package org.springframework.security.web.access;
 
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 
 import jakarta.servlet.ServletContext;
 import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletRequestWrapper;
 
 import org.springframework.security.core.Authentication;
 import org.springframework.security.web.FilterInvocation;
 import org.springframework.security.web.util.matcher.RequestMatcherEntry;
 import org.springframework.util.Assert;
 import org.springframework.web.context.ServletContextAware;
+import org.springframework.web.util.ServletRequestPathUtils;
 
 /**
  * A {@link WebInvocationPrivilegeEvaluator} which delegates to a list of
@@ -116,8 +120,10 @@ public final class RequestMatcherDelegatingWebInvocationPrivilegeEvaluator
 
 	private List<WebInvocationPrivilegeEvaluator> getDelegate(String contextPath, String uri, String method) {
 		FilterInvocation filterInvocation = new FilterInvocation(contextPath, uri, method, this.servletContext);
+		HttpServletRequest request = new AttributesSupportingHttpServletRequest(filterInvocation.getHttpRequest());
+		ServletRequestPathUtils.parseAndCache(request);
 		for (RequestMatcherEntry<List<WebInvocationPrivilegeEvaluator>> delegate : this.delegates) {
-			if (delegate.getRequestMatcher().matches(filterInvocation.getHttpRequest())) {
+			if (delegate.getRequestMatcher().matches(request)) {
 				return delegate.getEntry();
 			}
 		}
@@ -129,4 +135,29 @@ public final class RequestMatcherDelegatingWebInvocationPrivilegeEvaluator
 		this.servletContext = servletContext;
 	}
 
+	private static final class AttributesSupportingHttpServletRequest extends HttpServletRequestWrapper {
+
+		private final Map<String, Object> attributes = new HashMap<>();
+
+		AttributesSupportingHttpServletRequest(HttpServletRequest request) {
+			super(request);
+		}
+
+		@Override
+		public Object getAttribute(String name) {
+			return this.attributes.get(name);
+		}
+
+		@Override
+		public void setAttribute(String name, Object value) {
+			this.attributes.put(name, value);
+		}
+
+		@Override
+		public void removeAttribute(String name) {
+			this.attributes.remove(name);
+		}
+
+	}
+
 }

+ 23 - 0
web/src/test/java/org/springframework/security/web/access/RequestMatcherDelegatingWebInvocationPrivilegeEvaluatorTests.java

@@ -24,12 +24,16 @@ import jakarta.servlet.http.HttpServletRequest;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.mockito.ArgumentCaptor;
+import org.mockito.MockedStatic;
+import org.mockito.Mockito;
 
 import org.springframework.mock.web.MockServletContext;
 import org.springframework.security.authentication.TestingAuthenticationToken;
 import org.springframework.security.core.Authentication;
+import org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher;
 import org.springframework.security.web.util.matcher.RequestMatcher;
 import org.springframework.security.web.util.matcher.RequestMatcherEntry;
+import org.springframework.web.util.ServletRequestPathUtils;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
@@ -198,4 +202,23 @@ class RequestMatcherDelegatingWebInvocationPrivilegeEvaluatorTests {
 			.withMessageContaining("requestMatcher cannot be null");
 	}
 
+	// gh-16771
+	@Test
+	void isAllowedWhenInvokesDelegateThenCachesRequestPath() {
+		PathPatternRequestMatcher path = PathPatternRequestMatcher.withDefaults().matcher("/path/**");
+		PathPatternRequestMatcher any = PathPatternRequestMatcher.withDefaults().matcher("/**");
+		WebInvocationPrivilegeEvaluator delegating = new RequestMatcherDelegatingWebInvocationPrivilegeEvaluator(
+				List.of(deny(path), deny(any)));
+		try (MockedStatic<ServletRequestPathUtils> utils = Mockito.mockStatic(ServletRequestPathUtils.class,
+				Mockito.CALLS_REAL_METHODS)) {
+			delegating.isAllowed("/uri", null);
+			utils.verify(() -> ServletRequestPathUtils.parseAndCache(any()), times(1));
+		}
+	}
+
+	private RequestMatcherEntry<List<WebInvocationPrivilegeEvaluator>> deny(RequestMatcher requestMatcher) {
+		return new RequestMatcherEntry<>(requestMatcher,
+				Collections.singletonList(TestWebInvocationPrivilegeEvaluator.alwaysDeny()));
+	}
+
 }