This patch looks good. It's unfortunate that setAccessible was not final
to begin with. I agree that this fix is a good compromise with a simple
fix and low incompatibility. Is there a CSR to review?
Mandy
On 2/19/18 8:57 AM, Alan Bateman wrote:
> AccessibleObject's setAccessible(boolean) is currently not caller >
sensitive but the overrides in Method/Field/Constructor are. This >
awkwardness stems from its constructor being protected and the method >
not being final. It is thus possible to extend the class outside of >
the java.lang.reflect package and override this method (at least one >
popular library does this). Ideally the constructor should have been >
package private and/or the method be final but it's not possible to >
change this after 20 years. > > The consequence of the method in the
base class not being caller > sensitive is that it's possible to use a
minimally trusted lookup to > get a method handle to the method. Paul,
Mandy and I chatted about > this one recently. We prototyped changes to
the MH implementation to > special case this method and treat it "as if"
it is caller sensitive. > This maximizes compatibility but has the
downside that it makes it > harder to audit and somewhat fragile. In the
end, we concluded it > would be simpler to add the @CS annotation to
this method so that it > is treated consistently. The downside of this
is that custom > AccessibleObject implementations need to override
setAccessible if > they want to be invoked using method handles obtained
from a > minimally trusted lookup. > > The proposed changes are simple.
The removal of "checkMemberAccess" > from canBeCalledVirtual is just a
clean-up because this method is no > longer needs special casing (it was
degraded for Java SE 10 as > envisaged in JEP 176). It's not the goal
here to improve the > performance of canBeCalledVirtual but there may be
opportunities to > look at that with a separate issue: >
http://cr.openjdk.java.net/~alanb/8196830/webrev/ > > -Alan
