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

Document that CGLIB cannot proxy package private methods declared in a superclass is in a different package #28973

Closed
sasavilic opened this issue Aug 17, 2022 · 2 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: documentation A documentation task
Milestone

Comments

@sasavilic
Copy link

sasavilic commented Aug 17, 2022

This is issue is somewhat hard to explain. I created a small project where this issue can be demonstrated.

Given following classes:

package com.flightkeys.cglibbug.base;

public class BaseClass {

    private final ObjectMapper mapper = new ObjectMapper();

    String serialize(Object value) {
        return serialize(getMapper(), value);
    }

    private ObjectMapper getMapper() {
        return mapper;
    }

    private String serialize(ObjectMapper mapper, Object value) {
        return mapper.writeToString(value);
    }
}
package com.flightkeys.cglibbug.derived;

import com.flightkeys.cglibbug.base.BaseClass;
import org.springframework.context.annotation.Scope;
import org.springframework.context.annotation.ScopedProxyMode;
import org.springframework.stereotype.Component;

@Component
@Scope(proxyMode = ScopedProxyMode.TARGET_CLASS)
public class DerivedClass extends BaseClass {
}

If we pass around instance of DerivedClass and on that instance BaseClass.serialize is called, then return mapper.writeToString(value); will throw null pointer exception.

It seems that CGLIB is not proxying the String serialize(Object value) method. However, if we move DerivedClass into same package or either String serialize(Object value) or private ObjectMapper getMapper() is made protected, then it works.

NOTE: We noticed this when upgrading from spring-boot 2.1.18 to 2.2.13, but it is also reproducible on 2.7.2.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Aug 17, 2022
@rstoyanchev rstoyanchev added the in: core Issues in core modules (aop, beans, core, context, expression) label Jan 24, 2023
@snicoll
Copy link
Member

snicoll commented Oct 2, 2023

This is the expected behavior. You can't proxy a private method, see the documentation.

@snicoll snicoll closed this as not planned Won't fix, can't repro, duplicate, stale Oct 2, 2023
@snicoll snicoll added status: invalid An issue that we don't feel is valid and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Oct 2, 2023
@sbrannen
Copy link
Member

sbrannen commented Oct 2, 2023

This is the expected behavior. You can't proxy a private method, see the documentation and

That is indeed the expected behavior.

However, I can see how this particular scenario can be somewhat confusing.

CGLIB can actually proxy a package-private method, but the sample application does not work as-is, because BaseClass.serialize(Object) is "effectively private" relative to DerivedClass, and the CGLIB generated subclass is in the same package as DerivedClass.

That explains why the sample application works without modifying the visibility of BaseClass.serialize(Object) when moving BaseClass to the same package as DerivedClass, and it also explains why it works if the visibility of BaseClass.serialize(Object) is changed to protected.

Since the documentation explicitly only mentions that private methods cannot be proxied, I think it's worth updating the documentation to mention that "effectively private" methods also cannot be proxied.

In light of that, I am reopening this issue to improve the documentation.

@sbrannen sbrannen reopened this Oct 2, 2023
@sbrannen sbrannen added type: documentation A documentation task and removed status: invalid An issue that we don't feel is valid labels Oct 2, 2023
@sbrannen sbrannen self-assigned this Oct 2, 2023
@sbrannen sbrannen added this to the 6.1.0-RC1 milestone Oct 2, 2023
@sbrannen sbrannen changed the title CGLIB Proxies do not work for package private methods when derived class is in different package Document that CGLIB cannot proxy package private methods declared in a superclass is in a different package Oct 2, 2023
@sbrannen sbrannen modified the milestones: 6.1.0-RC1, 6.1.0-RC2, 6.1.x Oct 3, 2023
@sbrannen sbrannen modified the milestones: 6.1.x, 6.1.2 Nov 25, 2023
@sbrannen sbrannen modified the milestones: 6.1.2, 6.1.x Dec 11, 2023
@snicoll snicoll modified the milestones: 6.1.x, 6.1.3 Dec 22, 2023
@snicoll snicoll assigned snicoll and unassigned sbrannen Dec 22, 2023
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: documentation A documentation task
Projects
None yet
Development

No branches or pull requests

5 participants