2 candidates generated for a method where 1 was expected

TheAmg commented 3 months ago

Describe the bug

2 candidates are generated in the place of 1 if the following curl is invoked with unlogged-spring-maven-demo. curl - 1 ("findById")

curl --location 'http://localhost:8080/property/find?id=1' \
--header 'Authorization: Bearer eyJhbGciOiJIUzI1NiJ9.eyJzdWIiOiJhbWdAYW1nLmNvbSIsImV4cCI6MTkyNTYzOTMwN30.OBu-67QShlyZdY3sm4BxKbdye4nAsxeHvr5o85aG2YM'

On hitting this, and candidates load there will be 2 candidates for the method getById from PropertyServiceCEImpl. 1 with the correct return type PropertyDetails and the other with returnType Object.

This behaviour is not seen if the curl for findAll is used (there will only be 1 controller related candidate and 1 service related candidate).

curl - 2 ("findAll")

curl --location 'http://localhost:8080/property/findAll' \
--header 'Authorization: Bearer eyJhbGciOiJIUzI1NiJ9.eyJzdWIiOiJhbWdAYW1nLmNvbSIsImV4cCI6MTkyNTYzOTMwN30.OBu-67QShlyZdY3sm4BxKbdye4nAsxeHvr5o85aG2YM'

This might be happening due to CrudService (interface) methods that PropertyServiceCEImpl implements. The only distinguishing factor I see is that findAll() has no args and getById has an arg (parametrized).

Reproduction steps

Start unlogged-spring-maven-demo with 0.6.2.
Hit curl-1 from postman and check the list of candidates generated.

https://github.com/unloggedio/unlogged-sdk/assets/68552461/bfb16073-38e8-4a6f-bf28-d39024aa195c

Expected behavior

For only the candidate with the correct return type to be shown.

Additional context

No response

kartikeytewari-ul commented 2 months ago

RCA

When code is compiled by Java two methods are generated:
1. getById
  - Input type: Long
  - Output type: PropertyDetails
2. getById (ACC_SYNTHETIC)
  - Input type: Object
  - Output type: Object
This happens for some other methods in the class and happens irrespective of compilation with SDK.
The access bit of second method is set as ACC_SYNTHETIC. This implies that it is emitted by the compiler, and ignored by decompilers like fern-flower. Thus, the method does not comes up in decompiled output by it. Though, when the class is checked by javap -v the bytecode comes up in output.
When compiling with SDK classVisitor is invoked on class, and methodVisitor is invoked on both methods. Due to this probes are added to both of them.
When the method is invoked from curl, both methods are called and test candidates are formed for both of them.

Decompiled Method

 public java.lang.Object getById(java.lang.Object);
    descriptor: (Ljava/lang/Object;)Ljava/lang/Object;
    flags: (0x1041) ACC_PUBLIC, ACC_BRIDGE, ACC_SYNTHETIC
    Code:
      stack=2, locals=2, args_size=2
         0: aload_0
         1: aload_1
         2: checkcast     #11                 // class java/lang/Long
         5: invokevirtual #187                // Method getById:(Ljava/lang/Long;)Lorg/unlogged/demo/models/abstraction/PropertyDetails;
         8: areturn
      LineNumberTable:
        line 9: 0
      LocalVariableTable:
        Start  Length  Slot  Name   Signature
            0       9     0  this   Lorg/unlogged/demo/service/abstractions/PropertyServiceCEImpl;
    MethodParameters:
      Name                           Flags
      id                             synthetic

Proposed Fix

According to the spec sheet, ACC_SYNTHETIC is added to methods that are emitted by the compiler. This is not done on some exceptional cases like the constructor, which we need to probe always.
So a check can be added to see if the method has ACC_SYNTHETIC access bit set. If so then the method is not probed, a shadow unprobed method is not made and the key for it is not added to HashMap.

kartikeytewari-ul commented 2 months ago

Example of Synthetic injection is here.

kartikeytewari-ul commented 1 month ago

This is fixed with PR #112

unloggedio / unlogged-sdk