Skip to content

Fail explicitly if a final method is invoked on a CGLIB proxy #26729

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

Open
rpelissier opened this issue Mar 25, 2021 · 1 comment
Open

Fail explicitly if a final method is invoked on a CGLIB proxy #26729

rpelissier opened this issue Mar 25, 2021 · 1 comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement

Comments

@rpelissier
Copy link

rpelissier commented Mar 25, 2021

Started from spring-projects/spring-retry#238

In org.springframework.cglib.proxy.Enhancer.getMethods():
CollectionUtils.filter(methods, new RejectModifierPredicate(Constants.ACC_FINAL));
If the method is final it is SILENTLY rejected from being proxied.

Shouldn't we warn someone who is implementing a proxy on top of a final class ?
I believe it is a contract break of making the proxy and should throw a NotProxiableElementException("The method or field is final.").

This issue is particularly severe in Kotlin where final is the default.
This example is using Spring-Retry to generate the Proxy and demonstrate the issue.

import org.junit.Assert
import org.junit.jupiter.api.Test
import org.springframework.beans.factory.annotation.Autowired
import org.springframework.boot.test.context.SpringBootTest
import org.springframework.retry.annotation.EnableRetry
import org.springframework.retry.annotation.Retryable
import org.springframework.stereotype.Component


private const val COUNT=1000

@SpringBootTest
@EnableRetry
class SpringOpenIssueTest {

    @Autowired
    private lateinit var bean: ChildBean

    @Test
    fun `proxies should work in kotlin with all-open plugin`(){
        bean.assertOkayFromTheInside()
        Assert.assertEquals(COUNT, bean.openCount)
        Assert.assertEquals(COUNT, bean.closedCount) //java.lang.AssertionError: expected:<1000> but was:<0>
    }

}

@Component //This class is automatically opened by all-open plugin
class ChildBean : AbstractBean(){
    @Retryable
    fun assertOkayFromTheInside(){
        Assert.assertEquals(COUNT, openCount)
        Assert.assertEquals(COUNT, closedCount)
    }
}

//This class is open because abstract
abstract class AbstractBean{
    var closedCount = COUNT //This field is not automatically opened
    open var openCount = COUNT
}
@rpelissier rpelissier changed the title Silently breaking CGLIB proxy when methods or properties are final, causing null/zero being returned Silently breaking CGLIB proxy when methods or properties are final, causing null/zero to be returned Mar 25, 2021
@rpelissier rpelissier changed the title Silently breaking CGLIB proxy when methods or properties are final, causing null/zero to be returned Silently broken CGLIB proxy when methods or properties are final, causing null/zero to be returned Mar 25, 2021
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Mar 25, 2021
@mdeinum
Copy link
Contributor

mdeinum commented Mar 26, 2021

Generating a more clear warning would be nice as it is also a cause for weird behaviour (see https://deinum.biz/2020-07-03-Autowired-Field-Null/). It doesn't only apply to final methods but also private and default modifier methods leading to this.

I have been answering too many questions on StackOverflow on this kind of issues, so a nice warning would probably trigger people to watch closer.

@rstoyanchev rstoyanchev added the in: core Issues in core modules (aop, beans, core, context, expression) label Nov 10, 2021
@snicoll snicoll changed the title Silently broken CGLIB proxy when methods or properties are final, causing null/zero to be returned Fail explicitly if a final method is invoked on a CGLIB proxy Nov 22, 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 Nov 22, 2023
@snicoll snicoll added this to the 6.x Backlog milestone Nov 22, 2023
@jhoeller jhoeller modified the milestones: 6.x Backlog, General Backlog Dec 29, 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: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

6 participants