changeset 19355:d1759eda4acd

More cleanup and test for --permit-illegal-access
author alanb
date Tue, 21 Mar 2017 13:42:15 +0000
parents 38a48583e0d8
children 4f0101b769a9
files src/java.base/share/classes/java/lang/invoke/MethodHandles.java src/java.base/share/classes/java/lang/reflect/AccessibleObject.java src/java.base/share/classes/jdk/internal/module/IllegalAccessLogger.java src/java.base/share/classes/jdk/internal/reflect/Reflection.java src/java.base/share/classes/sun/launcher/LauncherHelper.java src/java.base/share/classes/sun/launcher/resources/launcher.properties test/tools/launcher/modules/permit/AttemptAccess.java test/tools/launcher/modules/permit/PermitIllegalAccess.java
diffstat 8 files changed, 306 insertions(+), 23 deletions(-) [+]
line wrap: on
line diff
--- a/src/java.base/share/classes/java/lang/invoke/MethodHandles.java	Mon Mar 20 19:05:02 2017 +0000
+++ b/src/java.base/share/classes/java/lang/invoke/MethodHandles.java	Tue Mar 21 13:42:15 2017 +0000
@@ -200,7 +200,7 @@
         if (!callerModule.isNamed() && targetModule.isNamed()) {
             IllegalAccessLogger logger = IllegalAccessLogger.illegalAccessLogger();
             if (logger != null) {
-                logger.logIfOpenByBackdoor(lookup, targetClass);
+                logger.logIfOpenedByBackdoor(lookup, targetClass);
             }
         }
         return new Lookup(targetClass);
--- a/src/java.base/share/classes/java/lang/reflect/AccessibleObject.java	Mon Mar 20 19:05:02 2017 +0000
+++ b/src/java.base/share/classes/java/lang/reflect/AccessibleObject.java	Tue Mar 21 13:42:15 2017 +0000
@@ -319,7 +319,7 @@
 
         // package is open to caller
         if (declaringModule.isOpen(pn, callerModule)) {
-            logIfOpenByBackdoor(caller, declaringClass);
+            logIfOpenedByBackdoor(caller, declaringClass);
             return true;
         }
 
@@ -353,14 +353,14 @@
         return false;
     }
 
-    private void logIfOpenByBackdoor(Class<?> caller, Class<?> declaringClass) {
+    private void logIfOpenedByBackdoor(Class<?> caller, Class<?> declaringClass) {
         Module callerModule = caller.getModule();
         Module targetModule = declaringClass.getModule();
         // callerModule is null during early startup
         if (callerModule != null && !callerModule.isNamed() && targetModule.isNamed()) {
             IllegalAccessLogger logger = IllegalAccessLogger.illegalAccessLogger();
             if (logger != null) {
-                logger.logIfOpenByBackdoor(caller, declaringClass, this::toShortString);
+                logger.logIfOpenedByBackdoor(caller, declaringClass, this::toShortString);
             }
         }
     }
@@ -628,14 +628,14 @@
     private boolean slowVerifyAccess(Class<?> caller, Class<?> memberClass,
                                      Class<?> targetClass, int modifiers)
     {
-        if (Reflection.verifyMemberAccess(caller, memberClass, targetClass, modifiers)) {
-            // access okay
-            logIfExportedByBackdoor(caller, memberClass);
-        } else {
+        if (!Reflection.verifyMemberAccess(caller, memberClass, targetClass, modifiers)) {
             // access denied
             return false;
         }
 
+        // access okay
+        logIfExportedByBackdoor(caller, memberClass);
+
         // Success: Update the cache.
         Object cache = (targetClass != null
                         && Modifier.isProtected(modifiers)
--- a/src/java.base/share/classes/jdk/internal/module/IllegalAccessLogger.java	Mon Mar 20 19:05:02 2017 +0000
+++ b/src/java.base/share/classes/jdk/internal/module/IllegalAccessLogger.java	Tue Mar 21 13:42:15 2017 +0000
@@ -109,9 +109,9 @@
      *
      * The {@code what} parameter supplies the message that describes the member.
      */
-    public void logIfOpenByBackdoor(Class<?> caller,
-                                    Class<?> target,
-                                    Supplier<String> whatSupplier) {
+    public void logIfOpenedByBackdoor(Class<?> caller,
+                                      Class<?> target,
+                                      Supplier<String> whatSupplier) {
         Map<String, String> packages = opened.get(target.getModule());
         if (packages != null) {
             String how = packages.get(target.getPackageName());
@@ -144,7 +144,7 @@
      * Logs access to caller class if the class is in a package that is opened via
      * a backdoor mechanism.
      */
-    public void logIfOpenByBackdoor(MethodHandles.Lookup caller, Class<?> target) {
+    public void logIfOpenedByBackdoor(MethodHandles.Lookup caller, Class<?> target) {
         Map<String, String> packages = opened.get(target.getModule());
         if (packages != null) {
             String how = packages.get(target.getPackageName());
--- a/src/java.base/share/classes/jdk/internal/reflect/Reflection.java	Mon Mar 20 19:05:02 2017 +0000
+++ b/src/java.base/share/classes/jdk/internal/reflect/Reflection.java	Tue Mar 21 13:42:15 2017 +0000
@@ -336,11 +336,10 @@
      * Returns an IllegalAccessException with an exception message based on
      * the access that is denied.
      */
-    public static IllegalAccessException newIllegalAccessException(
-        Class<?> currentClass,
-        Class<?> memberClass,
-        Class<?> targetClass,
-        int modifiers)
+    public static IllegalAccessException newIllegalAccessException(Class<?> currentClass,
+                                                                   Class<?> memberClass,
+                                                                   Class<?> targetClass,
+                                                                   int modifiers)
         throws IllegalAccessException
     {
         String currentSuffix = "";
--- a/src/java.base/share/classes/sun/launcher/LauncherHelper.java	Mon Mar 20 19:05:02 2017 +0000
+++ b/src/java.base/share/classes/sun/launcher/LauncherHelper.java	Tue Mar 21 13:42:15 2017 +0000
@@ -429,14 +429,20 @@
                 abort(null, "java.launcher.jar.error3", jarname);
             }
 
-            // Add-Exports and Add-Opens to break encapsulation
+            // Add-Exports and Add-Opens to allow illegal access
             String exports = mainAttrs.getValue(ADD_EXPORTS);
             if (exports != null) {
-                addExportsOrOpens(exports, false);
+                String warn = getLocalizedMessage("java.launcher.permitaccess.warning",
+                                                  jarname, ADD_EXPORTS);
+                System.err.println(warn);
+                addExportsOrOpens(exports, false, ADD_EXPORTS);
             }
             String opens = mainAttrs.getValue(ADD_OPENS);
             if (opens != null) {
-                addExportsOrOpens(opens, true);
+                String warn = getLocalizedMessage("java.launcher.permitaccess.warning",
+                                                   jarname, ADD_OPENS);
+                System.err.println(warn);
+                addExportsOrOpens(opens, true, ADD_OPENS);
             }
 
             /*
@@ -461,7 +467,7 @@
      * Process the Add-Exports or Add-Opens value. The value is
      * {@code <module>/<package> ( <module>/<package>)*}.
      */
-    static void addExportsOrOpens(String value, boolean open) {
+    static void addExportsOrOpens(String value, boolean open, String how) {
         IllegalAccessLogger.Builder builder;
         IllegalAccessLogger logger = IllegalAccessLogger.illegalAccessLogger();
         if (logger == null) {
@@ -479,10 +485,10 @@
                 Layer.boot().findModule(mn).ifPresent(m -> {
                     if (m.getDescriptor().packages().contains(pn)) {
                         if (open) {
-                            builder.logAccessToOpenPackage(m, pn, ADD_OPENS);
+                            builder.logAccessToOpenPackage(m, pn, how);
                             Modules.addOpensToAllUnnamed(m, pn);
                         } else {
-                            builder.logAccessToExportedPackage(m, pn, ADD_EXPORTS);
+                            builder.logAccessToExportedPackage(m, pn, how);
                             Modules.addExportsToAllUnnamed(m, pn);
                         }
                     }
--- a/src/java.base/share/classes/sun/launcher/resources/launcher.properties	Mon Mar 20 19:05:02 2017 +0000
+++ b/src/java.base/share/classes/sun/launcher/resources/launcher.properties	Tue Mar 21 13:42:15 2017 +0000
@@ -211,4 +211,6 @@
 java.launcher.module.error3=\
     Error: Unable to load main class {0} from module {1}\n\
     \t{2}
+java.launcher.permitaccess.warning=\
+    WARNING: Main manifest of {0} contains {1} attribute to permit illegal access
 
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/tools/launcher/modules/permit/AttemptAccess.java	Tue Mar 21 13:42:15 2017 +0000
@@ -0,0 +1,68 @@
+/*
+ * Copyright (c) 2017, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+import java.lang.reflect.Constructor;
+import java.lang.reflect.Method;
+
+/**
+ * Launched by PermitIllegalAccess to attempt illegal access.
+ */
+
+public class AttemptAccess {
+
+    public static void main(String[] args) throws Exception {
+        String action = args[0];
+        int count = Integer.parseInt(args[1]);
+
+        for (int i=0; i<count; i++) {
+            switch (action) {
+                case "access":
+                    tryAccess();
+                    break;
+                case "setAccessible":
+                    trySetAccessible();
+                    break;
+                case "trySetAccessible":
+                    tryTrySetAccessible();
+                    break;
+            }
+        }
+    }
+
+    static void tryAccess() throws Exception {
+        Class<?> clazz = Class.forName("sun.security.x509.X500Name");
+        Constructor<?> ctor = clazz.getConstructor(String.class);
+        Object name = ctor.newInstance("CN=user");
+    }
+
+    static void trySetAccessible() throws Exception {
+        Method find = ClassLoader.class.getDeclaredMethod("findClass", String.class);
+        find.setAccessible(true);
+    }
+
+    static void tryTrySetAccessible() throws Exception {
+        Method find = ClassLoader.class.getDeclaredMethod("findClass", String.class);
+        find.trySetAccessible();
+    }
+
+}
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/tools/launcher/modules/permit/PermitIllegalAccess.java	Tue Mar 21 13:42:15 2017 +0000
@@ -0,0 +1,208 @@
+/*
+ * Copyright (c) 2017, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/**
+ * @test
+ * @library /lib/testlibrary
+ * @build PermitIllegalAccess AttemptAccess jdk.testlibrary.*
+ * @run testng PermitIllegalAccess
+ * @summary Basic test for java --permit-illegal-access
+ */
+
+import java.util.List;
+
+import jdk.testlibrary.ProcessTools;
+import jdk.testlibrary.OutputAnalyzer;
+
+import org.testng.annotations.Test;
+import static org.testng.Assert.*;
+
+/**
+ * Basic test of --permit-illegal-access to ensure that it permits access
+ * via core reflection and setAccessible/trySetAccessible.
+ */
+
+@Test
+public class PermitIllegalAccess {
+
+    static final String TEST_CLASSES = System.getProperty("test.classes");
+    static final String TEST_MAIN = "AttemptAccess";
+
+    static final String WARNING = "WARNING";
+    static final String STARTUP_WARNING =
+        "WARNING: --permit-illegal-access will be removed in the next major release";
+    static final String ILLEGAL_ACCESS_WARNING =
+        "WARNING: Illegal access by " + TEST_MAIN;
+
+    /**
+     * Launches AttemptAccess to execute an action, returning the OutputAnalyzer
+     * to analyze the output/exitCode.
+     */
+    private OutputAnalyzer tryAction(String action, int count) throws Exception {
+        String arg = "" + count;
+        return ProcessTools
+                .executeTestJava("-cp", TEST_CLASSES, TEST_MAIN, action, arg)
+                .outputTo(System.out)
+                .errorTo(System.out);
+    }
+
+    /**
+     * Launches AttemptAccess with --permit-illegal-access to execute an action,
+     * returning the OutputAnalyzer to analyze the output/exitCode.
+     */
+    private OutputAnalyzer tryActionPermittingIllegalAccess(String action,
+                                                            int count)
+        throws Exception
+    {
+        String arg = "" + count;
+        return ProcessTools
+                .executeTestJava("-cp", TEST_CLASSES, "--permit-illegal-access",
+                                 TEST_MAIN, action, arg)
+                .outputTo(System.out)
+                .errorTo(System.out);
+    }
+
+    /**
+     * Sanity check to ensure that IllegalAccessException is thrown.
+     */
+    public void testAccessFail() throws Exception {
+        int exitValue = tryAction("access", 1)
+                .stdoutShouldNotContain(WARNING)
+                .stdoutShouldNotContain("IllegalAccessException")
+                .stderrShouldNotContain(WARNING)
+                .stderrShouldContain("IllegalAccessException")
+                .getExitValue();
+        assertTrue(exitValue != 0);
+    }
+
+    /**
+     * Sanity check to ensure that InaccessibleObjectException is thrown.
+     */
+    public void testSetAccessibleFail() throws Exception {
+        int exitValue = tryAction("setAccessible", 1)
+                .stdoutShouldNotContain(WARNING)
+                .stdoutShouldNotContain("InaccessibleObjectException")
+                .stderrShouldNotContain(WARNING)
+                .stderrShouldContain("InaccessibleObjectException")
+                .getExitValue();
+        assertTrue(exitValue != 0);
+    }
+
+    /**
+     * Permit illegal access to succeed
+     */
+    public void testAccessPermitted() throws Exception {
+        tryActionPermittingIllegalAccess("access", 1)
+                .stdoutShouldNotContain(WARNING)
+                .stdoutShouldNotContain("IllegalAccessException")
+                .stderrShouldContain(STARTUP_WARNING)
+                .stderrShouldNotContain("IllegalAccessException")
+                .stderrShouldContain(ILLEGAL_ACCESS_WARNING)
+                .shouldHaveExitValue(0);
+    }
+
+    /**
+     * Permit repeated illegal access to succeed
+     */
+    public void testRepeatedAccessPermitted() throws Exception {
+        OutputAnalyzer outputAnalyzer = tryActionPermittingIllegalAccess("access", 10)
+                .stdoutShouldNotContain(WARNING)
+                .stdoutShouldNotContain("IllegalAccessException")
+                .stderrShouldContain(STARTUP_WARNING)
+                .stderrShouldNotContain("IllegalAccessException")
+                .stderrShouldContain(ILLEGAL_ACCESS_WARNING)
+                .shouldHaveExitValue(0);;
+
+        // should only have one illegal access warning
+        assertTrue(containsCount(outputAnalyzer.asLines(), ILLEGAL_ACCESS_WARNING) == 1);
+    }
+
+    /**
+     * Permit setAccessible to succeed
+     */
+    public void testSetAccessiblePermitted() throws Exception {
+        tryActionPermittingIllegalAccess("setAccessible", 1)
+                .stdoutShouldNotContain(WARNING)
+                .stdoutShouldNotContain("InaccessibleObjectException")
+                .stderrShouldContain(STARTUP_WARNING)
+                .stderrShouldNotContain("InaccessibleObjectException")
+                .stderrShouldContain(ILLEGAL_ACCESS_WARNING)
+                .shouldHaveExitValue(0);
+    }
+
+    /**
+     * Permit repeated calls to setAccessible to succeed
+     */
+    public void testRepeatedSetAccessiblePermitted() throws Exception {
+        OutputAnalyzer outputAnalyzer = tryActionPermittingIllegalAccess("setAccessible", 10)
+                .stdoutShouldNotContain(WARNING)
+                .stdoutShouldNotContain("InaccessibleObjectException")
+                .stderrShouldContain(STARTUP_WARNING)
+                .stderrShouldNotContain("InaccessibleObjectException")
+                .stderrShouldContain(ILLEGAL_ACCESS_WARNING)
+                .shouldHaveExitValue(0);
+
+        // should only have one illegal access warning
+        assertTrue(containsCount(outputAnalyzer.asLines(), ILLEGAL_ACCESS_WARNING) == 1);
+    }
+
+    /**
+     * Permit trySetAccessible to succeed
+     */
+    public void testTrySetAccessiblePermitted() throws Exception {
+        tryActionPermittingIllegalAccess("trySetAccessible", 1)
+                .stdoutShouldNotContain(WARNING)
+                .stderrShouldContain(STARTUP_WARNING)
+                .stderrShouldContain(ILLEGAL_ACCESS_WARNING)
+                .shouldHaveExitValue(0);
+    }
+
+    /**
+     * Permit repeated calls to trySetAccessible to succeed
+     */
+    public void testRepeatedTrySetAccessiblePermitted() throws Exception {
+        OutputAnalyzer outputAnalyzer = tryActionPermittingIllegalAccess("trySetAccessible", 10)
+                .stdoutShouldNotContain(WARNING)
+                .stdoutShouldNotContain("InaccessibleObjectException")
+                .stderrShouldContain(STARTUP_WARNING)
+                .stderrShouldNotContain("InaccessibleObjectException")
+                .stderrShouldContain(ILLEGAL_ACCESS_WARNING)
+                .shouldHaveExitValue(0);
+
+        // should only have one illegal access warning
+        assertTrue(containsCount(outputAnalyzer.asLines(), ILLEGAL_ACCESS_WARNING) == 1);
+
+    }
+
+    /**
+     * Returns the number of lines in the given input that contain the
+     * given char sequence.
+     */
+    private int containsCount(List<String> lines, CharSequence cs) {
+        int count = 0;
+        for (String line : lines) {
+            if (line.contains(cs)) count++;
+        }
+        return count;
+    }
+}