OWB-1465 - Preserve ACC_BRIDGE / ACC_SYNTHETIC on bridge methods re-declared by normal scope proxies#140
OWB-1465 - Preserve ACC_BRIDGE / ACC_SYNTHETIC on bridge methods re-declared by normal scope proxies#140rzo1 wants to merge 2 commits into
Conversation
…eclared by normal scope proxies NormalScopeProxyFactory re-declares the JVM bridge methods of the proxied class but masked the method modifiers with PROTECTED | PUBLIC | VARARGS, dropping the ACC_BRIDGE flag. The proxy then exposed two regular methods with the same name and assignable parameter types, which breaks overload resolution in EL implementations (jakarta.el / Tomcat's ReflectionUtil tie-break on Method#isBridge()) with 'Unable to find unambiguous method'. Keep ACC_BRIDGE and ACC_SYNTHETIC in both generation paths (delegateNonInterceptedMethods for public methods, generateDelegationMethod for protected ones). Introduces MODIFIER_BRIDGE / MODIFIER_SYNTHETIC constants next to the existing MODIFIER_VARARGS as java.lang.reflect.Modifier does not expose them.
| @@ -395,7 +395,10 @@ protected void delegateNonInterceptedMethods(ClassLoader classLoader, ClassWrite | |||
| exceptionTypeNames[i] = Type.getType(exceptionTypes[i]).getInternalName(); | |||
There was a problem hiding this comment.
do we have the list of modifiers we want to drop? from the dozen of modifiers I did review we already filter the ones we dont' want before so we can almost passthrough or just force the ones we don't want if any no?
side note: we might extract the constant (Modifier.PROTECTED | Modifier.PUBLIC | MODIFIER_VARARGS | MODIFIER_BRIDGE | MODIFIER_SYNTHETIC for ex) instead of inlining it everytime.
There was a problem hiding this comment.
Concretely that would be something like modifiers & ~(Modifier.ABSTRACT | Modifier.SYNCHRONIZED | Modifier.NATIVE): ABSTRACT must go because the proxy method gets a body (interface methods arrive abstract), SYNCHRONIZED shouldn't be copied onto a delegating method (it would sync on the proxy instance, a semantics change; the real method still synchronizes), NATIVE for safety even though it's pre-filtered (didn't actually look that up). WDYT @struberg @tandraschko ?
Meanwhile, I have extracted the modifiers set in a constant.
There was a problem hiding this comment.
Perhaps the "allowlist" approach would also be a good follow-up but I let you decide how we handle it :)
There was a problem hiding this comment.
but it can't be abstract, cant be native, can't be final, can't be static, can't be private at that point in code. I'm not 100% sure about synchronized (guess you're right but also that the impact is almost 0 in practise)
so from my window we just forward them
maybe final has a particular case but it should be explicitly handled if the input method is final only IMHO
… feedback) Replace the four inlined modifier masks in NormalScopeProxyFactory and InterceptorDecoratorProxyFactory with a single documented constant in AbstractProxyFactory. No behavioural change for the interceptor proxy paths: bridge methods never reach them (unproxyableMethod skips them), and synthetic non-bridge methods (lambda$, access$) are private or static and filtered out before generation.
3 participants
NormalScopeProxyFactory re-declares the JVM bridge methods of the proxied class but masked the method modifiers with PROTECTED | PUBLIC | VARARGS, dropping the ACC_BRIDGE flag. The proxy then exposed two regular methods with the same name and assignable parameter types, which breaks overload resolution in EL implementations (jakarta.el / Tomcat's ReflectionUtil tie-break on Method#isBridge()) with 'Unable to find unambiguous method'.
Keep ACC_BRIDGE and ACC_SYNTHETIC in both generation paths (delegateNonInterceptedMethods for public methods, generateDelegationMethod for protected ones). Introduces MODIFIER_BRIDGE / MODIFIER_SYNTHETIC constants next to the existing MODIFIER_VARARGS as java.lang.reflect.Modifier does not expose them.