spring-projects / spring-framework

Spring Framework
https://spring.io/projects/spring-framework
Apache License 2.0
56.26k stars 37.98k forks source link

Invoking private method on a CGLIB proxy should trigger a dedicated exception #30938

Open samuelfac opened 1 year ago

samuelfac commented 1 year ago

I created a ExecutionTimeAdvice.java to log the runtime of all my controller methods using @Aspect.

When i call the public (http://localhost:8080/) method works perfectly, but when I call the private method (http://localhost:8080/2) my @Autowired service is not instantiated:

java.lang.NullPointerException: Cannot invoke "com.example.demo.service.MyService.sayHello()" because "this.myService" is null
    at com.example.demo.controller.MyController.get2(MyController.java:24) ~[classes/:na]
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[na:na]
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77) ~[na:na]
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[na:na]
    at java.base/java.lang.reflect.Method.invoke(Method.java:568) ~[na:na]
    at org.springframework.web.method.support.InvocableHandlerMethod.doInvoke(InvocableHandlerMethod.java:205) ~[spring-web-6.0.11.jar:6.0.11]

but if I remove the filter everything works.

You can download the project here: demo.zip

Tests were performed on the following versions:: 2.7.14 (java8) and 3.1.2 (java17)

snicoll commented 1 year ago

Thanks for the sample. Your aspect does not work on the private method and it looks like the call to the private method is done on the raw instance, and said instance has not been initialized properly. Perhaps the creation of the aspect lead to an early initialization of the bean. It's a bit odd so we'd need to investigate a bit more.

snicoll commented 1 year ago

So the Aspect has nothing to do with the issue, except the fact it triggers the creation of a proxy. The same. could happen with any stereotype on the controller that trigger the same thing (e.g. @Transactional). The problem is that invoking a private method on a cglib proxy is not supported. Our dispatching algorithm does not take that into account and so we end up in the weird state that you've discovered.

We'll revisit this to trigger a proper exception upfront, rather than calling the private method on the proxy via reflection. Either way, this scenario won't be supported so you'll have to revisit your controller so that the method isn't private.

samuelfac commented 1 year ago

Yes, I already changed all my methods to public, but it took me a few hours to find out what the problem was because I didn't find anything about it, so I opened the issue.

Thanks

Ventura2 commented 1 year ago

I have created a simple test that reproduces the error:


public class CGLibProxyPrivateMethodTest {

    ClassWithAutowireAndPrivateMethod proxy;

    @BeforeEach
    public void setup() {
        ApplicationContext context = new AnnotationConfigApplicationContext(ClassWithAutowireAndPrivateMethod.class, MySecondService.class);
        ClassWithAutowireAndPrivateMethod bean1 = context.getBean(ClassWithAutowireAndPrivateMethod.class);
        ProxyFactoryBean proxyFactoryBean = new ProxyFactoryBean();
        proxyFactoryBean.setTarget(bean1);
        proxy = (ClassWithAutowireAndPrivateMethod) proxyFactoryBean.getObject();
    }
    @Test
    public void publicMethodCallAutowired_Pass() throws InterruptedException {
        proxy.get1();
    }

    @Test
    public void privateMethodCallAutowired_Fail() throws InterruptedException {
        proxy.get2();
    }

    public static class ClassWithAutowireAndPrivateMethod {
        @Autowired
        MySecondService mySecondService;

        public String get1() throws InterruptedException {
            return mySecondService.sayHello();
        }

        private String get2() throws InterruptedException {
            return mySecondService.sayHello();
        }

    }

    static class MySecondService{
        public String sayHello() throws InterruptedException {
            return "hello";
        }
    }
}
esperar commented 6 months ago

I also had a similar situation. While developing a Handler, when I applied an AOP-annotated method to a simple operation (for example, returning the string "success"), it executed successfully. However, when debugging, I could see that it was being executed from the Enhancer class. But when it comes to executing operations that depend on classes like Service through dependency injection, the injected class ends up being null.

I believe that the statement about a proxy object being created for a private method itself is incorrect. If there is any related information, please provide a posting,

+ My guess is that the generated proxy object does not have dependency injection, but the original object has dependency injection, so the enhancer calls the method's behavior, which seems to be working as null. This seems to need modification.

esperar commented 6 months ago

I have registered the above comment as a cglib issue. https://github.com/cglib/cglib/issues/223