From 6fe00b34338f8576b253da5e0635e48dd293cd9f Mon Sep 17 00:00:00 2001
From: Luke Taylor <luke.taylor@springsource.com>
Date: Tue, 28 Aug 2007 16:53:05 +0000
Subject: [PATCH] SEC-501: Fix. Convert secure url paths to lower case if
 convertUrlToLowercaseBeforeComparison is true.

Also removed unnecessary assertions from PathBasedFilterDefinitionMapTests.
---
 ...athBasedFilterInvocationDefinitionMap.java | 14 +++++---
 .../PathBasedFilterDefinitionMapTests.java    | 36 ++++++++++++-------
 2 files changed, 33 insertions(+), 17 deletions(-)

diff --git a/core/src/main/java/org/acegisecurity/intercept/web/PathBasedFilterInvocationDefinitionMap.java b/core/src/main/java/org/acegisecurity/intercept/web/PathBasedFilterInvocationDefinitionMap.java
index 1615f2760b..f54aa44127 100644
--- a/core/src/main/java/org/acegisecurity/intercept/web/PathBasedFilterInvocationDefinitionMap.java
+++ b/core/src/main/java/org/acegisecurity/intercept/web/PathBasedFilterInvocationDefinitionMap.java
@@ -34,17 +34,17 @@ import java.util.Vector;
  * Maintains a <code>List</code> of <code>ConfigAttributeDefinition</code>s associated with different HTTP request
  * URL Apache Ant path-based patterns.<p>Apache Ant path expressions are used to match a HTTP request URL against a
  * <code>ConfigAttributeDefinition</code>.</p>
- *  <p>The order of registering the Ant paths using the {@link #addSecureUrl(String, ConfigAttributeDefinition)} is
+ * <p>The order of registering the Ant paths using the {@link #addSecureUrl(String,ConfigAttributeDefinition)} is
  * very important. The system will identify the <b>first</b>  matching path for a given HTTP URL. It will not proceed
  * to evaluate later paths if a match has already been found. Accordingly, the most specific paths should be
  * registered first, with the most general paths registered last.</p>
- *  <p>If no registered paths match the HTTP URL, <code>null</code> is returned.</p>
+ * <p>If no registered paths match the HTTP URL, <code>null</code> is returned.</p>
  *
  * @author Ben Alex
  * @version $Id$
  */
 public class PathBasedFilterInvocationDefinitionMap extends AbstractFilterInvocationDefinitionSource
-    implements FilterInvocationDefinition {
+        implements FilterInvocationDefinition {
     //~ Static fields/initializers =====================================================================================
 
     private static final Log logger = LogFactory.getLog(PathBasedFilterInvocationDefinitionMap.class);
@@ -58,6 +58,12 @@ public class PathBasedFilterInvocationDefinitionMap extends AbstractFilterInvoca
     //~ Methods ========================================================================================================
 
     public void addSecureUrl(String antPath, ConfigAttributeDefinition attr) {
+        // SEC-501: If using lower case comparison, we should convert the paths to lower case
+        // as any upper case characters included by mistake will prevent the URL from ever being matched.
+        if (convertUrlToLowercaseBeforeComparison) {
+            antPath = antPath.toLowerCase();
+        }
+
         requestMap.add(new EntryHolder(antPath, attr));
 
         if (logger.isDebugEnabled()) {
@@ -110,7 +116,7 @@ public class PathBasedFilterInvocationDefinitionMap extends AbstractFilterInvoca
 
             if (logger.isDebugEnabled()) {
                 logger.debug("Candidate is: '" + url + "'; pattern is " + entryHolder.getAntPath() + "; matched="
-                    + matched);
+                        + matched);
             }
 
             if (matched) {
diff --git a/core/src/test/java/org/acegisecurity/intercept/web/PathBasedFilterDefinitionMapTests.java b/core/src/test/java/org/acegisecurity/intercept/web/PathBasedFilterDefinitionMapTests.java
index 37aabbb846..af30f6f08e 100644
--- a/core/src/test/java/org/acegisecurity/intercept/web/PathBasedFilterDefinitionMapTests.java
+++ b/core/src/test/java/org/acegisecurity/intercept/web/PathBasedFilterDefinitionMapTests.java
@@ -36,7 +36,6 @@ public class PathBasedFilterDefinitionMapTests extends TestCase {
     //~ Constructors ===================================================================================================
 
     public PathBasedFilterDefinitionMapTests() {
-        super();
     }
 
     public PathBasedFilterDefinitionMapTests(String arg0) {
@@ -59,7 +58,6 @@ public class PathBasedFilterDefinitionMapTests extends TestCase {
     public void testLookupNotRequiringExactMatchSuccessIfNotMatching() {
         PathBasedFilterInvocationDefinitionMap map = new PathBasedFilterInvocationDefinitionMap();
         map.setConvertUrlToLowercaseBeforeComparison(true);
-        assertTrue(map.isConvertUrlToLowercaseBeforeComparison());
 
         ConfigAttributeDefinition def = new ConfigAttributeDefinition();
         def.addConfigAttribute(new SecurityConfig("ROLE_ONE"));
@@ -71,10 +69,26 @@ public class PathBasedFilterDefinitionMapTests extends TestCase {
         assertEquals(def, response);
     }
 
+    /**
+     * SEC-501
+     */
+    public void testLookupNotRequiringExactMatchSucceedsIfSecureUrlPathContainsUpperCase() {
+        PathBasedFilterInvocationDefinitionMap map = new PathBasedFilterInvocationDefinitionMap();
+        map.setConvertUrlToLowercaseBeforeComparison(true);
+
+        ConfigAttributeDefinition def = new ConfigAttributeDefinition();
+        def.addConfigAttribute(new SecurityConfig("ROLE_ONE"));
+        map.addSecureUrl("/SeCuRE/super/**", def);
+
+        FilterInvocation fi = createFilterinvocation("/secure/super/somefile.html");
+
+        ConfigAttributeDefinition response = map.lookupAttributes(fi.getRequestUrl());
+        assertEquals(def, response);
+    }
+
+
     public void testLookupRequiringExactMatchFailsIfNotMatching() {
         PathBasedFilterInvocationDefinitionMap map = new PathBasedFilterInvocationDefinitionMap();
-        assertFalse(map.isConvertUrlToLowercaseBeforeComparison());
-
         ConfigAttributeDefinition def = new ConfigAttributeDefinition();
         def.addConfigAttribute(new SecurityConfig("ROLE_ONE"));
         map.addSecureUrl("/secure/super/**", def);
@@ -87,13 +101,11 @@ public class PathBasedFilterDefinitionMapTests extends TestCase {
 
     public void testLookupRequiringExactMatchIsSuccessful() {
         PathBasedFilterInvocationDefinitionMap map = new PathBasedFilterInvocationDefinitionMap();
-        assertFalse(map.isConvertUrlToLowercaseBeforeComparison());
-
         ConfigAttributeDefinition def = new ConfigAttributeDefinition();
         def.addConfigAttribute(new SecurityConfig("ROLE_ONE"));
-        map.addSecureUrl("/secure/super/**", def);
+        map.addSecureUrl("/SeCurE/super/**", def);
 
-        FilterInvocation fi = createFilterinvocation("/secure/super/somefile.html");
+        FilterInvocation fi = createFilterinvocation("/SeCurE/super/somefile.html");
 
         ConfigAttributeDefinition response = map.lookupAttributes(fi.getRequestUrl());
         assertEquals(def, response);
@@ -101,8 +113,6 @@ public class PathBasedFilterDefinitionMapTests extends TestCase {
 
     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);
@@ -113,11 +123,11 @@ public class PathBasedFilterDefinitionMapTests extends TestCase {
         assertEquals(def, response); // see SEC-161 (it should truncate after ? sign)
     }
 
-    /** Check fixes for SEC-321 */
+    /**
+     * Check fixes for SEC-321
+     */
     public void testExtraQuestionMarkStillMatches() {
         PathBasedFilterInvocationDefinitionMap map = new PathBasedFilterInvocationDefinitionMap();
-        assertFalse(map.isConvertUrlToLowercaseBeforeComparison());
-
         ConfigAttributeDefinition def = new ConfigAttributeDefinition();
         def.addConfigAttribute(new SecurityConfig("ROLE_ONE"));
         map.addSecureUrl("/someAdminPage.html*", def);