SEC-1584: Addition of HttpFirewall strategy to FilterChainProxy to reject un-normalized requests and wrap the incoming request object before processing by the security filter chain to provide a more consistent representation of paths than is guaranteed by the servlet spec. The wrapper strips path parameters from pathInfo and servletPath to provide consistency of URL matching across servlet containers and protect against bypassing security constraints by the malicious addition of such parameters to the URL. The paths are canonicalized further by replacing of multiple sequences of "/" characters with a single "/".

web/src/main/java/org/springframework/security/web/FilterChainProxy.java

@@ -15,37 +15,37 @@
 
 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.FilterConfig;
-import javax.servlet.ServletException;
-import javax.servlet.ServletRequest;
-import javax.servlet.ServletResponse;
-
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.springframework.security.web.access.intercept.FilterInvocationSecurityMetadataSource;
+import org.springframework.security.web.firewall.DefaultHttpFirewall;
+import org.springframework.security.web.firewall.FirewalledRequest;
+import org.springframework.security.web.firewall.HttpFirewall;
 import org.springframework.security.web.util.AntUrlPathMatcher;
 import org.springframework.security.web.util.UrlMatcher;
+import org.springframework.security.web.util.UrlUtils;
 import org.springframework.util.Assert;
 import org.springframework.web.filter.DelegatingFilterProxy;
 import org.springframework.web.filter.GenericFilterBean;
 
+import javax.servlet.Filter;
+import javax.servlet.FilterChain;
+import javax.servlet.FilterConfig;
+import javax.servlet.ServletException;
+import javax.servlet.ServletRequest;
+import javax.servlet.ServletRequestWrapper;
+import javax.servlet.ServletResponse;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import java.io.IOException;
+import java.util.*;
+
 
 /**
- * Delegates <code>Filter</code> requests to a list of Spring-managed beans.
- * As of version 2.0, you shouldn't need to explicitly configure a <tt>FilterChainProxy</tt> bean in your application
+ * Delegates {@code Filter} requests to a list of Spring-managed filter beans.
+ * As of version 2.0, you shouldn't need to explicitly configure a {@code FilterChainProxy} bean in your application
  * context unless you need very fine control over the filter chain contents. Most cases should be adequately covered
- * by the default <tt>&lt;security:http /&gt</tt> namespace configuration options.
+ * by the default <tt>&lt;security:http /&gt;</tt> namespace configuration options.
  *
  * <p>The <code>FilterChainProxy</code> is loaded via a standard Spring {@link DelegatingFilterProxy} declaration in
  * <code>web.xml</code>. <code>FilterChainProxy</code> will then pass {@link #init(FilterConfig)}, {@link #destroy()}
@@ -71,7 +71,7 @@ import org.springframework.web.filter.GenericFilterBean;
  &lt;/bean>
  * </pre>
  *
- * The names "filter1", "filter2", "filter3" should be the bean names of <tt>Filter</tt> instances defined in the
+ * The names "filter1", "filter2", "filter3" should be the bean names of {@code Filter} instances defined in the
  * application context. The order of the names defines the order in which the filters will be applied. As shown above,
  * use of the value "none" for the "filters" can be used to exclude
  * Please consult the security namespace schema file for a full list of available configuration options.
@@ -86,17 +86,32 @@ import org.springframework.web.filter.GenericFilterBean;
  *
  * <p><code>FilterChainProxy</code> respects normal handling of <code>Filter</code>s that elect not to call {@link
  * javax.servlet.Filter#doFilter(javax.servlet.ServletRequest, javax.servlet.ServletResponse,
- * javax.servlet.FilterChain)}, in that the remainder of the original or <code>FilterChainProxy</code>-declared filter
+ * javax.servlet.FilterChain)}, in that the remainder of the original or {@code FilterChainProxy}-declared filter
  * chain will not be called.
  *
- * <p>Note the <code>Filter</code> lifecycle mismatch between the servlet container and IoC
- * container. As described in the {@link DelegatingFilterProxy} JavaDocs, we recommend you allow the IoC
- * container to manage the lifecycle instead of the servlet container. By default the <code>DelegatingFilterProxy</code>
- * will never call this class' {@link #init(FilterConfig)} and {@link #destroy()} methods, which in turns means that
- * the corresponding methods on the filter beans managed by this class will never be called. If you do need your filters to be
- * initialized and destroyed, please set the <tt>targetFilterLifecycle</tt> initialization parameter against the
- * <code>DelegatingFilterProxy</code> to specify that servlet container lifecycle management should be used. You don't
- * need to worry about this in most cases.
+ * <h3>Request Firewalling</h3>
+ *
+ * An {@link HttpFirewall} instance is used to validate incoming requests and create a wrapped request which provides
+ * consistent path values for matching against. See {@link DefaultHttpFirewall}, for more information on the type of
+ * attacks which the default implementation protects against. A custom implementation can be injected to provide
+ * stricter control over the request contents or if an application needs to support certain types of request which
+ * are rejected by default.
+ * <p>
+ * Note that this means that you must use the Spring Security filters in combination with a {@code FilterChainProxy}
+ * if you want this protection. Don't define them explicitly in your {@code web.xml} file.
+ * <p>
+ * {@code FilterChainProxy} will use the firewall instance to obtain both request and response objects which will be
+ * fed down the filter chain, so it is also possible to use this functionality to control the functionality of the
+ * response. When the request has passed through the security filter chain, the {@code reset} method will be called.
+ * With the default implementation this means that the original values of {@code servletPath} and {@code pathInfo} will
+ * be returned thereafter, instead of the modified ones used for security pattern matching.
+ *
+ * <h2>Filter Lifecycle</h2>
+ * <p>
+ * Note the {@code Filter} lifecycle mismatch between the servlet container and IoC
+ * container. As described in the {@link DelegatingFilterProxy} Javadocs, we recommend you allow the IoC
+ * container to manage the lifecycle instead of the servlet container. {@code FilterChainProxy} does not invoke the
+ * standard filter lifecycle methods on any filter beans that you add to the application context.
  *
  * @author Carlos Sanchez
  * @author Ben Alex
@@ -118,6 +133,7 @@ public class FilterChainProxy extends GenericFilterBean {
     private Map<Object, List<Filter>> filterChainMap;
     private UrlMatcher matcher = new AntUrlPathMatcher();
     private boolean stripQueryStringFromUrls = true;
+    private HttpFirewall firewall = new DefaultHttpFirewall();
     private FilterChainValidator filterChainValidator = new NullFilterChainValidator();
 
     //~ Methods ========================================================================================================
@@ -131,22 +147,24 @@ public class FilterChainProxy extends GenericFilterBean {
     public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain)
             throws IOException, ServletException {
 
-        FilterInvocation fi = new FilterInvocation(request, response, chain);
-        List<Filter> filters = getFilters(fi.getRequestUrl());
+        FirewalledRequest fwRequest = firewall.getFirewalledRequest((HttpServletRequest) request);
+        HttpServletResponse fwResponse = firewall.getFirewalledResponse((HttpServletResponse) response);
+        String url = UrlUtils.buildRequestUrl(fwRequest);
+
+        List<Filter> filters = getFilters(url);
 
         if (filters == null || filters.size() == 0) {
             if (logger.isDebugEnabled()) {
-                logger.debug(fi.getRequestUrl() +
-                        filters == null ? " has no matching filters" : " has an empty filter list");
+                logger.debug(url + (filters == null ? " has no matching filters" : " has an empty filter list"));
             }
 
-            chain.doFilter(request, response);
+            chain.doFilter(fwRequest, fwResponse);
 
             return;
         }
 
-        VirtualFilterChain virtualFilterChain = new VirtualFilterChain(fi, filters);
-        virtualFilterChain.doFilter(fi.getRequest(), fi.getResponse());
+        VirtualFilterChain vfc = new VirtualFilterChain(url, chain, filters);
+        vfc.doFilter(fwRequest, fwResponse);
     }
 
     /**
@@ -213,14 +231,14 @@ public class FilterChainProxy extends GenericFilterBean {
     /**
      * Sets the mapping of URL patterns to filter chains.
      *
-     * The map keys should be the paths and the values should be arrays of <tt>Filter</tt> objects.
+     * The map keys should be the paths and the values should be arrays of {@code Filter} objects.
      * It's VERY important that the type of map used preserves ordering - the order in which the iterator
      * returns the entries must be the same as the order they were added to the map, otherwise you have no way
      * of guaranteeing that the most specific patterns are returned before the more general ones. So make sure
-     * the Map used is an instance of <tt>LinkedHashMap</tt> or an equivalent, rather than a plain <tt>HashMap</tt>, for
+     * the Map used is an instance of {@code LinkedHashMap} or an equivalent, rather than a plain {@code HashMap}, for
      * example.
      *
-     * @param filterChainMap the map of path Strings to <tt>List&lt;Filter&gt;</tt>s.
+     * @param filterChainMap the map of path Strings to {@code List&lt;Filter&gt;}s.
      */
     @SuppressWarnings("unchecked")
     public void setFilterChainMap(Map filterChainMap) {
@@ -270,7 +288,7 @@ public class FilterChainProxy extends GenericFilterBean {
 
     /**
      * Returns a copy of the underlying filter chain map. Modifications to the map contents
-     * will not affect the FilterChainProxy state - to change the map call <tt>setFilterChainMap</tt>.
+     * will not affect the FilterChainProxy state - to change the map call {@code setFilterChainMap}.
      *
      * @return the map of path pattern Strings to filter chain lists (with ordering guaranteed).
      */
@@ -317,42 +335,53 @@ public class FilterChainProxy extends GenericFilterBean {
     //~ Inner Classes ==================================================================================================
 
     /**
-     * A <code>FilterChain</code> that records whether or not {@link
-     * FilterChain#doFilter(javax.servlet.ServletRequest, javax.servlet.ServletResponse)} is called.
-     * <p>
-     * This <code>FilterChain</code> is used by <code>FilterChainProxy</code> to determine if the next
-     * <code>Filter</code> should be called or not.</p>
+     * Internal {@code FilterChain} implementation that is used to pass a request through the additional
+     * internal list of filters which match the request.
      */
     private static class VirtualFilterChain implements FilterChain {
-        private FilterInvocation fi;
-        private List<Filter> additionalFilters;
+        private final FilterChain originalChain;
+        private final List<Filter> additionalFilters;
+        private final String url;
         private int currentPosition = 0;
 
-        private VirtualFilterChain(FilterInvocation filterInvocation, List<Filter> additionalFilters) {
-            this.fi = filterInvocation;
+        private VirtualFilterChain(String url, FilterChain chain, List<Filter> additionalFilters) {
+            this.originalChain = chain;
+            this.url = url;
             this.additionalFilters = additionalFilters;
         }
 
-        public void doFilter(ServletRequest request, ServletResponse response) throws IOException, ServletException {
+        public void doFilter(final ServletRequest request, final ServletResponse response) throws IOException, ServletException {
             if (currentPosition == additionalFilters.size()) {
                 if (logger.isDebugEnabled()) {
-                    logger.debug(fi.getRequestUrl()
-                        + " reached end of additional filter chain; proceeding with original chain");
+                    logger.debug(url + " reached end of additional filter chain; proceeding with original chain");
                 }
 
-                fi.getChain().doFilter(request, response);
+                // Deactivate path stripping as we exit the security filter chain
+                resetWrapper(request);
+
+                originalChain.doFilter(request, response);
             } else {
                 currentPosition++;
 
                 Filter nextFilter = additionalFilters.get(currentPosition - 1);
 
                 if (logger.isDebugEnabled()) {
-                    logger.debug(fi.getRequestUrl() + " at position " + currentPosition + " of "
+                    logger.debug(url + " at position " + currentPosition + " of "
                         + additionalFilters.size() + " in additional filter chain; firing Filter: '"
-                        + nextFilter + "'");
+                        + nextFilter.getClass().getSimpleName() + "'");
                 }
 
-               nextFilter.doFilter(request, response, this);
+                nextFilter.doFilter(request, response, this);
+            }
+        }
+
+        private void resetWrapper(ServletRequest request) {
+            while (request instanceof ServletRequestWrapper) {
+                if (request instanceof FirewalledRequest) {
+                    ((FirewalledRequest)request).reset();
+                    break;
+                }
+                request = ((ServletRequestWrapper)request).getRequest();
             }
         }
     }

web/src/main/java/org/springframework/security/web/firewall/DefaultHttpFirewall.java

@@ -0,0 +1,67 @@
+package org.springframework.security.web.firewall;
+
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+/**
+ * Default implementation which wraps requests in order to provide consistent values of the {@code servletPath} and
+ * {@code pathInfo}, which do not contain path parameters (as defined in
+ * <a href="http://www.ietf.org/rfc/rfc2396.txt">RFC 2396</a>). Different servlet containers
+ * interpret the servlet spec differently as to how path parameters are treated and it is possible they might be added
+ * in order to bypass particular security constraints. When using this implementation, they will be removed for all
+ * requests as the request passes through the security filter chain. Note that this means that any segments in the
+ * decoded path which contain a semi-colon, will have the part following the semi-colon removed for
+ * request matching. Your application should not contain any valid paths which contain semi-colons.
+ * <p>
+ * If any un-normalized paths are found (containing directory-traversal character sequences), the request will be
+ * rejected immediately. Most containers normalize the paths before performing the servlet-mapping, but again this is
+ * not guaranteed by the servlet spec.
+ *
+ * @author Luke Taylor
+ */
+public class DefaultHttpFirewall implements HttpFirewall {
+
+    public FirewalledRequest getFirewalledRequest(HttpServletRequest request) throws RequestRejectedException {
+        FirewalledRequest fwr = new RequestWrapper(request);
+
+        if (!isNormalized(fwr.getServletPath()) || !isNormalized(fwr.getPathInfo())) {
+            throw new RequestRejectedException("Un-normalized paths are not supported: " + fwr.getServletPath() +
+                (fwr.getPathInfo() != null ? fwr.getPathInfo() : ""));
+        }
+
+        return fwr;
+    }
+
+    public HttpServletResponse getFirewalledResponse(HttpServletResponse response) {
+        return response;
+    }
+
+    /**
+     * Checks whether a path is normalized (doesn't contain path traversal sequences like "./", "/../" or "/.")
+     *
+     * @param path the path to test
+     * @return true if the path doesn't contain any path-traversal character sequences.
+     */
+    private boolean isNormalized(String path) {
+        if (path == null) {
+            return true;
+        }
+
+        for (int j = path.length(); j > 0;) {
+            int i = path.lastIndexOf('/', j - 1);
+            int gap = j - i;
+
+            if (gap == 2 && path.charAt(i+1) == '.') {
+                // ".", "/./" or "/."
+                return false;
+            } else if (gap == 3 && path.charAt(i+1) == '.'&& path.charAt(i+2) == '.') {
+                return false;
+            }
+
+            j = i;
+        }
+
+        return true;
+    }
+
+}

web/src/main/java/org/springframework/security/web/firewall/FirewalledRequest.java

@@ -0,0 +1,34 @@
+package org.springframework.security.web.firewall;
+
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletRequestWrapper;
+
+/**
+ * Request wrapper which is returned by the {@code HttpFirewall} interface.
+ * <p>
+ * The only difference is the {@code reset} method which allows some
+ * or all of the state to be reset by the {@code FilterChainProxy} when the
+ * request leaves the security filter chain.
+ *
+ * @author Luke Taylor
+ */
+public abstract class FirewalledRequest extends HttpServletRequestWrapper {
+    /**
+     * Constructs a request object wrapping the given request.
+     *
+     * @throws IllegalArgumentException if the request is null
+     */
+    public FirewalledRequest(HttpServletRequest request) {
+        super(request);
+    }
+
+    /**
+     * This method will be called once the request has passed through the
+     * security filter chain, when it is about to proceed to the application
+     * proper.
+     * <p>
+     * An implementation can thus choose to modify the state of the request
+     * for the security infrastructure, while still maintaining the
+     */
+    public abstract void reset();
+}

web/src/main/java/org/springframework/security/web/firewall/HttpFirewall.java

@@ -0,0 +1,32 @@
+package org.springframework.security.web.firewall;
+
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+/**
+ * Interface which can be used to reject potentially dangerous requests and/or wrap them to
+ * control their behaviour.
+ * <p>
+ * The implementation is injected into the {@code FilterChainProxy} and will be invoked before
+ * sending any request through the filter chain. It can also provide a response wrapper if the response
+ * behaviour should also be restricted.
+ *
+ * @author Luke Taylor
+ */
+public interface HttpFirewall {
+
+    /**
+     * Provides the request object which will be passed through the filter chain.
+     *
+     * @throws RequestRejectedException if the request should be rejected immediately
+     */
+    FirewalledRequest getFirewalledRequest(HttpServletRequest request) throws RequestRejectedException;
+
+    /**
+     * Provides the response which will be passed through the filter chain.
+     *
+     * @param response the original response
+     * @return either the original response or a replacement/wrapper.
+     */
+    HttpServletResponse getFirewalledResponse(HttpServletResponse response);
+}

web/src/main/java/org/springframework/security/web/firewall/RequestRejectedException.java

@@ -0,0 +1,10 @@
+package org.springframework.security.web.firewall;
+
+/**
+ * @author Luke Taylor
+ */
+public class RequestRejectedException extends RuntimeException {
+    public RequestRejectedException(String message) {
+        super(message);
+    }
+}

web/src/main/java/org/springframework/security/web/firewall/RequestWrapper.java

@@ -0,0 +1,100 @@
+package org.springframework.security.web.firewall;
+
+import javax.servlet.http.HttpServletRequest;
+import java.util.*;
+
+/**
+ * Request wrapper which ensures values of {@code servletPath} and {@code pathInfo} are returned which are suitable for
+ * pattern matching against. It strips out path parameters and extra consecutive '/' characters.
+ *
+ * <h3>Path Parameters</h3>
+ * Parameters (as defined in <a href="http://www.ietf.org/rfc/rfc2396.txt">RFC 2396</a>) are stripped from the path
+ * segments of the {@code servletPath} and {@code pathInfo} values of the request.
+ * <p>
+ * The parameter sequence is demarcated by a semi-colon, so each segment is checked for the occurrence of a ";"
+ * character and truncated at that point if it is present.
+ * <p>
+ * The behaviour differs between servlet containers in how they interpret the servlet spec, which
+ * does not clearly state what the behaviour should be. For consistency, we make sure they are always removed, to
+ * avoid the risk of URL matching rules being bypassed by the malicious addition of parameters to the path component.
+ *
+ * @author Luke Taylor
+ */
+final class RequestWrapper extends FirewalledRequest {
+    private final String strippedServletPath;
+    private final String strippedPathInfo;
+    private boolean stripPaths = true;
+
+    public RequestWrapper(HttpServletRequest request) {
+        super(request);
+        strippedServletPath = strip(request.getServletPath());
+        String pathInfo = strip(request.getPathInfo());
+        if (pathInfo != null && pathInfo.length() == 0) {
+            pathInfo = null;
+        }
+        strippedPathInfo = pathInfo;
+    }
+
+    /**
+     * Removes path parameters from each path segment in the supplied path and truncates sequences of multiple '/'
+     * characters to a single '/'.
+     *
+     * @param path either the {@code servletPath} and {@code pathInfo} from the original request
+     *
+     * @return the supplied value, with path parameters removed and sequences of multiple '/' characters truncated,
+     *  or null if the supplied path was null.
+     */
+    private String strip(String path) {
+        if (path == null) {
+            return null;
+        }
+
+        int scIndex = path.indexOf(';');
+
+        if (scIndex < 0) {
+            int doubleSlashIndex = path.indexOf("//");
+            if (doubleSlashIndex < 0) {
+                // Most likely case, no parameters in any segment and no '//', so no stripping required
+                return path;
+            }
+        }
+
+        StringTokenizer st = new StringTokenizer(path, "/");
+        StringBuilder stripped = new StringBuilder(path.length());
+
+        if (path.charAt(0) == '/') {
+            stripped.append('/');
+        }
+
+        while(st.hasMoreTokens()) {
+            String segment = st.nextToken();
+            scIndex = segment.indexOf(';');
+
+            if (scIndex >= 0) {
+                segment = segment.substring(0, scIndex);
+            }
+            stripped.append(segment).append('/');
+        }
+
+        // Remove the trailing slash if the original path didn't have one
+        if (path.charAt(path.length() - 1) != '/') {
+            stripped.deleteCharAt(stripped.length() - 1);
+        }
+
+        return stripped.toString();
+    }
+
+    @Override
+    public String getPathInfo() {
+        return stripPaths ? strippedPathInfo : super.getPathInfo();
+    }
+
+    @Override
+    public String getServletPath() {
+        return stripPaths ? strippedServletPath : super.getServletPath();
+    }
+
+    public void reset() {
+        this.stripPaths = false;
+    }
+}

web/src/test/java/org/springframework/security/web/FilterChainProxyTests.java

@@ -0,0 +1,103 @@
+package org.springframework.security.web;
+
+import static org.junit.Assert.assertEquals;
+import static org.mockito.Mockito.*;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.stubbing.Answer;
+import org.springframework.mock.web.MockHttpServletRequest;
+import org.springframework.mock.web.MockHttpServletResponse;
+import org.springframework.security.web.firewall.FirewalledRequest;
+
+import javax.servlet.Filter;
+import javax.servlet.FilterChain;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import java.util.*;
+
+/**
+ * @author Luke Taylor
+ */
+@SuppressWarnings({"unchecked"})
+public class FilterChainProxyTests {
+    private FilterChainProxy fcp;
+    private MockHttpServletRequest request;
+    private MockHttpServletResponse response;
+    private FilterChain chain;
+    private Filter filter;
+
+    @Before
+    public void setup() throws Exception {
+        fcp = new FilterChainProxy();
+        filter = mock(Filter.class);
+        doAnswer(new Answer() {
+                    public Object answer(InvocationOnMock inv) throws Throwable {
+                        Object[] args = inv.getArguments();
+                        FilterChain fc = (FilterChain) args[2];
+                        fc.doFilter((HttpServletRequest) args[0], (HttpServletResponse) args[1]);
+                        return null;
+                    }
+                }).when(filter).doFilter(any(HttpServletRequest.class), any(HttpServletResponse.class), any(FilterChain.class));
+        LinkedHashMap map = new LinkedHashMap();
+        map.put("/match", Arrays.asList(filter));
+        fcp.setFilterChainMap(map);
+        request = new MockHttpServletRequest();
+        request.setServletPath("/match");
+        response = new MockHttpServletResponse();
+        chain = mock(FilterChain.class);
+    }
+
+    @Test
+    public void toStringCallSucceeds() throws Exception {
+        fcp.afterPropertiesSet();
+        fcp.toString();
+    }
+
+    @Test
+    public void securityFilterChainIsNotInvokedIfMatchFails() throws Exception {
+        request.setServletPath("/nomatch");
+        fcp.doFilter(request, response, chain);
+        assertEquals(1, fcp.getFilterChainMap().size());
+
+        verifyZeroInteractions(filter);
+        // The actual filter chain should be invoked though
+        verify(chain).doFilter(any(HttpServletRequest.class), any(HttpServletResponse.class));
+    }
+
+    @Test
+    public void originalChainIsInvokedAfterSecurityChainIfMatchSucceeds() throws Exception {
+        fcp.doFilter(request, response, chain);
+
+        verify(filter).doFilter(any(HttpServletRequest.class), any(HttpServletResponse.class), any(FilterChain.class));
+        verify(chain).doFilter(any(HttpServletRequest.class), any(HttpServletResponse.class));
+    }
+
+    @Test
+    public void originalFilterChainIsInvokedIfMatchingSecurityChainIsEmpty() throws Exception {
+        LinkedHashMap map = new LinkedHashMap();
+        map.put("/match", Collections.emptyList());
+        fcp.setFilterChainMap(map);
+
+        fcp.doFilter(request, response, chain);
+
+        verify(chain).doFilter(any(HttpServletRequest.class), any(HttpServletResponse.class));
+    }
+
+    @Test
+    public void requestIsWrappedForFilteringWhenMatchIsFound() throws Exception {
+        fcp.doFilter(request, response, chain);
+        verify(filter).doFilter(any(FirewalledRequest.class), any(HttpServletResponse.class), any(FilterChain.class));
+        verify(chain).doFilter(any(FirewalledRequest.class), any(HttpServletResponse.class));
+    }
+
+    @Test
+    public void requestIsWrappedForFilteringWhenMatchIsNotFound() throws Exception {
+        request.setServletPath("/nomatch");
+        fcp.doFilter(request, response, chain);
+        verifyZeroInteractions(filter);
+        verify(chain).doFilter(any(FirewalledRequest.class), any(HttpServletResponse.class));
+    }
+
+}

web/src/test/java/org/springframework/security/web/firewall/DefaultHttpFirewallTests.java

@@ -0,0 +1,44 @@
+package org.springframework.security.web.firewall;
+
+import static org.junit.Assert.fail;
+
+import org.junit.Test;
+import org.springframework.mock.web.MockHttpServletRequest;
+
+/**
+ * @author Luke Taylor
+ */
+public class DefaultHttpFirewallTests {
+    public String[] unnormalizedPaths = {
+            "/..",
+            "/./path/",
+            "/path/path/.",
+            "/path/path//.",
+            "./path/../path//.",
+            "./path",
+            ".//path",
+            "."
+    };
+
+    @Test
+    public void unnormalizedPathsAreRejected() throws Exception {
+        DefaultHttpFirewall fw = new DefaultHttpFirewall();
+
+        MockHttpServletRequest request;
+        for (String path : unnormalizedPaths) {
+            request = new MockHttpServletRequest();
+            request.setServletPath(path);
+            try {
+                fw.getFirewalledRequest(request);
+                fail(path + " is un-normalized");
+            } catch (RequestRejectedException expected) {
+            }
+            request.setPathInfo(path);
+            try {
+                fw.getFirewalledRequest(request);
+                fail(path + " is un-normalized");
+            } catch (RequestRejectedException expected) {
+            }
+        }
+    }
+}

web/src/test/java/org/springframework/security/web/firewall/RequestWrapperTests.java

@@ -0,0 +1,62 @@
+package org.springframework.security.web.firewall;
+
+import static org.junit.Assert.assertEquals;
+
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.springframework.mock.web.MockHttpServletRequest;
+
+import java.util.*;
+
+/**
+ * @author Luke Taylor
+ */
+public class RequestWrapperTests {
+    private static Map<String, String> testPaths = new LinkedHashMap<String,String>();
+
+    @BeforeClass
+    // Some of these may be unrealistic values, but we can't be sure because of the
+    // inconsistency in the spec.
+    public static void createTestMap() {
+        testPaths.put("/path1;x=y;z=w/path2;x=y/path3;x=y", "/path1/path2/path3");
+        testPaths.put("/path1;x=y/path2;x=y/", "/path1/path2/");
+        testPaths.put("/path1//path2/", "/path1/path2/");
+        testPaths.put("//path1/path2//", "/path1/path2/");
+        testPaths.put(";x=y;z=w", "");
+    }
+
+    @Test
+    public void pathParametersAreRemovedFromServletPath() {
+        MockHttpServletRequest request = new MockHttpServletRequest();
+
+        for (Map.Entry<String,String> entry : testPaths.entrySet()) {
+            String path = entry.getKey();
+            String expectedResult = entry.getValue();
+            request.setServletPath(path);
+            RequestWrapper wrapper = new RequestWrapper(request);
+            assertEquals(expectedResult, wrapper.getServletPath());
+            wrapper.reset();
+            assertEquals(path, wrapper.getServletPath());
+        }
+    }
+
+    @Test
+    public void pathParametersAreRemovedFromPathInfo() {
+        MockHttpServletRequest request = new MockHttpServletRequest();
+
+        for (Map.Entry<String,String> entry : testPaths.entrySet()) {
+            String path = entry.getKey();
+            String expectedResult = entry.getValue();
+            // Should be null when stripped value is empty
+            if (expectedResult.length() == 0) {
+                expectedResult = null;
+            }
+            request.setPathInfo(path);
+            RequestWrapper wrapper = new RequestWrapper(request);
+            assertEquals(expectedResult, wrapper.getPathInfo());
+            wrapper.reset();
+            assertEquals(path, wrapper.getPathInfo());
+        }
+    }
+
+}