Looks good, especially the test. Paul.
> On Feb 20, 2018, at 10:21 AM, mandy chung <mandy.ch...@oracle.com> wrote: > > 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
