Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
samuelfac opened this issue Jul 24, 2023 · 6 comments
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Milestone

Comments

@samuelfac
Copy link

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)

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jul 24, 2023
@snicoll snicoll transferred this issue from spring-projects/spring-boot Jul 24, 2023
@snicoll
Copy link
Member

snicoll commented Jul 24, 2023

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
Copy link
Member

snicoll commented Jul 25, 2023

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.

@snicoll snicoll changed the title AOP Aspect does not work with private methods Invoking private method on a CGLIB proxy should trigger a dedicated exception Jul 25, 2023
@snicoll snicoll added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jul 25, 2023
@snicoll snicoll added this to the 6.x Backlog milestone Jul 25, 2023
@snicoll snicoll added the in: core Issues in core modules (aop, beans, core, context, expression) label Jul 25, 2023
@samuelfac
Copy link
Author

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
Copy link

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
Copy link
Contributor

esperar commented Mar 7, 2024

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.

@spring-projects spring-projects deleted a comment from esperar Mar 7, 2024
@esperar
Copy link
Contributor

esperar commented Mar 7, 2024

I have registered the above comment as a cglib issue. cglib/cglib#223

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

6 participants