diff --git a/config/src/test/java/org/springframework/security/config/FilterChainProxyConfigTests.java b/config/src/test/java/org/springframework/security/config/FilterChainProxyConfigTests.java index 8e96d386b1..ceaeb41120 100644 --- a/config/src/test/java/org/springframework/security/config/FilterChainProxyConfigTests.java +++ b/config/src/test/java/org/springframework/security/config/FilterChainProxyConfigTests.java @@ -73,43 +73,43 @@ public class FilterChainProxyConfigTests { @Test public void normalOperation() throws Exception { - FilterChainProxy filterChainProxy = (FilterChainProxy) appCtx.getBean("filterChain", FilterChainProxy.class); + FilterChainProxy filterChainProxy = appCtx.getBean("filterChain", FilterChainProxy.class); doNormalOperation(filterChainProxy); } @Test public void normalOperationWithNewConfig() throws Exception { - FilterChainProxy filterChainProxy = (FilterChainProxy) appCtx.getBean("newFilterChainProxy", FilterChainProxy.class); + FilterChainProxy filterChainProxy = appCtx.getBean("newFilterChainProxy", FilterChainProxy.class); checkPathAndFilterOrder(filterChainProxy); doNormalOperation(filterChainProxy); } @Test public void normalOperationWithNewConfigRegex() throws Exception { - FilterChainProxy filterChainProxy = (FilterChainProxy) appCtx.getBean("newFilterChainProxyRegex", FilterChainProxy.class); + FilterChainProxy filterChainProxy = appCtx.getBean("newFilterChainProxyRegex", FilterChainProxy.class); checkPathAndFilterOrder(filterChainProxy); doNormalOperation(filterChainProxy); } @Test public void normalOperationWithNewConfigNonNamespace() throws Exception { - FilterChainProxy filterChainProxy = (FilterChainProxy) appCtx.getBean("newFilterChainProxyNonNamespace", FilterChainProxy.class); + FilterChainProxy filterChainProxy = appCtx.getBean("newFilterChainProxyNonNamespace", FilterChainProxy.class); checkPathAndFilterOrder(filterChainProxy); doNormalOperation(filterChainProxy); } @Test public void pathWithNoMatchHasNoFilters() throws Exception { - FilterChainProxy filterChainProxy = (FilterChainProxy) appCtx.getBean("newFilterChainProxyNoDefaultPath", FilterChainProxy.class); + FilterChainProxy filterChainProxy = appCtx.getBean("newFilterChainProxyNoDefaultPath", FilterChainProxy.class); assertEquals(null, filterChainProxy.getFilters("/nomatch")); } // SEC-1235 @Test public void mixingPatternsAndPlaceholdersDoesntCauseOrderingIssues() throws Exception { - FilterChainProxy filterChainProxy = (FilterChainProxy) appCtx.getBean("sec1235FilterChainProxy", FilterChainProxy.class); + FilterChainProxy fcp = appCtx.getBean("sec1235FilterChainProxy", FilterChainProxy.class); - RequestMatcher[] matchers = filterChainProxy.getFilterChainMap().keySet().toArray(new RequestMatcher[0]); + RequestMatcher[] matchers = fcp.getFilterChainMap().keySet().toArray(new RequestMatcher[fcp.getFilterChainMap().keySet().size()]); assertEquals("/login*", ((AntPathRequestMatcher)matchers[0]).getPattern()); assertEquals("/logout", ((AntPathRequestMatcher)matchers[1]).getPattern()); assertTrue(matchers[2] instanceof AnyRequestMatcher); diff --git a/web/src/main/java/org/springframework/security/web/FilterChainProxy.java b/web/src/main/java/org/springframework/security/web/FilterChainProxy.java index e369c0148c..27edef4515 100644 --- a/web/src/main/java/org/springframework/security/web/FilterChainProxy.java +++ b/web/src/main/java/org/springframework/security/web/FilterChainProxy.java @@ -15,22 +15,6 @@ package org.springframework.security.web; -import java.io.IOException; -import java.util.Collection; -import java.util.Iterator; -import java.util.LinkedHashMap; -import java.util.LinkedHashSet; -import java.util.List; -import java.util.Map; -import java.util.Set; - -import javax.servlet.Filter; -import javax.servlet.FilterChain; -import javax.servlet.ServletException; -import javax.servlet.ServletRequest; -import javax.servlet.ServletResponse; -import javax.servlet.http.HttpServletRequest; - import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.security.web.util.AnyRequestMatcher; @@ -39,6 +23,11 @@ import org.springframework.util.Assert; import org.springframework.web.filter.DelegatingFilterProxy; import org.springframework.web.filter.GenericFilterBean; +import javax.servlet.*; +import javax.servlet.http.HttpServletRequest; +import java.io.IOException; +import java.util.*; + /** * Delegates Filter requests to a list of Spring-managed beans. @@ -96,7 +85,6 @@ public class FilterChainProxy extends GenericFilterBean { //~ Static fields/initializers ===================================================================================== private static final Log logger = LogFactory.getLog(FilterChainProxy.class); - public static final String TOKEN_NONE = "#NONE#"; //~ Instance fields ================================================================================================ @@ -121,7 +109,7 @@ public class FilterChainProxy extends GenericFilterBean { if (filters == null || filters.size() == 0) { if (logger.isDebugEnabled()) { logger.debug(fi.getRequestUrl() + - filters == null ? " has no matching filters" : " has an empty filter list"); + (filters == null ? " has no matching filters" : " has an empty filter list")); } chain.doFilter(request, response); @@ -162,26 +150,6 @@ public class FilterChainProxy extends GenericFilterBean { return getFilters(new FilterInvocation(url, null).getRequest()); } - /** - * Obtains all of the uniqueFilter instances registered in the map of - * filter chains. - *

This is useful in ensuring a Filter is not initialized or destroyed twice.

- * - * @return all of the Filter instances in the application context which have an entry - * in the map (only one entry is included in the array for - * each Filter that actually exists in application context, even if a given - * Filter is defined multiples times in the filter chain map) - */ - protected Collection obtainAllDefinedFilters() { - Set allFilters = new LinkedHashSet(); - - for (List filters : filterChainMap.values()) { - allFilters.addAll(filters); - } - - return allFilters; - } - /** * Sets the mapping of URL patterns to filter chains. * @@ -217,10 +185,10 @@ public class FilterChainProxy extends GenericFilterBean { private void checkPathOrder() { // Check that the universal pattern is listed at the end, if at all - RequestMatcher[] matchers = filterChainMap.keySet().toArray(new RequestMatcher[0]); + Iterator matchers = filterChainMap.keySet().iterator(); - for (int i=0; i < matchers.length-1; i++) { - if (matchers[i] instanceof AnyRequestMatcher) { + while(matchers.hasNext()) { + if ((matchers.next() instanceof AnyRequestMatcher && matchers.hasNext())) { throw new IllegalArgumentException("A universal match pattern ('/**') is defined " + " before other patterns in the filter chain, causing them to be ignored. Please check the " + "ordering in your namespace or FilterChainProxy bean configuration"); @@ -241,7 +209,8 @@ public class FilterChainProxy extends GenericFilterBean { /** * Used (internally) to specify a validation strategy for the filters in each configured chain. * - * @param filterChainValidator + * @param filterChainValidator the validator instance which will be invoked on during initialization + * to check the {@code FilterChainProxy} instance. */ public void setFilterChainValidator(FilterChainValidator filterChainValidator) { this.filterChainValidator = filterChainValidator; @@ -260,24 +229,25 @@ public class FilterChainProxy extends GenericFilterBean { //~ Inner Classes ================================================================================================== /** - * A FilterChain that records whether or not {@link - * FilterChain#doFilter(javax.servlet.ServletRequest, javax.servlet.ServletResponse)} is called. - *

- * This FilterChain is used by FilterChainProxy to determine if the next - * Filter should be called or not.

+ * Internal {@code FilterChain} implementation that is used to pass a request through the additional + * internal list of filters which match the request. Records the position in the additional filter chain and, when + * completed, passes the request back to the original {@code FilterChain} supplied by the servlet container. */ private static class VirtualFilterChain implements FilterChain { private final FilterInvocation fi; private final List additionalFilters; + private final int size; private int currentPosition = 0; + private VirtualFilterChain(FilterInvocation filterInvocation, List additionalFilters) { this.fi = filterInvocation; this.additionalFilters = additionalFilters; + this.size = additionalFilters.size(); } public void doFilter(ServletRequest request, ServletResponse response) throws IOException, ServletException { - if (currentPosition == additionalFilters.size()) { + if (currentPosition == size) { if (logger.isDebugEnabled()) { logger.debug(fi.getRequestUrl() + " reached end of additional filter chain; proceeding with original chain"); @@ -291,7 +261,7 @@ public class FilterChainProxy extends GenericFilterBean { if (logger.isDebugEnabled()) { logger.debug(fi.getRequestUrl() + " at position " + currentPosition + " of " - + additionalFilters.size() + " in additional filter chain; firing Filter: '" + + size + " in additional filter chain; firing Filter: '" + nextFilter + "'"); }