From 8547893efe1c9d129f420cbca3db246563dad475 Mon Sep 17 00:00:00 2001 From: Alan Bateman Date: Fri, 17 Jun 2016 08:41:39 +0100 Subject: [PATCH] 8158456: ModuleDescriptor.read does not verify dependence on java.base in module-info.class 8159248: ModuleFinder.of not clear that FindException thrown if module descriptor cannot be derived for automatic module Reviewed-by: chegar, mchung --- .../java/lang/module/ModuleDescriptor.java | 17 +- .../java/lang/module/ModuleFinder.java | 8 + .../classes/java/lang/module/ModuleInfo.java | 16 +- .../classes/java/lang/module/ModulePath.java | 55 +++-- .../classes/java/lang/module/Resolver.java | 131 +++++------ .../lang/module/AutomaticModulesTest.java | 211 ++++++++++++++---- .../lang/module/ModuleDescriptorTest.java | 33 +++ 7 files changed, 338 insertions(+), 133 deletions(-) diff --git a/jdk/src/java.base/share/classes/java/lang/module/ModuleDescriptor.java b/jdk/src/java.base/share/classes/java/lang/module/ModuleDescriptor.java index 778b977c232..e76c2f59200 100644 --- a/jdk/src/java.base/share/classes/java/lang/module/ModuleDescriptor.java +++ b/jdk/src/java.base/share/classes/java/lang/module/ModuleDescriptor.java @@ -1095,7 +1095,7 @@ public class ModuleDescriptor public static final class Builder { final String name; - final boolean automatic; + boolean automatic; boolean synthetic; final Map requires = new HashMap<>(); final Set uses = new HashSet<>(); @@ -1120,12 +1120,19 @@ public class ModuleDescriptor * identifier */ public Builder(String name) { - this(name, false); + this.name = requireModuleName(name); } - /* package */ Builder(String name, boolean automatic) { - this.name = requireModuleName(name); - this.automatic = automatic; + /** + * Updates the builder so that it builds an automatic module. + * + * @return This builder + * + * @see ModuleDescriptor#isAutomatic() + */ + /* package */ Builder automatic() { + this.automatic = true; + return this; } /** diff --git a/jdk/src/java.base/share/classes/java/lang/module/ModuleFinder.java b/jdk/src/java.base/share/classes/java/lang/module/ModuleFinder.java index a6924484c0e..a108e467a33 100644 --- a/jdk/src/java.base/share/classes/java/lang/module/ModuleFinder.java +++ b/jdk/src/java.base/share/classes/java/lang/module/ModuleFinder.java @@ -42,6 +42,7 @@ import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.Set; + import sun.security.action.GetPropertyAction; /** @@ -267,6 +268,13 @@ public interface ModuleFinder { * * * + *

If a {@code ModuleDescriptor} cannot be created (by means of the + * {@link ModuleDescriptor.Builder ModuleDescriptor.Builder} API) for an + * automatic module then {@code FindException} is thrown. This can arise, + * for example, when a legal Java identifier name cannot be derived from + * the file name of the JAR file or where a package name derived from an + * entry ending with {@code .class} is not a legal Java identifier.

+ * *

In addition to JAR files, an implementation may also support modules * that are packaged in other implementation specific module formats. When * a file is encountered that is not recognized as a packaged module then diff --git a/jdk/src/java.base/share/classes/java/lang/module/ModuleInfo.java b/jdk/src/java.base/share/classes/java/lang/module/ModuleInfo.java index 7bf9342ca88..db9a3e000b9 100644 --- a/jdk/src/java.base/share/classes/java/lang/module/ModuleInfo.java +++ b/jdk/src/java.base/share/classes/java/lang/module/ModuleInfo.java @@ -276,10 +276,7 @@ final class ModuleInfo { throws IOException { int requires_count = in.readUnsignedShort(); - if (requires_count == 0 && !mn.equals("java.base")) { - throw invalidModuleDescriptor("The requires table must have" - + " at least one entry"); - } + boolean requiresJavaBase = false; for (int i=0; i 0) { + throw invalidModuleDescriptor("The requires table for java.base" + + " must be 0 length"); + } + } else if (!requiresJavaBase) { + throw invalidModuleDescriptor("The requires table must have" + + " an entry for java.base"); } int exports_count = in.readUnsignedShort(); diff --git a/jdk/src/java.base/share/classes/java/lang/module/ModulePath.java b/jdk/src/java.base/share/classes/java/lang/module/ModulePath.java index 250de159d74..43d8e56974d 100644 --- a/jdk/src/java.base/share/classes/java/lang/module/ModulePath.java +++ b/jdk/src/java.base/share/classes/java/lang/module/ModulePath.java @@ -55,7 +55,6 @@ import java.util.stream.Collectors; import java.util.zip.ZipEntry; import java.util.zip.ZipFile; -import jdk.internal.module.Checks; import jdk.internal.module.ConfigurableModuleFinder; import jdk.internal.perf.PerfCounter; @@ -343,8 +342,7 @@ class ModulePath implements ConfigurableModuleFinder { String prefix = cf.substring(0, index); if (prefix.equals(SERVICES_PREFIX)) { String sn = cf.substring(index); - if (Checks.isJavaIdentifier(sn)) - return Optional.of(sn); + return Optional.of(sn); } } return Optional.empty(); @@ -378,9 +376,6 @@ class ModulePath implements ConfigurableModuleFinder { * to "provides" declarations * 5. The Main-Class attribute in the main attributes of the JAR manifest * is mapped to the module descriptor mainClass - * - * @apiNote This needs to move to somewhere where it can be used by tools, - * maybe even a standard API if automatic modules are a Java SE feature. */ private ModuleDescriptor deriveModuleDescriptor(JarFile jf) throws IOException @@ -397,7 +392,7 @@ class ModulePath implements ConfigurableModuleFinder { String vs = null; // find first occurrence of -${NUMBER}. or -${NUMBER}$ - Matcher matcher = Pattern.compile("-(\\d+(\\.|$))").matcher(mn); + Matcher matcher = Patterns.DASH_VERSION.matcher(mn); if (matcher.find()) { int start = matcher.start(); @@ -412,16 +407,13 @@ class ModulePath implements ConfigurableModuleFinder { } // finally clean up the module name - mn = mn.replaceAll("[^A-Za-z0-9]", ".") // replace non-alphanumeric - .replaceAll("(\\.)(\\1)+", ".") // collapse repeating dots - .replaceAll("^\\.", "") // drop leading dots - .replaceAll("\\.$", ""); // drop trailing dots - + mn = cleanModuleName(mn); // Builder throws IAE if module name is empty or invalid ModuleDescriptor.Builder builder - = new ModuleDescriptor.Builder(mn, true) - .requires(Requires.Modifier.MANDATED, "java.base"); + = new ModuleDescriptor.Builder(mn) + .automatic() + .requires(Set.of(Requires.Modifier.MANDATED), "java.base"); if (vs != null) builder.version(vs); @@ -457,7 +449,7 @@ class ModulePath implements ConfigurableModuleFinder { = new BufferedReader(new InputStreamReader(in, "UTF-8")); String cn; while ((cn = nextLine(reader)) != null) { - if (Checks.isJavaIdentifier(cn)) { + if (cn.length() > 0) { providerClasses.add(cn); } } @@ -478,6 +470,39 @@ class ModulePath implements ConfigurableModuleFinder { return builder.build(); } + /** + * Patterns used to derive the module name from a JAR file name. + */ + private static class Patterns { + static final Pattern DASH_VERSION = Pattern.compile("-(\\d+(\\.|$))"); + static final Pattern NON_ALPHANUM = Pattern.compile("[^A-Za-z0-9]"); + static final Pattern REPEATING_DOTS = Pattern.compile("(\\.)(\\1)+"); + static final Pattern LEADING_DOTS = Pattern.compile("^\\."); + static final Pattern TRAILING_DOTS = Pattern.compile("\\.$"); + } + + /** + * Clean up candidate module name derived from a JAR file name. + */ + private static String cleanModuleName(String mn) { + // replace non-alphanumeric + mn = Patterns.NON_ALPHANUM.matcher(mn).replaceAll("."); + + // collapse repeating dots + mn = Patterns.REPEATING_DOTS.matcher(mn).replaceAll("."); + + // drop leading dots + if (mn.length() > 0 && mn.charAt(0) == '.') + mn = Patterns.LEADING_DOTS.matcher(mn).replaceAll(""); + + // drop trailing dots + int len = mn.length(); + if (len > 0 && mn.charAt(len-1) == '.') + mn = Patterns.TRAILING_DOTS.matcher(mn).replaceAll(""); + + return mn; + } + private Set jarPackages(JarFile jf) { return jf.stream() .filter(e -> e.getName().endsWith(".class")) diff --git a/jdk/src/java.base/share/classes/java/lang/module/Resolver.java b/jdk/src/java.base/share/classes/java/lang/module/Resolver.java index cefcf19af70..adc60da8892 100644 --- a/jdk/src/java.base/share/classes/java/lang/module/Resolver.java +++ b/jdk/src/java.base/share/classes/java/lang/module/Resolver.java @@ -36,7 +36,6 @@ import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; -import java.util.Map.Entry; import java.util.Objects; import java.util.Optional; import java.util.Set; @@ -454,17 +453,13 @@ final class Resolver { /** - * Computes and sets the readability graph for the modules in the given - * Resolution object. + * Computes the readability graph for the modules in the given Configuration. * * The readability graph is created by propagating "requires" through the * "public requires" edges of the module dependence graph. So if the module * dependence graph has m1 requires m2 && m2 requires public m3 then the - * resulting readability graph will contain m1 reads m2, m1 - * reads m3, and m2 reads m3. - * - * TODO: Use a more efficient algorithm, maybe cache the requires public - * in parent configurations. + * resulting readability graph will contain m1 reads m2, m1 reads m3, and + * m2 reads m3. */ private Map> makeGraph(Configuration cf) { @@ -483,16 +478,15 @@ final class Resolver { Configuration p = parent; while (p != null) { for (ModuleDescriptor descriptor : p.descriptors()) { - ResolvedModule x = p.findModule(descriptor.name()).orElse(null); - if (x == null) - throw new InternalError(); + String name = descriptor.name(); + ResolvedModule m1 = p.findModule(name) + .orElseThrow(() -> new InternalError(name + " not found")); for (ModuleDescriptor.Requires requires : descriptor.requires()) { if (requires.modifiers().contains(Modifier.PUBLIC)) { String dn = requires.name(); - ResolvedModule y = p.findModule(dn).orElse(null); - if (y == null) - throw new InternalError(dn + " not found"); - g2.computeIfAbsent(x, k -> new HashSet<>()).add(y); + ResolvedModule m2 = p.findModule(dn) + .orElseThrow(() -> new InternalError(dn + " not found")); + g2.computeIfAbsent(m1, k -> new HashSet<>()).add(m2); } } } @@ -501,53 +495,60 @@ final class Resolver { } // populate g1 and g2 with the dependences from the selected modules + + Map nameToResolved = new HashMap<>(); + for (ModuleReference mref : nameToReference.values()) { ModuleDescriptor descriptor = mref.descriptor(); - ResolvedModule x = new ResolvedModule(cf, mref); + String name = descriptor.name(); + + ResolvedModule m1 = computeIfAbsent(nameToResolved, name, cf, mref); Set reads = new HashSet<>(); - g1.put(x, reads); - Set requiresPublic = new HashSet<>(); - g2.put(x, requiresPublic); for (ModuleDescriptor.Requires requires : descriptor.requires()) { String dn = requires.name(); - ResolvedModule y; - ModuleReference other = nameToReference.get(dn); - if (other != null) { - y = new ResolvedModule(cf, other); // cache? + ResolvedModule m2; + ModuleReference mref2 = nameToReference.get(dn); + if (mref2 != null) { + // same configuration + m2 = computeIfAbsent(nameToResolved, dn, cf, mref2); } else { - y = parent.findModule(dn).orElse(null); - if (y == null) - throw new InternalError("unable to find " + dn); + // parent configuration + m2 = parent.findModule(dn).orElse(null); + if (m2 == null) { + continue; + } } - // m requires other => m reads other - reads.add(y); + // m1 requires m2 => m1 reads m2 + reads.add(m2); - // m requires public other + // m1 requires public m2 if (requires.modifiers().contains(Modifier.PUBLIC)) { - requiresPublic.add(y); + requiresPublic.add(m2); } } - // automatic modules reads all selected modules and all modules + // automatic modules read all selected modules and all modules // in parent configurations if (descriptor.isAutomatic()) { - String name = descriptor.name(); // reads all selected modules - // requires public` all selected automatic modules + // `requires public` all selected automatic modules for (ModuleReference mref2 : nameToReference.values()) { ModuleDescriptor descriptor2 = mref2.descriptor(); - if (!name.equals(descriptor2.name())) { - ResolvedModule m = new ResolvedModule(cf, mref2); - reads.add(m); + String name2 = descriptor2.name(); + + if (!name.equals(name2)) { + ResolvedModule m2 + = computeIfAbsent(nameToResolved, name2, cf, mref2); + reads.add(m2); if (descriptor2.isAutomatic()) - requiresPublic.add(m); + requiresPublic.add(m2); } } @@ -565,54 +566,58 @@ final class Resolver { } + g1.put(m1, reads); + g2.put(m1, requiresPublic); } // Iteratively update g1 until there are no more requires public to propagate boolean changed; - Map> changes = new HashMap<>(); + Set toAdd = new HashSet<>(); do { changed = false; - for (Entry> entry : g1.entrySet()) { - - ResolvedModule m1 = entry.getKey(); - Set m1Reads = entry.getValue(); - + for (Set m1Reads : g1.values()) { for (ResolvedModule m2 : m1Reads) { Set m2RequiresPublic = g2.get(m2); if (m2RequiresPublic != null) { for (ResolvedModule m3 : m2RequiresPublic) { if (!m1Reads.contains(m3)) { - - // computeIfAbsent - Set s = changes.get(m1); - if (s == null) { - s = new HashSet<>(); - changes.put(m1, s); - } - s.add(m3); - changed = true; - + // m1 reads m2, m2 requires public m3 + // => need to add m1 reads m3 + toAdd.add(m3); } } } } - } - - if (changed) { - for (Map.Entry> e : - changes.entrySet()) { - ResolvedModule m = e.getKey(); - g1.get(m).addAll(e.getValue()); + if (!toAdd.isEmpty()) { + m1Reads.addAll(toAdd); + toAdd.clear(); + changed = true; } - changes.clear(); } - } while (changed); - return g1; } + /** + * Equivalent to + * 

{@code
+     *     map.computeIfAbsent(name, k -> new ResolvedModule(cf, mref))
+     *
} + */ + private ResolvedModule computeIfAbsent(Map map, + String name, + Configuration cf, + ModuleReference mref) + { + ResolvedModule m = map.get(name); + if (m == null) { + m = new ResolvedModule(cf, mref); + map.put(name, m); + } + return m; + } + /** * Checks the readability graph to ensure that no two modules export the diff --git a/jdk/test/java/lang/module/AutomaticModulesTest.java b/jdk/test/java/lang/module/AutomaticModulesTest.java index 93dd5f7685d..680a727127e 100644 --- a/jdk/test/java/lang/module/AutomaticModulesTest.java +++ b/jdk/test/java/lang/module/AutomaticModulesTest.java @@ -24,14 +24,14 @@ /** * @test * @library /lib/testlibrary - * @build AutomaticModulesTest ModuleUtils + * @build AutomaticModulesTest ModuleUtils JarUtils * @run testng AutomaticModulesTest * @summary Basic tests for automatic modules */ import java.io.IOException; -import java.io.OutputStream; import java.lang.module.Configuration; +import java.lang.module.FindException; import java.lang.module.ModuleDescriptor; import java.lang.module.ModuleDescriptor.Exports; import java.lang.module.ModuleDescriptor.Requires.Modifier; @@ -46,11 +46,9 @@ import java.nio.file.Paths; import java.util.Optional; import java.util.Set; import java.util.jar.Attributes; -import java.util.jar.JarEntry; -import java.util.jar.JarFile; -import java.util.jar.JarOutputStream; import java.util.jar.Manifest; import java.util.stream.Collectors; +import java.util.stream.Stream; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; @@ -62,10 +60,8 @@ public class AutomaticModulesTest { private static final Path USER_DIR = Paths.get(System.getProperty("user.dir")); - @DataProvider(name = "names") public Object[][] createNames() { - return new Object[][] { // JAR file name module-name[/version] @@ -98,12 +94,22 @@ public class AutomaticModulesTest { }; } + // JAR file names that do not map to a legal module name + @DataProvider(name = "badnames") + public Object[][] createBadNames() { + return new Object[][]{ + + { ".jar", null }, + { "_.jar", null } + + }; + } + /** * Test mapping of JAR file names to module names */ @Test(dataProvider = "names") public void testNames(String fn, String mid) throws IOException { - String[] s = mid.split("/"); String mn = s[0]; String vs = (s.length == 2) ? s[1] : null; @@ -112,7 +118,7 @@ public class AutomaticModulesTest { Path jf = dir.resolve(fn); // create empty JAR file - createJarFile(jf); + createDummyJarFile(jf); // create a ModuleFinder to find modules in the directory ModuleFinder finder = ModuleFinder.of(dir); @@ -128,17 +134,32 @@ public class AutomaticModulesTest { } else { assertEquals(descriptor.version().get().toString(), vs); } + } + + /** + * Test impossible mapping of JAR files to modules names + */ + @Test(dataProvider = "badnames", expectedExceptions = FindException.class) + public void testBadNames(String fn, String ignore) throws IOException { + Path dir = Files.createTempDirectory(USER_DIR, "mods"); + Path jf = dir.resolve(fn); + + // create empty JAR file + createDummyJarFile(jf); + + // should throw FindException + ModuleFinder.of(dir).findAll(); } /** * Test all packages are exported */ - public void testExports() throws IOException { + public void testPackages() throws IOException { Path dir = Files.createTempDirectory(USER_DIR, "mods"); - createJarFile(dir.resolve("m1.jar"), - "p/C1.class", "p/C2.class", "q/C1.class"); + createDummyJarFile(dir.resolve("m1.jar"), + "p/C1.class", "p/C2.class", "q/C1.class"); ModuleFinder finder = ModuleFinder.of(dir); @@ -157,6 +178,84 @@ public class AutomaticModulesTest { } + /** + * Test class file in JAR file where the entry does not correspond to a + * legal package name. + */ + @Test(expectedExceptions = FindException.class) + public void testBadPackage() throws IOException { + Path dir = Files.createTempDirectory(USER_DIR, "mods"); + createDummyJarFile(dir.resolve("m1.jar"), "p-/T.class"); + + // should throw FindException + ModuleFinder.of(dir).findAll(); + } + + + /** + * Test JAR file with META-INF/services configuration file + */ + public void testServicesConfiguration() throws IOException { + String service = "p.S"; + String provider = "p.S1"; + + Path tmpdir = Files.createTempDirectory(USER_DIR, "tmp"); + Path services = tmpdir.resolve("META-INF").resolve("services"); + Files.createDirectories(services); + Files.write(services.resolve(service), Set.of(provider)); + Path dir = Files.createTempDirectory(USER_DIR, "mods"); + JarUtils.createJarFile(dir.resolve("m1.jar"), tmpdir); + + ModuleFinder finder = ModuleFinder.of(dir); + + Optional mref = finder.find("m1"); + assertTrue(mref.isPresent(), "m1 not found"); + + ModuleDescriptor descriptor = mref.get().descriptor(); + assertTrue(descriptor.provides().size() == 1); + assertTrue(descriptor.provides().containsKey(service)); + ModuleDescriptor.Provides provides = descriptor.provides().get(service); + assertTrue(provides.providers().size() == 1); + assertTrue(provides.providers().contains((provider))); + } + + + // META-INF/services configuration file/entries that are not legal + @DataProvider(name = "badproviders") + public Object[][] createProviders() { + return new Object[][] { + + // service type provider type + + { "p.S", "-" }, + { "p.S", ".S1" }, + { "p.S", "S1." }, + + { "-", "p.S1" }, + { ".S", "p.S1" }, + }; + } + + /** + * Test JAR file with META-INF/services configuration file with bad + * values or names. + */ + @Test(dataProvider = "badproviders", expectedExceptions = FindException.class) + public void testBadServicesConfiguration(String service, String provider) + throws IOException + { + Path tmpdir = Files.createTempDirectory(USER_DIR, "tmp"); + Path services = tmpdir.resolve("META-INF").resolve("services"); + Files.createDirectories(services); + Files.write(services.resolve(service), Set.of(provider)); + Path dir = Files.createTempDirectory(USER_DIR, "mods"); + JarUtils.createJarFile(dir.resolve("m1.jar"), tmpdir); + + // should throw FindException + ModuleFinder.of(dir).findAll(); + } + + /** * Test that a JAR file with a Main-Class attribute results * in a module with a main class. @@ -170,7 +269,7 @@ public class AutomaticModulesTest { attrs.put(Attributes.Name.MAIN_CLASS, mainClass); Path dir = Files.createTempDirectory(USER_DIR, "mods"); - createJarFile(dir.resolve("m1.jar"), man); + createDummyJarFile(dir.resolve("m1.jar"), man); ModuleFinder finder = ModuleFinder.of(dir); @@ -184,6 +283,36 @@ public class AutomaticModulesTest { } + // Main-Class files that do not map to a legal Java identifier + @DataProvider(name = "badmainclass") + public Object[][] createBadMainClass() { + return new Object[][]{ + + { "p-.Main", null }, + { ".Main", null } + + }; + } + + /** + * Test that a JAR file with a Main-Class attribute that is not a valid + * Java identifier + */ + @Test(dataProvider = "badmainclass", expectedExceptions = FindException.class) + public void testBadMainClass(String mainClass, String ignore) throws IOException { + Manifest man = new Manifest(); + Attributes attrs = man.getMainAttributes(); + attrs.put(Attributes.Name.MANIFEST_VERSION, "1.0.0"); + attrs.put(Attributes.Name.MAIN_CLASS, mainClass); + + Path dir = Files.createTempDirectory(USER_DIR, "mods"); + createDummyJarFile(dir.resolve("m1.jar"), man); + + // should throw FindException + ModuleFinder.of(dir).findAll(); + } + + /** * Basic test of a configuration created with automatic modules. * m1 requires m2* @@ -201,8 +330,8 @@ public class AutomaticModulesTest { // m2 and m3 are automatic modules Path dir = Files.createTempDirectory(USER_DIR, "mods"); - createJarFile(dir.resolve("m2.jar"), "p/T.class"); - createJarFile(dir.resolve("m3.jar"), "q/T.class"); + createDummyJarFile(dir.resolve("m2.jar"), "p/T.class"); + createDummyJarFile(dir.resolve("m3.jar"), "q/T.class"); // module finder locates m1 and the modules in the directory ModuleFinder finder @@ -252,7 +381,6 @@ public class AutomaticModulesTest { * m4* */ public void testInConfiguration2() throws IOException { - ModuleDescriptor descriptor1 = new ModuleDescriptor.Builder("m1") .requires("m2") @@ -267,8 +395,8 @@ public class AutomaticModulesTest { // m3 and m4 are automatic modules Path dir = Files.createTempDirectory(USER_DIR, "mods"); - createJarFile(dir.resolve("m3.jar"), "p/T.class"); - createJarFile(dir.resolve("m4.jar"), "q/T.class"); + createDummyJarFile(dir.resolve("m3.jar"), "p/T.class"); + createDummyJarFile(dir.resolve("m4.jar"), "q/T.class"); // module finder locates m1 and the modules in the directory ModuleFinder finder @@ -315,7 +443,6 @@ public class AutomaticModulesTest { assertTrue(m4.reads().contains(m2)); assertTrue(m4.reads().contains(m3)); testReadAllBootModules(cf, "m4"); // m4 reads all modules in boot layer - } @@ -327,7 +454,6 @@ public class AutomaticModulesTest { * m4* */ public void testInConfiguration3() throws IOException { - ModuleDescriptor descriptor1 = new ModuleDescriptor.Builder("m1") .requires("m2") @@ -342,8 +468,8 @@ public class AutomaticModulesTest { // m3 and m4 are automatic modules Path dir = Files.createTempDirectory(USER_DIR, "mods"); - createJarFile(dir.resolve("m3.jar"), "p/T.class"); - createJarFile(dir.resolve("m4.jar"), "q/T.class"); + createDummyJarFile(dir.resolve("m3.jar"), "p/T.class"); + createDummyJarFile(dir.resolve("m4.jar"), "q/T.class"); // module finder locates m1 and the modules in the directory ModuleFinder finder @@ -396,7 +522,6 @@ public class AutomaticModulesTest { assertTrue(m4.reads().contains(m2)); assertTrue(m4.reads().contains(m3)); testReadAllBootModules(cf, "m4"); // m4 reads all modules in boot layer - } @@ -412,8 +537,8 @@ public class AutomaticModulesTest { // m2 and m3 are simple JAR files Path dir = Files.createTempDirectory(USER_DIR, "mods"); - createJarFile(dir.resolve("m2.jar"), "p/T.class"); - createJarFile(dir.resolve("m3.jar"), "q/T2.class"); + createDummyJarFile(dir.resolve("m2.jar"), "p/T.class"); + createDummyJarFile(dir.resolve("m3.jar"), "q/T2.class"); // module finder locates m1 and the modules in the directory ModuleFinder finder @@ -447,7 +572,7 @@ public class AutomaticModulesTest { */ public void testMisc() throws IOException { Path dir = Files.createTempDirectory(USER_DIR, "mods"); - Path m1_jar = createJarFile(dir.resolve("m1.jar"), "p/T.class"); + Path m1_jar = createDummyJarFile(dir.resolve("m1.jar"), "p/T.class"); ModuleFinder finder = ModuleFinder.of(m1_jar); @@ -535,38 +660,32 @@ public class AutomaticModulesTest { * Creates a JAR file, optionally with a manifest, and with the given * entries. The entries will be empty in the resulting JAR file. */ - static Path createJarFile(Path file, Manifest man, String... entries) + static Path createDummyJarFile(Path jarfile, Manifest man, String... entries) throws IOException { - try (OutputStream out = Files.newOutputStream(file)) { - try (JarOutputStream jos = new JarOutputStream(out)) { + Path dir = Files.createTempDirectory(USER_DIR, "tmp"); - if (man != null) { - JarEntry je = new JarEntry(JarFile.MANIFEST_NAME); - jos.putNextEntry(je); - man.write(jos); - jos.closeEntry(); - } - - for (String entry : entries) { - JarEntry je = new JarEntry(entry); - jos.putNextEntry(je); - jos.closeEntry(); - } - - } + for (String entry : entries) { + Path file = dir.resolve(entry); + Path parent = file.getParent(); + if (parent != null) + Files.createDirectories(parent); + Files.createFile(file); } - return file; + + Path[] paths = Stream.of(entries).map(Paths::get).toArray(Path[]::new); + JarUtils.createJarFile(jarfile, man, dir, paths); + return jarfile; } /** * Creates a JAR file and with the given entries. The entries will be empty * in the resulting JAR file. */ - static Path createJarFile(Path file, String... entries) + static Path createDummyJarFile(Path jarfile, String... entries) throws IOException { - return createJarFile(file, null, entries); + return createDummyJarFile(jarfile, null, entries); } } diff --git a/jdk/test/java/lang/module/ModuleDescriptorTest.java b/jdk/test/java/lang/module/ModuleDescriptorTest.java index 61dfc4ea315..57de9dd6c69 100644 --- a/jdk/test/java/lang/module/ModuleDescriptorTest.java +++ b/jdk/test/java/lang/module/ModuleDescriptorTest.java @@ -23,6 +23,7 @@ /** * @test + * @modules java.base/jdk.internal.module * @run testng ModuleDescriptorTest * @summary Basic test for java.lang.module.ModuleDescriptor and its builder */ @@ -47,6 +48,7 @@ import java.util.Set; import static java.lang.module.ModuleDescriptor.Requires.Modifier.*; +import jdk.internal.module.ModuleInfoWriter; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; import static org.testng.Assert.*; @@ -668,6 +670,37 @@ public class ModuleDescriptorTest { ModuleDescriptor.read(bb); } + // The requires table for java.base must be 0 length + @Test(expectedExceptions = InvalidModuleDescriptorException.class) + public void testReadOfJavaBaseWithRequires() { + ModuleDescriptor descriptor + = new ModuleDescriptor.Builder("java.base") + .requires("other") + .build(); + ByteBuffer bb = ModuleInfoWriter.toByteBuffer(descriptor); + ModuleDescriptor.read(bb); + } + + // The requires table must have an entry for java.base + @Test(expectedExceptions = InvalidModuleDescriptorException.class) + public void testReadWithEmptyRequires() { + ModuleDescriptor descriptor = new ModuleDescriptor.Builder("m1").build(); + ByteBuffer bb = ModuleInfoWriter.toByteBuffer(descriptor); + ModuleDescriptor.read(bb); + } + + // The requires table must have an entry for java.base + @Test(expectedExceptions = InvalidModuleDescriptorException.class) + public void testReadWithNoRequiresBase() { + ModuleDescriptor descriptor + = new ModuleDescriptor.Builder("m1") + .requires("m2") + .build(); + ByteBuffer bb = ModuleInfoWriter.toByteBuffer(descriptor); + ModuleDescriptor.read(bb); + } + + public void testReadWithNull() throws Exception { Module base = Object.class.getModule();