RFR: JDK-8216558: Lookup.unreflectSetter(Field) fails to throw IllegalAccessException for final fields
adam.farley at uk.ibm.com
Thu Jan 31 12:18:05 UTC 2019
I've made the changes to the webrev and uploaded the corrected version.
I've also uploaded the zips in the csr and bug.
As for the test, I have mentioned that the test is attached to the bug. I
attached the same test to the CSR.
Are you saying that the test supplied is not a suitable test?
Mandy Chung <mandy.chung at oracle.com> wrote on 30/01/2019 18:22:40:
> From: Mandy Chung <mandy.chung at oracle.com>
> To: Adam Farley8 <adam.farley at uk.ibm.com>
> Cc: core-libs-dev <core-libs-dev at openjdk.java.net>
> Date: 30/01/2019 18:22
> Subject: Re: RFR: JDK-8216558: Lookup.unreflectSetter(Field) fails
> to throw IllegalAccessException for final fields
> Hi Adam,
> On 1/30/19 7:52 AM, Adam Farley8 wrote:
> > Hi Mandy,
> > CSR has been raised: https://urldefense.proofpoint.com/v2/url?
> 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
> > one-line-IF,
> > and I also took the liberty of moving the unreflectField method
> > the unreflectSetter method, because it seemed strange that it was
> > between unreflectSetter and unreflectGetter.
> > Online webrev has been updated to include these changes:
> > https://urldefense.proofpoint.com/v2/url?
> 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);
Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
More information about the core-libs-dev