Path Variables fail with different case

Fixes gh-3329

web/src/main/java/org/springframework/security/web/access/expression/AbstractVariableEvaluationContextPostProcessor.java

@@ -0,0 +1,55 @@
+/*
+ * Copyright 2002-2015 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.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.springframework.security.web.access.expression;
+
+import java.util.Map;
+
+import javax.servlet.http.HttpServletRequest;
+
+import org.springframework.expression.EvaluationContext;
+import org.springframework.security.web.FilterInvocation;
+
+/**
+ * Exposes URI template variables as variables on the {@link EvaluationContext}. For
+ * example, the pattern "/user/{username}/**" would expose a variable named username based
+ * on the current URI.
+ *
+ * <p>
+ * NOTE: This API is intentionally kept package scope as it may change in the future. It
+ * may be nice to allow users to augment expressions and queries
+ * </p>
+ *
+ * @author Rob Winch
+ * @since 4.1
+ */
+abstract class AbstractVariableEvaluationContextPostProcessor
+		implements EvaluationContextPostProcessor<FilterInvocation> {
+
+	@Override
+	public final EvaluationContext postProcess(EvaluationContext context,
+			FilterInvocation invocation) {
+		HttpServletRequest request = invocation.getHttpRequest();
+		Map<String, String> variables = extractVariables(request);
+		for (Map.Entry<String, String> entry : variables.entrySet()) {
+			context.setVariable(entry.getKey(), entry.getValue());
+		}
+		return context;
+	}
+
+	protected abstract Map<String, String> extractVariables(
+			HttpServletRequest request);
+
+}

web/src/main/java/org/springframework/security/web/access/expression/EvaluationContextPostProcessor.java

@@ -29,7 +29,7 @@ import org.springframework.expression.EvaluationContext;
  * @since 4.1
  * @param <I> the invocation to use for post processing
  */
-interface SecurityEvaluationContextPostProcessor<I> {
+interface EvaluationContextPostProcessor<I> {
 
 	/**
 	 * Allows post processing of the {@link EvaluationContext}. Implementations

web/src/main/java/org/springframework/security/web/access/expression/ExpressionBasedFilterInvocationSecurityMetadataSource.java

@@ -20,8 +20,11 @@ import java.util.Collection;
 import java.util.LinkedHashMap;
 import java.util.Map;
 
+import javax.servlet.http.HttpServletRequest;
+
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+
 import org.springframework.expression.ExpressionParser;
 import org.springframework.expression.ParseException;
 import org.springframework.security.access.ConfigAttribute;
@@ -38,8 +41,8 @@ import org.springframework.util.Assert;
  * @author Luke Taylor
  * @since 3.0
  */
-public final class ExpressionBasedFilterInvocationSecurityMetadataSource extends
-		DefaultFilterInvocationSecurityMetadataSource {
+public final class ExpressionBasedFilterInvocationSecurityMetadataSource
+		extends DefaultFilterInvocationSecurityMetadataSource {
 	private final static Log logger = LogFactory
 			.getLog(ExpressionBasedFilterInvocationSecurityMetadataSource.class);
 
@@ -67,20 +70,18 @@ public final class ExpressionBasedFilterInvocationSecurityMetadataSource extends
 			ArrayList<ConfigAttribute> attributes = new ArrayList<ConfigAttribute>(1);
 			String expression = entry.getValue().toArray(new ConfigAttribute[1])[0]
 					.getAttribute();
-			logger.debug("Adding web access control expression '" + expression
-					+ "', for " + request);
+			logger.debug("Adding web access control expression '" + expression + "', for "
+					+ request);
 
-			String pattern = null;
-			if(request instanceof AntPathRequestMatcher) {
-				pattern = ((AntPathRequestMatcher)request).getPattern();
-			}
+			AbstractVariableEvaluationContextPostProcessor postProcessor = createPostProcessor(
+					request);
 			try {
-				attributes.add(new WebExpressionConfigAttribute(parser
-						.parseExpression(expression), new PathVariableSecurityEvaluationContextPostProcessor(pattern)));
+				attributes.add(new WebExpressionConfigAttribute(
+						parser.parseExpression(expression), postProcessor));
 			}
 			catch (ParseException e) {
-				throw new IllegalArgumentException("Failed to parse expression '"
-						+ expression + "'");
+				throw new IllegalArgumentException(
+						"Failed to parse expression '" + expression + "'");
 			}
 
 			requestToExpressionAttributesMap.put(request, attributes);
@@ -89,4 +90,29 @@ public final class ExpressionBasedFilterInvocationSecurityMetadataSource extends
 		return requestToExpressionAttributesMap;
 	}
 
+	private static AbstractVariableEvaluationContextPostProcessor createPostProcessor(
+			Object request) {
+		if (request instanceof AntPathRequestMatcher) {
+			return new AntPathMatcherEvaluationContextPostProcessor(
+					(AntPathRequestMatcher) request);
+		}
+		return null;
+	}
+
+	static class AntPathMatcherEvaluationContextPostProcessor
+			extends AbstractVariableEvaluationContextPostProcessor {
+		private final AntPathRequestMatcher matcher;
+
+		public AntPathMatcherEvaluationContextPostProcessor(
+				AntPathRequestMatcher matcher) {
+			this.matcher = matcher;
+		}
+
+		@Override
+		protected Map<String, String> extractVariables(
+				HttpServletRequest request) {
+			return this.matcher.extractUriTemplateVariables(request);
+		}
+	}
+
 }

web/src/main/java/org/springframework/security/web/access/expression/PathVariableSecurityEvaluationContextPostProcessor.java

@@ -1,74 +0,0 @@
-/*
- * Copyright 2002-2015 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.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.springframework.security.web.access.expression;
-
-import java.util.Map;
-
-import javax.servlet.http.HttpServletRequest;
-
-import org.springframework.expression.EvaluationContext;
-import org.springframework.security.web.FilterInvocation;
-import org.springframework.util.AntPathMatcher;
-import org.springframework.util.PathMatcher;
-
-/**
- * Exposes URI template variables as variables on the {@link EvaluationContext}.
- * For example, the pattern "/user/{username}/**" would expose a variable named
- * username based on the current URI.
- *
- * <p>
- * NOTE: This API is intentionally kept package scope as it may change in the future. It may be nice to allow users to augment expressions and queries
- * </p>
- *
- * @author Rob Winch
- * @since 4.1
- */
-class PathVariableSecurityEvaluationContextPostProcessor implements SecurityEvaluationContextPostProcessor<FilterInvocation> {
-	private final PathMatcher matcher = new AntPathMatcher();
-	private final String antPattern;
-
-	/**
-	 * Creates a new instance.
-	 *
-	 * @param antPattern the ant pattern that may have template variables (i.e. "/user/{username}/**)
-	 */
-	public PathVariableSecurityEvaluationContextPostProcessor(String antPattern) {
-		this.antPattern = antPattern;
-	}
-
-	public EvaluationContext postProcess(EvaluationContext context, FilterInvocation invocation) {
-		if(antPattern == null) {
-			return context;
-		}
-
-		String path = getRequestPath(invocation.getHttpRequest());
-		Map<String, String> variables = matcher.extractUriTemplateVariables(antPattern, path);
-		for(Map.Entry<String, String> entry : variables.entrySet()) {
-			context.setVariable(entry.getKey(), entry.getValue());
-		}
-		return context;
-	}
-
-	private String getRequestPath(HttpServletRequest request) {
-		String url = request.getServletPath();
-
-		if (request.getPathInfo() != null) {
-			url += request.getPathInfo();
-		}
-
-		return url;
-	}
-}

web/src/main/java/org/springframework/security/web/access/expression/WebExpressionConfigAttribute.java

@@ -26,29 +26,34 @@ import org.springframework.security.web.FilterInvocation;
  * @author Luke Taylor
  * @since 3.0
  */
-class WebExpressionConfigAttribute implements ConfigAttribute, SecurityEvaluationContextPostProcessor<FilterInvocation> {
+class WebExpressionConfigAttribute implements ConfigAttribute,
+		EvaluationContextPostProcessor<FilterInvocation> {
 	private final Expression authorizeExpression;
-	private final SecurityEvaluationContextPostProcessor<FilterInvocation> postProcessor;
+	private final EvaluationContextPostProcessor<FilterInvocation> postProcessor;
 
-	public WebExpressionConfigAttribute(Expression authorizeExpression, SecurityEvaluationContextPostProcessor<FilterInvocation> postProcessor) {
+	public WebExpressionConfigAttribute(Expression authorizeExpression,
+			EvaluationContextPostProcessor<FilterInvocation> postProcessor) {
 		this.authorizeExpression = authorizeExpression;
 		this.postProcessor = postProcessor;
 	}
 
 	Expression getAuthorizeExpression() {
-		return authorizeExpression;
+		return this.authorizeExpression;
 	}
 
+	@Override
 	public EvaluationContext postProcess(EvaluationContext context, FilterInvocation fi) {
-		return postProcessor.postProcess(context, fi);
+		return this.postProcessor == null ? context
+				: this.postProcessor.postProcess(context, fi);
 	}
 
+	@Override
 	public String getAttribute() {
 		return null;
 	}
 
 	@Override
 	public String toString() {
-		return authorizeExpression.getExpressionString();
+		return this.authorizeExpression.getExpressionString();
 	}
 }

web/src/main/java/org/springframework/security/web/util/matcher/AntPathRequestMatcher.java

@@ -15,12 +15,15 @@
  */
 package org.springframework.security.web.util.matcher;
 
+import java.util.Collections;
+import java.util.Map;
+
 import javax.servlet.http.HttpServletRequest;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+
 import org.springframework.http.HttpMethod;
-import org.springframework.security.web.util.matcher.RequestMatcher;
 import org.springframework.util.AntPathMatcher;
 import org.springframework.util.Assert;
 import org.springframework.util.StringUtils;
@@ -89,13 +92,14 @@ public final class AntPathRequestMatcher implements RequestMatcher {
 	 * the incoming request doesn't doesn't have the same method.
 	 * @param caseSensitive true if the matcher should consider case, else false
 	 */
-	public AntPathRequestMatcher(String pattern, String httpMethod, boolean caseSensitive) {
+	public AntPathRequestMatcher(String pattern, String httpMethod,
+			boolean caseSensitive) {
 		Assert.hasText(pattern, "Pattern cannot be null or empty");
 		this.caseSensitive = caseSensitive;
 
 		if (pattern.equals(MATCH_ALL) || pattern.equals("**")) {
 			pattern = MATCH_ALL;
-			matcher = null;
+			this.matcher = null;
 		}
 		else {
 			if (!caseSensitive) {
@@ -105,19 +109,20 @@ public final class AntPathRequestMatcher implements RequestMatcher {
 			// 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)
-					&& (pattern.indexOf('?') == -1 && pattern.indexOf('{') == -1 && pattern
-							.indexOf('}') == -1)
+					&& (pattern.indexOf('?') == -1 && pattern.indexOf('{') == -1
+							&& pattern.indexOf('}') == -1)
 					&& pattern.indexOf("*") == pattern.length() - 2) {
-				matcher = new SubpathMatcher(pattern.substring(0, pattern.length() - 3));
+				this.matcher = new SubpathMatcher(
+						pattern.substring(0, pattern.length() - 3));
 			}
 			else {
-				matcher = new SpringAntMatcher(pattern);
+				this.matcher = new SpringAntMatcher(pattern);
 			}
 		}
 
 		this.pattern = pattern;
-		this.httpMethod = StringUtils.hasText(httpMethod) ? HttpMethod
-				.valueOf(httpMethod) : null;
+		this.httpMethod = StringUtils.hasText(httpMethod) ? HttpMethod.valueOf(httpMethod)
+				: null;
 	}
 
 	/**
@@ -127,19 +132,20 @@ public final class AntPathRequestMatcher implements RequestMatcher {
 	 * @param request the request to match against. The ant pattern will be matched
 	 * against the {@code servletPath} + {@code pathInfo} of the request.
 	 */
+	@Override
 	public boolean matches(HttpServletRequest request) {
-		if (httpMethod != null && StringUtils.hasText(request.getMethod())
-				&& httpMethod != valueOf(request.getMethod())) {
+		if (this.httpMethod != null && StringUtils.hasText(request.getMethod())
+				&& this.httpMethod != valueOf(request.getMethod())) {
 			if (logger.isDebugEnabled()) {
 				logger.debug("Request '" + request.getMethod() + " "
-						+ getRequestPath(request) + "'" + " doesn't match '" + httpMethod
-						+ " " + pattern);
+						+ getRequestPath(request) + "'" + " doesn't match '"
+						+ this.httpMethod + " " + this.pattern);
 			}
 
 			return false;
 		}
 
-		if (pattern.equals(MATCH_ALL)) {
+		if (this.pattern.equals(MATCH_ALL)) {
 			if (logger.isDebugEnabled()) {
 				logger.debug("Request '" + getRequestPath(request)
 						+ "' matched by universal pattern '/**'");
@@ -151,11 +157,19 @@ public final class AntPathRequestMatcher implements RequestMatcher {
 		String url = getRequestPath(request);
 
 		if (logger.isDebugEnabled()) {
-			logger.debug("Checking match of request : '" + url + "'; against '" + pattern
-					+ "'");
+			logger.debug("Checking match of request : '" + url + "'; against '"
+					+ this.pattern + "'");
 		}
 
-		return matcher.matches(url);
+		return this.matcher.matches(url);
+	}
+
+	public Map<String, String> extractUriTemplateVariables(HttpServletRequest request) {
+		if (this.matcher == null || !matches(request)) {
+			return Collections.emptyMap();
+		}
+		String url = getRequestPath(request);
+		return this.matcher.extractUriTemplateVariables(url);
 	}
 
 	private String getRequestPath(HttpServletRequest request) {
@@ -165,7 +179,7 @@ public final class AntPathRequestMatcher implements RequestMatcher {
 			url += request.getPathInfo();
 		}
 
-		if (!caseSensitive) {
+		if (!this.caseSensitive) {
 			url = url.toLowerCase();
 		}
 
@@ -173,7 +187,7 @@ public final class AntPathRequestMatcher implements RequestMatcher {
 	}
 
 	public String getPattern() {
-		return pattern;
+		return this.pattern;
 	}
 
 	@Override
@@ -189,9 +203,9 @@ public final class AntPathRequestMatcher implements RequestMatcher {
 
 	@Override
 	public int hashCode() {
-		int code = 31 ^ pattern.hashCode();
-		if (httpMethod != null) {
-			code ^= httpMethod.hashCode();
+		int code = 31 ^ this.pattern.hashCode();
+		if (this.httpMethod != null) {
+			code ^= this.httpMethod.hashCode();
 		}
 		return code;
 	}
@@ -199,10 +213,10 @@ public final class AntPathRequestMatcher implements RequestMatcher {
 	@Override
 	public String toString() {
 		StringBuilder sb = new StringBuilder();
-		sb.append("Ant [pattern='").append(pattern).append("'");
+		sb.append("Ant [pattern='").append(this.pattern).append("'");
 
-		if (httpMethod != null) {
-			sb.append(", ").append(httpMethod);
+		if (this.httpMethod != null) {
+			sb.append(", ").append(this.httpMethod);
 		}
 
 		sb.append("]");
@@ -230,6 +244,8 @@ public final class AntPathRequestMatcher implements RequestMatcher {
 
 	private static interface Matcher {
 		boolean matches(String path);
+
+		Map<String, String> extractUriTemplateVariables(String path);
 	}
 
 	private static class SpringAntMatcher implements Matcher {
@@ -241,8 +257,14 @@ public final class AntPathRequestMatcher implements RequestMatcher {
 			this.pattern = pattern;
 		}
 
+		@Override
 		public boolean matches(String path) {
-			return antMatcher.match(pattern, path);
+			return antMatcher.match(this.pattern, path);
+		}
+
+		@Override
+		public Map<String, String> extractUriTemplateVariables(String path) {
+			return antMatcher.extractUriTemplateVariables(this.pattern, path);
 		}
 	}
 
@@ -254,14 +276,20 @@ public final class AntPathRequestMatcher implements RequestMatcher {
 		private final int length;
 
 		private SubpathMatcher(String subpath) {
-			assert !subpath.contains("*");
+			assert!subpath.contains("*");
 			this.subpath = subpath;
 			this.length = subpath.length();
 		}
 
+		@Override
 		public boolean matches(String path) {
-			return path.startsWith(subpath)
-					&& (path.length() == length || path.charAt(length) == '/');
+			return path.startsWith(this.subpath)
+					&& (path.length() == this.length || path.charAt(this.length) == '/');
+		}
+
+		@Override
+		public Map<String, String> extractUriTemplateVariables(String path) {
+			return Collections.emptyMap();
 		}
 	}
 }

web/src/test/java/org/springframework/security/web/access/expression/AbstractVariableEvaluationContextPostProcessorTests.java

@@ -15,20 +15,28 @@
  */
 package org.springframework.security.web.access.expression;
 
+import java.util.Collections;
+import java.util.Map;
+
+import javax.servlet.http.HttpServletRequest;
+
 import org.junit.Before;
 import org.junit.Test;
+
 import org.springframework.expression.spel.support.StandardEvaluationContext;
 import org.springframework.mock.web.MockFilterChain;
 import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.mock.web.MockHttpServletResponse;
 import org.springframework.security.web.FilterInvocation;
 
+import static org.assertj.core.api.Assertions.assertThat;
+
 /**
  * @author Rob Winch
  *
  */
-public class PathVariableSecurityEvaluationContextPostProcessorTests {
-	PathVariableSecurityEvaluationContextPostProcessor processor;
+public class AbstractVariableEvaluationContextPostProcessorTests {
+	AbstractVariableEvaluationContextPostProcessor processor;
 
 	FilterInvocation invocation;
 
@@ -38,19 +46,34 @@ public class PathVariableSecurityEvaluationContextPostProcessorTests {
 
 	@Before
 	public void setup() {
-		processor = new PathVariableSecurityEvaluationContextPostProcessor("/");
+		this.processor = new VariableEvaluationContextPostProcessor();
 
-		request = new MockHttpServletRequest();
-		request.setServletPath("/");
-		response = new MockHttpServletResponse();
-		invocation = new FilterInvocation(request,response, new MockFilterChain());
-		context = new StandardEvaluationContext();
+		this.request = new MockHttpServletRequest();
+		this.request.setServletPath("/");
+		this.response = new MockHttpServletResponse();
+		this.invocation = new FilterInvocation(this.request, this.response,
+				new MockFilterChain());
+		this.context = new StandardEvaluationContext();
 	}
 
 	@Test
-	public void queryIgnored() {
-		request.setQueryString("logout");
-		processor.postProcess(context, invocation);
+	public void postProcess() {
+		this.processor.postProcess(this.context, this.invocation);
+
+		for (String key : VariableEvaluationContextPostProcessor.RESULTS.keySet()) {
+			assertThat(this.context.lookupVariable(key))
+					.isEqualTo(VariableEvaluationContextPostProcessor.RESULTS.get(key));
+		}
 	}
 
+	static class VariableEvaluationContextPostProcessor
+			extends AbstractVariableEvaluationContextPostProcessor {
+		static final Map<String, String> RESULTS = Collections.singletonMap("a", "b");
+
+		@Override
+		protected Map<String, String> extractVariables(HttpServletRequest request) {
+			return RESULTS;
+		}
+
+	}
 }

web/src/test/java/org/springframework/security/web/access/expression/WebExpressionVoterTests.java

@@ -51,7 +51,7 @@ public class WebExpressionVoterTests {
 	public void supportsWebConfigAttributeAndFilterInvocation() throws Exception {
 		WebExpressionVoter voter = new WebExpressionVoter();
 		assertThat(voter.supports(new WebExpressionConfigAttribute(mock(Expression.class),
-				mock(SecurityEvaluationContextPostProcessor.class)))).isTrue();
+				mock(EvaluationContextPostProcessor.class)))).isTrue();
 		assertThat(voter.supports(FilterInvocation.class)).isTrue();
 		assertThat(voter.supports(MethodInvocation.class)).isFalse();
 
@@ -69,8 +69,8 @@ public class WebExpressionVoterTests {
 	public void grantsAccessIfExpressionIsTrueDeniesIfFalse() {
 		WebExpressionVoter voter = new WebExpressionVoter();
 		Expression ex = mock(Expression.class);
-		SecurityEvaluationContextPostProcessor postProcessor = mock(
-				SecurityEvaluationContextPostProcessor.class);
+		EvaluationContextPostProcessor postProcessor = mock(
+				EvaluationContextPostProcessor.class);
 		when(postProcessor.postProcess(any(EvaluationContext.class),
 				any(FilterInvocation.class))).thenAnswer(new Answer<EvaluationContext>() {