* If the field is static, and @@ -1857,38 +1863,51 @@ public MethodHandle unreflectGetter(Field f) throws IllegalAccessException { return unreflectField(f, false); } - private MethodHandle unreflectField(Field f, boolean isSetter) throws IllegalAccessException { - MemberName field = new MemberName(f, isSetter); - assert(isSetter - ? MethodHandleNatives.refKindIsSetter(field.getReferenceKind()) - : MethodHandleNatives.refKindIsGetter(field.getReferenceKind())); - @SuppressWarnings("deprecation") - Lookup lookup = f.isAccessible() ? IMPL_LOOKUP : this; - return lookup.getDirectFieldNoSecurityManager(field.getReferenceKind(), f.getDeclaringClass(), field); - } /** * Produces a method handle giving write access to a reflected field. * The type of the method handle will have a void return type. - * If the field is static, the method handle will take a single + * If the field is {@code static}, the method handle will take a single * argument, of the field's value type, the value to be stored. * Otherwise, the two arguments will be the instance containing * the field, and the value to be stored. - * 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. *
- * 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. + *
+ * 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 store values into 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 */ public MethodHandle unreflectSetter(Field f) throws IllegalAccessException { return unreflectField(f, true); } + private MethodHandle unreflectField(Field f, boolean isSetter) throws IllegalAccessException { + MemberName field = new MemberName(f, isSetter); + if (isSetter && field.isStatic() && field.isFinal()) + throw field.makeAccessException("static final field has no write access", this); + assert(isSetter + ? MethodHandleNatives.refKindIsSetter(field.getReferenceKind()) + : MethodHandleNatives.refKindIsGetter(field.getReferenceKind())); + @SuppressWarnings("deprecation") + Lookup lookup = f.isAccessible() ? IMPL_LOOKUP : this; + return lookup.getDirectFieldNoSecurityManager(field.getReferenceKind(), f.getDeclaringClass(), field); + } + /** * Produces a VarHandle giving access to a reflected field {@code f} * of type {@code T} declared in a class of type {@code R}. --- old/test/jdk/java/lang/invoke/MethodHandlesGeneralTest.java 2019-03-22 12:00:35.332298961 +0000 +++ new/test/jdk/java/lang/invoke/MethodHandlesGeneralTest.java 2019-03-22 12:00:35.000293356 +0000 @@ -664,6 +664,7 @@ boolean testNPE = ((testMode0 & TEST_NPE) != 0); int testMode = testMode0 & ~(TEST_SETTER | TEST_BOUND | TEST_NPE); boolean positive = positive0 && !testNPE; + boolean isFinal; boolean isStatic; Class fclass; String fname; @@ -671,12 +672,14 @@ Field f = (fieldRef instanceof Field ? (Field)fieldRef : null); if (f != null) { isStatic = Modifier.isStatic(f.getModifiers()); + isFinal = Modifier.isFinal(f.getModifiers()); fclass = f.getDeclaringClass(); fname = f.getName(); ftype = f.getType(); } else { Object[] scnt = (Object[]) fieldRef; isStatic = (Boolean) scnt[0]; + isFinal = false; fclass = (Class) scnt[1]; fname = (String) scnt[2]; ftype = (Class) scnt[3]; @@ -720,18 +723,21 @@ ? NoSuchFieldException.class : IllegalAccessException.class, noAccess); + if (((testMode0 & TEST_SETTER) != 0) && (isFinal && isStatic)) return; // Final static field setter test failed as intended. if (verbosity >= 5) ex.printStackTrace(System.out); } if (verbosity >= 3) - System.out.println("find"+(isStatic?"Static":"")+(isGetter?"Getter":"Setter")+" "+fclass.getName()+"."+fname+"/"+ftype - +" => "+mh - +(noAccess == null ? "" : " !! "+noAccess)); + System.out.println((((testMode0 & TEST_UNREFLECT) != 0)?"unreflect":"find") + +(((testMode0 & TEST_FIND_STATIC) != 0)?"Static":"") + +(isGetter?"Getter":"Setter") + +" "+fclass.getName()+"."+fname+"/"+ftype + +" => "+mh + +(noAccess == null ? "" : " !! "+noAccess)); if (positive && !testNPE && noAccess != null) throw new RuntimeException(noAccess); assertEquals(positive0 ? "positive test" : "negative test erroneously passed", positive0, mh != null); + assertFalse("Setter methods should throw an exception if passed a final static field.", ((testMode0 & TEST_SETTER) != 0) && (isFinal && isStatic)); if (!positive && !testNPE) return; // negative access test failed as expected assertEquals((isStatic ? 0 : 1)+(isGetter ? 0 : 1), mh.type().parameterCount()); - - assertSame(mh.type(), expType); //assertNameStringContains(mh, fname); // This does not hold anymore with LFs HasFields fields = new HasFields(); @@ -778,8 +784,7 @@ } } assertEquals(sawValue, expValue); - if (f != null && f.getDeclaringClass() == HasFields.class - && !Modifier.isFinal(f.getModifiers())) { + if (f != null && f.getDeclaringClass() == HasFields.class && !isFinal) { Object random = randomArg(ftype); f.set(fields, random); expValue = random; @@ -813,8 +818,8 @@ } } } - if (f != null && f.getDeclaringClass() == HasFields.class) { - f.set(fields, value); // put it back + if ((f != null) && (f.getDeclaringClass() == HasFields.class) && !isFinal) { + f.set(fields, value); // put it back if we changed it. } if (testNPE) { if (caughtEx == null || !(caughtEx instanceof NullPointerException)) @@ -862,7 +867,6 @@ public void testSetter(int testMode) throws Throwable { Lookup lookup = PRIVATE; // FIXME: test more lookups than this one - startTest("unreflectSetter"); for (Object[] c : HasFields.CASES) { boolean positive = (c[1] != Error.class); testSetter(positive, lookup, c[0], c[1], testMode); --- old/test/jdk/java/lang/invoke/MethodHandlesTest.java 2019-03-22 12:00:35.988310037 +0000 +++ new/test/jdk/java/lang/invoke/MethodHandlesTest.java 2019-03-22 12:00:35.732305715 +0000 @@ -545,14 +545,14 @@ } public static class HasFields { - boolean fZ = false; - byte fB = (byte)'B'; - short fS = (short)'S'; - char fC = 'C'; - int fI = 'I'; - long fJ = 'J'; - float fF = 'F'; - double fD = 'D'; + boolean iZ = false; + byte iB = (byte)'B'; + short iS = (short)'S'; + char iC = 'C'; + int iI = 'I'; + long iJ = 'J'; + float iF = 'F'; + double iD = 'D'; static boolean sZ = true; static byte sB = 1+(byte)'B'; static short sS = 1+(short)'S'; @@ -561,11 +561,21 @@ static long sJ = 1+'J'; static float sF = 1+'F'; static double sD = 1+'D'; + final static boolean fsZ = false; + final static byte fsB = 2+(byte)'B'; + final static short fsS = 2+(short)'S'; + final static char fsC = 2+'C'; + final static int fsI = 2+'I'; + final static long fsJ = 2+'J'; + final static float fsF = 2+'F'; + final static double fsD = 2+'D'; - Object fL = 'L'; - String fR = "R"; + Object iL = 'L'; + String iR = "R"; static Object sL = 'M'; static String sR = "S"; + final static Object fsL = 'N'; + final static String fsR = "T"; static final Object[][] CASES; static { @@ -579,14 +589,16 @@ }; HasFields fields = new HasFields(); for (Object[] t : types) { - for (int kind = 0; kind <= 1; kind++) { + for (int kind = 0; kind <= 2; kind++) { boolean isStatic = (kind != 0); + boolean isFinal = (kind == 2); char btc = (Character)t[0]; - String name = (isStatic ? "s" : "f") + btc; + String name = (isStatic ? "s" : "i") + btc; + if (isFinal) name = "f" + name; Class type = (Class) t[1]; Object value; Field field; - try { + try { field = HasFields.class.getDeclaredField(name); } catch (NoSuchFieldException | SecurityException ex) { throw new InternalError("no field HasFields."+name); @@ -599,11 +611,14 @@ if (type == float.class) { float v = 'F'; if (isStatic) v++; + if (isFinal) v++; assertTrue(value.equals(v)); } + if (isFinal && isStatic) field.setAccessible(true); assertTrue(name.equals(field.getName())); assertTrue(type.equals(field.getType())); assertTrue(isStatic == (Modifier.isStatic(field.getModifiers()))); + assertTrue(isFinal == (Modifier.isFinal(field.getModifiers()))); cases.add(new Object[]{ field, value }); } } --- old/test/jdk/java/lang/invoke/VarHandles/accessibility/TestFieldLookupAccessibility.java 2019-03-22 12:00:36.624320776 +0000 +++ new/test/jdk/java/lang/invoke/VarHandles/accessibility/TestFieldLookupAccessibility.java 2019-03-22 12:00:36.380316655 +0000 @@ -110,6 +110,10 @@ Object lookup(MethodHandles.Lookup l, Field f) throws Exception { return l.unreflectSetter(cloneAndSetAccessible(f)); } + + boolean isAccessible(Field f) { + return !(Modifier.isStatic(f.getModifiers()) && Modifier.isFinal(f.getModifiers())); + } }, VH() { Object lookup(MethodHandles.Lookup l, Field f) throws Exception { @@ -210,7 +214,9 @@ } } else { - Assert.assertEquals(actualFieldNames, expected, "Accessibility failures differ:"); + //If this is an MH_UNREFLECT_SETTER_ACCESSIBLE test, then we only expect the static final fields to cause exceptions. + if (fl != FieldLookup.MH_UNREFLECT_SETTER_ACCESSIBLE && actualFieldNames.stream().anyMatch(s->!(s.contains("static")&&(s.contains("final"))))) + Assert.assertEquals(actualFieldNames, expected, "Accessibility failures differ:"); } } else {