SEC-161: Truncate everything after ? in URL.

core/src/main/java/org/acegisecurity/intercept/web/PathBasedFilterInvocationDefinitionMap.java

@@ -1,4 +1,4 @@
-/* Copyright 2004, 2005 Acegi Technology Pty Limited
+/* Copyright 2004, 2005, 2006 Acegi Technology Pty Limited
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -20,8 +20,8 @@ import org.acegisecurity.ConfigAttributeDefinition;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
-import org.springframework.util.PathMatcher;
 import org.springframework.util.AntPathMatcher;
+import org.springframework.util.PathMatcher;
 
 import java.util.HashSet;
 import java.util.Iterator;
@@ -65,11 +65,19 @@ public class PathBasedFilterInvocationDefinitionMap
     //~ Instance fields ========================================================
 
     private List requestMap = new Vector();
-    private boolean convertUrlToLowercaseBeforeComparison = false;
     private PathMatcher pathMatcher = new AntPathMatcher();
+    private boolean convertUrlToLowercaseBeforeComparison = false;
 
     //~ Methods ================================================================
 
+    public void addSecureUrl(String antPath, ConfigAttributeDefinition attr) {
+        requestMap.add(new EntryHolder(antPath, attr));
+
+        if (logger.isDebugEnabled()) {
+            logger.debug("Added Ant path: " + antPath + "; attributes: " + attr);
+        }
+    }
+
     public Iterator getConfigAttributeDefinitions() {
         Set set = new HashSet();
         Iterator iter = requestMap.iterator();
@@ -82,29 +90,21 @@ public class PathBasedFilterInvocationDefinitionMap
         return set.iterator();
     }
 
-    public void setConvertUrlToLowercaseBeforeComparison(
+    public int getMapSize() {
-        boolean convertUrlToLowercaseBeforeComparison) {
+        return this.requestMap.size();
-        this.convertUrlToLowercaseBeforeComparison = convertUrlToLowercaseBeforeComparison;
     }
 
     public boolean isConvertUrlToLowercaseBeforeComparison() {
         return convertUrlToLowercaseBeforeComparison;
     }
 
-    public int getMapSize() {
+    public ConfigAttributeDefinition lookupAttributes(String url) {
-        return this.requestMap.size();
+        // Strip anything after a question mark symbol, as per SEC-161.
-    }
+        int firstQuestionMarkIndex = url.lastIndexOf("?");
-
-    public void addSecureUrl(String antPath, ConfigAttributeDefinition attr) {
-        requestMap.add(new EntryHolder(antPath, attr));
 
-        if (logger.isDebugEnabled()) {
+        if (firstQuestionMarkIndex != -1) {
-            logger.debug("Added Ant path: " + antPath + "; attributes: " + attr);
+            url = url.substring(0, firstQuestionMarkIndex);
         }
-    }
-
-    public ConfigAttributeDefinition lookupAttributes(String url) {
-        Iterator iter = requestMap.iterator();
 
         if (convertUrlToLowercaseBeforeComparison) {
             url = url.toLowerCase();
@@ -115,6 +115,8 @@ public class PathBasedFilterInvocationDefinitionMap
             }
         }
 
+        Iterator iter = requestMap.iterator();
+
         while (iter.hasNext()) {
             EntryHolder entryHolder = (EntryHolder) iter.next();
 
@@ -133,6 +135,11 @@ public class PathBasedFilterInvocationDefinitionMap
         return null;
     }
 
+    public void setConvertUrlToLowercaseBeforeComparison(
+        boolean convertUrlToLowercaseBeforeComparison) {
+        this.convertUrlToLowercaseBeforeComparison = convertUrlToLowercaseBeforeComparison;
+    }
+
     //~ Inner Classes ==========================================================
 
     protected class EntryHolder {

core/src/test/java/org/acegisecurity/intercept/web/PathBasedFilterDefinitionMapTests.java

@@ -1,4 +1,4 @@
-/* Copyright 2004 Acegi Technology Pty Limited
+/* Copyright 2004, 2005, 2006 Acegi Technology Pty Limited
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -19,8 +19,6 @@ import junit.framework.TestCase;
 
 import org.acegisecurity.ConfigAttributeDefinition;
 import org.acegisecurity.MockFilterChain;
-
-
 import org.acegisecurity.SecurityConfig;
 
 import org.springframework.mock.web.MockHttpServletRequest;
@@ -47,14 +45,14 @@ public class PathBasedFilterDefinitionMapTests extends TestCase {
 
     //~ Methods ================================================================
 
-    public final void setUp() throws Exception {
-        super.setUp();
-    }
-
     public static void main(String[] args) {
         junit.textui.TestRunner.run(PathBasedFilterDefinitionMapTests.class);
     }
 
+    public final void setUp() throws Exception {
+        super.setUp();
+    }
+
     public void testConvertUrlToLowercaseIsFalseByDefault() {
         PathBasedFilterInvocationDefinitionMap map = new PathBasedFilterInvocationDefinitionMap();
         assertFalse(map.isConvertUrlToLowercaseBeforeComparison());
@@ -78,6 +76,7 @@ public class PathBasedFilterDefinitionMapTests extends TestCase {
         // Build a HTTP request
         MockHttpServletRequest request = new MockHttpServletRequest();
         request.setRequestURI(null);
+
         MockHttpServletRequest req = request;
         req.setServletPath("/SeCuRE/super/somefile.html");
 
@@ -100,6 +99,7 @@ public class PathBasedFilterDefinitionMapTests extends TestCase {
         // Build a HTTP request
         MockHttpServletRequest request = new MockHttpServletRequest();
         request.setRequestURI(null);
+
         MockHttpServletRequest req = request;
         req.setServletPath("/SeCuRE/super/somefile.html");
 
@@ -122,6 +122,7 @@ public class PathBasedFilterDefinitionMapTests extends TestCase {
         // Build a HTTP request
         MockHttpServletRequest request = new MockHttpServletRequest();
         request.setRequestURI(null);
+
         MockHttpServletRequest req = request;
         req.setServletPath("/secure/super/somefile.html");
 
@@ -132,4 +133,27 @@ public class PathBasedFilterDefinitionMapTests extends TestCase {
                 .getRequestUrl());
         assertEquals(def, response);
     }
+
+    public void testLookupRequiringExactMatchWithAdditionalSlashesIsSuccessful() {
+        PathBasedFilterInvocationDefinitionMap map = new PathBasedFilterInvocationDefinitionMap();
+        assertFalse(map.isConvertUrlToLowercaseBeforeComparison());
+
+        ConfigAttributeDefinition def = new ConfigAttributeDefinition();
+        def.addConfigAttribute(new SecurityConfig("ROLE_ONE"));
+        map.addSecureUrl("/someAdminPage.html**", def);
+
+        // Build a HTTP request
+        MockHttpServletRequest request = new MockHttpServletRequest();
+        request.setRequestURI(null);
+
+        MockHttpServletRequest req = request;
+        req.setServletPath("/someAdminPage.html?a=/test");
+
+        FilterInvocation fi = new FilterInvocation(req,
+                new MockHttpServletResponse(), new MockFilterChain());
+
+        ConfigAttributeDefinition response = map.lookupAttributes(fi
+                .getRequestUrl());
+        assertEquals(def, response); // see SEC-161 (it should truncate after ? sign)
+    }
 }