8033278: Missed access checks for Lookup.unreflect* after 8032585

Reviewed-by: jrose, twisti

src/share/classes/sun/invoke/util/VerifyAccess.java

@@ -90,33 +90,26 @@ public class VerifyAccess {
         if (allowedModes == 0)  return false;
         assert((allowedModes & PUBLIC) != 0 &&
                (allowedModes & ~(ALL_ACCESS_MODES|PACKAGE_ALLOWED)) == 0);
-        // Usually refc and defc are the same, but if they differ, verify them both.
-        if (refc != defc) {
-            if (!isClassAccessible(refc, lookupClass, allowedModes)) {
-                // Note that defc is verified in the switch below.
-                return false;
-            }
-            if ((mods & (ALL_ACCESS_MODES|STATIC)) == (PROTECTED|STATIC) &&
-                (allowedModes & PROTECTED_OR_PACKAGE_ALLOWED) != 0) {
-                // Apply the special rules for refc here.
-                if (!isRelatedClass(refc, lookupClass))
-                    return isSamePackage(defc, lookupClass);
-                // If refc == defc, the call to isPublicSuperClass will do
-                // the whole job, since in that case refc (as defc) will be
-                // a superclass of the lookup class.
-            }
+        // The symbolic reference class (refc) must always be fully verified.
+        if (!isClassAccessible(refc, lookupClass, allowedModes)) {
+            return false;
         }
+        // Usually refc and defc are the same, but verify defc also in case they differ.
         if (defc == lookupClass &&
             (allowedModes & PRIVATE) != 0)
             return true;        // easy check; all self-access is OK
         switch (mods & ALL_ACCESS_MODES) {
         case PUBLIC:
-            if (refc != defc)  return true;  // already checked above
-            return isClassAccessible(refc, lookupClass, allowedModes);
+            return true;  // already checked above
         case PROTECTED:
             if ((allowedModes & PROTECTED_OR_PACKAGE_ALLOWED) != 0 &&
                 isSamePackage(defc, lookupClass))
                 return true;
+            if ((allowedModes & PROTECTED) == 0)
+                return false;
+            if ((mods & STATIC) != 0 &&
+                !isRelatedClass(refc, lookupClass))
+                return false;
             if ((allowedModes & PROTECTED) != 0 &&
                 isSuperClass(defc, lookupClass))
                 return true;

test/java/lang/invoke/ProtectedMemberDifferentPackage/Test.java

@@ -24,7 +24,7 @@
 
 /**
  * @test
- * @bug 8032585
+ * @bug 8032585 8033278
  * @summary JSR292: IllegalAccessError when attempting to invoke protected method from different package
  *
  * @compile p1/T2.java p2/T3.java

test/java/lang/invoke/ProtectedMemberDifferentPackage/p1/T2.java

@@ -23,8 +23,57 @@
  */
 package p1;
 
+import p2.T3;
+
+import java.lang.invoke.MethodHandle;
+import java.lang.invoke.MethodHandles;
+import java.lang.invoke.MethodHandles.Lookup;
+import java.lang.invoke.MethodType;
+import java.util.concurrent.Callable;
+
 class T1 {
-    protected void m() { System.out.println("T1.m");}
+    protected        void m1() {}
+    protected static void m2() {}
 }
 
-public class T2 extends T1 {}
+public class T2 extends T1 {
+    public static void main(String[] args) throws Throwable {
+        Lookup LOOKUP = T3.lookup();
+        Class<IllegalAccessException> IAE = IllegalAccessException.class;
+
+        assertFailure(IAE, () -> LOOKUP.findVirtual(T1.class, "m1", MethodType.methodType(void.class)));
+        assertFailure(IAE, () -> LOOKUP.findStatic(T1.class, "m2", MethodType.methodType(void.class)));
+
+        assertSuccess(() -> LOOKUP.findVirtual(T2.class, "m1", MethodType.methodType(void.class)));
+        assertSuccess(() -> LOOKUP.findVirtual(T3.class, "m1", MethodType.methodType(void.class)));
+
+        assertSuccess(() -> LOOKUP.findStatic(T2.class, "m2", MethodType.methodType(void.class)));
+        assertSuccess(() -> LOOKUP.findStatic(T3.class, "m2", MethodType.methodType(void.class)));
+
+        assertFailure(IAE, () -> LOOKUP.unreflect(T1.class.getDeclaredMethod("m1")));
+        assertFailure(IAE, () -> LOOKUP.unreflect(T1.class.getDeclaredMethod("m2")));
+
+        System.out.println("TEST PASSED");
+    }
+
+    public static void assertFailure(Class<? extends Throwable> expectedError, Callable r) {
+        try {
+            r.call();
+        } catch(Throwable e) {
+            if (expectedError.isAssignableFrom(e.getClass())) {
+                return; // expected error
+            } else {
+                throw new Error("Unexpected error type: "+e.getClass()+"; expected type: "+expectedError, e);
+            }
+        }
+        throw new Error("No error");
+    }
+
+    public static void assertSuccess(Callable r) {
+        try {
+            r.call();
+        } catch(Throwable e) {
+            throw new Error("Unexpected error", e);
+        }
+    }
+}

test/java/lang/invoke/ProtectedMemberDifferentPackage/p2/T3.java

@@ -25,13 +25,8 @@ package p2;
 
 import p1.T2;
 
-import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodHandles;
-import java.lang.invoke.MethodType;
 
 public class T3 extends T2 {
-    public static void main(String[] args) throws Throwable {
-        MethodHandles.lookup().findVirtual(T3.class, "m", MethodType.methodType(void.class));
-        System.out.println("TEST PASSED");
-    }
+    public static MethodHandles.Lookup lookup() { return MethodHandles.lookup(); }
 }