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);
}
