SEC-2690: String[]->List<String>

Use Collections rather than Arrays since Collections can be immutable.

ldap/src/integration-test/java/org/springframework/security/ldap/SpringSecurityLdapTemplateITests.java

@@ -17,6 +17,7 @@ package org.springframework.security.ldap;
 
 import static org.junit.Assert.*;
 
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
@@ -102,14 +103,14 @@ public class SpringSecurityLdapTemplateITests extends AbstractLdapIntegrationTes
 
 	@Test
     public void testMultiAttributeRetrievalWithNullAttributeNames() {
-        Set<Map<String, String[]>> values =
+        Set<Map<String, List<String>>> values =
                 template.searchForMultipleAttributeValues(
                         "ou=people",
                         "(uid={0})",
                         new String[]{"bob"},
                         null);
         assertEquals(1, values.size());
-        Map<String, String[]> record = (Map<String, String[]>) values.toArray()[0];
+        Map<String, List<String>> record = values.iterator().next();
         assertAttributeValue(record, "uid", "bob");
         assertAttributeValue(record, "objectclass", "top", "person", "organizationalPerson", "inetOrgPerson");
         assertAttributeValue(record, "cn", "Bob Hamilton");
@@ -119,14 +120,14 @@ public class SpringSecurityLdapTemplateITests extends AbstractLdapIntegrationTes
 
     @Test
     public void testMultiAttributeRetrievalWithZeroLengthAttributeNames() {
-        Set<Map<String, String[]>> values =
+        Set<Map<String, List<String>>> values =
                 template.searchForMultipleAttributeValues(
                         "ou=people",
                         "(uid={0})",
                         new String[]{"bob"},
                         new String[0]);
         assertEquals(1, values.size());
-        Map<String, String[]> record = (Map<String, String[]>) values.toArray()[0];
+        Map<String, List<String>> record = values.iterator().next();
         assertAttributeValue(record, "uid", "bob");
         assertAttributeValue(record, "objectclass", "top", "person", "organizationalPerson", "inetOrgPerson");
         assertAttributeValue(record, "cn", "Bob Hamilton");
@@ -136,7 +137,7 @@ public class SpringSecurityLdapTemplateITests extends AbstractLdapIntegrationTes
 
     @Test
     public void testMultiAttributeRetrievalWithSpecifiedAttributeNames() {
-        Set<Map<String, String[]>> values =
+        Set<Map<String, List<String>>> values =
                 template.searchForMultipleAttributeValues(
                         "ou=people",
                         "(uid={0})",
@@ -147,7 +148,7 @@ public class SpringSecurityLdapTemplateITests extends AbstractLdapIntegrationTes
                                 "sn"
                         });
         assertEquals(1, values.size());
-        Map<String, String[]> record = (Map<String, String[]>) values.toArray()[0];
+        Map<String, List<String>> record = values.iterator().next();
         assertAttributeValue(record, "uid", "bob");
         assertAttributeValue(record, "cn", "Bob Hamilton");
         assertAttributeValue(record, "sn", "Hamilton");
@@ -155,11 +156,11 @@ public class SpringSecurityLdapTemplateITests extends AbstractLdapIntegrationTes
         assertFalse(record.containsKey("objectclass"));
     }
 
-    protected void assertAttributeValue(Map<String, String[]> record, String attributeName, String... values) {
+    protected void assertAttributeValue(Map<String, List<String>> record, String attributeName, String... values) {
         assertTrue(record.containsKey(attributeName));
-        assertEquals(values.length, record.get(attributeName).length);
+        assertEquals(values.length, record.get(attributeName).size());
         for (int i = 0; i < values.length; i++) {
-            assertEquals(values[i], record.get(attributeName)[i]);
+            assertEquals(values[i], record.get(attributeName).get(i));
         }
     }
 

ldap/src/integration-test/java/org/springframework/security/ldap/userdetails/NestedLdapAuthoritiesPopulatorTests.java

@@ -104,13 +104,13 @@ public class NestedLdapAuthoritiesPopulatorTests extends AbstractLdapIntegration
         //closure group
         assertTrue(ldapAuthorities[0].getAttributes().containsKey("member"));
         assertNotNull(ldapAuthorities[0].getAttributes().get("member"));
-        assertEquals(1, ldapAuthorities[0].getAttributes().get("member").length);
+        assertEquals(1, ldapAuthorities[0].getAttributes().get("member").size());
         assertEquals("uid=closuredude,ou=people,dc=springframework,dc=org", ldapAuthorities[0].getFirstAttributeValue("member"));
 
         //java group
         assertTrue(ldapAuthorities[1].getAttributes().containsKey("member"));
         assertNotNull(ldapAuthorities[1].getAttributes().get("member"));
-        assertEquals(3, ldapAuthorities[1].getAttributes().get("member").length);
+        assertEquals(3, ldapAuthorities[1].getAttributes().get("member").size());
         assertEquals(groovyDevelopers.getDn(), ldapAuthorities[1].getFirstAttributeValue("member"));
         assertEquals(
                 new String[]{
@@ -124,7 +124,7 @@ public class NestedLdapAuthoritiesPopulatorTests extends AbstractLdapIntegration
         //test non existent attribute
         assertNull(ldapAuthorities[2].getFirstAttributeValue("test"));
         assertNotNull(ldapAuthorities[2].getAttributeValues("test"));
-        assertEquals(0, ldapAuthorities[2].getAttributeValues("test").length);
+        assertEquals(0, ldapAuthorities[2].getAttributeValues("test").size());
         //test role name
         assertEquals(jDevelopers.getAuthority(), ldapAuthorities[3].getAuthority());
     }

ldap/src/main/java/org/springframework/security/ldap/SpringSecurityLdapTemplate.java

@@ -153,12 +153,12 @@ public class SpringSecurityLdapTemplate extends LdapTemplate {
     public Set<String> searchForSingleAttributeValues(final String base, final String filter, final Object[] params,
             final String attributeName) {
         String[] attributeNames = new String[]{attributeName};
-        Set<Map<String, String[]>> multipleAttributeValues = searchForMultipleAttributeValues(base, filter, params, attributeNames);
+        Set<Map<String, List<String>>> multipleAttributeValues = searchForMultipleAttributeValues(base, filter, params, attributeNames);
         Set<String> result = new HashSet<String>();
-        for (Map<String, String[]> map : multipleAttributeValues) {
-            String[] values = map.get(attributeName);
-            if (values != null && values.length > 0) {
-                result.addAll(Arrays.asList(values));
+        for (Map<String, List<String>> map : multipleAttributeValues) {
+            List<String> values = map.get(attributeName);
+            if (values != null) {
+                result.addAll(values);
             }
         }
         return result;
@@ -178,7 +178,7 @@ public class SpringSecurityLdapTemplate extends LdapTemplate {
      * The attribute name is the key for each set of values. In addition each map contains the DN as a String
      * with the key predefined key {@link #DN_KEY}.
      */
-    public Set<Map<String, String[]>> searchForMultipleAttributeValues(final String base, final String filter, final Object[] params,
+    public Set<Map<String, List<String>>> searchForMultipleAttributeValues(final String base, final String filter, final Object[] params,
             final String[] attributeNames) {
         // Escape the params acording to RFC2254
         Object[] encodedParams = new String[params.length];
@@ -190,12 +190,12 @@ public class SpringSecurityLdapTemplate extends LdapTemplate {
         String formattedFilter = MessageFormat.format(filter, encodedParams);
         logger.debug("Using filter: " + formattedFilter);
 
-        final HashSet<Map<String, String[]>> set = new HashSet<Map<String, String[]>>();
+        final HashSet<Map<String, List<String>>> set = new HashSet<Map<String, List<String>>>();
 
         ContextMapper roleMapper = new ContextMapper() {
             public Object mapFromContext(Object ctx) {
                 DirContextAdapter adapter = (DirContextAdapter) ctx;
-                Map<String, String[]> record = new HashMap<String, String[]>();
+                Map<String, List<String>> record = new HashMap<String, List<String>>();
                 if (attributeNames == null || attributeNames.length == 0) {
                     try {
                         for (NamingEnumeration ae = adapter.getAttributes().getAll(); ae.hasMore(); ) {
@@ -210,7 +210,7 @@ public class SpringSecurityLdapTemplate extends LdapTemplate {
                         extractStringAttributeValues(adapter, record, attributeName);
                     }
                 }
-                record.put(DN_KEY, new String[]{getAdapterDN(adapter)});
+                record.put(DN_KEY, Arrays.asList(getAdapterDN(adapter)));
                 set.add(record);
                 return null;
             }
@@ -246,7 +246,7 @@ public class SpringSecurityLdapTemplate extends LdapTemplate {
      * @param record        - the map holding the attribute names and values
      * @param attributeName - the name for which to fetch the values from
      */
-    protected void extractStringAttributeValues(DirContextAdapter adapter, Map<String, String[]> record, String attributeName) {
+    protected void extractStringAttributeValues(DirContextAdapter adapter, Map<String, List<String>> record, String attributeName) {
         Object[] values = adapter.getObjectAttributes(attributeName);
         if (values == null || values.length == 0) {
             logger.debug("No attribute value found for '" + attributeName + "'");
@@ -265,7 +265,7 @@ public class SpringSecurityLdapTemplate extends LdapTemplate {
                 }
             }
         }
-        record.put(attributeName, svalues.toArray(new String[svalues.size()]));
+        record.put(attributeName, svalues);
     }
 
     /**

ldap/src/main/java/org/springframework/security/ldap/userdetails/LdapAuthority.java

@@ -17,6 +17,8 @@ package org.springframework.security.ldap.userdetails;
 
 import org.springframework.security.core.GrantedAuthority;
 
+import java.util.Collections;
+import java.util.List;
 import java.util.Map;
 
 /**
@@ -30,7 +32,7 @@ public class LdapAuthority implements GrantedAuthority {
 
     private String dn;
     private String role;
-    private Map<String, String[]> attributes;
+    private Map<String, List<String>> attributes;
 
     /**
      * Constructs an LdapAuthority that has a role and a DN but no other attributes
@@ -49,7 +51,7 @@ public class LdapAuthority implements GrantedAuthority {
      * @param dn
      * @param attributes
      */
-    public LdapAuthority(String role, String dn, Map<String, String[]> attributes) {
+    public LdapAuthority(String role, String dn, Map<String, List<String>> attributes) {
         if (role == null) throw new NullPointerException("role can not be null");
         this.role = role;
         this.dn = dn;
@@ -61,7 +63,7 @@ public class LdapAuthority implements GrantedAuthority {
      *
      * @return the LDAP attributes, map can be null
      */
-    public Map<String, String[]> getAttributes() {
+    public Map<String, List<String>> getAttributes() {
         return attributes;
     }
 
@@ -80,13 +82,13 @@ public class LdapAuthority implements GrantedAuthority {
      * @param name the attribute name
      * @return a String array, never null but may be zero length
      */
-    public String[] getAttributeValues(String name) {
-        String[] result = null;
+    public List<String> getAttributeValues(String name) {
+        List<String> result = null;
         if (attributes != null) {
             result = attributes.get(name);
         }
         if (result == null) {
-            result = new String[0];
+            result = Collections.emptyList();
         }
         return result;
     }
@@ -98,11 +100,11 @@ public class LdapAuthority implements GrantedAuthority {
      * @return the first attribute value for a specified attribute, may be null
      */
     public String getFirstAttributeValue(String name) {
-        String[] result = getAttributeValues(name);
-        if (result.length > 0) {
-            return result[0];
-        } else {
+        List<String> result = getAttributeValues(name);
+        if (result.isEmpty()) {
             return null;
+        } else {
+            return result.get(0);
         }
     }
 

ldap/src/main/java/org/springframework/security/ldap/userdetails/NestedLdapAuthoritiesPopulator.java

@@ -22,10 +22,7 @@ import org.springframework.security.core.GrantedAuthority;
 import org.springframework.security.ldap.SpringSecurityLdapTemplate;
 import org.springframework.util.StringUtils;
 
-import java.util.Arrays;
-import java.util.HashSet;
-import java.util.Map;
-import java.util.Set;
+import java.util.*;
 
 /**
  * A LDAP authority populator that can recursively search static nested groups. <p>An example of nested groups can be
@@ -185,7 +182,7 @@ public class NestedLdapAuthoritiesPopulator extends DefaultLdapAuthoritiesPopula
             getAttributeNames().add(getGroupRoleAttribute());
         }
 
-        Set<Map<String, String[]>> userRoles = getLdapTemplate().searchForMultipleAttributeValues(
+        Set<Map<String, List<String>>> userRoles = getLdapTemplate().searchForMultipleAttributeValues(
                 getGroupSearchBase(),
                 getGroupSearchFilter(),
                 new String[]{userDn, username},
@@ -195,12 +192,14 @@ public class NestedLdapAuthoritiesPopulator extends DefaultLdapAuthoritiesPopula
             logger.debug("Roles from search: " + userRoles);
         }
 
-        for (Map<String, String[]> record : userRoles) {
+        for (Map<String, List<String>> record : userRoles) {
             boolean circular = false;
-            String dn = record.get(SpringSecurityLdapTemplate.DN_KEY)[0];
-            String[] roleValues = record.get(getGroupRoleAttribute());
+            String dn = record.get(SpringSecurityLdapTemplate.DN_KEY).get(0);
+            List<String> roleValues = record.get(getGroupRoleAttribute());
             Set<String> roles = new HashSet<String>();
-            roles.addAll(Arrays.asList(roleValues != null ? roleValues : new String[0]));
+            if(roleValues != null) {
+                roles.addAll(roleValues);
+            }
             for (String role : roles) {
                 if (isConvertToUpperCase()) {
                     role = role.toUpperCase();

ldap/src/test/java/org/springframework/security/ldap/userdetails/LdapAuthorityTests.java

@@ -4,7 +4,9 @@ import org.junit.Before;
 import org.junit.Test;
 import org.springframework.security.ldap.SpringSecurityLdapTemplate;
 
+import java.util.Arrays;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 
 import static org.junit.Assert.assertEquals;
@@ -20,9 +22,9 @@ public class LdapAuthorityTests {
 
     @Before
     public void setUp() {
-        Map<String, String[]> attributes = new HashMap<String, String[]>();
-        attributes.put(SpringSecurityLdapTemplate.DN_KEY, new String[]{DN});
-        attributes.put("mail", new String[]{"filip@ldap.test.org", "filip@ldap.test2.org"});
+        Map<String, List<String>> attributes = new HashMap<String, List<String>>();
+        attributes.put(SpringSecurityLdapTemplate.DN_KEY, Arrays.asList(DN));
+        attributes.put("mail", Arrays.asList("filip@ldap.test.org", "filip@ldap.test2.org"));
         authority = new LdapAuthority("testRole", DN, attributes);
     }
 
@@ -30,7 +32,7 @@ public class LdapAuthorityTests {
     public void testGetDn() throws Exception {
         assertEquals(DN, authority.getDn());
         assertNotNull(authority.getAttributeValues(SpringSecurityLdapTemplate.DN_KEY));
-        assertEquals(1, authority.getAttributeValues(SpringSecurityLdapTemplate.DN_KEY).length);
+        assertEquals(1, authority.getAttributeValues(SpringSecurityLdapTemplate.DN_KEY).size());
         assertEquals(DN, authority.getFirstAttributeValue(SpringSecurityLdapTemplate.DN_KEY));
     }
 
@@ -38,10 +40,10 @@ public class LdapAuthorityTests {
     public void testGetAttributes() throws Exception {
         assertNotNull(authority.getAttributes());
         assertNotNull(authority.getAttributeValues("mail"));
-        assertEquals(2, authority.getAttributeValues("mail").length);
+        assertEquals(2, authority.getAttributeValues("mail").size());
         assertEquals("filip@ldap.test.org", authority.getFirstAttributeValue("mail"));
-        assertEquals("filip@ldap.test.org", authority.getAttributeValues("mail")[0]);
-        assertEquals("filip@ldap.test2.org", authority.getAttributeValues("mail")[1]);
+        assertEquals("filip@ldap.test.org", authority.getAttributeValues("mail").get(0));
+        assertEquals("filip@ldap.test2.org", authority.getAttributeValues("mail").get(1));
     }
 
     @Test