8186087: jar tool fails to create a multi-release jar when validating nested classes

Reviewed-by: psandoz

src/jdk.jartool/share/classes/sun/tools/jar/FingerPrint.java

@@ -43,7 +43,7 @@ import java.util.Set;
  * a class, it also contains information to (1) describe the public API;
  * (2) compare the public API of this class with another class;  (3) determine
  * whether or not it's a nested class and, if so, the name of the associated
- * top level class; and (4) for an canonically ordered set of classes determine
+ * outer class; and (4) for an canonically ordered set of classes determine
  * if the class versions are compatible.  A set of classes is canonically
  * ordered if the classes in the set have the same name, and the base class
  * precedes the versioned classes and if each versioned class with version
@@ -53,10 +53,13 @@ import java.util.Set;
 final class FingerPrint {
     private static final MessageDigest MD;
 
+    private final String basename;
+    private final String entryName;
+    private final int mrversion;
+
     private final byte[] sha1;
     private final ClassAttributes attrs;
     private final boolean isClassEntry;
-    private final String entryName;
 
     static {
         try {
@@ -67,16 +70,19 @@ final class FingerPrint {
         }
     }
 
-    public FingerPrint(String entryName,byte[] bytes) throws IOException {
+    public FingerPrint(String basename, String entryName, int mrversion, byte[] bytes)
+            throws IOException {
+        this.basename = basename;
         this.entryName = entryName;
-        if (entryName.endsWith(".class") && isCafeBabe(bytes)) {
+        this.mrversion = mrversion;
+        if (isCafeBabe(bytes)) {
             isClassEntry = true;
             sha1 = sha1(bytes, 8);  // skip magic number and major/minor version
             attrs = getClassAttributes(bytes);
         } else {
             isClassEntry = false;
-            sha1 = sha1(bytes);
+            sha1 = null;
-            attrs = new ClassAttributes();   // empty class
+            attrs = null;
         }
     }
 
@@ -107,14 +113,24 @@ final class FingerPrint {
         return attrs.equals(that.attrs);
     }
 
-    public String name() {
+    public String basename() {
-        String name = attrs.name;
+        return basename;
-        return name == null ? entryName : name;
     }
 
-    public String topLevelName() {
+    public String entryName() {
-        String name = attrs.topLevelName;
+        return entryName;
-        return name == null ? name() : name;
+    }
+
+    public String className() {
+        return attrs.name;
+    }
+
+    public int mrversion() {
+        return mrversion;
+    }
+
+    public String outerClassName() {
+        return attrs.outerClassName;
     }
 
     private byte[] sha1(byte[] entry) {
@@ -218,7 +234,7 @@ final class FingerPrint {
 
     private static final class ClassAttributes extends ClassVisitor {
         private String name;
-        private String topLevelName;
+        private String outerClassName;
         private String superName;
         private int version;
         private int access;
@@ -228,7 +244,7 @@ final class FingerPrint {
         private final Set<Method> methods = new HashSet<>();
 
         public ClassAttributes() {
-            super(Opcodes.ASM5);
+            super(Opcodes.ASM6);
         }
 
         private boolean isPublic(int access) {
@@ -250,7 +266,7 @@ final class FingerPrint {
         @Override
         public void visitOuterClass(String owner, String name, String desc) {
             if (!this.nestedClass) return;
-            this.topLevelName = owner;
+            this.outerClassName = owner;
         }
 
         @Override
@@ -259,7 +275,7 @@ final class FingerPrint {
             if (!this.nestedClass) return;
             if (outerName == null) return;
             if (!this.name.equals(name)) return;
-            if (this.topLevelName == null) this.topLevelName = outerName;
+            if (this.outerClassName == null) this.outerClassName = outerName;
         }
 
         @Override
@@ -294,7 +310,7 @@ final class FingerPrint {
 
         @Override
         public void visitEnd() {
-            this.nestedClass = this.topLevelName != null;
+            this.nestedClass = this.outerClassName != null;
         }
 
         @Override

src/jdk.jartool/share/classes/sun/tools/jar/Main.java

@@ -71,7 +71,6 @@ import static jdk.internal.util.jar.JarIndex.INDEX_NAME;
 import static java.util.jar.JarFile.MANIFEST_NAME;
 import static java.util.stream.Collectors.joining;
 import static java.nio.file.StandardCopyOption.REPLACE_EXISTING;
-import static sun.tools.jar.Validator.ENTRYNAME_COMPARATOR;
 
 /**
  * This class implements a simple utility for creating files in the JAR
@@ -444,8 +443,8 @@ public class Main {
 
     private void validateAndClose(File tmpfile) throws IOException {
         if (ok && isMultiRelease) {
-            try (JarFile jf = new JarFile(tmpfile)) {
+            try (ZipFile zf = new ZipFile(tmpfile)) {
-                ok = Validator.validate(this, jf);
+                ok = Validator.validate(this, zf);
                 if (!ok) {
                     error(formatMsg("error.validator.jarfile.invalid", fname));
                 }
@@ -1784,7 +1783,7 @@ public class Main {
     private boolean describeModule(ZipFile zipFile) throws IOException {
         ZipFileModuleInfoEntry[] infos = zipFile.stream()
                 .filter(e -> isModuleInfoEntry(e.getName()))
-                .sorted(Validator.ENTRY_COMPARATOR)
+                .sorted(ENTRY_COMPARATOR)
                 .map(e -> new ZipFileModuleInfoEntry(zipFile, e))
                 .toArray(ZipFileModuleInfoEntry[]::new);
 
@@ -2216,4 +2215,48 @@ public class Main {
             return hashesBuilder.computeHashes(Set.of(name)).get(name);
         }
     }
+
+    // sort base entries before versioned entries, and sort entry classes with
+    // nested classes so that the outter class appears before the associated
+    // nested class
+    static Comparator<String> ENTRYNAME_COMPARATOR = (s1, s2) ->  {
+
+        if (s1.equals(s2)) return 0;
+        boolean b1 = s1.startsWith(VERSIONS_DIR);
+        boolean b2 = s2.startsWith(VERSIONS_DIR);
+        if (b1 && !b2) return 1;
+        if (!b1 && b2) return -1;
+        int n = 0; // starting char for String compare
+        if (b1 && b2) {
+            // normally strings would be sorted so "10" goes before "9", but
+            // version number strings need to be sorted numerically
+            n = VERSIONS_DIR.length();   // skip the common prefix
+            int i1 = s1.indexOf('/', n);
+            int i2 = s2.indexOf('/', n);
+            if (i1 == -1) throw new Validator.InvalidJarException(s1);
+            if (i2 == -1) throw new Validator.InvalidJarException(s2);
+            // shorter version numbers go first
+            if (i1 != i2) return i1 - i2;
+            // otherwise, handle equal length numbers below
+        }
+        int l1 = s1.length();
+        int l2 = s2.length();
+        int lim = Math.min(l1, l2);
+        for (int k = n; k < lim; k++) {
+            char c1 = s1.charAt(k);
+            char c2 = s2.charAt(k);
+            if (c1 != c2) {
+                // change natural ordering so '.' comes before '$'
+                // i.e. outer classes come before nested classes
+                if (c1 == '$' && c2 == '.') return 1;
+                if (c1 == '.' && c2 == '$') return -1;
+                return c1 - c2;
+            }
+        }
+        return l1 - l2;
+    };
+
+    static Comparator<ZipEntry> ENTRY_COMPARATOR =
+        Comparator.comparing(ZipEntry::getName, ENTRYNAME_COMPARATOR);
+
 }

src/jdk.jartool/share/classes/sun/tools/jar/Validator.java

@@ -28,24 +28,22 @@ package sun.tools.jar;
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
-import java.lang.module.InvalidModuleDescriptorException;
 import java.lang.module.ModuleDescriptor;
 import java.lang.module.ModuleDescriptor.Exports;
-import java.lang.module.InvalidModuleDescriptorException;
 import java.lang.module.ModuleDescriptor.Opens;
 import java.lang.module.ModuleDescriptor.Provides;
 import java.lang.module.ModuleDescriptor.Requires;
 import java.util.Collections;
-import java.util.Comparator;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-import java.util.function.Consumer;
+import java.util.TreeMap;
-import java.util.jar.JarEntry;
+import java.util.function.Function;
-import java.util.jar.JarFile;
+import java.util.stream.Collectors;
 import java.util.zip.ZipEntry;
+import java.util.zip.ZipFile;
 
 import static java.util.jar.JarFile.MANIFEST_NAME;
 import static sun.tools.jar.Main.VERSIONS_DIR;
@@ -55,269 +53,184 @@ import static sun.tools.jar.Main.getMsg;
 import static sun.tools.jar.Main.formatMsg;
 import static sun.tools.jar.Main.formatMsg2;
 import static sun.tools.jar.Main.toBinaryName;
-import static sun.tools.jar.Main.isModuleInfoEntry;
 
 final class Validator {
-    private final static boolean DEBUG = Boolean.getBoolean("jar.debug");
+
-    private final  Map<String,FingerPrint> fps = new HashMap<>();
+    private final Map<String,FingerPrint> classes = new HashMap<>();
     private final Main main;
-    private final JarFile jf;
+    private final ZipFile zf;
-    private int oldVersion = -1;
-    private String currentTopLevelName;
     private boolean isValid = true;
     private Set<String> concealedPkgs = Collections.emptySet();
     private ModuleDescriptor md;
     private String mdName;
 
-    private Validator(Main main, JarFile jf) {
+    private Validator(Main main, ZipFile zf) {
         this.main = main;
-        this.jf = jf;
+        this.zf = zf;
         checkModuleDescriptor(MODULE_INFO);
     }
 
-    static boolean validate(Main main, JarFile jf) throws IOException {
+    static boolean validate(Main main, ZipFile zf) throws IOException {
-        return new Validator(main, jf).validate();
+        return new Validator(main, zf).validate();
     }
 
     private boolean validate() {
         try {
-            jf.stream()
+            zf.stream()
-              .filter(e -> !e.isDirectory() &&
+              .filter(e -> e.getName().endsWith(".class"))
-                      !e.getName().equals(MANIFEST_NAME))
+              .map(this::getFingerPrint)
-              .sorted(ENTRY_COMPARATOR)
+              .filter(FingerPrint::isClass)    // skip any non-class entry
-              .forEachOrdered(e -> validate(e));
+              .collect(Collectors.groupingBy(
-            return isValid;
+                      FingerPrint::mrversion,
+                      TreeMap::new,
+                      Collectors.toMap(FingerPrint::className,
+                                       Function.identity(),
+                                       this::sameNameFingerPrint)))
+              .forEach((version, entries) -> {
+                      if (version == 0)
+                          validateBase(entries);
+                      else
+                          validateVersioned(entries);
+                  });
         } catch (InvalidJarException e) {
-            error(formatMsg("error.validator.bad.entry.name", e.getMessage()));
+            errorAndInvalid(e.getMessage());
         }
-        return false;
+        return isValid;
     }
 
-    private static class InvalidJarException extends RuntimeException {
+    static class InvalidJarException extends RuntimeException {
         private static final long serialVersionUID = -3642329147299217726L;
         InvalidJarException(String msg) {
             super(msg);
         }
     }
 
-    // sort base entries before versioned entries, and sort entry classes with
+    private FingerPrint sameNameFingerPrint(FingerPrint fp1, FingerPrint fp2) {
-    // nested classes so that the top level class appears before the associated
+        checkClassName(fp1);
-    // nested class
+        checkClassName(fp2);
-    static Comparator<String> ENTRYNAME_COMPARATOR = (s1, s2) ->  {
+        // entries/classes with same name, return fp2 for now ?
+        return fp2;
+    }
 
-        if (s1.equals(s2)) return 0;
+    private FingerPrint getFingerPrint(ZipEntry ze) {
-        boolean b1 = s1.startsWith(VERSIONS_DIR);
+        // figure out the version and basename from the ZipEntry
-        boolean b2 = s2.startsWith(VERSIONS_DIR);
+        String ename = ze.getName();
-        if (b1 && !b2) return 1;
+        String bname = ename;
-        if (!b1 && b2) return -1;
+        int version = 0;
-        int n = 0; // starting char for String compare
+
-        if (b1 && b2) {
+        if (ename.startsWith(VERSIONS_DIR)) {
-            // normally strings would be sorted so "10" goes before "9", but
+            int n = ename.indexOf("/", VERSIONS_DIR_LENGTH);
-            // version number strings need to be sorted numerically
+            if (n == -1) {
-            n = VERSIONS_DIR.length();   // skip the common prefix
+                throw new InvalidJarException(
-            int i1 = s1.indexOf('/', n);
+                    formatMsg("error.validator.version.notnumber", ename));
-            int i2 = s2.indexOf('/', n);
-            if (i1 == -1) throw new InvalidJarException(s1);
-            if (i2 == -1) throw new InvalidJarException(s2);
-            // shorter version numbers go first
-            if (i1 != i2) return i1 - i2;
-            // otherwise, handle equal length numbers below
-        }
-        int l1 = s1.length();
-        int l2 = s2.length();
-        int lim = Math.min(l1, l2);
-        for (int k = n; k < lim; k++) {
-            char c1 = s1.charAt(k);
-            char c2 = s2.charAt(k);
-            if (c1 != c2) {
-                // change natural ordering so '.' comes before '$'
-                // i.e. top level classes come before nested classes
-                if (c1 == '$' && c2 == '.') return 1;
-                if (c1 == '.' && c2 == '$') return -1;
-                return c1 - c2;
             }
+            try {
+                version = Integer.parseInt(ename, VERSIONS_DIR_LENGTH, n, 10);
+            } catch (NumberFormatException x) {
+                throw new InvalidJarException(
+                    formatMsg("error.validator.version.notnumber", ename));
+            }
+            if (n == ename.length()) {
+                throw new InvalidJarException(
+                    formatMsg("error.validator.entryname.tooshort", ename));
+            }
+            bname = ename.substring(n + 1);
         }
-        return l1 - l2;
-    };
 
-    static Comparator<ZipEntry> ENTRY_COMPARATOR =
+        // return the cooresponding fingerprint entry
-        Comparator.comparing(ZipEntry::getName, ENTRYNAME_COMPARATOR);
+        try (InputStream is = zf.getInputStream(ze)) {
+            return new FingerPrint(bname, ename, version, is.readAllBytes());
+        } catch (IOException x) {
+           throw new InvalidJarException(x.getMessage());
+        }
+    }
 
     /*
-     *  Validator has state and assumes entries provided to accept are ordered
+     *  Validates (a) if there is any isolated nested class, and (b) if the
-     *  from base entries first and then through the versioned entries in
+     *  class name in class file (by asm) matches the entry's basename.
-     *  ascending version order.  Also, to find isolated nested classes,
+     */
-     *  classes must be ordered so that the top level class is before the associated
+    public void validateBase(Map<String, FingerPrint> fps) {
-     *  nested class(es).
+        fps.values().forEach( fp -> {
-    */
+            if (!checkClassName(fp)) {
-    public void validate(JarEntry je) {
-        String entryName = je.getName();
-
-        // directories are always accepted
-        if (entryName.endsWith("/")) {
-            debug("%s is a directory", entryName);
-            return;
-        }
-
-        // validate the versioned module-info
-        if (isModuleInfoEntry(entryName)) {
-            if (!entryName.equals(mdName))
-                checkModuleDescriptor(entryName);
-            return;
-        }
-
-        // figure out the version and basename from the JarEntry
-        int version;
-        String basename;
-        String versionStr = null;;
-        if (entryName.startsWith(VERSIONS_DIR)) {
-            int n = entryName.indexOf("/", VERSIONS_DIR_LENGTH);
-            if (n == -1) {
-                error(formatMsg("error.validator.version.notnumber", entryName));
                 isValid = false;
                 return;
             }
-            versionStr = entryName.substring(VERSIONS_DIR_LENGTH, n);
-            try {
-                version = Integer.parseInt(versionStr);
-            } catch (NumberFormatException x) {
-                error(formatMsg("error.validator.version.notnumber", entryName));
-                isValid = false;
-                return;
-            }
-            if (n == entryName.length()) {
-                error(formatMsg("error.validator.entryname.tooshort", entryName));
-                isValid = false;
-                return;
-            }
-            basename = entryName.substring(n + 1);
-        } else {
-            version = 0;
-            basename = entryName;
-        }
-        debug("\n===================\nversion %d %s", version, entryName);
-
-        if (oldVersion != version) {
-            oldVersion = version;
-            currentTopLevelName = null;
-            if (md == null && versionStr != null) {
-                // don't have a base module-info.class yet, try to see if
-                // a versioned one exists
-                checkModuleDescriptor(VERSIONS_DIR + versionStr + "/" + MODULE_INFO);
-            }
-        }
-
-        // analyze the entry, keeping key attributes
-        FingerPrint fp;
-        try (InputStream is = jf.getInputStream(je)) {
-            fp = new FingerPrint(basename, is.readAllBytes());
-        } catch (IOException x) {
-            error(x.getMessage());
-            isValid = false;
-            return;
-        }
-        String internalName = fp.name();
-
-        // process a base entry paying attention to nested classes
-        if (version == 0) {
-            debug("base entry found");
             if (fp.isNestedClass()) {
-                debug("nested class found");
+                if (!checkNestedClass(fp, fps)) {
-                if (fp.topLevelName().equals(currentTopLevelName)) {
+                    isValid = false;
-                    fps.put(internalName, fp);
-                    return;
                 }
-                error(formatMsg("error.validator.isolated.nested.class", entryName));
+            }
-                isValid = false;
+            classes.put(fp.className(), fp);
+        });
+    }
+
+    public void validateVersioned(Map<String, FingerPrint> fps) {
+
+        fps.values().forEach( fp -> {
+
+            // validate the versioned module-info
+            if (MODULE_INFO.equals(fp.basename())) {
+                checkModuleDescriptor(fp.entryName());
                 return;
             }
-            // top level class or resource entry
+            // process a versioned entry, look for previous entry with same name
-            if (fp.isClass()) {
+            FingerPrint matchFp = classes.get(fp.className());
-                currentTopLevelName = fp.topLevelName();
+            if (matchFp == null) {
-                if (!checkInternalName(entryName, basename, internalName)) {
+                // no match found
-                    isValid = false;
-                    return;
-                }
-            }
-            fps.put(internalName, fp);
-            return;
-        }
-
-        // process a versioned entry, look for previous entry with same name
-        FingerPrint matchFp = fps.get(internalName);
-        debug("looking for match");
-        if (matchFp == null) {
-            debug("no match found");
-            if (fp.isClass()) {
                 if (fp.isNestedClass()) {
-                    if (!checkNestedClass(version, entryName, internalName, fp)) {
+                    if (!checkNestedClass(fp, fps)) {
                         isValid = false;
                     }
                     return;
                 }
                 if (fp.isPublicClass()) {
-                    if (!isConcealed(internalName)) {
+                    if (!isConcealed(fp.className())) {
-                        error(Main.formatMsg("error.validator.new.public.class", entryName));
+                        errorAndInvalid(formatMsg("error.validator.new.public.class",
-                        isValid = false;
+                                                  fp.entryName()));
                         return;
-                    }
+                     }
-                    warn(formatMsg("warn.validator.concealed.public.class", entryName));
+                     // entry is a public class entry in a concealed package
-                    debug("%s is a public class entry in a concealed package", entryName);
+                     warn(formatMsg("warn.validator.concealed.public.class",
+                                   fp.entryName()));
                 }
-                debug("%s is a non-public class entry", entryName);
+                classes.put(fp.className(), fp);
-                fps.put(internalName, fp);
-                currentTopLevelName = fp.topLevelName();
                 return;
             }
-            debug("%s is a resource entry");
-            fps.put(internalName, fp);
-            return;
-        }
-        debug("match found");
 
-        // are the two classes/resources identical?
+            // are the two classes/resources identical?
-        if (fp.isIdentical(matchFp)) {
+            if (fp.isIdentical(matchFp)) {
-            warn(formatMsg("warn.validator.identical.entry", entryName));
+                warn(formatMsg("warn.validator.identical.entry", fp.entryName()));
-            return;  // it's okay, just takes up room
+                return;    // it's okay, just takes up room
-        }
+            }
-        debug("sha1 not equal -- different bytes");
 
-        // ok, not identical, check for compatible class version and api
+            // ok, not identical, check for compatible class version and api
-        if (fp.isClass()) {
             if (fp.isNestedClass()) {
-                if (!checkNestedClass(version, entryName, internalName, fp)) {
+                if (!checkNestedClass(fp, fps)) {
                     isValid = false;
                 }
-                return;
+                return;    // fall through, need check nested public class??
             }
-            debug("%s is a class entry", entryName);
             if (!fp.isCompatibleVersion(matchFp)) {
-                error(formatMsg("error.validator.incompatible.class.version", entryName));
+                errorAndInvalid(formatMsg("error.validator.incompatible.class.version",
-                isValid = false;
+                                          fp.entryName()));
                 return;
             }
             if (!fp.isSameAPI(matchFp)) {
-                error(formatMsg("error.validator.different.api", entryName));
+                errorAndInvalid(formatMsg("error.validator.different.api",
+                                          fp.entryName()));
+                return;
+            }
+            if (!checkClassName(fp)) {
                 isValid = false;
                 return;
             }
-            if (!checkInternalName(entryName, basename, internalName)) {
+            classes.put(fp.className(), fp);
-                isValid = false;
-                return;
-            }
-            debug("fingerprints same -- same api");
-            fps.put(internalName, fp);
-            currentTopLevelName = fp.topLevelName();
-            return;
-        }
-        debug("%s is a resource", entryName);
 
-        warn(formatMsg("warn.validator.resources.with.same.name", entryName));
+            return;
-        fps.put(internalName, fp);
+        });
-        return;
     }
 
-    /**
+    /*
      * Checks whether or not the given versioned module descriptor's attributes
      * are valid when compared against the root/base module descriptor.
      *
@@ -326,12 +239,12 @@ final class Validator {
      *  - A versioned descriptor can have different non-public `requires`
      *    clauses of platform ( `java.*` and `jdk.*` ) modules, and
      *  - A versioned descriptor can have different `uses` clauses, even of
-     *    service types defined outside of the platform modules.
+    *    service types defined outside of the platform modules.
      */
     private void checkModuleDescriptor(String miName) {
-        ZipEntry je = jf.getEntry(miName);
+        ZipEntry ze = zf.getEntry(miName);
-        if (je != null) {
+        if (ze != null) {
-            try (InputStream jis = jf.getInputStream(je)) {
+            try (InputStream jis = zf.getInputStream(ze)) {
                 ModuleDescriptor md = ModuleDescriptor.read(jis);
                 // Initialize the base md if it's not yet. A "base" md can be either the
                 // root module-info.class or the first versioned module-info.class
@@ -344,7 +257,7 @@ final class Validator {
                     // must have the implementation class of the services it 'provides'.
                     if (md.provides().stream().map(Provides::providers)
                           .flatMap(List::stream)
-                          .filter(p -> jf.getEntry(toBinaryName(p)) == null)
+                          .filter(p -> zf.getEntry(toBinaryName(p)) == null)
                           .peek(p -> error(formatMsg("error.missing.provider", p)))
                           .count() != 0) {
                         isValid = false;
@@ -356,8 +269,7 @@ final class Validator {
                 }
 
                 if (!base.name().equals(md.name())) {
-                    error(getMsg("error.validator.info.name.notequal"));
+                    errorAndInvalid(getMsg("error.validator.info.name.notequal"));
-                    isValid = false;
                 }
                 if (!base.requires().equals(md.requires())) {
                     Set<Requires> baseRequires = base.requires();
@@ -365,11 +277,9 @@ final class Validator {
                         if (baseRequires.contains(r))
                             continue;
                         if (r.modifiers().contains(Requires.Modifier.TRANSITIVE)) {
-                            error(getMsg("error.validator.info.requires.transitive"));
+                            errorAndInvalid(getMsg("error.validator.info.requires.transitive"));
-                            isValid = false;
                         } else if (!isPlatformModule(r.name())) {
-                            error(getMsg("error.validator.info.requires.added"));
+                            errorAndInvalid(getMsg("error.validator.info.requires.added"));
-                            isValid = false;
                         }
                     }
                     for (Requires r : baseRequires) {
@@ -377,87 +287,78 @@ final class Validator {
                         if (mdRequires.contains(r))
                             continue;
                         if (!isPlatformModule(r.name())) {
-                            error(getMsg("error.validator.info.requires.dropped"));
+                            errorAndInvalid(getMsg("error.validator.info.requires.dropped"));
-                            isValid = false;
                         }
                     }
                 }
                 if (!base.exports().equals(md.exports())) {
-                    error(getMsg("error.validator.info.exports.notequal"));
+                    errorAndInvalid(getMsg("error.validator.info.exports.notequal"));
-                    isValid = false;
                 }
                 if (!base.opens().equals(md.opens())) {
-                    error(getMsg("error.validator.info.opens.notequal"));
+                    errorAndInvalid(getMsg("error.validator.info.opens.notequal"));
-                    isValid = false;
                 }
                 if (!base.provides().equals(md.provides())) {
-                    error(getMsg("error.validator.info.provides.notequal"));
+                    errorAndInvalid(getMsg("error.validator.info.provides.notequal"));
-                    isValid = false;
                 }
                 if (!base.mainClass().equals(md.mainClass())) {
-                    error(formatMsg("error.validator.info.manclass.notequal", je.getName()));
+                    errorAndInvalid(formatMsg("error.validator.info.manclass.notequal",
-                    isValid = false;
+                                              ze.getName()));
                 }
                 if (!base.version().equals(md.version())) {
-                    error(formatMsg("error.validator.info.version.notequal", je.getName()));
+                    errorAndInvalid(formatMsg("error.validator.info.version.notequal",
-                    isValid = false;
+                                              ze.getName()));
                 }
             } catch (Exception x) {
-                error(x.getMessage() + " : " + miName);
+                errorAndInvalid(x.getMessage() + " : " + miName);
-                this.isValid = false;
             }
         }
     }
 
+    private boolean checkClassName(FingerPrint fp) {
+        if (fp.className().equals(className(fp.basename()))) {
+            return true;
+        }
+        error(formatMsg2("error.validator.names.mismatch",
+                         fp.entryName(), fp.className().replace("/", ".")));
+        return false;
+    }
+
+    private boolean checkNestedClass(FingerPrint fp, Map<String, FingerPrint> outerClasses) {
+        if (outerClasses.containsKey(fp.outerClassName())) {
+            return true;
+        }
+        // outer class was not available
+        error(formatMsg("error.validator.isolated.nested.class", fp.entryName()));
+        return false;
+    }
+
+    private boolean isConcealed(String className) {
+        if (concealedPkgs.isEmpty()) {
+            return false;
+        }
+        int idx = className.lastIndexOf('/');
+        String pkgName = idx != -1 ? className.substring(0, idx).replace('/', '.') : "";
+        return concealedPkgs.contains(pkgName);
+    }
+
     private static boolean isPlatformModule(String name) {
         return name.startsWith("java.") || name.startsWith("jdk.");
     }
 
-    private boolean checkInternalName(String entryName, String basename, String internalName) {
+    private static String className(String entryName) {
-        String className = className(basename);
-        if (internalName.equals(className)) {
-            return true;
-        }
-        error(formatMsg2("error.validator.names.mismatch",
-                entryName, internalName.replace("/", ".")));
-        return false;
-    }
-
-    private boolean checkNestedClass(int version, String entryName, String internalName, FingerPrint fp) {
-        debug("%s is a nested class entry in top level class %s", entryName, fp.topLevelName());
-        if (fp.topLevelName().equals(currentTopLevelName)) {
-            debug("%s (top level class) was accepted", fp.topLevelName());
-            fps.put(internalName, fp);
-            return true;
-        }
-        debug("top level class was not accepted");
-        error(formatMsg("error.validator.isolated.nested.class", entryName));
-        return false;
-    }
-
-    private String className(String entryName) {
         return entryName.endsWith(".class") ? entryName.substring(0, entryName.length() - 6) : null;
     }
 
-    private boolean isConcealed(String internalName) {
-        if (concealedPkgs.isEmpty()) {
-            return false;
-        }
-        int idx = internalName.lastIndexOf('/');
-        String pkgName = idx != -1 ? internalName.substring(0, idx).replace('/', '.') : "";
-        return concealedPkgs.contains(pkgName);
-    }
-
-    private void debug(String fmt, Object... args) {
-        if (DEBUG) System.err.format(fmt, args);
-    }
-
     private void error(String msg) {
         main.error(msg);
     }
 
+    private void errorAndInvalid(String msg) {
+        main.error(msg);
+        isValid = false;
+    }
+
     private void warn(String msg) {
         main.warn(msg);
     }
-
 }

test/jdk/tools/jar/multiRelease/Basic.java

@@ -23,6 +23,7 @@
 
 /*
  * @test
+ # @bug 8186087
  * @library /test/lib
  * @modules java.base/jdk.internal.misc
  *          jdk.compiler
@@ -345,39 +346,6 @@ public class Basic extends MRTestBase {
         FileUtils.deleteFileTreeWithRetry(Paths.get(usr, "classes"));
     }
 
-    @Test
-    // resources with same name in different versions
-    // this is okay but produces warning
-    public void test08() throws Throwable {
-        String jarfile = "test.jar";
-
-        compile("test01");  //use same data as test01
-
-        Path classes = Paths.get("classes");
-
-        // add a resource to the base
-        Path source = Paths.get(src, "data", "test01", "base", "version");
-        Files.copy(source.resolve("Version.java"), classes.resolve("base")
-                .resolve("version").resolve("Version.java"));
-
-        jar("cf", jarfile, "-C", classes.resolve("base").toString(), ".",
-                "--release", "9", "-C", classes.resolve("v9").toString(), ".")
-                .shouldHaveExitValue(SUCCESS)
-                .shouldBeEmpty();
-
-        // now add a different resource with same name to META-INF/version/9
-        Files.copy(source.resolve("Main.java"), classes.resolve("v9")
-                .resolve("version").resolve("Version.java"));
-
-        jar("cf", jarfile, "-C", classes.resolve("base").toString(), ".",
-                "--release", "9", "-C", classes.resolve("v9").toString(), ".")
-                .shouldHaveExitValue(SUCCESS)
-                .shouldContain("multiple resources with same name");
-
-        FileUtils.deleteFileIfExistsWithRetry(Paths.get(jarfile));
-        FileUtils.deleteFileTreeWithRetry(Paths.get(usr, "classes"));
-    }
-
     @Test
     // a class with an internal name different from the external name
     public void test09() throws Throwable {
@@ -451,7 +419,9 @@ public class Basic extends MRTestBase {
                 .shouldNotHaveExitValue(SUCCESS)
                 .asLines();
 
+        /* "META-INF/versions/9/version/Nested$nested.class" is really NOT isolated
         assertTrue(output.size() == 4);
+        assertTrue(output.size() == 3);
         assertTrue(output.get(0).contains("an isolated nested class"),
                 output.get(0));
         assertTrue(output.get(1).contains("contains a new public class"),
@@ -460,6 +430,17 @@ public class Basic extends MRTestBase {
                 output.get(2));
         assertTrue(output.get(3).contains("invalid multi-release jar file"),
                 output.get(3));
+        assertTrue(output.get(2).contains("invalid multi-release jar file"),
+               output.get(2));
+        */
+
+        assertTrue(output.size() == 3);
+        assertTrue(output.get(0).contains("an isolated nested class"),
+                output.get(0));
+        assertTrue(output.get(1).contains("contains a new public class"),
+                output.get(1));
+        assertTrue(output.get(2).contains("invalid multi-release jar file"),
+               output.get(2));
 
         FileUtils.deleteFileIfExistsWithRetry(Paths.get(jarfile));
         FileUtils.deleteFileTreeWithRetry(Paths.get(usr, "classes"));
@@ -494,6 +475,31 @@ public class Basic extends MRTestBase {
         FileUtils.deleteFileTreeWithRetry(Paths.get(usr, "classes"));
     }
 
+    @Test
+    // assure the nested-nested classes are acceptable
+    public void test13() throws Throwable {
+        String jarfile = "test.jar";
+
+        compile("test01");  //use same data as test01
+
+        Path classes = Paths.get("classes");
+
+        // add a base class with a nested and nested-nested class
+        Path source = Paths.get(src, "data", "test13", "base", "version");
+        javac(classes.resolve("base"), source.resolve("Nested.java"));
+
+        // add a versioned class with a nested and nested-nested class
+        source = Paths.get(src, "data", "test13", "v10", "version");
+        javac(classes.resolve("v10"), source.resolve("Nested.java"));
+
+        jar("cf", jarfile, "-C", classes.resolve("base").toString(), ".",
+                "--release", "10", "-C", classes.resolve("v10").toString(), ".")
+                .shouldHaveExitValue(SUCCESS);
+
+        FileUtils.deleteFileIfExistsWithRetry(Paths.get(jarfile));
+        FileUtils.deleteFileTreeWithRetry(Paths.get(usr, "classes"));
+    }
+
     @Test
     public void testCustomManifest() throws Throwable {
         String jarfile = "test.jar";

test/jdk/tools/jar/multiRelease/data/test13/base/version/Nested.java

@@ -0,0 +1,22 @@
+package version;
+
+public class Nested {
+    public int getVersion() {
+        return 9;
+    }
+
+    protected void doNothing() {
+    }
+
+    private void anyName() {
+    }
+
+    class nested {
+        int save;
+
+        class nestnested {
+            int save;
+        }
+
+    }
+}

test/jdk/tools/jar/multiRelease/data/test13/v10/version/Nested.java

@@ -0,0 +1,18 @@
+package version;
+
+public class Nested {
+    public int getVersion() {
+        return 10;
+    }
+
+    protected void doNothing() {
+    }
+
+    class nested {
+        int save = getVersion();
+
+        class nestnested {
+            int save = getVersion();;
+        }
+    }
+}

test/jdk/tools/jar/multiRelease/whitebox/jdk.jartool/sun/tools/jar/ValidatorComparatorTest.java

@@ -29,7 +29,7 @@ package sun.tools.jar;
 
 import java.util.List;
 import static java.util.stream.Collectors.toList;
-import static sun.tools.jar.Validator.ENTRYNAME_COMPARATOR;
+import static sun.tools.jar.Main.ENTRYNAME_COMPARATOR;
 
 import org.testng.Assert;
 import org.testng.annotations.Test;