From 9d50944cb2a5849ac46e115de3eb8290447a4d43 Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Tue, 5 Jul 2016 08:26:15 -0500 Subject: [PATCH] AntPathRequestMatcher implements RequestVariableExtractor Issue gh-3964 --- ...ariableEvaluationContextPostProcessor.java | 3 -- ...ilterInvocationSecurityMetadataSource.java | 14 ------- .../util/matcher/AntPathRequestMatcher.java | 41 +++++++++---------- ...leEvaluationContextPostProcessorTests.java | 12 ------ .../matcher/AntPathRequestMatcherTests.java | 15 ------- 5 files changed, 19 insertions(+), 66 deletions(-) diff --git a/web/src/main/java/org/springframework/security/web/access/expression/AbstractVariableEvaluationContextPostProcessor.java b/web/src/main/java/org/springframework/security/web/access/expression/AbstractVariableEvaluationContextPostProcessor.java index f0f70e0cfb..89016238ea 100644 --- a/web/src/main/java/org/springframework/security/web/access/expression/AbstractVariableEvaluationContextPostProcessor.java +++ b/web/src/main/java/org/springframework/security/web/access/expression/AbstractVariableEvaluationContextPostProcessor.java @@ -54,7 +54,6 @@ abstract class AbstractVariableEvaluationContextPostProcessor if (this.variables == null) { this.variables = extractVariables(request); } - name = postProcessVariableName(name); return this.variables.get(name); } @@ -63,6 +62,4 @@ abstract class AbstractVariableEvaluationContextPostProcessor abstract Map extractVariables(HttpServletRequest request); - abstract String postProcessVariableName(String variableName); - } diff --git a/web/src/main/java/org/springframework/security/web/access/expression/ExpressionBasedFilterInvocationSecurityMetadataSource.java b/web/src/main/java/org/springframework/security/web/access/expression/ExpressionBasedFilterInvocationSecurityMetadataSource.java index d988a14ef9..c38bb09b1e 100644 --- a/web/src/main/java/org/springframework/security/web/access/expression/ExpressionBasedFilterInvocationSecurityMetadataSource.java +++ b/web/src/main/java/org/springframework/security/web/access/expression/ExpressionBasedFilterInvocationSecurityMetadataSource.java @@ -93,10 +93,6 @@ public final class ExpressionBasedFilterInvocationSecurityMetadataSource private static AbstractVariableEvaluationContextPostProcessor createPostProcessor( Object request) { - if (request instanceof AntPathRequestMatcher) { - return new AntPathMatcherEvaluationContextPostProcessor( - (AntPathRequestMatcher) request); - } if (request instanceof RequestVariablesExtractor) { return new RequestVariablesExtractorEvaluationContextPostProcessor( (RequestVariablesExtractor) request); @@ -117,11 +113,6 @@ public final class ExpressionBasedFilterInvocationSecurityMetadataSource Map extractVariables(HttpServletRequest request) { return this.matcher.extractUriTemplateVariables(request); } - - @Override - String postProcessVariableName(String variableName) { - return this.matcher.postProcessVariableName(variableName); - } } static class RequestVariablesExtractorEvaluationContextPostProcessor @@ -137,11 +128,6 @@ public final class ExpressionBasedFilterInvocationSecurityMetadataSource Map extractVariables(HttpServletRequest request) { return this.matcher.extractUriTemplateVariables(request); } - - @Override - String postProcessVariableName(String variableName) { - return variableName; - } } } diff --git a/web/src/main/java/org/springframework/security/web/util/matcher/AntPathRequestMatcher.java b/web/src/main/java/org/springframework/security/web/util/matcher/AntPathRequestMatcher.java index 17536b892e..b1b0659135 100644 --- a/web/src/main/java/org/springframework/security/web/util/matcher/AntPathRequestMatcher.java +++ b/web/src/main/java/org/springframework/security/web/util/matcher/AntPathRequestMatcher.java @@ -52,7 +52,8 @@ import org.springframework.util.StringUtils; * * @see org.springframework.util.AntPathMatcher */ -public final class AntPathRequestMatcher implements RequestMatcher { +public final class AntPathRequestMatcher + implements RequestMatcher, RequestVariablesExtractor { private static final Log logger = LogFactory.getLog(AntPathRequestMatcher.class); private static final String MATCH_ALL = "/**"; @@ -102,10 +103,6 @@ public final class AntPathRequestMatcher implements RequestMatcher { this.matcher = null; } else { - if (!caseSensitive) { - pattern = pattern.toLowerCase(); - } - // If the pattern ends with {@code /**} and has no other wildcards or path // variables, then optimize to a sub-path match if (pattern.endsWith(MATCH_ALL) @@ -113,10 +110,10 @@ public final class AntPathRequestMatcher implements RequestMatcher { && pattern.indexOf('}') == -1) && pattern.indexOf("*") == pattern.length() - 2) { this.matcher = new SubpathMatcher( - pattern.substring(0, pattern.length() - 3)); + pattern.substring(0, pattern.length() - 3), caseSensitive); } else { - this.matcher = new SpringAntMatcher(pattern); + this.matcher = new SpringAntMatcher(pattern, caseSensitive); } } @@ -164,6 +161,7 @@ public final class AntPathRequestMatcher implements RequestMatcher { return this.matcher.matches(url); } + @Override public Map extractUriTemplateVariables(HttpServletRequest request) { if (this.matcher == null || !matches(request)) { return Collections.emptyMap(); @@ -172,10 +170,6 @@ public final class AntPathRequestMatcher implements RequestMatcher { return this.matcher.extractUriTemplateVariables(url); } - public String postProcessVariableName(String variableName) { - return this.caseSensitive ? variableName : variableName.toLowerCase(); - } - private String getRequestPath(HttpServletRequest request) { String url = request.getServletPath(); @@ -183,10 +177,6 @@ public final class AntPathRequestMatcher implements RequestMatcher { url += request.getPathInfo(); } - if (!this.caseSensitive) { - url = url.toLowerCase(); - } - return url; } @@ -253,27 +243,29 @@ public final class AntPathRequestMatcher implements RequestMatcher { } private static class SpringAntMatcher implements Matcher { - private static final AntPathMatcher antMatcher = createMatcher(); + private final AntPathMatcher antMatcher; private final String pattern; - private SpringAntMatcher(String pattern) { + private SpringAntMatcher(String pattern, boolean caseSensitive) { this.pattern = pattern; + this.antMatcher = createMatcher(caseSensitive); } @Override public boolean matches(String path) { - return antMatcher.match(this.pattern, path); + return this.antMatcher.match(this.pattern, path); } @Override public Map extractUriTemplateVariables(String path) { - return antMatcher.extractUriTemplateVariables(this.pattern, path); + return this.antMatcher.extractUriTemplateVariables(this.pattern, path); } - private static AntPathMatcher createMatcher() { + private static AntPathMatcher createMatcher(boolean caseSensitive) { AntPathMatcher matcher = new AntPathMatcher(); matcher.setTrimTokens(false); + matcher.setCaseSensitive(caseSensitive); return matcher; } } @@ -284,15 +276,20 @@ public final class AntPathRequestMatcher implements RequestMatcher { private static class SubpathMatcher implements Matcher { private final String subpath; private final int length; + private final boolean caseSensitive; - private SubpathMatcher(String subpath) { + private SubpathMatcher(String subpath, boolean caseSensitive) { assert!subpath.contains("*"); - this.subpath = subpath; + this.subpath = caseSensitive ? subpath : subpath.toLowerCase(); this.length = subpath.length(); + this.caseSensitive = caseSensitive; } @Override public boolean matches(String path) { + if (!this.caseSensitive) { + path = path.toLowerCase(); + } return path.startsWith(this.subpath) && (path.length() == this.length || path.charAt(this.length) == '/'); } diff --git a/web/src/test/java/org/springframework/security/web/access/expression/AbstractVariableEvaluationContextPostProcessorTests.java b/web/src/test/java/org/springframework/security/web/access/expression/AbstractVariableEvaluationContextPostProcessorTests.java index b7eda64420..f737506f07 100644 --- a/web/src/test/java/org/springframework/security/web/access/expression/AbstractVariableEvaluationContextPostProcessorTests.java +++ b/web/src/test/java/org/springframework/security/web/access/expression/AbstractVariableEvaluationContextPostProcessorTests.java @@ -65,13 +65,6 @@ public class AbstractVariableEvaluationContextPostProcessorTests { assertThat(this.context.lookupVariable(KEY)).isEqualTo(VALUE); } - @Test - public void postProcessVariableName() { - this.context = this.processor.postProcess(this.context, this.invocation); - - assertThat(this.context.lookupVariable("nothing")).isEqualTo(VALUE); - } - @Test public void extractVariablesOnlyUsedOnce() { this.context = this.processor.postProcess(this.context, this.invocation); @@ -89,10 +82,5 @@ public class AbstractVariableEvaluationContextPostProcessorTests { protected Map extractVariables(HttpServletRequest request) { return this.results; } - - @Override - String postProcessVariableName(String variableName) { - return KEY; - } } } diff --git a/web/src/test/java/org/springframework/security/web/util/matcher/AntPathRequestMatcherTests.java b/web/src/test/java/org/springframework/security/web/util/matcher/AntPathRequestMatcherTests.java index edbb1c9f21..3d07612d5d 100644 --- a/web/src/test/java/org/springframework/security/web/util/matcher/AntPathRequestMatcherTests.java +++ b/web/src/test/java/org/springframework/security/web/util/matcher/AntPathRequestMatcherTests.java @@ -210,21 +210,6 @@ public class AntPathRequestMatcherTests { assertThat(matcher.matches(request)).isFalse(); } - @Test - public void postProcessVariableNameCaseInsensitive() { - AntPathRequestMatcher matcher = new AntPathRequestMatcher("/**", null, false); - String variableName = "userName"; - assertThat(matcher.postProcessVariableName(variableName)) - .isEqualTo(variableName.toLowerCase()); - } - - @Test - public void postProcessVariableNameCaseSensitive() { - AntPathRequestMatcher matcher = new AntPathRequestMatcher("/**", null, true); - String variableName = "userName"; - assertThat(matcher.postProcessVariableName(variableName)).isEqualTo(variableName); - } - private HttpServletRequest createRequestWithNullMethod(String path) { when(this.request.getQueryString()).thenReturn("doesntMatter"); when(this.request.getServletPath()).thenReturn(path);