Clarification: Spring AOP pointcuts match protected methods when CGLIB is used [SPR-15354]

[spring-projects-issues]

Alexander Kriegisch opened SPR-15354 and commented

Disclaimer: I am not a Spring user, just an AspectJ expert trying to answer a StackOverflow question related to Spring AOP.

The Spring documentation in chapter 11.2.3 says:

Due to the proxy-based nature of Spring’s AOP framework, protected methods are by definition not intercepted, neither for JDK proxies (where this isn’t applicable) nor for CGLIB proxies (where this is technically possible but not recommendable for AOP purposes). As a consequence, any given pointcut will be matched against public methods only!
If your interception needs include protected/private methods or even constructors, consider the use of Spring-driven native AspectJ weaving instead of Spring’s proxy-based AOP framework. This constitutes a different mode of AOP usage with different characteristics, so be sure to make yourself familiar with weaving first before making a decision.

Anyway, after I set up a little Spring Boot project and tried to use Spring AOP on a class, I saw that at least for CGLIB proxies pointcuts match protected methods. I was really surprised because the documentation says otherwise. So either the documentation is not up to date with this new "feature" or this is a regression - since which version I cannot say.

When debugging into the advice with @EnableAspectJAutoProxy(proxyTargetClass = true, exposeProxy = true) and inspecting AopContext.currentProxy() I also see that a CGLIB proxy is created for a class which only has one protected method and for which the pointcut should not even match, thus not create a proxy.

Just try something like this:

package spring.aop;

import java.lang.annotation.*;
import java.util.concurrent.TimeUnit;

@Documented
@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.METHOD)
public @interface RetryOnFailure {
    int attempts() default 2;
    long delay() default 1;
    TimeUnit unit() default TimeUnit.SECONDS;
}

package spring.aop;

import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RestController;

import java.util.concurrent.TimeUnit;

@RestController("/")
public class HomeController {

    static int counter = 0;

    @RequestMapping
    @RetryOnFailure(attempts = 3, delay = 2, unit = TimeUnit.SECONDS)
    protected String index() {
        throw new RuntimeException("Exception in try " + ++counter);
    }
}

package spring.aop;

import org.aspectj.lang.ProceedingJoinPoint;
import org.aspectj.lang.annotation.Around;
import org.aspectj.lang.annotation.Aspect;
import org.springframework.stereotype.Component;

@Aspect
@Component
public final class MethodRepeater {

    @Around("execution(* spring.aop..*(..)) && @annotation(retryOnFailure)")
    public Object wrap(final ProceedingJoinPoint joinPoint, RetryOnFailure retryOnFailure) throws Throwable {
        System.out.println(joinPoint);
        return proceed(joinPoint, retryOnFailure);
    }

    private Object proceed(ProceedingJoinPoint joinPoint, RetryOnFailure retryOnFailure) throws Throwable {
        int attempt = 1;
        while (true) {
            try {
                return joinPoint.proceed();
            } catch (final Throwable ex) {
                System.out.println("Try #" + attempt + " failed: " + ex);
                if (++attempt >= retryOnFailure.attempts())
                    return "OK";
                if (retryOnFailure.delay() > 0L)
                    retryOnFailure.unit().sleep(retryOnFailure.delay());
            }
        }
    }
}

package spring.aop;

import org.springframework.boot.SpringApplication;
import org.springframework.boot.autoconfigure.SpringBootApplication;

@SpringBootApplication
public class DemoApplication {
    public static void main(String[] args) {
        SpringApplication.run(DemoApplication.class, args);
    }
}

---

Affects: 4.3.7

Reference URL: https://docs.spring.io/spring/docs/current/spring-framework-reference/html/aop.html#aop-pointcuts-designators

Issue Links:

• Cannot intercept calls to protected methods of CGLIB proxied class [SPR-1611] #6308 Cannot intercept calls to protected methods of CGLIB proxied class
• "CglibAopProxy: Unable to proxy method" WARN when bean class contains static final method [SPR-11107] #15733 "CglibAopProxy: Unable to proxy method" WARN when bean class contains static final method
• CglibAopProxy needs to detect package-visible methods when defined in a different ClassLoader [SPR-11618] #16241 CglibAopProxy needs to detect package-visible methods when defined in a different ClassLoader
• Revisit CGLIB AOP proxy warnings for final methods [SPR-15436] #19997 Revisit CGLIB AOP proxy warnings for final methods

Referenced from: commits b90d3d0, 66670cf

[spring-projects-issues]

Alexander Kriegisch commented

@Juergen Hoeller, would you mind briefly commenting on why you relabled this issue a documentation issue and not a bug? My guess was bug because for many years the behaviour for proxies said that they are only applicable for public methods. I thought that was intentional. Has the policy changed now for Spring AOP? Have you even checked if this is maybe a regression breaking behaviour in older Spring versions for CGLIB proxies? If the behaviour has changed this could cause serious side effects because more joinpoints would suddenly be matched by existing aspects after a Spring upgrade. Or have you already verified that protected methods have indeed always been intercepted by Spring AOP pointcuts and the documentation was wrong since day 1? I would not take this lightly.

[spring-projects-issues]

Juergen Hoeller commented

This has been relaxed for protected methods on CGLIB proxies a long time ago: see #6308. Unfortunately the documentation never caught up with that.

The more important distinction is between calls through the proxy (on public or protected methods, consistently intercepted) and calls within the target class (never intercepted). The latter can only be addressed through weaving.

[spring-projects-issues]

Alexander Kriegisch commented

The latter I know and it is out of scope for this question. What I wanted to know you have clarified with the link to #6308, thank you very much. So really since the old days of Spring 1.2.7 this was changed on purpose, but never documented. Wow! Now I fully agree that this is a documentation issue.

[spring-projects-issues]

Juergen Hoeller commented

I've revised that documentation section towards the following, even mentioning that package-visible methods will be intercepted as well. Essentially, we're trying to intercept any method call on the proxy, logging a warning in the validate-class step when not possible, since otherwise such calls would end up in the uninitalized proxy instance (typically leading to NPEs when interacting with uninitialized fields etc).

Due to the proxy-based nature of Spring's AOP framework, calls within the target object
are by definition not intercepted. For JDK proxies, only public interface method
calls on the proxy can be intercepted. With CGLIB, public and protected method calls on
the proxy will be intercepted, and even package-visible methods if necessary. However,
common interactions through proxies should always be designed through public signatures.

Note that pointcut definitions are generally matched against any intercepted method.
If a pointcut is strictly meant to be public-only, even in a CGLIB proxy scenario with
potential non-public interactions through proxies, it needs to be defined accordingly.

If your interception needs include method calls or even constructors within the target
class, consider the use of Spring-driven <<aop-aj-ltw,native AspectJ weaving>> instead
of Spring's proxy-based AOP framework. This constitutes a different mode of AOP usage
with different characteristics, so be sure to make yourself familiar with weaving first
before making a decision.