8256299: Implement JEP 396: Strongly Encapsulate JDK Internals by Default

Co-authored-by: Alan Bateman <alanb@openjdk.org>
Reviewed-by: mchung, alanb
This commit is contained in:
Mark Reinhold 2020-12-08 22:12:24 +00:00
parent c47ab5f6b7
commit ed4c4ee73b
5 changed files with 90 additions and 139 deletions

View file

@ -2461,6 +2461,7 @@ jint Arguments::parse_each_vm_init_arg(const JavaVMInitArgs* args, bool* patch_m
return res; return res;
} }
} else if (match_option(option, "--illegal-access=", &tail)) { } else if (match_option(option, "--illegal-access=", &tail)) {
warning("Option --illegal-access is deprecated and will be removed in a future release.");
if (!create_module_property("jdk.module.illegalAccess", tail, ExternalProperty)) { if (!create_module_property("jdk.module.illegalAccess", tail, ExternalProperty)) {
return JNI_ENOMEM; return JNI_ENOMEM;
} }

View file

@ -27,35 +27,27 @@ package jdk.internal.module;
import jdk.internal.misc.CDS; import jdk.internal.misc.CDS;
/** /**
* Used by ModuleBootstrap for archiving the boot layer and the builder needed to * Used by ModuleBootstrap for archiving the boot layer.
* set the IllegalAccessLogger.
*/ */
class ArchivedBootLayer { class ArchivedBootLayer {
private static ArchivedBootLayer archivedBootLayer; private static ArchivedBootLayer archivedBootLayer;
private final ModuleLayer bootLayer; private final ModuleLayer bootLayer;
private final IllegalAccessLogger.Builder builder;
private ArchivedBootLayer(ModuleLayer bootLayer, private ArchivedBootLayer(ModuleLayer bootLayer) {
IllegalAccessLogger.Builder builder) {
this.bootLayer = bootLayer; this.bootLayer = bootLayer;
this.builder = builder;
} }
ModuleLayer bootLayer() { ModuleLayer bootLayer() {
return bootLayer; return bootLayer;
} }
IllegalAccessLogger.Builder illegalAccessLoggerBuilder() {
return builder;
}
static ArchivedBootLayer get() { static ArchivedBootLayer get() {
return archivedBootLayer; return archivedBootLayer;
} }
static void archive(ModuleLayer layer, IllegalAccessLogger.Builder builder) { static void archive(ModuleLayer layer) {
archivedBootLayer = new ArchivedBootLayer(layer, builder); archivedBootLayer = new ArchivedBootLayer(layer);
} }
static { static {

View file

@ -24,8 +24,6 @@
*/ */
package jdk.internal.module; package jdk.internal.module;
import java.util.Map;
import java.util.Set;
import java.util.function.Function; import java.util.function.Function;
import java.lang.module.Configuration; import java.lang.module.Configuration;
import java.lang.module.ModuleFinder; import java.lang.module.ModuleFinder;
@ -33,7 +31,7 @@ import jdk.internal.misc.CDS;
/** /**
* Used by ModuleBootstrap for archiving the configuration for the boot layer, * Used by ModuleBootstrap for archiving the configuration for the boot layer,
* the system module finder, and the maps used to create the IllegalAccessLogger. * and the system module finder.
*/ */
class ArchivedModuleGraph { class ArchivedModuleGraph {
private static ArchivedModuleGraph archivedModuleGraph; private static ArchivedModuleGraph archivedModuleGraph;
@ -43,23 +41,17 @@ class ArchivedModuleGraph {
private final ModuleFinder finder; private final ModuleFinder finder;
private final Configuration configuration; private final Configuration configuration;
private final Function<String, ClassLoader> classLoaderFunction; private final Function<String, ClassLoader> classLoaderFunction;
private final Map<String, Set<String>> concealedPackagesToOpen;
private final Map<String, Set<String>> exportedPackagesToOpen;
private ArchivedModuleGraph(boolean hasSplitPackages, private ArchivedModuleGraph(boolean hasSplitPackages,
boolean hasIncubatorModules, boolean hasIncubatorModules,
ModuleFinder finder, ModuleFinder finder,
Configuration configuration, Configuration configuration,
Function<String, ClassLoader> classLoaderFunction, Function<String, ClassLoader> classLoaderFunction) {
Map<String, Set<String>> concealedPackagesToOpen,
Map<String, Set<String>> exportedPackagesToOpen) {
this.hasSplitPackages = hasSplitPackages; this.hasSplitPackages = hasSplitPackages;
this.hasIncubatorModules = hasIncubatorModules; this.hasIncubatorModules = hasIncubatorModules;
this.finder = finder; this.finder = finder;
this.configuration = configuration; this.configuration = configuration;
this.classLoaderFunction = classLoaderFunction; this.classLoaderFunction = classLoaderFunction;
this.concealedPackagesToOpen = concealedPackagesToOpen;
this.exportedPackagesToOpen = exportedPackagesToOpen;
} }
ModuleFinder finder() { ModuleFinder finder() {
@ -74,14 +66,6 @@ class ArchivedModuleGraph {
return classLoaderFunction; return classLoaderFunction;
} }
Map<String, Set<String>> concealedPackagesToOpen() {
return concealedPackagesToOpen;
}
Map<String, Set<String>> exportedPackagesToOpen() {
return exportedPackagesToOpen;
}
boolean hasSplitPackages() { boolean hasSplitPackages() {
return hasSplitPackages; return hasSplitPackages;
} }
@ -110,16 +94,12 @@ class ArchivedModuleGraph {
boolean hasIncubatorModules, boolean hasIncubatorModules,
ModuleFinder finder, ModuleFinder finder,
Configuration configuration, Configuration configuration,
Function<String, ClassLoader> classLoaderFunction, Function<String, ClassLoader> classLoaderFunction) {
Map<String, Set<String>> concealedPackagesToOpen,
Map<String, Set<String>> exportedPackagesToOpen) {
archivedModuleGraph = new ArchivedModuleGraph(hasSplitPackages, archivedModuleGraph = new ArchivedModuleGraph(hasSplitPackages,
hasIncubatorModules, hasIncubatorModules,
finder, finder,
configuration, configuration,
classLoaderFunction, classLoaderFunction);
concealedPackagesToOpen,
exportedPackagesToOpen);
} }
static { static {

View file

@ -141,14 +141,14 @@ public final class ModuleBootstrap {
private static boolean canUseArchivedBootLayer() { private static boolean canUseArchivedBootLayer() {
return getProperty("jdk.module.upgrade.path") == null && return getProperty("jdk.module.upgrade.path") == null &&
getProperty("jdk.module.path") == null && getProperty("jdk.module.path") == null &&
getProperty("jdk.module.patch.0") == null && // --patch-module getProperty("jdk.module.patch.0") == null && // --patch-module
getProperty("jdk.module.main") == null && getProperty("jdk.module.main") == null && // --module
getProperty("jdk.module.addmods.0") == null && // --add-modules getProperty("jdk.module.addmods.0") == null && // --add-modules
getProperty("jdk.module.limitmods") == null && getProperty("jdk.module.limitmods") == null && // --limit-modules
getProperty("jdk.module.addreads.0") == null && // --add-reads getProperty("jdk.module.addreads.0") == null && // --add-reads
getProperty("jdk.module.addexports.0") == null && // --add-exports getProperty("jdk.module.addexports.0") == null && // --add-exports
getProperty("jdk.module.addopens.0") == null && // --add-opens getProperty("jdk.module.addopens.0") == null && // --add-opens
getProperty("jdk.module.illegalAccess") == null; getProperty("jdk.module.illegalAccess") == null; // --illegal-access
} }
/** /**
@ -172,12 +172,6 @@ public final class ModuleBootstrap {
// assume boot layer has at least one module providing a service // assume boot layer has at least one module providing a service
// that is mapped to the application class loader. // that is mapped to the application class loader.
JLA.bindToLoader(bootLayer, ClassLoaders.appClassLoader()); JLA.bindToLoader(bootLayer, ClassLoaders.appClassLoader());
// IllegalAccessLogger needs to be set
var illegalAccessLoggerBuilder = archivedBootLayer.illegalAccessLoggerBuilder();
if (illegalAccessLoggerBuilder != null) {
illegalAccessLoggerBuilder.complete();
}
} else { } else {
bootLayer = boot2(); bootLayer = boot2();
} }
@ -192,10 +186,10 @@ public final class ModuleBootstrap {
ModuleFinder upgradeModulePath = finderFor("jdk.module.upgrade.path"); ModuleFinder upgradeModulePath = finderFor("jdk.module.upgrade.path");
ModuleFinder appModulePath = finderFor("jdk.module.path"); ModuleFinder appModulePath = finderFor("jdk.module.path");
boolean isPatched = patcher.hasPatches(); boolean isPatched = patcher.hasPatches();
String mainModule = System.getProperty("jdk.module.main"); String mainModule = System.getProperty("jdk.module.main");
Set<String> addModules = addModules(); Set<String> addModules = addModules();
Set<String> limitModules = limitModules(); Set<String> limitModules = limitModules();
String illegalAccess = getAndRemoveProperty("jdk.module.illegalAccess");
PrintStream traceOutput = null; PrintStream traceOutput = null;
String trace = getAndRemoveProperty("jdk.module.showModuleResolution"); String trace = getAndRemoveProperty("jdk.module.showModuleResolution");
@ -227,7 +221,8 @@ public final class ModuleBootstrap {
&& !haveModulePath && !haveModulePath
&& addModules.isEmpty() && addModules.isEmpty()
&& limitModules.isEmpty() && limitModules.isEmpty()
&& !isPatched) { && !isPatched
&& illegalAccess == null) {
systemModuleFinder = archivedModuleGraph.finder(); systemModuleFinder = archivedModuleGraph.finder();
hasSplitPackages = archivedModuleGraph.hasSplitPackages(); hasSplitPackages = archivedModuleGraph.hasSplitPackages();
hasIncubatorModules = archivedModuleGraph.hasIncubatorModules(); hasIncubatorModules = archivedModuleGraph.hasIncubatorModules();
@ -464,21 +459,15 @@ public final class ModuleBootstrap {
addExtraReads(bootLayer); addExtraReads(bootLayer);
boolean extraExportsOrOpens = addExtraExportsAndOpens(bootLayer); boolean extraExportsOrOpens = addExtraExportsAndOpens(bootLayer);
Map<String, Set<String>> concealedPackagesToOpen; if (illegalAccess != null) {
Map<String, Set<String>> exportedPackagesToOpen; assert systemModules != null;
if (archivedModuleGraph != null) { addIllegalAccess(illegalAccess,
concealedPackagesToOpen = archivedModuleGraph.concealedPackagesToOpen(); systemModules,
exportedPackagesToOpen = archivedModuleGraph.exportedPackagesToOpen(); upgradeModulePath,
} else {
concealedPackagesToOpen = systemModules.concealedPackagesToOpen();
exportedPackagesToOpen = systemModules.exportedPackagesToOpen();
}
IllegalAccessLogger.Builder builder =
addIllegalAccess(upgradeModulePath,
concealedPackagesToOpen,
exportedPackagesToOpen,
bootLayer, bootLayer,
extraExportsOrOpens); extraExportsOrOpens);
}
Counters.add("jdk.module.boot.7.adjustModulesTime"); Counters.add("jdk.module.boot.7.adjustModulesTime");
// save module finders for later use // save module finders for later use
@ -495,12 +484,9 @@ public final class ModuleBootstrap {
hasIncubatorModules, hasIncubatorModules,
systemModuleFinder, systemModuleFinder,
cf, cf,
clf, clf);
concealedPackagesToOpen,
exportedPackagesToOpen);
if (!hasSplitPackages && !hasIncubatorModules) { if (!hasSplitPackages && !hasIncubatorModules) {
ArchivedBootLayer.archive(bootLayer, builder); ArchivedBootLayer.archive(bootLayer);
} }
} }
@ -794,38 +780,32 @@ public final class ModuleBootstrap {
} }
/** /**
* Process the --illegal-access option (and its default) to open packages * Process the --illegal-access option to open packages of system modules
* of system modules in the boot layer to code in unnamed modules. * in the boot layer to code in unnamed modules.
*/ */
private static IllegalAccessLogger.Builder private static void addIllegalAccess(String illegalAccess,
addIllegalAccess(ModuleFinder upgradeModulePath, SystemModules systemModules,
Map<String, Set<String>> concealedPackagesToOpen, ModuleFinder upgradeModulePath,
Map<String, Set<String>> exportedPackagesToOpen, ModuleLayer bootLayer,
ModuleLayer bootLayer, boolean extraExportsOrOpens) {
boolean extraExportsOrOpens) {
String value = getAndRemoveProperty("jdk.module.illegalAccess");
IllegalAccessLogger.Mode mode = IllegalAccessLogger.Mode.ONESHOT;
if (value != null) {
switch (value) {
case "deny":
return null;
case "permit":
break;
case "warn":
mode = IllegalAccessLogger.Mode.WARN;
break;
case "debug":
mode = IllegalAccessLogger.Mode.DEBUG;
break;
default:
fail("Value specified to --illegal-access not recognized:"
+ " '" + value + "'");
return null;
}
}
IllegalAccessLogger.Builder builder
= new IllegalAccessLogger.Builder(mode, System.err);
if (illegalAccess.equals("deny"))
return; // nothing to do
IllegalAccessLogger.Mode mode = switch (illegalAccess) {
case "permit" -> IllegalAccessLogger.Mode.ONESHOT;
case "warn" -> IllegalAccessLogger.Mode.WARN;
case "debug" -> IllegalAccessLogger.Mode.DEBUG;
default -> {
fail("Value specified to --illegal-access not recognized:"
+ " '" + illegalAccess + "'");
yield null;
}
};
var builder = new IllegalAccessLogger.Builder(mode, System.err);
Map<String, Set<String>> concealedPackagesToOpen = systemModules.concealedPackagesToOpen();
Map<String, Set<String>> exportedPackagesToOpen = systemModules.exportedPackagesToOpen();
if (concealedPackagesToOpen.isEmpty() && exportedPackagesToOpen.isEmpty()) { if (concealedPackagesToOpen.isEmpty() && exportedPackagesToOpen.isEmpty()) {
// need to generate (exploded build) // need to generate (exploded build)
IllegalAccessMaps maps = IllegalAccessMaps.generate(limitedFinder()); IllegalAccessMaps maps = IllegalAccessMaps.generate(limitedFinder());
@ -887,7 +867,6 @@ public final class ModuleBootstrap {
} }
builder.complete(); builder.complete();
return builder;
} }
/** /**

View file

@ -1,5 +1,5 @@
/* /*
* Copyright (c) 2017, 2018, Oracle and/or its affiliates. All rights reserved. * Copyright (c) 2017, 2020, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
* *
* This code is free software; you can redistribute it and/or modify it * This code is free software; you can redistribute it and/or modify it
@ -239,15 +239,15 @@ public class IllegalAccessTest {
} }
@Test(dataProvider = "denyCases") @Test(dataProvider = "denyCases")
public void testDeny(String action, Result expectedResult) throws Exception {
run(action, expectedResult, "--illegal-access=deny");
}
@Test(dataProvider = "permitCases")
public void testDefault(String action, Result expectedResult) throws Exception { public void testDefault(String action, Result expectedResult) throws Exception {
run(action, expectedResult); run(action, expectedResult);
} }
@Test(dataProvider = "denyCases")
public void testDeny(String action, Result expectedResult) throws Exception {
run(action, expectedResult, "--illegal-access=deny");
}
@Test(dataProvider = "permitCases") @Test(dataProvider = "permitCases")
public void testPermit(String action, Result expectedResult) throws Exception { public void testPermit(String action, Result expectedResult) throws Exception {
run(action, expectedResult, "--illegal-access=permit"); run(action, expectedResult, "--illegal-access=permit");
@ -267,41 +267,42 @@ public class IllegalAccessTest {
run(action, expectedResult, "--illegal-access=debug"); run(action, expectedResult, "--illegal-access=debug");
} }
/** /**
* Specify --add-exports to export a package * Specify --add-exports to export a package
*/ */
public void testWithAddExportsOption() throws Exception { public void testWithAddExportsOption() throws Exception {
// warning // not accessible
run("reflectPublicMemberNonExportedPackage", successWithWarning()); run("reflectPublicMemberNonExportedPackage", fail("IllegalAccessException"));
// no warning due to --add-exports // should succeed with --add-exports
run("reflectPublicMemberNonExportedPackage", successNoWarning(), run("reflectPublicMemberNonExportedPackage", successNoWarning(),
"--add-exports", "java.base/sun.security.x509=ALL-UNNAMED"); "--add-exports", "java.base/sun.security.x509=ALL-UNNAMED");
// attempt two illegal accesses, one allowed by --add-exports // not accessible
run("reflectPublicMemberNonExportedPackage" run("setAccessibleNonPublicMemberNonExportedPackage", fail("InaccessibleObjectException"));
+ ",setAccessibleNonPublicMemberExportedPackage",
successWithWarning(), // should fail as --add-exports does not open package
"--add-exports", "java.base/sun.security.x509=ALL-UNNAMED"); run("setAccessibleNonPublicMemberNonExportedPackage", fail("InaccessibleObjectException"),
"--add-exports", "java.base/sun.nio.ch=ALL-UNNAMED");
} }
/** /**
* Specify --add-open to open a package * Specify --add-open to open a package
*/ */
public void testWithAddOpensOption() throws Exception { public void testWithAddOpensOption() throws Exception {
// warning // not accessible
run("setAccessibleNonPublicMemberExportedPackage", successWithWarning()); run("reflectPublicMemberNonExportedPackage", fail("IllegalAccessException"));
// no warning due to --add-opens // should succeed with --add-opens
run("reflectPublicMemberNonExportedPackage", successNoWarning(),
"--add-opens", "java.base/sun.security.x509=ALL-UNNAMED");
// not accessible
run("setAccessibleNonPublicMemberExportedPackage", fail("InaccessibleObjectException"));
// should succeed with --add-opens
run("setAccessibleNonPublicMemberExportedPackage", successNoWarning(), run("setAccessibleNonPublicMemberExportedPackage", successNoWarning(),
"--add-opens", "java.base/java.lang=ALL-UNNAMED"); "--add-opens", "java.base/java.lang=ALL-UNNAMED");
// attempt two illegal accesses, one allowed by --add-opens
run("reflectPublicMemberNonExportedPackage"
+ ",setAccessibleNonPublicMemberExportedPackage",
successWithWarning(),
"--add-opens", "java.base/java.lang=ALL-UNNAMED");
} }
/** /**
@ -373,19 +374,20 @@ public class IllegalAccessTest {
Attributes attrs = man.getMainAttributes(); Attributes attrs = man.getMainAttributes();
attrs.put(Attributes.Name.MANIFEST_VERSION, "1.0"); attrs.put(Attributes.Name.MANIFEST_VERSION, "1.0");
attrs.put(Attributes.Name.MAIN_CLASS, "TryAccess"); attrs.put(Attributes.Name.MAIN_CLASS, "TryAccess");
attrs.put(new Attributes.Name("Add-Exports"), "java.base/sun.security.x509"); attrs.put(new Attributes.Name("Add-Exports"),
"java.base/sun.security.x509 java.base/sun.nio.ch");
Path jarfile = Paths.get("x.jar"); Path jarfile = Paths.get("x.jar");
Path classes = Paths.get(TEST_CLASSES); Path classes = Paths.get(TEST_CLASSES);
JarUtils.createJarFile(jarfile, man, classes, Paths.get("TryAccess.class")); JarUtils.createJarFile(jarfile, man, classes, Paths.get("TryAccess.class"));
run(jarfile, "reflectPublicMemberNonExportedPackage", successNoWarning()); run(jarfile, "reflectPublicMemberNonExportedPackage", successNoWarning());
run(jarfile, "setAccessibleNonPublicMemberExportedPackage", successWithWarning()); run(jarfile, "reflectPublicMemberNonExportedPackage", successNoWarning(),
"--illegal-access=permit");
// attempt two illegal accesses, one allowed by Add-Exports // should fail as Add-Exports does not open package
run(jarfile, "reflectPublicMemberNonExportedPackage," run(jarfile, "setAccessibleNonPublicMemberNonExportedPackage",
+ "setAccessibleNonPublicMemberExportedPackage", fail("InaccessibleObjectException"));
successWithWarning());
} }
/** /**
@ -403,29 +405,26 @@ public class IllegalAccessTest {
run(jarfile, "setAccessibleNonPublicMemberExportedPackage", successNoWarning()); run(jarfile, "setAccessibleNonPublicMemberExportedPackage", successNoWarning());
run(jarfile, "reflectPublicMemberNonExportedPackage", successWithWarning()); run(jarfile, "setAccessibleNonPublicMemberExportedPackage", successNoWarning(),
"--illegal-access=permit");
// attempt two illegal accesses, one allowed by Add-Opens
run(jarfile, "reflectPublicMemberNonExportedPackage,"
+ "setAccessibleNonPublicMemberExportedPackage",
successWithWarning());
} }
/** /**
* Test that default behavior is to print a warning on the first illegal * Test that --illegal-access=permit behavior is to print a warning on the
* access only. * first illegal access only.
*/ */
public void testWarnOnFirstIllegalAccess() throws Exception { public void testWarnOnFirstIllegalAccess() throws Exception {
String action1 = "reflectPublicMemberNonExportedPackage"; String action1 = "reflectPublicMemberNonExportedPackage";
String action2 = "setAccessibleNonPublicMemberExportedPackage"; String action2 = "setAccessibleNonPublicMemberExportedPackage";
int warningCount = count(run(action1).asLines(), "WARNING"); int warningCount = count(run(action1, "--illegal-access=permit").asLines(), "WARNING");
assertTrue(warningCount > 0); // multi line warning
// same illegal access // same illegal access
List<String> output1 = run(action1 + "," + action1).asLines(); List<String> output1 = run(action1 + "," + action1, "--illegal-access=permit").asLines();
assertTrue(count(output1, "WARNING") == warningCount); assertTrue(count(output1, "WARNING") == warningCount);
// different illegal access // different illegal access
List<String> output2 = run(action1 + "," + action2).asLines(); List<String> output2 = run(action1 + "," + action2, "--illegal-access=permit").asLines();
assertTrue(count(output2, "WARNING") == warningCount); assertTrue(count(output2, "WARNING") == warningCount);
} }