Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8218061

Lookup.unreflectSetter(Field) fails to throw IllegalAccessException for static final field

    XMLWordPrintable

    Details

    • Type: CSR
    • Status: Closed
    • Priority: P4
    • Resolution: Approved
    • Fix Version/s: 13
    • Component/s: core-libs
    • Labels:
      None
    • Subcomponent:
    • Compatibility Kind:
      behavioral
    • Compatibility Risk:
      low
    • Compatibility Risk Description:
      Hide
      If anyone is using Lookup.unreflectSetter(Field) to set static final fields via reflection, then this change will break their code and IllegalAccessException will be thrown.

      The compatibility risk is low because Field::set on a static final field throws IAE even setAccessible(true) and less likely that Field object will be passed in calling unreflectSetter.
      Show
      If anyone is using Lookup.unreflectSetter(Field) to set static final fields via reflection, then this change will break their code and IllegalAccessException will be thrown. The compatibility risk is low because Field::set on a static final field throws IAE even setAccessible(true) and less likely that Field object will be passed in calling unreflectSetter.
    • Interface Kind:
      Java API
    • Scope:
      SE

      Description

      Summary

      Lookup.unreflectSetter(Field) will now throw an IllegalAccessException for static final fields, regardless of whether setAccessible(boolean) has been set to true.

      Problem

      The problem is that, if setAccessible(boolean) has been set to true for a static final field, you can use Lookup.unreflectSetter(Field) to obtain a setter MethodHandle for the field. The original intent was to allow write access only if Field::set allows. This is correct behaviour for a final field, but not a field which is both static and final. Field::set does not allow setting a static final field even its accessible flag is set.

      A test case illustrating the bug will be attached, for clarity.

      Solution

      Change Lookup.unreflectSetter(Field) to throw IAE if the specified Field object is a static final field. Also clarify the specification of field setter functions findSetter, findStaticSetter, unreflectSetter w.r.t. access control on a final field and the special case of Field object.

      Proposed solution, includes a code change (a simple check on the field for static and final, followed by a throw) and some comment changes to improve spec clarity. They can be found on the web, here:

      http://hg.openjdk.java.net/jdk/jdk/rev/49c4b23d8d0a

      The method ordering change, and the annotation additions, should not affect functionality, and are purely for readability.

      Specification

      The detailed changes can be found in the webrev. The parts of the webrev relevant to this CSR are: - The behavioral change caused by the static final field check+throw - The comment changes clarifying what the code will do.

      The full webrev zip containing the current set of proposed changes will be attached to this issue.

      Spec Changes

      The spec in java.lang.invoke.MethodHandles.Lookup class, Lookup::findSetter, Lookup::fndStaticSetter and Lookup::unreflectSetter are updated to clarify that IAE will be thrown if the field has no write access.

      src/java.base/share/classes/java/lang/invoke/MethodHandles.java

      @@ -420,10 +420,14 @@
            * A lookup can fail, because
            * the containing class is not accessible to the lookup class, or
            * because the desired class member is missing, or because the
            * desired class member is not accessible to the lookup class, or
            * because the lookup object is not trusted enough to access the member.
      +     * In the case of a field setter function on a {@code final} field,
      +     * finality enforcement is treated as a kind of access control,
      +     * and the lookup will fail, except in special cases of
      +     * {@link Lookup#unreflectSetter Lookup.unreflectSetter}.
            * In any of these cases, a {@code ReflectiveOperationException} will be
            * thrown from the attempted lookup.  The exact class will be one of
            * the following:
            * <ul>
            * <li>NoSuchMethodException &mdash; if a method is requested but does not exist
      
      @@ -1436,10 +1440,11 @@
                * @param name the field's name
                * @param type the field's type
                * @return a method handle which can store values into the field
                * @throws NoSuchFieldException if the field does not exist
                * @throws IllegalAccessException if access checking fails, or if the field is {@code static}
      +         *                                or {@code final}
                * @exception SecurityException if a security manager is present and it
                *                              <a href="MethodHandles.Lookup.html#secmgr">refuses access</a>
                * @throws NullPointerException if any argument is null
                * @see #findVarHandle(Class, String, Class)
                */
      
      @@ -1559,10 +1564,11 @@
                * @param name the field's name
                * @param type the field's type
                * @return a method handle which can store values into the field
                * @throws NoSuchFieldException if the field does not exist
                * @throws IllegalAccessException if access checking fails, or if the field is not {@code static}
      +         *                                or is {@code final}
                * @exception SecurityException if a security manager is present and it
                *                              <a href="MethodHandles.Lookup.html#secmgr">refuses access</a>
                * @throws NullPointerException if any argument is null
                */
               public MethodHandle findStaticSetter(Class<?> refc, String name, Class<?> type) throws NoSuchFieldException, IllegalAccessException {
      
      @@ -1838,36 +1844,37 @@
      
               /**
                * Produces a method handle giving read access to a reflected field.
                * The type of the method handle will have a return type of the field's
                * value type.
      -         * If the field is static, the method handle will take no arguments.
      +         * If the field is {@code static}, the method handle will take no arguments.
                * Otherwise, its single argument will be the instance containing
                * the field.
      -         * If the field's {@code accessible} flag is not set,
      +         * If the {@code Field} object's {@code accessible} flag is not set,
                * access checking is performed immediately on behalf of the lookup class.
                * <p>
      -         * If the field is static, and
      +         * If the field is {@code final}, write access will not be
      +         * allowed and access checking will fail, except under certain
      +         * narrow circumstances documented for {@link Field#set Field.set}.
      +         * A method handle is returned only if a corresponding call to
      +         * the {@code Field} object's {@code set} method could return
      +         * normally.  In particular, fields which are both {@code static}
      +         * and {@code final} may never be set.
      +         * <p>
      +         * If the field is {@code static}, and
                * if the returned method handle is invoked, the field's class will
                * be initialized, if it has not already been initialized.
                * @param f the reflected field
                * @return a method handle which can load values from the reflected field
      -         * @throws IllegalAccessException if access checking fails
      +         * @throws IllegalAccessException if access checking fails,
      +         *         or if the field is {@code final} and write access
      +         *         is not enabled on the {@code Field} object
                * @throws NullPointerException if the argument is null
                */

      ...Further changes for this file are code only, and can be found in the webrev...

        Attachments

          Issue Links

            Activity

              People

              Assignee:
              afarley Adam Farley
              Reporter:
              afarley Adam Farley
              Reviewed By:
              David Holmes, Mandy Chung
              Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved: