Skip to content

Commit 3d96878

Browse files
committed
Cache RequestPath
In this way PathPatternRequestMatcher won't need to reparse for each request matcher. Issue gh-16771
1 parent 2a275b1 commit 3d96878

File tree

2 files changed

+55
-1
lines changed

2 files changed

+55
-1
lines changed

web/src/main/java/org/springframework/security/web/access/RequestMatcherDelegatingWebInvocationPrivilegeEvaluator.java

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,16 +17,20 @@
1717
package org.springframework.security.web.access;
1818

1919
import java.util.Collections;
20+
import java.util.HashMap;
2021
import java.util.List;
22+
import java.util.Map;
2123

2224
import jakarta.servlet.ServletContext;
2325
import jakarta.servlet.http.HttpServletRequest;
26+
import jakarta.servlet.http.HttpServletRequestWrapper;
2427

2528
import org.springframework.security.core.Authentication;
2629
import org.springframework.security.web.FilterInvocation;
2730
import org.springframework.security.web.util.matcher.RequestMatcherEntry;
2831
import org.springframework.util.Assert;
2932
import org.springframework.web.context.ServletContextAware;
33+
import org.springframework.web.util.ServletRequestPathUtils;
3034

3135
/**
3236
* A {@link WebInvocationPrivilegeEvaluator} which delegates to a list of
@@ -116,8 +120,10 @@ public boolean isAllowed(String contextPath, String uri, String method, Authenti
116120

117121
private List<WebInvocationPrivilegeEvaluator> getDelegate(String contextPath, String uri, String method) {
118122
FilterInvocation filterInvocation = new FilterInvocation(contextPath, uri, method, this.servletContext);
123+
HttpServletRequest request = new AttributesSupportingHttpServletRequest(filterInvocation.getHttpRequest());
124+
ServletRequestPathUtils.parseAndCache(request);
119125
for (RequestMatcherEntry<List<WebInvocationPrivilegeEvaluator>> delegate : this.delegates) {
120-
if (delegate.getRequestMatcher().matches(filterInvocation.getHttpRequest())) {
126+
if (delegate.getRequestMatcher().matches(request)) {
121127
return delegate.getEntry();
122128
}
123129
}
@@ -129,4 +135,29 @@ public void setServletContext(ServletContext servletContext) {
129135
this.servletContext = servletContext;
130136
}
131137

138+
private static final class AttributesSupportingHttpServletRequest extends HttpServletRequestWrapper {
139+
140+
private final Map<String, Object> attributes = new HashMap<>();
141+
142+
AttributesSupportingHttpServletRequest(HttpServletRequest request) {
143+
super(request);
144+
}
145+
146+
@Override
147+
public Object getAttribute(String name) {
148+
return this.attributes.get(name);
149+
}
150+
151+
@Override
152+
public void setAttribute(String name, Object value) {
153+
this.attributes.put(name, value);
154+
}
155+
156+
@Override
157+
public void removeAttribute(String name) {
158+
this.attributes.remove(name);
159+
}
160+
161+
}
162+
132163
}

web/src/test/java/org/springframework/security/web/access/RequestMatcherDelegatingWebInvocationPrivilegeEvaluatorTests.java

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,16 @@
2424
import org.junit.jupiter.api.BeforeEach;
2525
import org.junit.jupiter.api.Test;
2626
import org.mockito.ArgumentCaptor;
27+
import org.mockito.MockedStatic;
28+
import org.mockito.Mockito;
2729

2830
import org.springframework.mock.web.MockServletContext;
2931
import org.springframework.security.authentication.TestingAuthenticationToken;
3032
import org.springframework.security.core.Authentication;
33+
import org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher;
3134
import org.springframework.security.web.util.matcher.RequestMatcher;
3235
import org.springframework.security.web.util.matcher.RequestMatcherEntry;
36+
import org.springframework.web.util.ServletRequestPathUtils;
3337

3438
import static org.assertj.core.api.Assertions.assertThat;
3539
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
@@ -198,4 +202,23 @@ void constructorWhenRequestMatcherNullThenException() {
198202
.withMessageContaining("requestMatcher cannot be null");
199203
}
200204

205+
// gh-16771
206+
@Test
207+
void isAllowedWhenInvokesDelegateThenCachesRequestPath() {
208+
PathPatternRequestMatcher path = PathPatternRequestMatcher.withDefaults().matcher("/path/**");
209+
PathPatternRequestMatcher any = PathPatternRequestMatcher.withDefaults().matcher("/**");
210+
WebInvocationPrivilegeEvaluator delegating = new RequestMatcherDelegatingWebInvocationPrivilegeEvaluator(
211+
List.of(deny(path), deny(any)));
212+
try (MockedStatic<ServletRequestPathUtils> utils = Mockito.mockStatic(ServletRequestPathUtils.class,
213+
Mockito.CALLS_REAL_METHODS)) {
214+
delegating.isAllowed("/uri", null);
215+
utils.verify(() -> ServletRequestPathUtils.parseAndCache(any()), times(1));
216+
}
217+
}
218+
219+
private RequestMatcherEntry<List<WebInvocationPrivilegeEvaluator>> deny(RequestMatcher requestMatcher) {
220+
return new RequestMatcherEntry<>(requestMatcher,
221+
Collections.singletonList(TestWebInvocationPrivilegeEvaluator.alwaysDeny()));
222+
}
223+
201224
}

0 commit comments

Comments
 (0)