8243666: ModuleHashes attribute generated for JMOD and JAR files depends on timestamps

Reviewed-by: mchung

src/java.base/share/classes/jdk/internal/module/ModuleHashes.java

@@ -26,18 +26,21 @@
 package jdk.internal.module;
 
 import java.io.IOException;
+import java.io.InputStream;
 import java.io.UncheckedIOException;
-import java.nio.ByteBuffer;
+import java.lang.module.ModuleReader;
-import java.nio.channels.FileChannel;
+import java.lang.module.ModuleReference;
-import java.nio.file.Path;
+import java.nio.charset.StandardCharsets;
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
-import java.util.TreeMap;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
+import java.util.TreeMap;
+import java.util.function.Supplier;
 
 /**
  * The result of hashing the contents of a number of module artifacts.
@@ -61,8 +64,8 @@ public final class ModuleHashes {
      * @param algorithm   the algorithm used to create the hashes
      * @param nameToHash  the map of module name to hash value
      */
-    public ModuleHashes(String algorithm, Map<String, byte[]> nameToHash) {
+    ModuleHashes(String algorithm, Map<String, byte[]> nameToHash) {
-        this.algorithm = algorithm;
+        this.algorithm = Objects.requireNonNull(algorithm);
         this.nameToHash = Collections.unmodifiableMap(nameToHash);
     }
 
@@ -96,54 +99,125 @@ public final class ModuleHashes {
     }
 
     /**
-     * Computes the hash for the given file with the given message digest
+     * Computes a hash from the names and content of a module.
-     * algorithm.
      *
+     * @param reader the module reader to access the module content
+     * @param algorithm the name of the message digest algorithm to use
+     * @return the hash
+     * @throws IllegalArgumentException if digest algorithm is not supported
      * @throws UncheckedIOException if an I/O error occurs
-     * @throws RuntimeException if the algorithm is not available
      */
-    public static byte[] computeHash(Path file, String algorithm) {
+    private static byte[] computeHash(ModuleReader reader, String algorithm) {
+        MessageDigest md;
         try {
-            MessageDigest md = MessageDigest.getInstance(algorithm);
+            md = MessageDigest.getInstance(algorithm);
-
-            // Ideally we would just mmap the file but this consumes too much
-            // memory when jlink is running concurrently on very large jmods
-            try (FileChannel fc = FileChannel.open(file)) {
-                ByteBuffer bb = ByteBuffer.allocate(32*1024);
-                while (fc.read(bb) > 0) {
-                    bb.flip();
-                    md.update(bb);
-                    assert bb.remaining() == 0;
-                    bb.clear();
-                }
-            }
-
-            return md.digest();
         } catch (NoSuchAlgorithmException e) {
-            throw new RuntimeException(e);
+            throw new IllegalArgumentException(e);
+        }
+        try {
+            byte[] buf = new byte[32*1024];
+            reader.list().sorted().forEach(rn -> {
+                md.update(rn.getBytes(StandardCharsets.UTF_8));
+                try (InputStream in = reader.open(rn).orElseThrow()) {
+                    int n;
+                    while ((n = in.read(buf)) > 0) {
+                        md.update(buf, 0, n);
+                    }
+                } catch (IOException ioe) {
+                    throw new UncheckedIOException(ioe);
+                }
+            });
+        } catch (IOException ioe) {
+            throw new UncheckedIOException(ioe);
+        }
+        return md.digest();
+    }
+
+    /**
+     * Computes a hash from the names and content of a module.
+     *
+     * @param supplier supplies the module reader to access the module content
+     * @param algorithm the name of the message digest algorithm to use
+     * @return the hash
+     * @throws IllegalArgumentException if digest algorithm is not supported
+     * @throws UncheckedIOException if an I/O error occurs
+     */
+    static byte[] computeHash(Supplier<ModuleReader> supplier, String algorithm) {
+        try (ModuleReader reader = supplier.get()) {
+            return computeHash(reader, algorithm);
         } catch (IOException ioe) {
             throw new UncheckedIOException(ioe);
         }
     }
 
     /**
-     * Computes the hash for every entry in the given map, returning a
+     * Computes the hash from the names and content of a set of modules. Returns
-     * {@code ModuleHashes} to encapsulate the result. The map key is
+     * a {@code ModuleHashes} to encapsulate the result.
-     * the entry name, typically the module name. The map value is the file
-     * path to the entry (module artifact).
      *
+     * @param mrefs the set of modules
+     * @param algorithm the name of the message digest algorithm to use
      * @return ModuleHashes that encapsulates the hashes
+     * @throws IllegalArgumentException if digest algorithm is not supported
+     * @throws UncheckedIOException if an I/O error occurs
      */
-    public static ModuleHashes generate(Map<String, Path> map, String algorithm) {
+    static ModuleHashes generate(Set<ModuleReference> mrefs, String algorithm) {
         Map<String, byte[]> nameToHash = new TreeMap<>();
-        for (Map.Entry<String, Path> entry: map.entrySet()) {
+        for (ModuleReference mref : mrefs) {
-            String name = entry.getKey();
+            try (ModuleReader reader = mref.open()) {
-            Path path = entry.getValue();
+                byte[] hash = computeHash(reader, algorithm);
-            nameToHash.put(name, computeHash(path, algorithm));
+                nameToHash.put(mref.descriptor().name(), hash);
+            } catch (IOException ioe) {
+                throw new UncheckedIOException(ioe);
+            }
         }
         return new ModuleHashes(algorithm, nameToHash);
     }
 
+    @Override
+    public int hashCode() {
+        int h = algorithm.hashCode();
+        for (Map.Entry<String, byte[]> e : nameToHash.entrySet()) {
+            h = h * 31 + e.getKey().hashCode();
+            h = h * 31 + Arrays.hashCode(e.getValue());
+        }
+        return h;
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+        if (!(obj instanceof ModuleHashes))
+            return false;
+        ModuleHashes other = (ModuleHashes) obj;
+        if (!algorithm.equals(other.algorithm)
+                || nameToHash.size() != other.nameToHash.size())
+            return false;
+        for (Map.Entry<String, byte[]> e : nameToHash.entrySet()) {
+            String name = e.getKey();
+            byte[] hash = e.getValue();
+            if (!Arrays.equals(hash, other.nameToHash.get(name)))
+                return false;
+        }
+        return true;
+    }
+
+    @Override
+    public String toString() {
+        StringBuilder sb = new StringBuilder(algorithm);
+        sb.append(" ");
+        nameToHash.entrySet()
+                .stream()
+                .sorted(Map.Entry.comparingByKey())
+                .forEach(e -> {
+                    sb.append(e.getKey());
+                    sb.append("=");
+                    byte[] ba = e.getValue();
+                    for (byte b : ba) {
+                        sb.append(String.format("%02x", b & 0xff));
+                    }
+                });
+        return sb.toString();
+    }
+
     /**
      * This is used by jdk.internal.module.SystemModules class
      * generated at link time.

src/java.base/share/classes/jdk/internal/module/ModuleHashesBuilder.java

@@ -27,9 +27,8 @@ package jdk.internal.module;
 
 import java.io.PrintStream;
 import java.lang.module.Configuration;
+import java.lang.module.ModuleReference;
 import java.lang.module.ResolvedModule;
-import java.net.URI;
-import java.nio.file.Path;
 import java.util.ArrayDeque;
 import java.util.Collections;
 import java.util.Deque;
@@ -39,7 +38,6 @@ import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
 import java.util.function.Consumer;
-import java.util.function.Function;
 import java.util.stream.Stream;
 import static java.util.stream.Collectors.*;
 
@@ -114,27 +112,17 @@ public class ModuleHashesBuilder {
                    mods.addAll(ns);
 
                    if (!ns.isEmpty()) {
-                       Map<String, Path> moduleToPath = ns.stream()
+                       Set<ModuleReference> mrefs = ns.stream()
-                           .collect(toMap(Function.identity(), this::moduleToPath));
+                               .map(name -> configuration.findModule(name)
-                       hashes.put(mn, ModuleHashes.generate(moduleToPath, "SHA-256"));
+                                                         .orElseThrow(InternalError::new))
+                               .map(ResolvedModule::reference)
+                               .collect(toSet());
+                       hashes.put(mn, ModuleHashes.generate(mrefs, "SHA-256"));
                    }
                });
         return hashes;
     }
 
-    private Path moduleToPath(String name) {
-        ResolvedModule rm = configuration.findModule(name).orElseThrow(
-            () -> new InternalError("Selected module " + name + " not on module path"));
-
-        URI uri = rm.reference().location().get();
-        Path path = Path.of(uri);
-        String fn = path.getFileName().toString();
-        if (!fn.endsWith(".jar") && !fn.endsWith(".jmod")) {
-            throw new UnsupportedOperationException(path + " is not a modular JAR or jmod file");
-        }
-        return path;
-    }
-
     /*
      * Utility class
      */

src/java.base/share/classes/jdk/internal/module/ModuleInfoExtender.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 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
@@ -167,7 +167,9 @@ public final class ModuleInfoExtender {
 
                 // ModulePackages attribute
                 if (packages != null) {
-                    packages.forEach(pn -> mv.visitPackage(pn.replace('.', '/')));
+                    packages.stream()
+                            .sorted()
+                            .forEach(pn -> mv.visitPackage(pn.replace('.', '/')));
                 }
 
                 return new ModuleVisitor(Opcodes.ASM7, mv) {

src/java.base/share/classes/jdk/internal/module/ModuleReferences.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2016, 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
@@ -93,7 +93,7 @@ class ModuleReferences {
                                         Path file) {
         URI uri = file.toUri();
         Supplier<ModuleReader> supplier = () -> new JarModuleReader(file, uri);
-        HashSupplier hasher = (a) -> ModuleHashes.computeHash(file, a);
+        HashSupplier hasher = (a) -> ModuleHashes.computeHash(supplier, a);
         return newModule(attrs, uri, supplier, patcher, hasher);
     }
 
@@ -103,7 +103,7 @@ class ModuleReferences {
     static ModuleReference newJModModule(ModuleInfo.Attributes attrs, Path file) {
         URI uri = file.toUri();
         Supplier<ModuleReader> supplier = () -> new JModModuleReader(file, uri);
-        HashSupplier hasher = (a) -> ModuleHashes.computeHash(file, a);
+        HashSupplier hasher = (a) -> ModuleHashes.computeHash(supplier, a);
         return newModule(attrs, uri, supplier, null, hasher);
     }
 

test/jdk/tools/jmod/hashes/HashesTest.java

@@ -1,5 +1,5 @@
 /**
- * Copyright (c) 2015, 2018, 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 8160286
+ * @bug 8160286 8243666
  * @summary Test the recording and checking of module hashes
  * @library /test/lib
  * @modules java.base/jdk.internal.misc
@@ -54,6 +54,7 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 import java.util.spi.ToolProvider;
 import java.util.stream.Collectors;
@@ -84,6 +85,7 @@ public class HashesTest {
     private final Path srcDir;
     private final Path lib;
     private final ModuleInfoMaker builder;
+
     HashesTest(Path dest) throws IOException {
         if (Files.exists(dest)) {
             deleteDirectory(dest);
@@ -305,6 +307,29 @@ public class HashesTest {
         validateImageJmodsTest(ht, mpath);
     }
 
+    @Test
+    public static void testReproducibibleHash() throws Exception {
+        HashesTest ht = new HashesTest(Path.of("repro"));
+        ht.makeModule("m4");
+        ht.makeModule("m3", "m4");
+        ht.makeModule("m2");
+        ht.makeModule("m1", "m2", "m3");
+
+        // create JMOD files and run jmod hash
+        List.of("m1", "m2", "m3", "m4").forEach(ht::makeJmod);
+        Map<String, ModuleHashes> hashes1 = ht.runJmodHash();
+
+        // sleep a bit to be confident that the hashes aren't dependent on timestamps
+        Thread.sleep(2000);
+
+        // (re)create JMOD files and run jmod hash
+        List.of("m1", "m2", "m3", "m4").forEach(ht::makeJmod);
+        Map<String, ModuleHashes> hashes2 = ht.runJmodHash();
+
+        // hashes should be equal
+        assertEquals(hashes1, hashes2);
+    }
+
     private static void validateImageJmodsTest(HashesTest ht, Path mpath)
         throws IOException
     {
@@ -434,6 +459,25 @@ public class HashesTest {
         runJmod(args);
     }
 
+    /**
+     * Execute jmod hash on the modules in the lib directory. Returns a map of
+     * the modules, with the module name as the key, for the modules that have
+     * a ModuleHashes class file attribute.
+     */
+    private Map<String, ModuleHashes> runJmodHash() {
+        runJmod(List.of("hash",
+                "--module-path", lib.toString(),
+                "--hash-modules", ".*"));
+        HashesTest ht = this;
+        return ModulePath.of(Runtime.version(), true, lib)
+                .findAll()
+                .stream()
+                .map(ModuleReference::descriptor)
+                .map(ModuleDescriptor::name)
+                .filter(mn -> ht.hashes(mn) != null)
+                .collect(Collectors.toMap(mn -> mn, ht::hashes));
+    }
+
     private static void runJmod(List<String> args) {
         int rc = JMOD_TOOL.run(System.out, System.out, args.toArray(new String[args.size()]));
         System.out.println("jmod " + args.stream().collect(Collectors.joining(" ")));