8177474: Do not emit warnings when illegal access is allowed by --add-exports/--add-opens
Reviewed-by: chegar, mchung
--- a/jdk/src/java.base/share/classes/jdk/internal/module/IllegalAccessLogger.java Fri Mar 24 09:16:58 2017 -0700
+++ b/jdk/src/java.base/share/classes/jdk/internal/module/IllegalAccessLogger.java Fri Mar 24 16:35:51 2017 +0000
@@ -25,6 +25,7 @@
package jdk.internal.module;
+import java.io.PrintStream;
import java.lang.invoke.MethodHandles;
import java.lang.reflect.Module;
import java.net.URL;
@@ -42,6 +43,9 @@
import java.util.function.Supplier;
import java.util.stream.Collectors;
+import jdk.internal.loader.BootLoader;
+import sun.security.action.GetPropertyAction;
+
/**
* Supports logging of access to members of API packages that are exported or
* opened via backdoor mechanisms to code in unnamed modules.
@@ -49,16 +53,25 @@
public final class IllegalAccessLogger {
- // true to print stack trace
- private static final boolean PRINT_STACK_TRACE;
- static {
- String s = System.getProperty("sun.reflect.debugModuleAccessChecks");
- PRINT_STACK_TRACE = "access".equals(s);
+ /**
+ * Holder class to lazily create the StackWalker object and determine
+ * if the stack trace should be printed
+ */
+ static class Holder {
+ static final StackWalker STACK_WALKER;
+ static final boolean PRINT_STACK_TRACE;
+
+ static {
+ PrivilegedAction<StackWalker> pa = () ->
+ StackWalker.getInstance(StackWalker.Option.RETAIN_CLASS_REFERENCE);
+ STACK_WALKER = AccessController.doPrivileged(pa);
+
+ String name = "sun.reflect.debugModuleAccessChecks";
+ String value = GetPropertyAction.privilegedGetProperty(name, null);
+ PRINT_STACK_TRACE = "access" .equals(value);
+ }
}
- private static final StackWalker STACK_WALKER
- = StackWalker.getInstance(StackWalker.Option.RETAIN_CLASS_REFERENCE);
-
// the maximum number of frames to capture
private static final int MAX_STACK_FRAMES = 32;
@@ -72,10 +85,15 @@
private final Map<Module, Map<String, String>> exported;
private final Map<Module, Map<String, String>> opened;
+ // the print stream to send the warnings
+ private final PrintStream warningStream;
+
private IllegalAccessLogger(Map<Module, Map<String, String>> exported,
- Map<Module, Map<String, String>> opened) {
+ Map<Module, Map<String, String>> opened,
+ PrintStream warningStream) {
this.exported = deepCopy(exported);
this.opened = deepCopy(opened);
+ this.warningStream = warningStream;
}
/**
@@ -168,7 +186,7 @@
*/
private void log(Class<?> caller, String what, Supplier<String> msgSupplier) {
// stack trace without the top-most frames in java.base
- List<StackWalker.StackFrame> stack = STACK_WALKER.walk(s ->
+ List<StackWalker.StackFrame> stack = Holder.STACK_WALKER.walk(s ->
s.dropWhile(this::isJavaBase)
.limit(MAX_STACK_FRAMES)
.collect(Collectors.toList())
@@ -184,13 +202,13 @@
// log message if first usage
if (firstUsage) {
String msg = msgSupplier.get();
- if (PRINT_STACK_TRACE) {
+ if (Holder.PRINT_STACK_TRACE) {
synchronized (OUTPUT_LOCK) {
- System.err.println(msg);
- stack.forEach(f -> System.err.println("\tat " + f));
+ warningStream.println(msg);
+ stack.forEach(f -> warningStream.println("\tat " + f));
}
} else {
- System.err.println(msg);
+ warningStream.println(msg);
}
}
}
@@ -265,8 +283,10 @@
* A builder for IllegalAccessLogger objects.
*/
public static class Builder {
+ private final Module UNNAMED = BootLoader.getUnnamedModule();
private Map<Module, Map<String, String>> exported;
private Map<Module, Map<String, String>> opened;
+ private PrintStream warningStream = System.err;
public Builder() { }
@@ -276,30 +296,37 @@
this.opened = deepCopy(opened);
}
- public void logAccessToExportedPackage(Module m, String pn, String how) {
- if (!m.isExported(pn)) {
+ public Builder logAccessToExportedPackage(Module m, String pn, String how) {
+ if (!m.isExported(pn, UNNAMED)) {
if (exported == null)
exported = new HashMap<>();
exported.computeIfAbsent(m, k -> new HashMap<>()).putIfAbsent(pn, how);
}
+ return this;
}
- public void logAccessToOpenPackage(Module m, String pn, String how) {
+ public Builder logAccessToOpenPackage(Module m, String pn, String how) {
// opens implies exported at run-time.
logAccessToExportedPackage(m, pn, how);
- if (!m.isOpen(pn)) {
+ if (!m.isOpen(pn, UNNAMED)) {
if (opened == null)
opened = new HashMap<>();
opened.computeIfAbsent(m, k -> new HashMap<>()).putIfAbsent(pn, how);
}
+ return this;
+ }
+
+ public Builder warningStream(PrintStream warningStream) {
+ this.warningStream = Objects.requireNonNull(warningStream);
+ return this;
}
/**
* Builds the logger.
*/
public IllegalAccessLogger build() {
- return new IllegalAccessLogger(exported, opened);
+ return new IllegalAccessLogger(exported, opened, warningStream);
}
}
--- a/jdk/src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java Fri Mar 24 09:16:58 2017 -0700
+++ b/jdk/src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java Fri Mar 24 16:35:51 2017 +0000
@@ -515,44 +515,42 @@
* additional packages specified on the command-line.
*/
private static void addExtraExportsAndOpens(Layer bootLayer) {
- IllegalAccessLogger.Builder builder = new IllegalAccessLogger.Builder();
-
// --add-exports
String prefix = "jdk.module.addexports.";
Map<String, List<String>> extraExports = decode(prefix);
if (!extraExports.isEmpty()) {
- addExtraExportsOrOpens(bootLayer, extraExports, false, builder);
+ addExtraExportsOrOpens(bootLayer, extraExports, false);
}
// --add-opens
prefix = "jdk.module.addopens.";
Map<String, List<String>> extraOpens = decode(prefix);
if (!extraOpens.isEmpty()) {
- addExtraExportsOrOpens(bootLayer, extraOpens, true, builder);
+ addExtraExportsOrOpens(bootLayer, extraOpens, true);
}
// --permit-illegal-access
if (getAndRemoveProperty("jdk.module.permitIllegalAccess") != null) {
warn("--permit-illegal-access will be removed in the next major release");
+ IllegalAccessLogger.Builder builder = new IllegalAccessLogger.Builder();
+ Module unnamed = BootLoader.getUnnamedModule();
bootLayer.modules().stream().forEach(m -> {
m.getDescriptor()
.packages()
.stream()
- .filter(pn -> !m.isOpen(pn))
+ .filter(pn -> !m.isOpen(pn, unnamed)) // skip if opened by --add-opens
.forEach(pn -> {
builder.logAccessToOpenPackage(m, pn, "--permit-illegal-access");
Modules.addOpensToAllUnnamed(m, pn);
});
});
+ IllegalAccessLogger.setIllegalAccessLogger(builder.build());
}
-
- IllegalAccessLogger.setIllegalAccessLogger(builder.build());
}
private static void addExtraExportsOrOpens(Layer bootLayer,
Map<String, List<String>> map,
- boolean opens,
- IllegalAccessLogger.Builder builder)
+ boolean opens)
{
String option = opens ? ADD_OPENS : ADD_EXPORTS;
for (Map.Entry<String, List<String>> e : map.entrySet()) {
@@ -600,10 +598,8 @@
}
if (allUnnamed) {
if (opens) {
- builder.logAccessToOpenPackage(m, pn, option);
Modules.addOpensToAllUnnamed(m, pn);
} else {
- builder.logAccessToExportedPackage(m, pn, option);
Modules.addExportsToAllUnnamed(m, pn);
}
} else {
--- a/jdk/src/java.base/share/classes/sun/launcher/LauncherHelper.java Fri Mar 24 09:16:58 2017 -0700
+++ b/jdk/src/java.base/share/classes/sun/launcher/LauncherHelper.java Fri Mar 24 16:35:51 2017 +0000
@@ -85,7 +85,6 @@
import java.util.stream.Stream;
import jdk.internal.misc.VM;
-import jdk.internal.module.IllegalAccessLogger;
import jdk.internal.module.Modules;
@@ -429,20 +428,14 @@
abort(null, "java.launcher.jar.error3", jarname);
}
- // Add-Exports and Add-Opens to allow illegal access
+ // Add-Exports and Add-Opens
String exports = mainAttrs.getValue(ADD_EXPORTS);
if (exports != null) {
- String warn = getLocalizedMessage("java.launcher.permitaccess.warning",
- jarname, ADD_EXPORTS);
- System.err.println(warn);
- addExportsOrOpens(exports, false, ADD_EXPORTS);
+ addExportsOrOpens(exports, false);
}
String opens = mainAttrs.getValue(ADD_OPENS);
if (opens != null) {
- String warn = getLocalizedMessage("java.launcher.permitaccess.warning",
- jarname, ADD_OPENS);
- System.err.println(warn);
- addExportsOrOpens(opens, true, ADD_OPENS);
+ addExportsOrOpens(opens, true);
}
/*
@@ -467,15 +460,7 @@
* Process the Add-Exports or Add-Opens value. The value is
* {@code <module>/<package> ( <module>/<package>)*}.
*/
- static void addExportsOrOpens(String value, boolean open, String how) {
- IllegalAccessLogger.Builder builder;
- IllegalAccessLogger logger = IllegalAccessLogger.illegalAccessLogger();
- if (logger == null) {
- builder = new IllegalAccessLogger.Builder();
- } else {
- builder = logger.toBuilder();
- }
-
+ static void addExportsOrOpens(String value, boolean open) {
for (String moduleAndPackage : value.split(" ")) {
String[] s = moduleAndPackage.trim().split("/");
if (s.length == 2) {
@@ -485,18 +470,14 @@
Layer.boot().findModule(mn).ifPresent(m -> {
if (m.getDescriptor().packages().contains(pn)) {
if (open) {
- builder.logAccessToOpenPackage(m, pn, how);
Modules.addOpensToAllUnnamed(m, pn);
} else {
- builder.logAccessToExportedPackage(m, pn, how);
Modules.addExportsToAllUnnamed(m, pn);
}
}
});
}
}
-
- IllegalAccessLogger.setIllegalAccessLogger(builder.build());
}
// From src/share/bin/java.c:
--- a/jdk/src/java.base/share/classes/sun/launcher/resources/launcher.properties Fri Mar 24 09:16:58 2017 -0700
+++ b/jdk/src/java.base/share/classes/sun/launcher/resources/launcher.properties Fri Mar 24 16:35:51 2017 +0000
@@ -211,6 +211,4 @@
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
--- a/jdk/test/tools/launcher/modules/permit/PermitIllegalAccess.java Fri Mar 24 09:16:58 2017 -0700
+++ b/jdk/test/tools/launcher/modules/permit/PermitIllegalAccess.java Fri Mar 24 16:35:51 2017 +0000
@@ -29,7 +29,10 @@
* @summary Basic test for java --permit-illegal-access
*/
+import java.util.ArrayList;
+import java.util.Arrays;
import java.util.List;
+import java.util.stream.Stream;
import jdk.testlibrary.ProcessTools;
import jdk.testlibrary.OutputAnalyzer;
@@ -58,10 +61,13 @@
* 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)
+ private OutputAnalyzer tryAction(String action, int count, String... args)
+ throws Exception
+ {
+ Stream<String> s1 = Stream.of(args);
+ Stream<String> s2 = Stream.of("-cp", TEST_CLASSES, TEST_MAIN, action, "" + count);
+ String[] opts = Stream.concat(s1, s2).toArray(String[]::new);
+ return ProcessTools.executeTestJava(opts)
.outputTo(System.out)
.errorTo(System.out);
}
@@ -70,16 +76,10 @@
* 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)
+ 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);
+ return tryAction(action, count, "--permit-illegal-access");
}
/**
@@ -195,6 +195,61 @@
}
/**
+ * Permit access to succeed with --add-exports. No warning should be printed.
+ */
+ public void testAccessWithAddExports() throws Exception {
+ tryAction("access", 1, "--add-exports", "java.base/sun.security.x509=ALL-UNNAMED")
+ .stdoutShouldNotContain(WARNING)
+ .stdoutShouldNotContain("IllegalAccessException")
+ .stderrShouldNotContain(WARNING)
+ .stderrShouldNotContain("IllegalAccessException")
+ .shouldHaveExitValue(0);
+ }
+
+ /**
+ * Permit access to succeed with --add-exports and --permit-illegal-access.
+ * The only warning emitted should be the startup warning.
+ */
+ public void testAccessWithePermittedAddExports() throws Exception {
+ tryAction("access", 1, "--permit-illegal-access",
+ "--add-exports", "java.base/sun.security.x509=ALL-UNNAMED")
+ .stdoutShouldNotContain(WARNING)
+ .stdoutShouldNotContain("IllegalAccessException")
+ .stderrShouldContain(STARTUP_WARNING)
+ .stderrShouldNotContain("IllegalAccessException")
+ .stderrShouldNotContain(ILLEGAL_ACCESS_WARNING)
+ .shouldHaveExitValue(0);
+ }
+
+ /**
+ * Permit setAccessible to succeed with --add-opens. No warning should be printed.
+ */
+ public void testSetAccessibleWithAddOpens() throws Exception {
+ tryAction("setAccessible", 1, "--add-opens", "java.base/java.lang=ALL-UNNAMED")
+ .stdoutShouldNotContain(WARNING)
+ .stdoutShouldNotContain("InaccessibleObjectException")
+ .stderrShouldNotContain(WARNING)
+ .stderrShouldNotContain("InaccessibleObjectException")
+ .shouldHaveExitValue(0);
+ }
+
+ /**
+ * Permit setAccessible to succeed with both --add-opens and --permit-illegal-access.
+ * The only warning emitted should be the startup warning.
+ */
+ public void testSetAccessiblePermittedWithAddOpens() throws Exception {
+ tryAction("setAccessible", 1, "--permit-illegal-access",
+ "--add-opens", "java.base/java.lang=ALL-UNNAMED")
+ .stdoutShouldNotContain(WARNING)
+ .stdoutShouldNotContain("InaccessibleObjectException")
+ .stderrShouldContain(STARTUP_WARNING)
+ .stderrShouldNotContain("InaccessibleObjectException")
+ .stderrShouldNotContain(ILLEGAL_ACCESS_WARNING)
+ .shouldHaveExitValue(0);
+ }
+
+
+ /**
* Returns the number of lines in the given input that contain the
* given char sequence.
*/