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

Muzzle incorrectly constructs field references #2823

Closed
mateuszrzeszutek opened this issue Apr 20, 2021 · 2 comments · Fixed by #2870
Closed

Muzzle incorrectly constructs field references #2823

mateuszrzeszutek opened this issue Apr 20, 2021 · 2 comments · Fixed by #2870
Assignees
Labels
bug Something isn't working

Comments

@mateuszrzeszutek
Copy link
Member

Describe the bug
I've noticed that in #2819 muzzle should have prevented the instrumentation from being applied, but it didn't: a NoSuchFieldError was thrown instead.

When you print out the muzzle references you can see that it incorrectly assigns the qs field to our helper class:

  io.opentelemetry.instrumentation.rxjava2.TracingObserver extends<io.reactivex.internal.observers.BasicFuseableObserver>
    Field: PRIVATE_OR_HIGHER NON_STATIC FieldRef:qsio/reactivex/internal/fuseable/QueueDisposable
  io.reactivex.internal.observers.BasicFuseableObserver
    Source: io.opentelemetry.instrumentation.rxjava2.TracingObserver:0
    Source: io.opentelemetry.instrumentation.rxjava2.TracingObserver:39
    Method: NON_STATIC PROTECTED_OR_HIGHER <init>(Lio/reactivex/Observer;)V

And the BasicFuseableObserver which actually owns this field only has a reference to its constructor. We don't check field access in our helper classes, so muzzle fails to recognize a mismatch in the runtime.

What did you expect to see?
Muzzle should assign the field to the class that actually defines it.

@mateuszrzeszutek mateuszrzeszutek added the bug Something isn't working label Apr 20, 2021
@laurit
Copy link
Contributor

laurit commented Apr 20, 2021

For field access this.qs compiler generates bytecode where field owner is the type of this even when the field is in super class. This behaviour is the same for methods and static fields, owner is whatever the type is that access is performed on, actual field owner is resolved during runtime.
Are you suggesting that we somehow try to figure out the actual owner of field/method?

@mateuszrzeszutek
Copy link
Member Author

Yeah, figuring this out in compile time would be pretty pretty hard. We could do something else: in the runtime ReferenceMatcher we have access to the application classloader (via BB TypePool) and we can check if a field that we have a reference to exists in one of the super classes. If not, we have a mismatch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants