RFR: JDK-8216558: Lookup.unreflectSetter(Field) fails to throw IllegalAccessException for final fields
mandy.chung at oracle.com
Wed Jan 30 18:22:40 UTC 2019
On 1/30/19 7:52 AM, Adam Farley8 wrote:
> Hi Mandy,
> CSR has been raised: https://bugs.openjdk.java.net/browse/JDK-8218061
Thanks for doing it. A couple comments:
I think the compatibility risk should be low rather than minimal.
Even the code shouldn't be doing that, unreflectSetter does
allow to set a static final field in the current implementation.
It's low because Field::set throws exception if it's a static
final field and so it's expected that very few existing libraries
would use unreflectSetter.
FWIW. There are few existing libraries hacking the reflection
to change their static final fields but very few.
Since the spec diff is small, I suggest to cut-n-paste the diff
on the javadoc to the specification section and easier to see
the spec change. webrev is not generally needed for CSR but
welcome to include that if it helps the review.
I made some minor edit to the CSR to make the problem and solution
> The webrev I used includes John's comments, your additions, the
> and I also took the liberty of moving the unreflectField method underneath
> the unreflectSetter method, because it seemed strange that it was located
> between unreflectSetter and unreflectGetter.
> Online webrev has been updated to include these changes:
Looks good but the patch still needs a regression test. Please
include the regression test for code review.
Formatting nit: a space is missing between 'if' and '(' and
plase use the 4-space identation of the throw.
1901 if(isSetter && field.isStatic() && field.isFinal())
1902 throw field.makeAccessException("static final
field has no write access", this);
More information about the core-libs-dev