shiro-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From lhazlew...@apache.org
Subject svn commit: r1229884 - in /shiro/trunk/web/src: main/java/org/apache/shiro/web/filter/mgt/DefaultFilterChainManager.java test/groovy/org/apache/shiro/web/filter/mgt/DefaultFilterChainManagerTest.groovy
Date Wed, 11 Jan 2012 03:43:35 GMT
Author: lhazlewood
Date: Wed Jan 11 03:43:35 2012
New Revision: 1229884

URL: http://svn.apache.org/viewvc?rev=1229884&view=rev
Log:
SHIRO-205: Filter bracketed config with nested commas now no longer needs to be quoted.  E.g.
the following is now a valid chain definition:  foo, bar[a, b], baz[d, e, f]  (it was previously
required to be: foo, bar["a, b"], baz["d, e, f"] ).  Backwards compatibility is retained by
stripping quoted config if it exists.  Test cases added.

Modified:
    shiro/trunk/web/src/main/java/org/apache/shiro/web/filter/mgt/DefaultFilterChainManager.java
    shiro/trunk/web/src/test/groovy/org/apache/shiro/web/filter/mgt/DefaultFilterChainManagerTest.groovy

Modified: shiro/trunk/web/src/main/java/org/apache/shiro/web/filter/mgt/DefaultFilterChainManager.java
URL: http://svn.apache.org/viewvc/shiro/trunk/web/src/main/java/org/apache/shiro/web/filter/mgt/DefaultFilterChainManager.java?rev=1229884&r1=1229883&r2=1229884&view=diff
==============================================================================
--- shiro/trunk/web/src/main/java/org/apache/shiro/web/filter/mgt/DefaultFilterChainManager.java
(original)
+++ shiro/trunk/web/src/main/java/org/apache/shiro/web/filter/mgt/DefaultFilterChainManager.java
Wed Jan 11 03:43:35 2012
@@ -35,8 +35,6 @@ import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.Set;
 
-import static org.apache.shiro.util.StringUtils.split;
-
 /**
  * Default {@link FilterChainManager} implementation maintaining a map of {@link Filter Filter}
instances
  * (key: filter name, value: Filter) as well as a map of {@link NamedFilterList NamedFilterList}s
created from these
@@ -139,23 +137,95 @@ public class DefaultFilterChainManager i
         //
         //     { "authc", "roles[admin,user]", "perms[file:edit]" }
         //
-        String[] filterTokens = split(chainDefinition);
+        String[] filterTokens = splitChainDefinition(chainDefinition);
 
         //each token is specific to each filter.
         //strip the name and extract any filter-specific config between brackets [ ]
         for (String token : filterTokens) {
-            String[] nameAndConfig = token.split("\\[", 2);
-            String name = nameAndConfig[0];
+            String[] nameConfigPair = toNameConfigPair(token);
+
+            //now we have the filter name, path and (possibly null) path-specific config.
 Let's apply them:
+            addToChain(chainName, nameConfigPair[0], nameConfigPair[1]);
+        }
+    }
+
+    /**
+     * Splits the comma-delimited filter chain definition line into individual filter definition
tokens.
+     * <p/>
+     * Example Input:
+     * <pre>
+     *     foo, bar[baz], blah[x, y]
+     * </pre>
+     * Resulting Output:
+     * <pre>
+     *     output[0] == foo
+     *     output[1] == bar[baz]
+     *     output[2] == blah[x, y]
+     * </pre>
+     * @param chainDefinition the comma-delimited filter chain definition.
+     * @return an array of filter definition tokens
+     * @since 1.2
+     * @see <a href="https://issues.apache.org/jira/browse/SHIRO-205">SHIRO-205</a>
+     */
+    protected String[] splitChainDefinition(String chainDefinition) {
+        return StringUtils.split(chainDefinition, StringUtils.DEFAULT_DELIMITER_CHAR, '[',
']', true, true);
+    }
+
+    /**
+     * Based on the given filter chain definition token (e.g. 'foo' or 'foo[bar, baz]'),
this will return the token
+     * as a name/value pair, removing any brackets as necessary.  Examples:
+     * <table>
+     *     <tr>
+     *         <th>Input</th>
+     *         <th>Result</th>
+     *     </tr>
+     *     <tr>
+     *         <td>{@code foo}</td>
+     *         <td>returned[0] == {@code foo}<br/>returned[1] == {@code null}</td>
+     *     </tr>
+     *     <tr>
+     *         <td>{@code foo[bar, baz]}</td>
+     *         <td>returned[0] == {@code foo}<br/>returned[1] == {@code bar,
baz}</td>
+     *     </tr>
+     * </table>
+     * @param token the filter chain definition token
+     * @return A name/value pair representing the filter name and a (possibly null) config
value.
+     * @throws ConfigurationException if the token cannot be parsed
+     * @since 1.2
+     * @see <a href="https://issues.apache.org/jira/browse/SHIRO-205">SHIRO-205</a>
+     */
+    protected String[] toNameConfigPair(String token) throws ConfigurationException {
+
+        try {
+            String[] pair = token.split("\\[", 2);
+            String name = StringUtils.clean(pair[0]);
+
+            if (name == null) {
+                throw new IllegalArgumentException("Filter name not found for filter chain
definition token: " + token);
+            }
             String config = null;
 
-            if (nameAndConfig.length == 2) {
-                config = nameAndConfig[1];
-                //if there was an open bracket, there was a close bracket, so strip it too:
+            if (pair.length == 2) {
+                config = StringUtils.clean(pair[1]);
+                //if there was an open bracket, it assumed there is a closing bracket, so
strip it too:
                 config = config.substring(0, config.length() - 1);
+
+                //backwards compatibility prior to implmenting SHIRO-205:
+                //prior to SHIRO-205 being implemented, it was common for end-users to quote
the config inside brackets
+                //if that config required commas.  We need to strip those quotes to get to
the interior quoted definition
+                //to ensure any existing quoted definitions still function for end users:
+                if (config.startsWith("\"") && config.endsWith("\"")) {
+                    config = config.substring(1, config.length() - 1);
+                }
+                
+                config = StringUtils.clean(config);
             }
+            
+            return new String[]{name, config};
 
-            //now we have the filter name, path and (possibly null) path-specific config.
 Let's apply them:
-            addToChain(chainName, name, config);
+        } catch (Exception e) {
+            String msg = "Unable to parse filter chain definition token: " + token;
+            throw new ConfigurationException(msg, e);
         }
     }
 

Modified: shiro/trunk/web/src/test/groovy/org/apache/shiro/web/filter/mgt/DefaultFilterChainManagerTest.groovy
URL: http://svn.apache.org/viewvc/shiro/trunk/web/src/test/groovy/org/apache/shiro/web/filter/mgt/DefaultFilterChainManagerTest.groovy?rev=1229884&r1=1229883&r2=1229884&view=diff
==============================================================================
--- shiro/trunk/web/src/test/groovy/org/apache/shiro/web/filter/mgt/DefaultFilterChainManagerTest.groovy
(original)
+++ shiro/trunk/web/src/test/groovy/org/apache/shiro/web/filter/mgt/DefaultFilterChainManagerTest.groovy
Wed Jan 11 03:43:35 2012
@@ -25,25 +25,98 @@ import javax.servlet.ServletContext
 import org.apache.shiro.config.ConfigurationException
 import org.apache.shiro.web.filter.authz.SslFilter
 import org.apache.shiro.web.servlet.ShiroFilter
-import org.junit.Before
-import org.junit.Test
 import static org.easymock.EasyMock.*
-import static org.junit.Assert.*
 
 /**
  * Unit tests for the {@link DefaultFilterChainManager} implementation.
  */
-class DefaultFilterChainManagerTest {
+class DefaultFilterChainManagerTest extends GroovyTestCase {
 
     DefaultFilterChainManager manager;
 
-    @Before
-    public void setUp() {
+    void setUp() {
         this.manager = new DefaultFilterChainManager();
     }
 
-    @Test
-    public void testNewInstanceDefaultFilters() {
+    //SHIRO-205
+    void testToNameConfigPairNoBrackets() {
+        def token = "foo"
+
+        String[] pair = manager.toNameConfigPair(token);
+
+        assertNotNull pair
+        assertEquals 2, pair.length
+        assertEquals "foo", pair[0]
+        assertNull pair[1]
+    }
+
+    //SHIRO-205
+    void testToNameConfigPairWithEmptyBrackets() {
+        def token = "foo[]"
+
+        String[] pair = manager.toNameConfigPair(token);
+
+        assertNotNull pair
+        assertEquals 2, pair.length
+        assertEquals "foo", pair[0]
+        assertNull pair[1]
+    }
+
+    //SHIRO-205
+    void testToNameConfigPairWithPopulatedBrackets() {
+        def token = "foo[bar, baz]"
+
+        String[] pair = manager.toNameConfigPair(token);
+
+        assertNotNull pair
+        assertEquals 2, pair.length
+        assertEquals "foo", pair[0]
+        assertEquals "bar, baz", pair[1]
+    }
+
+    //SHIRO-205 - asserts backwards compatibility before SHIRO-205 was implemented:
+    void testToNameConfigPairWithNestedQuotesInBrackets() {
+        def token = 'roles["guest, admin"]'
+
+        String[] pair = manager.toNameConfigPair(token);
+
+        assertNotNull pair
+        assertEquals 2, pair.length
+        assertEquals "roles", pair[0]
+        assertEquals "guest, admin", pair[1]
+    }
+    
+    //SHIRO-205
+    void testFilterChainConfigWithNestedCommas() {
+        def chain = "a, b[c], d[e, f], g[h, i, j], k"
+
+        String[] tokens = manager.splitChainDefinition(chain);
+        
+        assertNotNull tokens
+        assertEquals 5, tokens.length
+        assertEquals "a", tokens[0]
+        assertEquals "b[c]", tokens[1]
+        assertEquals "d[e, f]", tokens[2]
+        assertEquals "g[h, i, j]", tokens[3]
+        assertEquals "k", tokens[4]
+    }
+
+    //SHIRO-205
+    void testFilterChainConfigWithNestedQuotedCommas() {
+        def chain = "a, b[c], d[e, f], g[h, i, j], k"
+
+        String[] tokens = manager.splitChainDefinition(chain);
+
+        assertNotNull tokens
+        assertEquals 5, tokens.length
+        assertEquals "a", tokens[0]
+        assertEquals "b[c]", tokens[1]
+        assertEquals "d[e, f]", tokens[2]
+        assertEquals "g[h, i, j]", tokens[3]
+        assertEquals "k", tokens[4]
+    }
+
+    void testNewInstanceDefaultFilters() {
         for (DefaultFilter defaultFilter : DefaultFilter.values()) {
             assertNotNull(manager.getFilter(defaultFilter.name()));
         }
@@ -57,8 +130,7 @@ class DefaultFilterChainManagerTest {
         return mock;
     }
 
-    @Test
-    public void testNewInstanceWithFilterConfig() {
+    void testNewInstanceWithFilterConfig() {
         FilterConfig mock = createNiceMockFilterConfig();
         replay(mock);
         this.manager = new DefaultFilterChainManager(mock);
@@ -69,8 +141,7 @@ class DefaultFilterChainManagerTest {
         verify(mock);
     }
 
-    @Test
-    public void testCreateChain() {
+    void testCreateChain() {
         try {
             manager.createChain(null, null);
         } catch (NullPointerException expected) {
@@ -110,15 +181,13 @@ class DefaultFilterChainManagerTest {
         assertEquals(DefaultFilter.perms.getFilterClass(), filter.getClass());
     }
 
-    @Test
-    public void testBeanMethods() {
+    void testBeanMethods() {
         Map<String, Filter> filters = manager.getFilters();
         assertEquals(filters.size(), DefaultFilter.values().length);
         manager.setFilters(filters);
     }
 
-    @Test
-    public void testAddFilter() {
+    void testAddFilter() {
         FilterConfig mockFilterConfig = createNiceMockFilterConfig();
         replay(mockFilterConfig);
         this.manager = new DefaultFilterChainManager(mockFilterConfig);
@@ -129,8 +198,7 @@ class DefaultFilterChainManagerTest {
         verify(mockFilterConfig);
     }
 
-    @Test
-    public void testAddFilterNoInit() {
+    void testAddFilterNoInit() {
         FilterConfig mockFilterConfig = createNiceMockFilterConfig();
         Filter mockFilter = createNiceMock(Filter.class);
 
@@ -146,15 +214,14 @@ class DefaultFilterChainManagerTest {
         verify mockFilterConfig, mockFilter
     }
 
-    public void testAddFilterNoFilterConfig() {
+    void testAddFilterNoFilterConfig() {
         SslFilter filter = new SslFilter();
         manager.addFilter("test", filter);
         assertNotNull manager.filters['test']
         assertSame manager.filters['test'], filter
     }
 
-    @Test
-    public void testAddToChain() {
+    void testAddToChain() {
         FilterConfig mockFilterConfig = createNiceMockFilterConfig();
         replay(mockFilterConfig);
         this.manager = new DefaultFilterChainManager(mockFilterConfig);
@@ -164,16 +231,17 @@ class DefaultFilterChainManagerTest {
 
         try {
             manager.addToChain("test", null);
+            fail "Should have thrown an IllegalArgumentException"
         } catch (IllegalArgumentException expected) {
         }
         try {
             manager.addToChain(null, "testSsl");
+            fail "Should have thrown an IllegalArgumentException"
         } catch (IllegalArgumentException expected) {
         }
     }
 
-    @Test
-    public void testAddToChainNotPathProcessor() {
+    void testAddToChainNotPathProcessor() {
         FilterConfig mockFilterConfig = createNiceMockFilterConfig();
         replay(mockFilterConfig);
         this.manager = new DefaultFilterChainManager(mockFilterConfig);
@@ -183,12 +251,12 @@ class DefaultFilterChainManagerTest {
 
         try {
             manager.addToChain("test", "nonPathProcessor", "dummyConfig");
+            fail "Should have thrown a ConfigurationException"
         } catch (ConfigurationException expected) {
         }
     }
 
-    @Test
-    public void testProxy() {
+    void testProxy() {
         FilterChain mock = createNiceMock(FilterChain.class);
         replay(mock);
         manager.createChain("test", "anon");
@@ -196,11 +264,14 @@ class DefaultFilterChainManagerTest {
         verify(mock);
     }
 
-    @Test(expected = IllegalArgumentException.class)
-    public void testProxyNoChain() {
+    void testProxyNoChain() {
         FilterChain mock = createNiceMock(FilterChain.class);
         replay(mock);
-        this.manager.proxy(mock, "blah");
+        try {
+            this.manager.proxy(mock, "blah");
+            fail "Should have thrown an IllegalArgumentException"
+        } catch (IllegalArgumentException expected) {
+        }
         verify(mock);
     }
 



Mime
View raw message