SEC-342: Change ObjectDefinitionSource to return a Collection instead of an Iterator.

core/src/main/java/org/springframework/security/intercept/AbstractSecurityInterceptor.java

@@ -52,6 +52,7 @@ import org.springframework.util.Assert;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.Set;
+import java.util.Collection;
 
 /**
  * Abstract class that implements security interception for secure objects.
@@ -212,14 +213,15 @@ public abstract class AbstractSecurityInterceptor implements InitializingBean, A
         }
 
         if (this.validateConfigAttributes) {
-            Iterator iter = this.obtainObjectDefinitionSource().getConfigAttributeDefinitions();
+            Collection attributeDefs = this.obtainObjectDefinitionSource().getConfigAttributeDefinitions();
 
-            if (iter == null) {
+            if (attributeDefs == null) {
                 logger.warn("Could not validate configuration attributes as the ObjectDefinitionSource did not return "
                         + "a ConfigAttributeDefinition Iterator");
                 return;
             }
 
+            Iterator iter = attributeDefs.iterator();
             Set unsupportedAttrs = new HashSet();
 
             while (iter.hasNext()) {

core/src/main/java/org/springframework/security/intercept/ObjectDefinitionSource.java

@@ -18,6 +18,7 @@ package org.springframework.security.intercept;
 import org.springframework.security.ConfigAttributeDefinition;
 
 import java.util.Iterator;
+import java.util.Collection;
 
 
 /**
@@ -42,17 +43,17 @@ public interface ObjectDefinitionSource {
      * @throws IllegalArgumentException if the passed object is not of a type supported by the
      *         <code>ObjectDefinitionSource</code> implementation
      */
-    ConfigAttributeDefinition getAttributes(Object object)
+    ConfigAttributeDefinition getAttributes(Object object) throws IllegalArgumentException;
-        throws IllegalArgumentException;
 
     /**
-     * If available, all of the <code>ConfigAttributeDefinition</code>s defined by the implementing class.<P>This
+     * If available, returns all of the <code>ConfigAttributeDefinition</code>s defined by the implementing class.
-     * is used by the {@link AbstractSecurityInterceptor} to perform startup time validation of each
+     * <p>
-     * <code>ConfigAttribute</code> configured against it.</p>
+     * This is used by the {@link AbstractSecurityInterceptor} to perform startup time validation of each
+     * <code>ConfigAttribute</code> configured against it.
      *
-     * @return an iterator over all the <code>ConfigAttributeDefinition</code>s or <code>null</code> if unsupported
+     * @return the <code>ConfigAttributeDefinition</code>s or <code>null</code> if unsupported
      */
-    Iterator getConfigAttributeDefinitions();
+    Collection getConfigAttributeDefinitions();
 
     /**
      * Indicates whether the <code>ObjectDefinitionSource</code> implementation is able to provide

core/src/main/java/org/springframework/security/intercept/method/MethodDefinitionAttributes.java

@@ -103,7 +103,7 @@ public class MethodDefinitionAttributes extends AbstractMethodDefinitionSource {
         }
     }
 
-    public Iterator getConfigAttributeDefinitions() {
+    public Collection getConfigAttributeDefinitions() {
         return null;
     }
 

core/src/main/java/org/springframework/security/intercept/method/MethodDefinitionMap.java

@@ -16,7 +16,6 @@
 package org.springframework.security.intercept.method;
 
 import org.springframework.security.ConfigAttributeDefinition;
-import org.springframework.security.SecurityConfig;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -28,6 +27,8 @@ import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Collection;
+import java.util.Collections;
 
 
 /**
@@ -177,8 +178,8 @@ public class MethodDefinitionMap extends AbstractMethodDefinitionSource {
      *
      * @return the attributes explicitly defined against this bean
      */
-    public Iterator getConfigAttributeDefinitions() {
+    public Collection getConfigAttributeDefinitions() {
-        return methodMap.values().iterator();
+        return Collections.unmodifiableCollection(methodMap.values());
     }
 
     /**

core/src/main/java/org/springframework/security/intercept/web/DefaultFilterInvocationDefinitionSource.java

@@ -16,7 +16,6 @@
 package org.springframework.security.intercept.web;
 
 import org.springframework.security.ConfigAttributeDefinition;
-import org.springframework.security.SecurityConfig;
 import org.springframework.security.util.UrlMatcher;
 
 import org.apache.commons.logging.Log;
@@ -29,7 +28,8 @@ import java.util.HashMap;
 import java.util.Set;
 import java.util.HashSet;
 import java.util.Arrays;
-import java.util.List;
+import java.util.Collection;
+import java.util.Collections;
 
 
 /**
@@ -136,8 +136,8 @@ public class DefaultFilterInvocationDefinitionSource implements FilterInvocation
         return methodRequestmap;
     }
 
-    public Iterator getConfigAttributeDefinitions() {
+    public Collection getConfigAttributeDefinitions() {
-        return getRequestMap().values().iterator();
+        return Collections.unmodifiableCollection(getRequestMap().values());
     }
 
     public ConfigAttributeDefinition getAttributes(Object object) throws IllegalArgumentException {

core/src/main/java/org/springframework/security/securechannel/ChannelProcessingFilter.java

@@ -35,6 +35,7 @@ import java.io.IOException;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.Set;
+import java.util.Collection;
 
 
 /**
@@ -66,9 +67,9 @@ public class ChannelProcessingFilter extends SpringSecurityFilter implements Ini
         Assert.notNull(filterInvocationDefinitionSource, "filterInvocationDefinitionSource must be specified");
         Assert.notNull(channelDecisionManager, "channelDecisionManager must be specified");
 
-        Iterator iter = this.filterInvocationDefinitionSource.getConfigAttributeDefinitions();
+        Collection attrDefs = this.filterInvocationDefinitionSource.getConfigAttributeDefinitions();
 
-        if (iter == null) {
+        if (attrDefs == null) {
             if (logger.isWarnEnabled()) {
                 logger.warn("Could not validate configuration attributes as the FilterInvocationDefinitionSource did "
                         + "not return a ConfigAttributeDefinition Iterator");
@@ -77,6 +78,7 @@ public class ChannelProcessingFilter extends SpringSecurityFilter implements Ini
             return;
         }
 
+        Iterator iter = attrDefs.iterator();
         Set set = new HashSet();
 
         while (iter.hasNext()) {

core/src/test/java/org/springframework/security/intercept/method/MethodDefinitionSourceEditorTests.java

@@ -137,7 +137,7 @@ public class MethodDefinitionSourceEditorTests extends TestCase {
             "org.springframework.security.TargetObject.countLength=ROLE_ONE,ROLE_TWO,RUN_AS_ENTRY\r\norg.springframework.security.TargetObject.make*=ROLE_NINE,ROLE_SUPERVISOR");
 
         MethodDefinitionMap map = (MethodDefinitionMap) editor.getValue();
-        Iterator iter = map.getConfigAttributeDefinitions();
+        Iterator iter = map.getConfigAttributeDefinitions().iterator();
         int counter = 0;
 
         while (iter.hasNext()) {

core/src/test/java/org/springframework/security/intercept/method/MockMethodDefinitionSource.java

@@ -16,13 +16,12 @@
 package org.springframework.security.intercept.method;
 
 import org.springframework.security.ConfigAttributeDefinition;
-import org.springframework.security.SecurityConfig;
 
 import java.lang.reflect.Method;
 
-import java.util.Iterator;
 import java.util.List;
 import java.util.Vector;
+import java.util.Collection;
 
 
 /**
@@ -34,12 +33,12 @@ public class MockMethodDefinitionSource extends AbstractMethodDefinitionSource {
     //~ Instance fields ================================================================================================
 
     private List list;
-    private boolean returnAnIterator;
+    private boolean returnACollection;
 
     //~ Constructors ===================================================================================================
 
-    public MockMethodDefinitionSource(boolean includeInvalidAttributes, boolean returnAnIteratorWhenRequested) {
+    public MockMethodDefinitionSource(boolean includeInvalidAttributes, boolean returnACollectionWhenRequested) {
-        returnAnIterator = returnAnIteratorWhenRequested;
+        returnACollection = returnACollectionWhenRequested;
         list = new Vector();
 
         ConfigAttributeDefinition def1 = new ConfigAttributeDefinition("MOCK_LOWER");
@@ -61,9 +60,9 @@ public class MockMethodDefinitionSource extends AbstractMethodDefinitionSource {
 
     //~ Methods ========================================================================================================
 
-    public Iterator getConfigAttributeDefinitions() {
+    public Collection getConfigAttributeDefinitions() {
-        if (returnAnIterator) {
+        if (returnACollection) {
-            return list.iterator();
+            return list;
         } else {
             return null;
         }

core/src/test/java/org/springframework/security/intercept/method/aopalliance/MethodSecurityInterceptorTests.java

@@ -48,7 +48,7 @@ import org.springframework.context.support.ClassPathXmlApplicationContext;
 
 import java.lang.reflect.Method;
 
-import java.util.Iterator;
+import java.util.Collection;
 
 
 /**
@@ -447,7 +447,7 @@ public class MethodSecurityInterceptorTests extends TestCase {
     }
 
     private class MockObjectDefinitionSourceWhichOnlySupportsStrings extends AbstractMethodDefinitionSource {
-        public Iterator getConfigAttributeDefinitions() {
+        public Collection getConfigAttributeDefinitions() {
             return null;
         }
 

core/src/test/java/org/springframework/security/intercept/web/FilterInvocationDefinitionSourceEditorTests.java

@@ -145,7 +145,7 @@ public class FilterInvocationDefinitionSourceEditorTests extends TestCase {
         editor.setAsText("\\A/secure/super.*\\Z=ROLE_WE_DONT_HAVE\r\n\\A/secure/.*\\Z=ROLE_SUPERVISOR,ROLE_TELLER");
 
         DefaultFilterInvocationDefinitionSource map = (DefaultFilterInvocationDefinitionSource) editor.getValue();
-        Iterator iter = map.getConfigAttributeDefinitions();
+        Iterator iter = map.getConfigAttributeDefinitions().iterator();
         int counter = 0;
 
         while (iter.hasNext()) {

core/src/test/java/org/springframework/security/intercept/web/FilterSecurityInterceptorTests.java

@@ -29,7 +29,6 @@ import org.springframework.security.MockApplicationContext;
 import org.springframework.security.MockAuthenticationManager;
 import org.springframework.security.MockRunAsManager;
 import org.springframework.security.RunAsManager;
-import org.springframework.security.SecurityConfig;
 import org.springframework.security.util.AntUrlPathMatcher;
 import org.springframework.security.util.RegexUrlPathMatcher;
 import org.springframework.security.context.SecurityContextHolder;
@@ -39,10 +38,7 @@ import org.springframework.mock.web.MockHttpServletResponse;
 
 import java.io.IOException;
 
-import java.util.ArrayList;
+import java.util.Collection;
-import java.util.Iterator;
-import java.util.List;
-import java.util.LinkedHashMap;
 
 import javax.servlet.FilterChain;
 import javax.servlet.ServletException;
@@ -281,7 +277,7 @@ public class FilterSecurityInterceptorTests extends TestCase {
             }
         }
 
-        public Iterator getConfigAttributeDefinitions() {
+        public Collection getConfigAttributeDefinitions() {
             return null;
         }
 

core/src/test/java/org/springframework/security/intercept/web/MockFilterInvocationDefinitionSource.java

@@ -16,12 +16,11 @@
 package org.springframework.security.intercept.web;
 
 import org.springframework.security.ConfigAttributeDefinition;
-import org.springframework.security.SecurityConfig;
 import org.springframework.security.util.AntUrlPathMatcher;
 
-import java.util.Iterator;
 import java.util.List;
 import java.util.Vector;
+import java.util.Collection;
 
 
 /**
@@ -62,9 +61,9 @@ public class MockFilterInvocationDefinitionSource extends DefaultFilterInvocatio
 
     //~ Methods ========================================================================================================
 
-    public Iterator getConfigAttributeDefinitions() {
+    public Collection getConfigAttributeDefinitions() {
         if (returnAnIterator) {
-            return list.iterator();
+            return list;
         } else {
             return null;
         }

core/src/test/java/org/springframework/security/securechannel/ChannelProcessingFilterTests.java

@@ -19,7 +19,6 @@ import junit.framework.TestCase;
 
 import org.springframework.security.ConfigAttribute;
 import org.springframework.security.ConfigAttributeDefinition;
-import org.springframework.security.SecurityConfig;
 
 import org.springframework.security.intercept.web.FilterInvocation;
 import org.springframework.security.intercept.web.FilterInvocationDefinitionSource;
@@ -29,9 +28,9 @@ import org.springframework.mock.web.MockHttpServletResponse;
 
 import java.io.IOException;
 
-import java.util.Iterator;
 import java.util.List;
 import java.util.Vector;
+import java.util.Collection;
 
 import javax.servlet.FilterChain;
 import javax.servlet.ServletException;
@@ -91,8 +90,7 @@ public class ChannelProcessingFilterTests extends TestCase {
         assertTrue(true);
     }
 
-    public void testDetectsUnsupportedConfigAttribute()
+    public void testDetectsUnsupportedConfigAttribute() throws Exception {
-        throws Exception {
         ChannelProcessingFilter filter = new ChannelProcessingFilter();
         filter.setChannelDecisionManager(new MockChannelDecisionManager(false, "SUPPORTS_MOCK_ONLY"));
 
@@ -109,8 +107,7 @@ public class ChannelProcessingFilterTests extends TestCase {
         }
     }
 
-    public void testDoFilterWhenManagerDoesCommitResponse()
+    public void testDoFilterWhenManagerDoesCommitResponse() throws Exception {
-        throws Exception {
         ChannelProcessingFilter filter = new ChannelProcessingFilter();
         filter.setChannelDecisionManager(new MockChannelDecisionManager(true, "SOME_ATTRIBUTE"));
 
@@ -131,8 +128,7 @@ public class ChannelProcessingFilterTests extends TestCase {
         assertTrue(true);
     }
 
-    public void testDoFilterWhenManagerDoesNotCommitResponse()
+    public void testDoFilterWhenManagerDoesNotCommitResponse() throws Exception {
-        throws Exception {
         ChannelProcessingFilter filter = new ChannelProcessingFilter();
         filter.setChannelDecisionManager(new MockChannelDecisionManager(false, "SOME_ATTRIBUTE"));
 
@@ -175,8 +171,7 @@ public class ChannelProcessingFilterTests extends TestCase {
         assertTrue(true);
     }
 
-    public void testDoFilterWithNonHttpServletRequestDetected()
+    public void testDoFilterWithNonHttpServletRequestDetected() throws Exception {
-        throws Exception {
         ChannelProcessingFilter filter = new ChannelProcessingFilter();
 
         try {
@@ -186,8 +181,7 @@ public class ChannelProcessingFilterTests extends TestCase {
         }
     }
 
-    public void testDoFilterWithNonHttpServletResponseDetected()
+    public void testDoFilterWithNonHttpServletResponseDetected() throws Exception {
-        throws Exception {
         ChannelProcessingFilter filter = new ChannelProcessingFilter();
 
         try {
@@ -293,7 +287,7 @@ public class ChannelProcessingFilterTests extends TestCase {
             }
         }
 
-        public Iterator getConfigAttributeDefinitions() {
+        public Collection getConfigAttributeDefinitions() {
             if (!provideIterator) {
                 return null;
             }
@@ -301,7 +295,7 @@ public class ChannelProcessingFilterTests extends TestCase {
             List list = new Vector();
             list.add(toReturn);
 
-            return list.iterator();
+            return list;
         }
 
         public boolean supports(Class clazz) {