From 021abb73694c5bbc68dc71c37f06f0ed1db00227 Mon Sep 17 00:00:00 2001
From: Luke Taylor <luke.taylor@springsource.com>
Date: Sat, 9 Apr 2005 22:50:06 +0000
Subject: [PATCH] Added check for "path parameters" to ensure the
 filterProcessesUrl matches rewritten URLs with a jsessionid included.
 Refactored property checking to use Spring Assert class.

---
 .../ui/AbstractProcessingFilter.java          | 40 ++++++++-----------
 .../ui/AbstractProcessingFilterTests.java     | 17 +++++++-
 2 files changed, 33 insertions(+), 24 deletions(-)

diff --git a/core/src/main/java/org/acegisecurity/ui/AbstractProcessingFilter.java b/core/src/main/java/org/acegisecurity/ui/AbstractProcessingFilter.java
index 17f785f7b3..7df86dc775 100644
--- a/core/src/main/java/org/acegisecurity/ui/AbstractProcessingFilter.java
+++ b/core/src/main/java/org/acegisecurity/ui/AbstractProcessingFilter.java
@@ -249,27 +249,10 @@ public abstract class AbstractProcessingFilter implements Filter,
     }
 
     public void afterPropertiesSet() throws Exception {
-        if ((filterProcessesUrl == null) || "".equals(filterProcessesUrl)) {
-            throw new IllegalArgumentException(
-                "filterProcessesUrl must be specified");
-        }
-
-        if ((defaultTargetUrl == null) || "".equals(defaultTargetUrl)) {
-            throw new IllegalArgumentException(
-                "defaultTargetUrl must be specified");
-        }
-
-        if ((authenticationFailureUrl == null)
-            || "".equals(authenticationFailureUrl)) {
-            throw new IllegalArgumentException(
-                "authenticationFailureUrl must be specified");
-        }
-
-        if (authenticationManager == null) {
-            throw new IllegalArgumentException(
-                "authenticationManager must be specified");
-        }
-
+        Assert.hasLength(filterProcessesUrl, "filterProcessesUrl must be specified");
+        Assert.hasLength(defaultTargetUrl, "defaultTargetUrl must be specified");
+        Assert.hasLength(authenticationFailureUrl, "authenticationFailureUrl must be specified");
+        Assert.notNull(authenticationManager, "authenticationManager must be specified");
         Assert.notNull(this.rememberMeServices);
     }
 
@@ -346,6 +329,10 @@ public abstract class AbstractProcessingFilter implements Filter,
      * Indicates whether this filter should attempt to process a login request
      * for the current invocation.
      * </p>
+     * <p>
+     * It strips any parameters from the "path" section of the request URL (such as the
+     * jsessionid parameter in <em>http://host/myapp/index.html;jsessionid=blah</em>)
+     * before matching against the <code>filterProcessesUrl</code> property.
      * 
      * <p>
      * Subclasses may override for special requirements, such as Tapestry
@@ -360,8 +347,15 @@ public abstract class AbstractProcessingFilter implements Filter,
      */
     protected boolean requiresAuthentication(HttpServletRequest request,
         HttpServletResponse response) {
-        return request.getRequestURL().toString().endsWith(request
-            .getContextPath() + filterProcessesUrl);
+        String uri = request.getRequestURI();
+        int pathParamIndex = uri.indexOf(';');
+
+        if(pathParamIndex > 0) {
+            // strip everything after the first semi-colon
+            uri = uri.substring(0, pathParamIndex);
+        }
+
+        return uri.endsWith(request.getContextPath() + filterProcessesUrl);
     }
 
     protected void successfulAuthentication(HttpServletRequest request,
diff --git a/core/src/test/java/org/acegisecurity/ui/AbstractProcessingFilterTests.java b/core/src/test/java/org/acegisecurity/ui/AbstractProcessingFilterTests.java
index 5f87c73a48..8705ebe443 100644
--- a/core/src/test/java/org/acegisecurity/ui/AbstractProcessingFilterTests.java
+++ b/core/src/test/java/org/acegisecurity/ui/AbstractProcessingFilterTests.java
@@ -41,6 +41,7 @@ import javax.servlet.ServletException;
 import javax.servlet.ServletRequest;
 import javax.servlet.ServletResponse;
 import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
 import java.io.IOException;
 import java.util.Properties;
 
@@ -242,6 +243,16 @@ public class AbstractProcessingFilterTests extends TestCase {
                               .getPrincipal().toString());
     }
 
+    public void testDefaultProcessesFilterUrlWithPathParameter() {
+        MockHttpServletRequest request = createMockRequest();
+        MockHttpServletResponse response = new MockHttpServletResponse();
+        MockAbstractProcessingFilter filter = new MockAbstractProcessingFilter();
+        filter.setFilterProcessesUrl("/j_acegi_security_check");        
+
+        request.setRequestURI("/mycontext/j_acegi_security_check;jsessionid=I8MIONOSTHOR");
+        assertTrue(filter.requiresAuthentication(request, response));
+    }
+
     public void testStartupDetectsInvalidAuthenticationFailureUrl()
         throws Exception {
         AbstractProcessingFilter filter = new MockAbstractProcessingFilter();
@@ -307,7 +318,7 @@ public class AbstractProcessingFilterTests extends TestCase {
         }
     }
 
-    public void testSuccessLoginThenFailureLoginResultsInSessionLoosingToken()
+    public void testSuccessLoginThenFailureLoginResultsInSessionLosingToken()
         throws Exception {
         // Setup our HTTP request
         MockHttpServletRequest request = createMockRequest();
@@ -451,6 +462,10 @@ public class AbstractProcessingFilterTests extends TestCase {
             this.exceptionToThrow = exceptionToThrow;
         }
 
+        public boolean requiresAuthentication(HttpServletRequest request, HttpServletResponse response) {
+            return super.requiresAuthentication(request, response);
+        }
+
         private MockAbstractProcessingFilter() {
             super();
         }