[nestmates] JDK-8235602: Re-examine if a hidden class should trust final non static fields

[Paul Sandoz]

I think this is a reasonable restriction.

AccessibleObject
--

 179      * <p> If this reflected object is a final field declared in a
 180      * {@linkplain Class#isHiddenClass() hidden class}, this method can
 181      * only be used to suppress checks for Java language access control
 182      * to gain read access but not write access even if the {@code accessible}
 183      * flag is {@code true}.  That is, final fields in a hidden class are
 184      * not modifiable.


Suggested:

This method cannot be used to enable /write/ access to a final field declared in a 
{@linkplain Class#isHiddenClass() hidden class}, since such fields are not 
modifiable.  The {@code accessible} flag when {@code true} 
suppresses Java language access control checks to *only* enable read access to such fields.


Field
—

      * <p>If the underlying field is final, the method throws an
      * {@code IllegalAccessException} unless {@code setAccessible(true)}
      * has succeeded for this {@code Field} object
-     * and the field is non-static. Setting a final field in this way
+     * and the field is non-static and its declaring class is not
+     * a {@linkplain Class#isHiddenClass() hidden} class.
+     * Setting a final field in this way
      * is meaningful only during deserialization or reconstruction of
      * instances of classes with blank final fields, before they are
      * made available for access by other parts of a program. Use in
      * any other context may have unpredictable effects, including cases
      * in which other parts of a program continue to use the original

Suggested

 the method throws …. unless the following conditions are met: 
{@code setAccessible(true)} has succeeded for this {@code Field} object;
the field is non-static; and the field's declaring class is not a {@linkplain Class#isHiddenClass() hidden} class.

Paul.

> On Jan 27, 2020, at 1:08 PM, Mandy Chung <mandy.chung at oracle.com> wrote:
> 
> Deserialization is the primary use case for core reflection to allow writing to final fields after object construction.  Serializable hidden classes are required to use its own custom serialization mechanism.  With the properties of hidden classes, "non-discoverable" and "non-modifiable", I propose to make hidden classes final fields not-writeable via reflection and enables frameworks and language implementors to benefit from the final fields optimization with the use of hidden classes.   Core platform classes like lambdas will not have to pay for the price just because a few libraries (e.g. mocking) might want to write to final fields.
> 
> java.lang.reflect.Field::set and Lookup::unreflectSetter already disallow the write-access to static final fields regardless of the accessible flag.  This proposes to disallow write-access to final non-static fields declared in a hidden class.
> 
> There is no change to AccessibleObject::setAccessible that can be used to suppress language access control check.   Most frameworks use setAccessible to break encapsulation and access a member and they should not be impacted.
> 
> I see that this spec change sets a precedence for JDK-8233873 [1] "final field values should be trusted constants", the general fix.
> 
> Webrev:
> http://cr.openjdk.java.net/~mchung/valhalla/webrevs/8235602/webrev.01/
> 
> This patch also puts a stop in using sun.misc.Unsafe to find field offsets of hidden class.  jdk.internal.misc.Unsafe::objectFieldOffset is used by reflection machinery that I will follow up next.
> 
> Mandy
> [1] https://bugs.openjdk.java.net/browse/JDK-8233873
>