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

Configuration property binding does not deal with bridge methods #33105

Closed
vkuzel opened this issue Nov 11, 2022 · 3 comments
Closed

Configuration property binding does not deal with bridge methods #33105

vkuzel opened this issue Nov 11, 2022 · 3 comments
Labels
type: bug A general bug
Milestone

Comments

@vkuzel
Copy link

vkuzel commented Nov 11, 2022

Summary

If a bean class contains multiple getter/setter methods with a same name but different types, the binding mechanism does not sort these methods properly which may lead to a non-deterministic/erroneous behaviour.

Affects v2.7.5 (current main).

Details

Let's have following bean classes:

static class Child extends Parent<ChildProperty> {
    @Override
    public ChildProperty getProperty() {
        return null;
    }
}

abstract static class Parent<T extends ParentProperty> {
    abstract public T getProperty();
}

static class ChildProperty extends ParentProperty {
}

abstract static class ParentProperty {
}

When compiled, the Child class does have two getter methods. One from the Parent class with the ParentProperty return type, another from it's own implementation with the ChildPropertyReturnType.

$ javap Child.class
Compiled from "Child.java"
class Child extends Parent<ChildProperty> {
  Child();
  public ChildProperty getProperty();
  public ParentProperty getProperty();
}

The binding mechanism in the JavaBeanBinder.Bean.addProperties() method, reads all declared methods via reflection and then tries to sort those methods by its name. Reason of sorting is non-determinism of the Class.getDeclaredMethods() reflection method. This issue was previously noted in #24068

https://github.com/spring-projects/spring-boot/blob/v2.7.5/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/JavaBeanBinder.java#L130-L143

The problems is: Sorting is solely based on method name which does not (fully) remove the aforementioned non-determinism.

Additional details

  • I've encountered this issue in Kotlin, where constructs like abstract var property: T are slightly more common.
  • Here is a test app FieldsOrderingTest.txt (please rename the txt extension to java)
  • Maybe unexpectedly Java compiler does not flag the public ParentProperty getProperty() on the Child class as abstract. The following code returns array of false values:
    Object[] abstracts = Arrays.stream(Child.class.getDeclaredMethods())
            .map(Method::getModifiers)
            .map(Modifier::isAbstract)
            .toArray();
    // abstracts = {Object[2]@710}
    // 0 = {Boolean@712} false
    // 1 = {Boolean@712} false
    This means the JavaBeanBinder.Bean.isCandidate() method returns true for both getProperty() methods in the Child class.
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Nov 11, 2022
@philwebb philwebb added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Nov 16, 2022
@philwebb philwebb added this to the 2.6.x milestone Nov 16, 2022
@philwebb philwebb changed the title Configuration property binding processes JavaBean methods in a non-deterministic order which may result in variable behavior - part 2 Configuration property binding does not deal with bridge methods Nov 16, 2022
@philwebb
Copy link
Member

Thanks very much for raising the issue @vkuzel.

I think that the problem might be being caused by synthetic bridge methods that are added by the compiler. I've attempted to fix this, but it's unfortunately quite hard to write a test that consistently works. I'd appreciate it If you could try the latest SNAPSHOT release and let me know if it actually solved your issue for real.

@philwebb philwebb modified the milestones: 2.6.x, 2.6.14 Nov 16, 2022
@vkuzel
Copy link
Author

vkuzel commented Nov 22, 2022

I'd appreciate it If you could try the latest SNAPSHOT release and let me know if it actually solved your issue for real.

Hey @philwebb I tested this in my environment and it works as expected. The non-bridge methods are filtered out, so the binding mechanism does not have issues with method resolution.

Thank you!

@wilkinsona
Copy link
Member

Thanks very much for taking the time to try a snapshot, @vkuzel. Much appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

No branches or pull requests

4 participants