SEC-1826: Empty attribute list should be treated the same as null in DelegatingMethodSecurityMetadataSource.

This commit is contained in:
Luke Taylor 2011-09-24 14:13:57 +01:00
parent be8ee61f82
commit 44364d0101
3 changed files with 9 additions and 12 deletions

View File

@ -35,8 +35,8 @@ public interface SecurityMetadataSource extends AopInfrastructureBean {
* *
* @param object the object being secured * @param object the object being secured
* *
* @return the attributes that apply to the passed in secured object. Can return either {@code null} or an * @return the attributes that apply to the passed in secured object. Should return an empty collection if there
* empty collection if there are no applicable attributes. * are no applicable attributes.
* *
* @throws IllegalArgumentException if the passed object is not of a type supported by the * @throws IllegalArgumentException if the passed object is not of a type supported by the
* <code>SecurityMetadataSource</code> implementation * <code>SecurityMetadataSource</code> implementation

View File

@ -15,7 +15,7 @@ import org.springframework.util.ObjectUtils;
/** /**
* Automatically tries a series of method definition sources, relying on the first source of metadata * Automatically tries a series of method definition sources, relying on the first source of metadata
* that provides a non-null response. Provides automatic caching of the retrieved metadata. * that provides a non-null/non-empty response. Provides automatic caching of the retrieved metadata.
* *
* @author Ben Alex * @author Ben Alex
* @author Luke Taylor * @author Luke Taylor
@ -41,9 +41,6 @@ public final class DelegatingMethodSecurityMetadataSource extends AbstractMethod
synchronized (attributeCache) { synchronized (attributeCache) {
Collection<ConfigAttribute> cached = attributeCache.get(cacheKey); Collection<ConfigAttribute> cached = attributeCache.get(cacheKey);
// Check for canonical value indicating there is no config attribute, // Check for canonical value indicating there is no config attribute,
if (cached == NULL_CONFIG_ATTRIBUTE) {
return null;
}
if (cached != null) { if (cached != null) {
return cached; return cached;
@ -59,13 +56,13 @@ public final class DelegatingMethodSecurityMetadataSource extends AbstractMethod
} }
// Put it in the cache. // Put it in the cache.
if (attributes == null) { if (attributes == null || attributes.isEmpty()) {
this.attributeCache.put(cacheKey, NULL_CONFIG_ATTRIBUTE); this.attributeCache.put(cacheKey, NULL_CONFIG_ATTRIBUTE);
return null; return NULL_CONFIG_ATTRIBUTE;
} }
if (logger.isDebugEnabled()) { if (logger.isDebugEnabled()) {
logger.debug("Adding security method [" + cacheKey + "] with attributes " + attributes); logger.debug("Caching method [" + cacheKey + "] with attributes " + attributes);
} }
this.attributeCache.put(cacheKey, attributes); this.attributeCache.put(cacheKey, attributes);

View File

@ -20,7 +20,7 @@ public class DelegatingMethodSecurityMetadataSourceTests {
DelegatingMethodSecurityMetadataSource mds; DelegatingMethodSecurityMetadataSource mds;
@Test @Test
public void returnsNullIfDelegateReturnsNull() throws Exception { public void returnsEmptyListIfDelegateReturnsNull() throws Exception {
List sources = new ArrayList(); List sources = new ArrayList();
MethodSecurityMetadataSource delegate = mock(MethodSecurityMetadataSource.class); MethodSecurityMetadataSource delegate = mock(MethodSecurityMetadataSource.class);
when(delegate.getAttributes(Matchers.<Method>any(), Matchers.any(Class.class))).thenReturn(null); when(delegate.getAttributes(Matchers.<Method>any(), Matchers.any(Class.class))).thenReturn(null);
@ -29,9 +29,9 @@ public class DelegatingMethodSecurityMetadataSourceTests {
assertSame(sources, mds.getMethodSecurityMetadataSources()); assertSame(sources, mds.getMethodSecurityMetadataSources());
assertTrue(mds.getAllConfigAttributes().isEmpty()); assertTrue(mds.getAllConfigAttributes().isEmpty());
MethodInvocation mi = new SimpleMethodInvocation(null, String.class.getMethod("toString")); MethodInvocation mi = new SimpleMethodInvocation(null, String.class.getMethod("toString"));
assertNull(mds.getAttributes(mi)); assertEquals(Collections.emptyList(), mds.getAttributes(mi));
// Exercise the cached case // Exercise the cached case
assertNull(mds.getAttributes(mi)); assertEquals(Collections.emptyList(), mds.getAttributes(mi));
} }
@Test @Test