SEC-1532: Add cache of previously matched beans to ProtectPointcutPostProcessor to ensure that it doesn't perform pointcut matching every time a new prototype bean is created.

config/src/main/java/org/springframework/security/config/method/ProtectPointcutPostProcessor.java

@@ -1,12 +1,7 @@
 package org.springframework.security.config.method;
 
 import java.lang.reflect.Method;
-import java.util.HashSet;
-import java.util.LinkedHashMap;
-import java.util.LinkedHashSet;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
+import java.util.*;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -54,6 +49,7 @@ final class ProtectPointcutPostProcessor implements BeanPostProcessor {
     private final MapBasedMethodSecurityMetadataSource mapBasedMethodSecurityMetadataSource;
     private final Set<PointcutExpression> pointCutExpressions = new LinkedHashSet<PointcutExpression>();
     private final PointcutParser parser;
+    private final Set<String> processedBeans = new HashSet<String>();
 
     public ProtectPointcutPostProcessor(MapBasedMethodSecurityMetadataSource mapBasedMethodSecurityMetadataSource) {
         Assert.notNull(mapBasedMethodSecurityMetadataSource, "MapBasedMethodSecurityMetadataSource to populate is required");
@@ -79,6 +75,11 @@ final class ProtectPointcutPostProcessor implements BeanPostProcessor {
     }
 
     public Object postProcessBeforeInitialization(Object bean, String beanName) throws BeansException {
+        if (processedBeans.contains(beanName)) {
+            // We already have the metadata for this bean
+            return bean;
+        }
+
         // Obtain methods for the present bean
         Method[] methods;
         try {
@@ -98,6 +99,8 @@ final class ProtectPointcutPostProcessor implements BeanPostProcessor {
             }
         }
 
+        processedBeans.add(beanName);
+
         return bean;
     }
 

core/src/main/java/org/springframework/security/access/method/MapBasedMethodSecurityMetadataSource.java

@@ -25,6 +25,7 @@ import java.util.Map;
 import java.util.Set;
 
 import org.springframework.beans.factory.BeanClassLoaderAware;
+import org.springframework.beans.factory.InitializingBean;
 import org.springframework.security.access.ConfigAttribute;
 import org.springframework.util.Assert;
 import org.springframework.util.ClassUtils;
@@ -42,7 +43,8 @@ import org.springframework.util.ClassUtils;
  * @author Ben Alex
  * @since 2.0
  */
-public class MapBasedMethodSecurityMetadataSource extends AbstractFallbackMethodSecurityMetadataSource implements BeanClassLoaderAware {
+public class MapBasedMethodSecurityMetadataSource extends AbstractFallbackMethodSecurityMetadataSource
+        implements BeanClassLoaderAware {
 
     //~ Instance fields ================================================================================================
     private ClassLoader beanClassLoader = ClassUtils.getDefaultClassLoader();
@@ -103,7 +105,7 @@ public class MapBasedMethodSecurityMetadataSource extends AbstractFallbackMethod
      * for matching multiple methods.
      *
      * @param name type and method name, separated by a dot
-     * @param attr required authorities associated with the method
+     * @param attr the security attributes associated with the method
      */
     private void addSecureMethod(String name, List<ConfigAttribute> attr) {
         int lastDotIndex = name.lastIndexOf(".");
@@ -175,7 +177,9 @@ public class MapBasedMethodSecurityMetadataSource extends AbstractFallbackMethod
      * Adds configuration attributes for a specific method, for example where the method has been
      * matched using a pointcut expression. If a match already exists in the map for the method, then
      * the existing match will be retained, so that if this method is called for a more general pointcut
-     * it will not override a more specific one which has already been added. This
+     * it will not override a more specific one which has already been added.
+     * <p>
+     * This method should only be called during initialization of the {@code BeanFactory}.
      */
     public void addSecureMethod(Class<?> javaType, Method method, List<ConfigAttribute> attr) {
         RegisteredMethod key = new RegisteredMethod(method, javaType);

itest/context/itest-context.gradle

@@ -4,6 +4,7 @@ dependencies {
     compile     project(':spring-security-core'),
                 'aopalliance:aopalliance:1.0',
                 'org.python:jython:2.5.0',
+                "org.springframework:spring-context:$springVersion",
                 "org.springframework:spring-aop:$springVersion",
                 "org.springframework:spring-tx:$springVersion",
                 "org.springframework:spring-beans:$springVersion"

itest/context/src/test/java/org/springframework/security/performance/ProtectPointcutPerformanceTests.java

@@ -0,0 +1,52 @@
+package org.springframework.security.performance;
+
+import static junit.framework.Assert.fail;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.springframework.beans.BeansException;
+import org.springframework.context.ApplicationContext;
+import org.springframework.context.ApplicationContextAware;
+import org.springframework.security.authentication.AuthenticationCredentialsNotFoundException;
+import org.springframework.security.core.context.SecurityContextHolder;
+import org.springframework.security.core.session.SessionRegistry;
+import org.springframework.test.context.ContextConfiguration;
+import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
+import org.springframework.util.StopWatch;
+
+/**
+ * @author Luke Taylor
+ */
+@ContextConfiguration(locations={"/protect-pointcut-performance-app-context.xml"})
+@RunWith(SpringJUnit4ClassRunner.class)
+public class ProtectPointcutPerformanceTests implements ApplicationContextAware {
+    ApplicationContext ctx;
+
+    @Before
+    public void clearContext() {
+        SecurityContextHolder.clearContext();
+    }
+
+    // Method for use with profiler
+    @Test
+    public void usingPrototypeDoesNotParsePointcutOnEachCall() {
+        StopWatch sw = new StopWatch();
+        sw.start();
+        for (int i = 0; i < 1000; i++) {
+            try {
+                SessionRegistry reg = (SessionRegistry) ctx.getBean("sessionRegistryPrototype");
+                reg.getAllPrincipals();
+                fail("Expected AuthenticationCredentialsNotFoundException");
+            } catch (AuthenticationCredentialsNotFoundException expected) {
+            }
+        }
+        sw.stop();
+//        assertTrue(sw.getTotalTimeMillis() < 1000);
+
+    }
+
+    public void setApplicationContext(ApplicationContext applicationContext) throws BeansException {
+        ctx = applicationContext;
+    }
+}

itest/context/src/test/resources/protect-pointcut-performance-app-context.xml

@@ -0,0 +1,26 @@
+<beans xmlns="http://www.springframework.org/schema/beans"
+    xmlns:sec="http://www.springframework.org/schema/security"
+    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xsi:schemaLocation="http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans-3.0.xsd
+                        http://www.springframework.org/schema/security http://www.springframework.org/schema/security/spring-security-3.0.xsd">
+
+    <sec:global-method-security>
+        <sec:protect-pointcut expression="execution(* org.springframework.security.core.session.SessionRegistry.refreshLastRequest(..))" access="ROLE_ADMIN" />
+        <sec:protect-pointcut expression="execution(* org.springframework.security.core.session.SessionRegistry.registerNewSession(..))" access="ROLE_ADMIN" />
+        <sec:protect-pointcut expression="execution(* org.springframework.security.core.session.SessionRegistry.removeSessionInformation(..))" access="ROLE_ADMIN" />
+        <sec:protect-pointcut expression="execution(* org.springframework.security.core.session.SessionRegistry.get*(..))" access="ROLE_ADMIN" />
+    </sec:global-method-security>
+
+    <bean id="sessionRegistry" class="org.springframework.security.core.session.SessionRegistryImpl" />
+
+    <bean id="sessionRegistryPrototype" class="org.springframework.security.core.session.SessionRegistryImpl" scope="prototype"/>
+
+    <sec:authentication-manager alias="authenticationManager">
+        <sec:authentication-provider>
+            <sec:user-service id="userService">
+                <sec:user name="notused" password="notused" authorities="ROLE_0,ROLE_1"/>
+            </sec:user-service>
+        </sec:authentication-provider>
+    </sec:authentication-manager>
+
+</beans>