8240910: jmod rejects duplicate entries in --class-path jars

Reviewed-by: alanb, lancea

src/jdk.jlink/share/classes/jdk/tools/jmod/JmodOutputStream.java

@@ -34,6 +34,10 @@ import java.io.UncheckedIOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
 import java.util.zip.ZipEntry;
 import java.util.zip.ZipOutputStream;
 import jdk.internal.jmod.JmodFile;
@@ -44,6 +48,8 @@ import static jdk.internal.jmod.JmodFile.*;
  * Output stream to write to JMOD file
  */
 class JmodOutputStream extends OutputStream implements AutoCloseable {
+    private final Map<Section, Set<String>> entries = new HashMap<>();
+
     /**
      * This method creates (or overrides, if exists) the JMOD file,
      * returning the the output stream to write to the JMOD file.
@@ -110,13 +116,22 @@ class JmodOutputStream extends OutputStream implements AutoCloseable {
         zos.closeEntry();
     }
 
-    private ZipEntry newEntry(Section section, String path) {
+    private ZipEntry newEntry(Section section, String path) throws IOException {
+        if (contains(section, path)) {
+            throw new IOException("duplicate entry: " + path + " in section " + section);
+        }
         String prefix = section.jmodDir();
         String name = Paths.get(prefix, path).toString()
                            .replace(File.separatorChar, '/');
+        entries.get(section).add(path);
         return new ZipEntry(name);
     }
 
+    public boolean contains(Section section, String path) {
+        Set<String> set = entries.computeIfAbsent(section, k -> new HashSet<>());
+        return set.contains(path);
+    }
+
     @Override
     public void write(int b) throws IOException {
         zos.write(b);

src/jdk.jlink/share/classes/jdk/tools/jmod/JmodTask.java

@@ -757,21 +757,17 @@ public class JmodTask {
                         throws IOException
                     {
                         Path relPath = path.relativize(file);
-                        if (relPath.toString().equals(MODULE_INFO)
+                        String name = relPath.toString();
-                                && !Section.CLASSES.equals(section))
+                        if (name.equals(MODULE_INFO)) {
-                            warning("warn.ignore.entry", MODULE_INFO, section);
+                            if (!Section.CLASSES.equals(section))
-
+                                warning("warn.ignore.entry", name, section);
-                        if (!relPath.toString().equals(MODULE_INFO)
+                        } else if (!matches(relPath, excludes)) {
-                                && !matches(relPath, excludes)) {
+                            if (out.contains(section, name)) {
-                            try (InputStream in = Files.newInputStream(file)) {
+                                warning("warn.ignore.duplicate.entry", name, section);
-                                out.writeEntry(in, section, relPath.toString());
+                            } else {
-                            } catch (IOException x) {
+                                try (InputStream in = Files.newInputStream(file)) {
-                                if (x.getMessage().contains("duplicate entry")) {
+                                    out.writeEntry(in, section, name);
-                                    warning("warn.ignore.duplicate.entry",
-                                            relPath.toString(), section);
-                                    return FileVisitResult.CONTINUE;
                                 }
-                                throw x;
                             }
                         }
                         return FileVisitResult.CONTINUE;
@@ -808,7 +804,14 @@ public class JmodTask {
             public boolean test(JarEntry je) {
                 String name = je.getName();
                 // ## no support for excludes. Is it really needed?
-                return !name.endsWith(MODULE_INFO) && !je.isDirectory();
+                if (name.endsWith(MODULE_INFO) || je.isDirectory()) {
+                    return false;
+                }
+                if (out.contains(Section.CLASSES, name)) {
+                    warning("warn.ignore.duplicate.entry", name, Section.CLASSES);
+                    return false;
+                }
+                return true;
             }
         }
     }

test/jdk/tools/jmod/JmodTest.java

@@ -1,5 +1,5 @@
 /**
- * Copyright (c) 2015, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2020, 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
@@ -23,7 +23,7 @@
 
 /*
  * @test
- * @bug 8142968 8166568 8166286 8170618 8168149
+ * @bug 8142968 8166568 8166286 8170618 8168149 8240910
  * @summary Basic test for jmod
  * @library /test/lib
  * @modules jdk.compiler
@@ -60,6 +60,10 @@ public class JmodTest {
         .orElseThrow(() ->
             new RuntimeException("jmod tool not found")
         );
+    static final ToolProvider JAR_TOOL = ToolProvider.findFirst("jar")
+        .orElseThrow(() ->
+            new RuntimeException("jar tool not found")
+        );
 
     static final String TEST_SRC = System.getProperty("test.src", ".");
     static final Path SRC_DIR = Paths.get(TEST_SRC, "src");
@@ -436,6 +440,25 @@ public class JmodTest {
              );
     }
 
+    @Test
+    public void testDuplicateEntriesFromJarFile() throws IOException {
+        String cp = EXPLODED_DIR.resolve("foo").resolve("classes").toString();
+        Path jar = Paths.get("foo.jar");
+        Path jmod = MODS_DIR.resolve("testDuplicates.jmod");
+        FileUtils.deleteFileIfExistsWithRetry(jar);
+        FileUtils.deleteFileIfExistsWithRetry(jmod);
+        // create JAR file
+        assertTrue(JAR_TOOL.run(System.out, System.err, "cf", jar.toString(), "-C", cp, ".") == 0);
+
+        jmod("create",
+             "--class-path", jar.toString() + pathSeparator + jar.toString(),
+             jmod.toString())
+             .assertSuccess()
+             .resultChecker(r ->
+                 assertContains(r.output, "Warning: ignoring duplicate entry")
+             );
+    }
+
     @Test
     public void testIgnoreModuleInfoInOtherSections() throws IOException {
         Path jmod = MODS_DIR.resolve("testIgnoreModuleInfoInOtherSections.jmod");