From 00125cddee481297b4c4b822d38dc05dfbcf34e8 Mon Sep 17 00:00:00 2001
From: Luke Taylor <luke.taylor@springsource.com>
Date: Wed, 17 Dec 2008 00:48:32 +0000
Subject: [PATCH] SEC-1016: Moved the MapBasedDefinitionSource to the top of
 the list of delegates (before expressions), but changed the code to only add
 it if there are pointcuts defined, so there should be no unnecessary
 overhead.

---
 ...balMethodSecurityBeanDefinitionParser.java | 22 +++++++++----------
 .../DelegatingMethodDefinitionSource.java     |  7 +++---
 2 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/core/src/main/java/org/springframework/security/config/GlobalMethodSecurityBeanDefinitionParser.java b/core/src/main/java/org/springframework/security/config/GlobalMethodSecurityBeanDefinitionParser.java
index fb7951fc5d..731d43ea9b 100644
--- a/core/src/main/java/org/springframework/security/config/GlobalMethodSecurityBeanDefinitionParser.java
+++ b/core/src/main/java/org/springframework/security/config/GlobalMethodSecurityBeanDefinitionParser.java
@@ -77,6 +77,17 @@ class GlobalMethodSecurityBeanDefinitionParser implements BeanDefinitionParser {
         boolean expressionsEnabled = "enabled".equals(element.getAttribute(ATT_USE_EXPRESSIONS));
         BeanDefinition expressionVoter = null;
 
+        // Now create a Map<String, ConfigAttribute> for each <protect-pointcut> sub-element
+        Map<String, List<ConfigAttribute>> pointcutMap = parseProtectPointcuts(parserContext,
+                DomUtils.getChildElementsByTagName(element, Elements.PROTECT_POINTCUT));
+
+        if (pointcutMap.size() > 0) {
+            // SEC-1016: Put the pointcut MDS first, but only add it if there are actually any pointcuts defined.
+            MapBasedMethodDefinitionSource mapBasedMethodDefinitionSource = new MapBasedMethodDefinitionSource();
+            delegates.add(mapBasedMethodDefinitionSource);
+            registerProtectPointcutPostProcessor(parserContext, pointcutMap, mapBasedMethodDefinitionSource, source);
+        }
+
         if (expressionsEnabled) {
             Element expressionHandlerElt = DomUtils.getChildElementByTagName(element, Elements.EXPRESSION_HANDLER);
             String expressionHandlerRef = expressionHandlerElt == null ? null : expressionHandlerElt.getAttribute("ref");
@@ -112,17 +123,6 @@ class GlobalMethodSecurityBeanDefinitionParser implements BeanDefinitionParser {
             delegates.add(BeanDefinitionBuilder.rootBeanDefinition(JSR_250_SECURITY_METHOD_DEFINITION_SOURCE_CLASS).getBeanDefinition());
         }
 
-        MapBasedMethodDefinitionSource mapBasedMethodDefinitionSource = new MapBasedMethodDefinitionSource();
-        delegates.add(mapBasedMethodDefinitionSource);
-
-        // Now create a Map<String, ConfigAttribute> for each <protect-pointcut> sub-element
-        Map<String, List<ConfigAttribute>> pointcutMap = parseProtectPointcuts(parserContext,
-                DomUtils.getChildElementsByTagName(element, Elements.PROTECT_POINTCUT));
-
-        if (pointcutMap.size() > 0) {
-            registerProtectPointcutPostProcessor(parserContext, pointcutMap, mapBasedMethodDefinitionSource, source);
-        }
-
         registerDelegatingMethodDefinitionSource(parserContext, delegates, source);
 
         String accessManagerId = element.getAttribute(ATT_ACCESS_MGR);
diff --git a/core/src/main/java/org/springframework/security/intercept/method/DelegatingMethodDefinitionSource.java b/core/src/main/java/org/springframework/security/intercept/method/DelegatingMethodDefinitionSource.java
index 394658da76..33ad0dba10 100644
--- a/core/src/main/java/org/springframework/security/intercept/method/DelegatingMethodDefinitionSource.java
+++ b/core/src/main/java/org/springframework/security/intercept/method/DelegatingMethodDefinitionSource.java
@@ -32,7 +32,7 @@ public final class DelegatingMethodDefinitionSource extends AbstractMethodDefini
     //~ Methods ========================================================================================================
 
     public void afterPropertiesSet() throws Exception {
-        Assert.notEmpty(methodDefinitionSources, "A list of MethodDefinitionSources is required");
+        Assert.notNull(methodDefinitionSources, "A list of MethodDefinitionSources is required");
     }
 
     public List<ConfigAttribute> getAttributes(Method method, Class<?> targetClass) {
@@ -86,7 +86,6 @@ public final class DelegatingMethodDefinitionSource extends AbstractMethodDefini
 
     @SuppressWarnings("unchecked")
     public void setMethodDefinitionSources(List methodDefinitionSources) {
-        Assert.notEmpty(methodDefinitionSources, "A list of MethodDefinitionSources is required");
         this.methodDefinitionSources = methodDefinitionSources;
     }
 
@@ -94,9 +93,9 @@ public final class DelegatingMethodDefinitionSource extends AbstractMethodDefini
 
     private static class DefaultCacheKey {
         private final Method method;
-        private final Class targetClass;
+        private final Class<?> targetClass;
 
-        public DefaultCacheKey(Method method, Class targetClass) {
+        public DefaultCacheKey(Method method, Class<?> targetClass) {
             this.method = method;
             this.targetClass = targetClass;
         }