From def15478ebc213eeff8eb4da9178f8bac4c72604 Mon Sep 17 00:00:00 2001 From: Steve Drach Date: Fri, 29 Jul 2016 09:58:28 -0700 Subject: [PATCH] 8158295: Add a multi-release jar validation mechanism to jar tool Reviewed-by: ogb, psandoz --- .../java.base/share/classes/module-info.java | 1 + .../classes/sun/tools/jar/FingerPrint.java | 324 ++++++++++++++++++ .../share/classes/sun/tools/jar/Main.java | 223 +++++++----- .../classes/sun/tools/jar/Validator.java | 242 +++++++++++++ .../sun/tools/jar/resources/jar.properties | 24 ++ jdk/test/tools/jar/multiRelease/Basic.java | 264 +++++++++++++- .../data/test04/v9/version/Version.java | 14 + .../data/test05/v9/version/Extra.java | 13 + .../data/test06/v9/version/Extra.java | 13 + .../data/test10/base/version/Nested.java | 17 + .../data/test10/v9/version/Nested.java | 14 + 11 files changed, 1069 insertions(+), 80 deletions(-) create mode 100644 jdk/src/jdk.jartool/share/classes/sun/tools/jar/FingerPrint.java create mode 100644 jdk/src/jdk.jartool/share/classes/sun/tools/jar/Validator.java create mode 100644 jdk/test/tools/jar/multiRelease/data/test04/v9/version/Version.java create mode 100644 jdk/test/tools/jar/multiRelease/data/test05/v9/version/Extra.java create mode 100644 jdk/test/tools/jar/multiRelease/data/test06/v9/version/Extra.java create mode 100644 jdk/test/tools/jar/multiRelease/data/test10/base/version/Nested.java create mode 100644 jdk/test/tools/jar/multiRelease/data/test10/v9/version/Nested.java diff --git a/jdk/src/java.base/share/classes/module-info.java b/jdk/src/java.base/share/classes/module-info.java index a6446ac111c..044f43b2877 100644 --- a/jdk/src/java.base/share/classes/module-info.java +++ b/jdk/src/java.base/share/classes/module-info.java @@ -128,6 +128,7 @@ module java.base { exports jdk.internal.logger to java.logging; exports jdk.internal.org.objectweb.asm to + jdk.jartool, jdk.jlink, jdk.scripting.nashorn, jdk.vm.ci; diff --git a/jdk/src/jdk.jartool/share/classes/sun/tools/jar/FingerPrint.java b/jdk/src/jdk.jartool/share/classes/sun/tools/jar/FingerPrint.java new file mode 100644 index 00000000000..5ae52909e81 --- /dev/null +++ b/jdk/src/jdk.jartool/share/classes/sun/tools/jar/FingerPrint.java @@ -0,0 +1,324 @@ +/* + * Copyright (c) 2016, 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 + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Oracle designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +package sun.tools.jar; + +import jdk.internal.org.objectweb.asm.*; + +import java.io.IOException; +import java.io.InputStream; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; + +/** + * A FingerPrint is an abstract representation of a JarFile entry that contains + * information to determine if the entry represents a class or a + * resource, and whether two entries are identical. If the FingerPrint represents + * 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 + * 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 + * {@code n} precedes classes with versions {@code > n} for all versions + * {@code n}. + */ +final class FingerPrint { + private static final MessageDigest MD; + + private final byte[] sha1; + private final ClassAttributes attrs; + private final boolean isClassEntry; + private final String entryName; + + static { + try { + MD = MessageDigest.getInstance("SHA-1"); + } catch (NoSuchAlgorithmException x) { + // log big problem? + throw new RuntimeException(x); + } + } + + public FingerPrint(String entryName,byte[] bytes) throws IOException { + this.entryName = entryName; + if (entryName.endsWith(".class") && isCafeBabe(bytes)) { + isClassEntry = true; + sha1 = sha1(bytes, 8); // skip magic number and major/minor version + attrs = getClassAttributes(bytes); + } else { + isClassEntry = false; + sha1 = sha1(bytes); + attrs = new ClassAttributes(); // empty class + } + } + + public boolean isClass() { + return isClassEntry; + } + + public boolean isNestedClass() { + return attrs.nestedClass; + } + + public boolean isPublicClass() { + return attrs.publicClass; + } + + public boolean isIdentical(FingerPrint that) { + if (that == null) return false; + if (this == that) return true; + return isEqual(this.sha1, that.sha1); + } + + public boolean isCompatibleVersion(FingerPrint that) { + return attrs.version >= that.attrs.version; + } + + public boolean isSameAPI(FingerPrint that) { + if (that == null) return false; + return attrs.equals(that.attrs); + } + + public String name() { + String name = attrs.name; + return name == null ? entryName : name; + } + + public String topLevelName() { + String name = attrs.topLevelName; + return name == null ? name() : name; + } + + private byte[] sha1(byte[] entry) { + MD.update(entry); + return MD.digest(); + } + + private byte[] sha1(byte[] entry, int offset) { + MD.update(entry, offset, entry.length - offset); + return MD.digest(); + } + + private boolean isEqual(byte[] sha1_1, byte[] sha1_2) { + return MessageDigest.isEqual(sha1_1, sha1_2); + } + + private static final byte[] cafeBabe = {(byte)0xca, (byte)0xfe, (byte)0xba, (byte)0xbe}; + + private boolean isCafeBabe(byte[] bytes) { + if (bytes.length < 4) return false; + for (int i = 0; i < 4; i++) { + if (bytes[i] != cafeBabe[i]) { + return false; + } + } + return true; + } + + private ClassAttributes getClassAttributes(byte[] bytes) { + ClassReader rdr = new ClassReader(bytes); + ClassAttributes attrs = new ClassAttributes(); + rdr.accept(attrs, 0); + return attrs; + } + + private static final class Field { + private final int access; + private final String name; + private final String desc; + + Field(int access, String name, String desc) { + this.access = access; + this.name = name; + this.desc = desc; + } + + @Override + public boolean equals(Object that) { + if (that == null) return false; + if (this == that) return true; + if (!(that instanceof Field)) return false; + Field field = (Field)that; + return (access == field.access) && name.equals(field.name) + && desc.equals(field.desc); + } + + @Override + public int hashCode() { + int result = 17; + result = 37 * result + access; + result = 37 * result + name.hashCode(); + result = 37 * result + desc.hashCode(); + return result; + } + } + + private static final class Method { + private final int access; + private final String name; + private final String desc; + private final Set exceptions; + + Method(int access, String name, String desc, Set exceptions) { + this.access = access; + this.name = name; + this.desc = desc; + this.exceptions = exceptions; + } + + @Override + public boolean equals(Object that) { + if (that == null) return false; + if (this == that) return true; + if (!(that instanceof Method)) return false; + Method method = (Method)that; + return (access == method.access) && name.equals(method.name) + && desc.equals(method.desc) + && exceptions.equals(method.exceptions); + } + + @Override + public int hashCode() { + int result = 17; + result = 37 * result + access; + result = 37 * result + name.hashCode(); + result = 37 * result + desc.hashCode(); + result = 37 * result + exceptions.hashCode(); + return result; + } + } + + private static final class ClassAttributes extends ClassVisitor { + private String name; + private String topLevelName; + private String superName; + private int version; + private int access; + private boolean publicClass; + private boolean nestedClass; + private final Set fields = new HashSet<>(); + private final Set methods = new HashSet<>(); + + public ClassAttributes() { + super(Opcodes.ASM5); + } + + private boolean isPublic(int access) { + return ((access & Opcodes.ACC_PUBLIC) == Opcodes.ACC_PUBLIC) + || ((access & Opcodes.ACC_PROTECTED) == Opcodes.ACC_PROTECTED); + } + + @Override + public void visit(int version, int access, String name, String signature, + String superName, String[] interfaces) { + this.version = version; + this.access = access; + this.name = name; + this.nestedClass = name.contains("$"); + this.superName = superName; + this.publicClass = isPublic(access); + } + + @Override + public void visitOuterClass(String owner, String name, String desc) { + if (!this.nestedClass) return; + this.topLevelName = owner; + } + + @Override + public void visitInnerClass(String name, String outerName, String innerName, + int access) { + if (!this.nestedClass) return; + if (outerName == null) return; + if (!this.name.equals(name)) return; + if (this.topLevelName == null) this.topLevelName = outerName; + } + + @Override + public FieldVisitor visitField(int access, String name, String desc, + String signature, Object value) { + if (isPublic(access)) { + fields.add(new Field(access, name, desc)); + } + return null; + } + + @Override + public MethodVisitor visitMethod(int access, String name, String desc, + String signature, String[] exceptions) { + if (isPublic(access)) { + Set exceptionSet = new HashSet<>(); + if (exceptions != null) { + for (String e : exceptions) { + exceptionSet.add(e); + } + } + // treat type descriptor as a proxy for signature because signature + // is usually null, need to strip off the return type though + int n; + if (desc != null && (n = desc.lastIndexOf(')')) != -1) { + desc = desc.substring(0, n + 1); + methods.add(new Method(access, name, desc, exceptionSet)); + } + } + return null; + } + + @Override + public void visitEnd() { + this.nestedClass = this.topLevelName != null; + } + + @Override + public boolean equals(Object that) { + if (that == null) return false; + if (this == that) return true; + if (!(that instanceof ClassAttributes)) return false; + ClassAttributes clsAttrs = (ClassAttributes)that; + boolean superNameOkay = superName != null + ? superName.equals(clsAttrs.superName) : true; + return access == clsAttrs.access + && superNameOkay + && fields.equals(clsAttrs.fields) + && methods.equals(clsAttrs.methods); + } + + @Override + public int hashCode() { + int result = 17; + result = 37 * result + access; + result = 37 * result + superName != null ? superName.hashCode() : 0; + result = 37 * result + fields.hashCode(); + result = 37 * result + methods.hashCode(); + return result; + } + } +} diff --git a/jdk/src/jdk.jartool/share/classes/sun/tools/jar/Main.java b/jdk/src/jdk.jartool/share/classes/sun/tools/jar/Main.java index f33bcf711ce..00796dee41e 100644 --- a/jdk/src/jdk.jartool/share/classes/sun/tools/jar/Main.java +++ b/jdk/src/jdk.jartool/share/classes/sun/tools/jar/Main.java @@ -42,6 +42,7 @@ import java.nio.ByteBuffer; import java.nio.file.Path; import java.nio.file.Files; import java.nio.file.Paths; +import java.nio.file.StandardCopyOption; import java.util.*; import java.util.function.Consumer; import java.util.function.Function; @@ -278,23 +279,20 @@ class Main { } } } + if (cflag) { Manifest manifest = null; - InputStream in = null; - if (!Mflag) { if (mname != null) { - in = new FileInputStream(mname); - manifest = new Manifest(new BufferedInputStream(in)); + try (InputStream in = new FileInputStream(mname)) { + manifest = new Manifest(new BufferedInputStream(in)); + } } else { manifest = new Manifest(); } addVersion(manifest); addCreatedBy(manifest); if (isAmbiguousMainClass(manifest)) { - if (in != null) { - in.close(); - } return false; } if (ename != null) { @@ -304,11 +302,13 @@ class Main { addMultiRelease(manifest); } } + Map moduleInfoPaths = new HashMap<>(); for (int version : filesMap.keySet()) { String[] files = filesMap.get(version); expand(null, files, false, moduleInfoPaths, version); } + Map moduleInfos = new LinkedHashMap<>(); if (!moduleInfoPaths.isEmpty()) { if (!checkModuleInfos(moduleInfoPaths)) @@ -332,84 +332,61 @@ class Main { return false; } - OutputStream out; - if (fname != null) { - out = new FileOutputStream(fname); - } else { - out = new FileOutputStream(FileDescriptor.out); - if (vflag) { - // Disable verbose output so that it does not appear - // on stdout along with file data - // error("Warning: -v option ignored"); - vflag = false; - } + if (vflag && fname == null) { + // Disable verbose output so that it does not appear + // on stdout along with file data + // error("Warning: -v option ignored"); + vflag = false; } - File tmpfile = null; - final OutputStream finalout = out; + final String tmpbase = (fname == null) ? "tmpjar" : fname.substring(fname.indexOf(File.separatorChar) + 1); - if (nflag) { - tmpfile = createTemporaryFile(tmpbase, ".jar"); - out = new FileOutputStream(tmpfile); - } - create(new BufferedOutputStream(out, 4096), manifest, moduleInfos); + File tmpfile = createTemporaryFile(tmpbase, ".jar"); - if (in != null) { - in.close(); + try (OutputStream out = new FileOutputStream(tmpfile)) { + create(new BufferedOutputStream(out, 4096), manifest, moduleInfos); } - out.close(); + if (nflag) { - JarFile jarFile = null; - File packFile = null; - JarOutputStream jos = null; + File packFile = createTemporaryFile(tmpbase, ".pack"); try { Packer packer = Pack200.newPacker(); Map p = packer.properties(); p.put(Packer.EFFORT, "1"); // Minimal effort to conserve CPU - jarFile = new JarFile(tmpfile.getCanonicalPath()); - packFile = createTemporaryFile(tmpbase, ".pack"); - out = new FileOutputStream(packFile); - packer.pack(jarFile, out); - jos = new JarOutputStream(finalout); - Unpacker unpacker = Pack200.newUnpacker(); - unpacker.unpack(packFile, jos); - } catch (IOException ioe) { - fatalError(ioe); - } finally { - if (jarFile != null) { - jarFile.close(); + try ( + JarFile jarFile = new JarFile(tmpfile.getCanonicalPath()); + OutputStream pack = new FileOutputStream(packFile) + ) { + packer.pack(jarFile, pack); } - if (out != null) { - out.close(); - } - if (jos != null) { - jos.close(); - } - if (tmpfile != null && tmpfile.exists()) { + if (tmpfile.exists()) { tmpfile.delete(); } - if (packFile != null && packFile.exists()) { - packFile.delete(); + tmpfile = createTemporaryFile(tmpbase, ".jar"); + try ( + OutputStream out = new FileOutputStream(tmpfile); + JarOutputStream jos = new JarOutputStream(out) + ) { + Unpacker unpacker = Pack200.newUnpacker(); + unpacker.unpack(packFile, jos); } + } finally { + Files.deleteIfExists(packFile.toPath()); } } + + validateAndClose(tmpfile); + } else if (uflag) { File inputFile = null, tmpFile = null; - FileInputStream in; - FileOutputStream out; if (fname != null) { inputFile = new File(fname); tmpFile = createTempFileInSameDirectoryAs(inputFile); - in = new FileInputStream(inputFile); - out = new FileOutputStream(tmpFile); } else { - in = new FileInputStream(FileDescriptor.in); - out = new FileOutputStream(FileDescriptor.out); vflag = false; + tmpFile = createTemporaryFile("tmpjar", ".jar"); } - InputStream manifest = (!Mflag && (mname != null)) ? - (new FileInputStream(mname)) : null; Map moduleInfoPaths = new HashMap<>(); for (int version : filesMap.keySet()) { @@ -421,8 +398,19 @@ class Main { for (Map.Entry e : moduleInfoPaths.entrySet()) moduleInfos.put(e.getKey(), readModuleInfo(e.getValue())); - boolean updateOk = update(in, new BufferedOutputStream(out), - manifest, moduleInfos, null); + try ( + FileInputStream in = (fname != null) ? new FileInputStream(inputFile) + : new FileInputStream(FileDescriptor.in); + FileOutputStream out = new FileOutputStream(tmpFile); + InputStream manifest = (!Mflag && (mname != null)) ? + (new FileInputStream(mname)) : null; + ) { + boolean updateOk = update(in, new BufferedOutputStream(out), + manifest, moduleInfos, null); + if (ok) { + ok = updateOk; + } + } // Consistency checks for modular jars. if (!moduleInfos.isEmpty()) { @@ -430,23 +418,8 @@ class Main { return false; } - if (ok) { - ok = updateOk; - } - in.close(); - out.close(); - if (manifest != null) { - manifest.close(); - } - if (ok && fname != null) { - // on Win32, we need this delete - inputFile.delete(); - if (!tmpFile.renameTo(inputFile)) { - tmpFile.delete(); - throw new IOException(getMsg("error.write.file")); - } - tmpFile.delete(); - } + validateAndClose(tmpFile); + } else if (tflag) { replaceFSC(filesMap); // For the "list table contents" action, access using the @@ -520,6 +493,28 @@ class Main { return ok; } + private void validateAndClose(File tmpfile) throws IOException { + if (ok && isMultiRelease) { + ok = validate(tmpfile.getCanonicalPath()); + if (!ok) { + error(formatMsg("error.validator.jarfile.invalid", fname)); + } + } + + Path path = tmpfile.toPath(); + try { + if (ok) { + if (fname != null) { + Files.move(path, Paths.get(fname), StandardCopyOption.REPLACE_EXISTING); + } else { + Files.copy(path, new FileOutputStream(FileDescriptor.out)); + } + } + } finally { + Files.deleteIfExists(path); + } + } + private String[] filesMapToFiles(Map filesMap) { if (filesMap.isEmpty()) return null; return filesMap.entrySet() @@ -534,6 +529,76 @@ class Main { .map(f -> (new EntryName(f, version)).entryName); } + // sort base entries before versioned entries, and sort entry classes with + // nested classes so that the top level class appears before the associated + // nested class + private Comparator entryComparator = (je1, je2) -> { + String s1 = je1.getName(); + String s2 = je2.getName(); + 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 = s1.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; + } + } + return l1 - l2; + }; + + private boolean validate(String fname) { + boolean valid; + + try (JarFile jf = new JarFile(fname)) { + Validator validator = new Validator(this, jf); + jf.stream() + .filter(e -> !e.isDirectory()) + .filter(e -> !e.getName().equals(MANIFEST_NAME)) + .filter(e -> !e.getName().endsWith(MODULE_INFO)) + .sorted(entryComparator) + .forEachOrdered(validator); + valid = validator.isValid(); + } catch (IOException e) { + error(formatMsg2("error.validator.jarfile.exception", fname, e.getMessage())); + valid = false; + } catch (InvalidJarException e) { + error(formatMsg("error.validator.bad.entry.name", e.getMessage())); + valid = false; + } + return valid; + } + + private static class InvalidJarException extends RuntimeException { + private static final long serialVersionUID = -3642329147299217726L; + InvalidJarException(String msg) { + super(msg); + } + } + /** * Parses command line arguments. */ diff --git a/jdk/src/jdk.jartool/share/classes/sun/tools/jar/Validator.java b/jdk/src/jdk.jartool/share/classes/sun/tools/jar/Validator.java new file mode 100644 index 00000000000..bf4fa8bf702 --- /dev/null +++ b/jdk/src/jdk.jartool/share/classes/sun/tools/jar/Validator.java @@ -0,0 +1,242 @@ +/* + * Copyright (c) 2016, 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 + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Oracle designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +package sun.tools.jar; + +import java.io.IOException; +import java.io.InputStream; +import java.util.HashMap; +import java.util.Map; +import java.util.function.Consumer; +import java.util.jar.JarEntry; +import java.util.jar.JarFile; + +final class Validator implements Consumer { + private final static boolean DEBUG = Boolean.getBoolean("jar.debug"); + private final Map fps = new HashMap<>(); + private final int vdlen = Main.VERSIONS_DIR.length(); + private final Main main; + private final JarFile jf; + private int oldVersion = -1; + private String currentTopLevelName; + private boolean isValid = true; + + Validator(Main main, JarFile jf) { + this.main = main; + this.jf = jf; + } + + boolean isValid() { + return isValid; + } + + /* + * Validator has state and assumes entries provided to accept are ordered + * from base entries first and then through the versioned entries in + * ascending version order. Also, to find isolated nested classes, + * classes must be ordered so that the top level class is before the associated + * nested class(es). + */ + public void accept(JarEntry je) { + String entryName = je.getName(); + + // directories are always accepted + if (entryName.endsWith("/")) { + debug("%s is a directory", entryName); + return; + } + + // figure out the version and basename from the JarEntry + int version; + String basename; + if (entryName.startsWith(Main.VERSIONS_DIR)) { + int n = entryName.indexOf("/", vdlen); + if (n == -1) { + main.error(Main.formatMsg("error.validator.version.notnumber", entryName)); + isValid = false; + return; + } + String v = entryName.substring(vdlen, n); + try { + version = Integer.parseInt(v); + } catch (NumberFormatException x) { + main.error(Main.formatMsg("error.validator.version.notnumber", entryName)); + isValid = false; + return; + } + if (n == entryName.length()) { + main.error(Main.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; + } + + // analyze the entry, keeping key attributes + FingerPrint fp; + try (InputStream is = jf.getInputStream(je)) { + fp = new FingerPrint(basename, is.readAllBytes()); + } catch (IOException x) { + main.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 (fp.topLevelName().equals(currentTopLevelName)) { + fps.put(internalName, fp); + return; + } + main.error(Main.formatMsg("error.validator.isolated.nested.class", entryName)); + isValid = false; + return; + } + // top level class or resource entry + if (fp.isClass()) { + currentTopLevelName = fp.topLevelName(); + if (!checkInternalName(entryName, basename, internalName)) { + 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)) { + isValid = false; + } + return; + } + if (fp.isPublicClass()) { + main.error(Main.formatMsg("error.validator.new.public.class", entryName)); + isValid = false; + return; + } + debug("%s is a non-public class entry", entryName); + 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? + if (fp.isIdentical(matchFp)) { + main.error(Main.formatMsg("error.validator.identical.entry", entryName)); + return; // it's okay, just takes up room + } + debug("sha1 not equal -- different bytes"); + + // ok, not identical, check for compatible class version and api + if (fp.isClass()) { + if (fp.isNestedClass()) { + if (!checkNestedClass(version, entryName, internalName, fp)) { + isValid = false; + } + return; + } + debug("%s is a class entry", entryName); + if (!fp.isCompatibleVersion(matchFp)) { + main.error(Main.formatMsg("error.validator.incompatible.class.version", entryName)); + isValid = false; + return; + } + if (!fp.isSameAPI(matchFp)) { + main.error(Main.formatMsg("error.validator.different.api", entryName)); + isValid = false; + return; + } + if (!checkInternalName(entryName, basename, internalName)) { + isValid = false; + return; + } + debug("fingerprints same -- same api"); + fps.put(internalName, fp); + currentTopLevelName = fp.topLevelName(); + return; + } + debug("%s is a resource", entryName); + + main.error(Main.formatMsg("error.validator.resources.with.same.name", entryName)); + fps.put(internalName, fp); + return; + } + + private boolean checkInternalName(String entryName, String basename, String internalName) { + String className = className(basename); + if (internalName.equals(className)) { + return true; + } + main.error(Main.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"); + main.error(Main.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 void debug(String fmt, Object... args) { + if (DEBUG) System.err.format(fmt, args); + } +} + diff --git a/jdk/src/jdk.jartool/share/classes/sun/tools/jar/resources/jar.properties b/jdk/src/jdk.jartool/share/classes/sun/tools/jar/resources/jar.properties index e00ee76610d..9c570c7ded8 100644 --- a/jdk/src/jdk.jartool/share/classes/sun/tools/jar/resources/jar.properties +++ b/jdk/src/jdk.jartool/share/classes/sun/tools/jar/resources/jar.properties @@ -85,6 +85,30 @@ error.release.value.notnumber=\ release {0} not valid error.release.value.toosmall=\ release {0} not valid, must be >= 9 +error.validator.jarfile.exception=\ + can not validate {0}: {1} +error.validator.jarfile.invalid=\ + invalid multi-release jar file {0} deleted +error.validator.bad.entry.name=\ + entry name malformed, {0} +error.validator.version.notnumber=\ + entry name: {0}, does not have a version number +error.validator.entryname.tooshort=\ + entry name: {0}, too short, not a directory +error.validator.isolated.nested.class=\ + entry: {0}, is an isolated nested class +error.validator.new.public.class=\ + entry: {0}, contains a new public class not found in base entries +error.validator.identical.entry=\ + warning - entry: {0} contains a class that is identical to an entry already in the jar +error.validator.incompatible.class.version=\ + entry: {0}, has a class version incompatible with an earlier version +error.validator.different.api=\ + entry: {0}, contains a class with different api from earlier version +error.validator.resources.with.same.name=\ + warning - entry: {0}, multiple resources with same name +error.validator.names.mismatch=\ + entry: {0}, contains a class with internal name {1}, names do not match out.added.manifest=\ added manifest out.added.module-info=\ diff --git a/jdk/test/tools/jar/multiRelease/Basic.java b/jdk/test/tools/jar/multiRelease/Basic.java index cd5c195a169..589615ad1e9 100644 --- a/jdk/test/tools/jar/multiRelease/Basic.java +++ b/jdk/test/tools/jar/multiRelease/Basic.java @@ -43,6 +43,7 @@ import java.util.stream.Stream; import java.util.zip.*; import jdk.test.lib.JDKToolFinder; +import jdk.test.lib.Utils; import static java.lang.String.format; import static java.lang.System.out; @@ -198,6 +199,262 @@ public class Basic { deleteDir(Paths.get(usr, "classes")); } + /* + * The following tests exercise the jar validator + */ + + @Test + // META-INF/versions/9 class has different api than base class + public void test04() throws IOException { + String jarfile = "test.jar"; + + compile("test01"); //use same data as test01 + + Path classes = Paths.get("classes"); + + // replace the v9 class + Path source = Paths.get(src, "data", "test04", "v9", "version"); + javac(classes.resolve("v9"), source.resolve("Version.java")); + + jar("cf", jarfile, "-C", classes.resolve("base").toString(), ".", + "--release", "9", "-C", classes.resolve("v9").toString(), ".") + .assertFailure() + .resultChecker(r -> + assertTrue(r.output.contains("different api from earlier"), r.output) + ); + + delete(jarfile); + deleteDir(Paths.get(usr, "classes")); + } + + @Test + // META-INF/versions/9 contains an extra public class + public void test05() throws IOException { + String jarfile = "test.jar"; + + compile("test01"); //use same data as test01 + + Path classes = Paths.get("classes"); + + // add the new v9 class + Path source = Paths.get(src, "data", "test05", "v9", "version"); + javac(classes.resolve("v9"), source.resolve("Extra.java")); + + jar("cf", jarfile, "-C", classes.resolve("base").toString(), ".", + "--release", "9", "-C", classes.resolve("v9").toString(), ".") + .assertFailure() + .resultChecker(r -> + assertTrue(r.output.contains("contains a new public class"), r.output) + ); + + delete(jarfile); + deleteDir(Paths.get(usr, "classes")); + } + + @Test + // META-INF/versions/9 contains an extra package private class -- this is okay + public void test06() throws IOException { + String jarfile = "test.jar"; + + compile("test01"); //use same data as test01 + + Path classes = Paths.get("classes"); + + // add the new v9 class + Path source = Paths.get(src, "data", "test06", "v9", "version"); + javac(classes.resolve("v9"), source.resolve("Extra.java")); + + jar("cf", jarfile, "-C", classes.resolve("base").toString(), ".", + "--release", "9", "-C", classes.resolve("v9").toString(), ".") + .assertSuccess(); + + delete(jarfile); + deleteDir(Paths.get(usr, "classes")); + } + + @Test + // META-INF/versions/9 contains an identical class to base entry class + // this is okay but produces warning + public void test07() throws IOException { + String jarfile = "test.jar"; + + compile("test01"); //use same data as test01 + + Path classes = Paths.get("classes"); + + // add the new v9 class + Path source = Paths.get(src, "data", "test01", "base", "version"); + javac(classes.resolve("v9"), source.resolve("Version.java")); + + jar("cf", jarfile, "-C", classes.resolve("base").toString(), ".", + "--release", "9", "-C", classes.resolve("v9").toString(), ".") + .assertSuccess() + .resultChecker(r -> + assertTrue(r.output.contains("contains a class that is identical"), r.output) + ); + + delete(jarfile); + deleteDir(Paths.get(usr, "classes")); + } + + @Test + // resources with same name in different versions + // this is okay but produces warning + public void test08() throws IOException { + 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(), ".") + .assertSuccess() + .resultChecker(r -> + assertTrue(r.output.isEmpty(), r.output) + ); + + // 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(), ".") + .assertSuccess() + .resultChecker(r -> + assertTrue(r.output.contains("multiple resources with same name"), r.output) + ); + + delete(jarfile); + deleteDir(Paths.get(usr, "classes")); + } + + @Test + // a class with an internal name different from the external name + public void test09() throws IOException { + String jarfile = "test.jar"; + + compile("test01"); //use same data as test01 + + Path classes = Paths.get("classes"); + + Path base = classes.resolve("base").resolve("version"); + + Files.copy(base.resolve("Main.class"), base.resolve("Foo.class")); + + jar("cf", jarfile, "-C", classes.resolve("base").toString(), ".", + "--release", "9", "-C", classes.resolve("v9").toString(), ".") + .assertFailure() + .resultChecker(r -> + assertTrue(r.output.contains("names do not match"), r.output) + ); + + delete(jarfile); + deleteDir(Paths.get(usr, "classes")); + } + + @Test + // assure that basic nested classes are acceptable + public void test10() throws IOException { + String jarfile = "test.jar"; + + compile("test01"); //use same data as test01 + + Path classes = Paths.get("classes"); + + // add a base class with a nested class + Path source = Paths.get(src, "data", "test10", "base", "version"); + javac(classes.resolve("base"), source.resolve("Nested.java")); + + // add a versioned class with a nested class + source = Paths.get(src, "data", "test10", "v9", "version"); + javac(classes.resolve("v9"), source.resolve("Nested.java")); + + jar("cf", jarfile, "-C", classes.resolve("base").toString(), ".", + "--release", "9", "-C", classes.resolve("v9").toString(), ".") + .assertSuccess(); + + delete(jarfile); + deleteDir(Paths.get(usr, "classes")); + } + + @Test + // a base entry contains a nested class that doesn't have a matching top level class + public void test11() throws IOException { + String jarfile = "test.jar"; + + compile("test01"); //use same data as test01 + + Path classes = Paths.get("classes"); + + // add a base class with a nested class + Path source = Paths.get(src, "data", "test10", "base", "version"); + javac(classes.resolve("base"), source.resolve("Nested.java")); + + // remove the top level class, thus isolating the nested class + Files.delete(classes.resolve("base").resolve("version").resolve("Nested.class")); + + // add a versioned class with a nested class + source = Paths.get(src, "data", "test10", "v9", "version"); + javac(classes.resolve("v9"), source.resolve("Nested.java")); + + jar("cf", jarfile, "-C", classes.resolve("base").toString(), ".", + "--release", "9", "-C", classes.resolve("v9").toString(), ".") + .assertFailure() + .resultChecker(r -> { + String[] msg = r.output.split("\\R"); + // There should be 3 error messages, cascading from the first. Once we + // remove the base top level class, the base nested class becomes isolated, + // also the versioned top level class becomes a new public class, thus ignored + // for subsequent checks, leading to the associated versioned nested class + // becoming an isolated nested class + assertTrue(msg.length == 4); + assertTrue(msg[0].contains("an isolated nested class"), msg[0]); + assertTrue(msg[1].contains("contains a new public class"), msg[1]); + assertTrue(msg[2].contains("an isolated nested class"), msg[2]); + assertTrue(msg[3].contains("invalid multi-release jar file"), msg[3]); + }); + + delete(jarfile); + deleteDir(Paths.get(usr, "classes")); + } + + @Test + // a versioned entry contains a nested class that doesn't have a matching top level class + public void test12() throws IOException { + String jarfile = "test.jar"; + + compile("test01"); //use same data as test01 + + Path classes = Paths.get("classes"); + + // add a base class with a nested class + Path source = Paths.get(src, "data", "test10", "base", "version"); + javac(classes.resolve("base"), source.resolve("Nested.java")); + + // add a versioned class with a nested class + source = Paths.get(src, "data", "test10", "v9", "version"); + javac(classes.resolve("v9"), source.resolve("Nested.java")); + + // remove the top level class, thus isolating the nested class + Files.delete(classes.resolve("v9").resolve("version").resolve("Nested.class")); + + jar("cf", jarfile, "-C", classes.resolve("base").toString(), ".", + "--release", "9", "-C", classes.resolve("v9").toString(), ".") + .assertFailure() + .resultChecker(r -> + assertTrue(r.output.contains("an isolated nested class"), r.output) + ); + + delete(jarfile); + deleteDir(Paths.get(usr, "classes")); + } + /* * Test Infrastructure */ @@ -243,7 +500,7 @@ public class Basic { } private void delete(String name) throws IOException { - Files.delete(Paths.get(usr, name)); + Files.deleteIfExists(Paths.get(usr, name)); } private void deleteDir(Path dir) throws IOException { @@ -271,6 +528,10 @@ public class Basic { List commands = new ArrayList<>(); commands.add(javac); + String opts = System.getProperty("test.compiler.opts"); + if (!opts.isEmpty()) { + commands.addAll(Arrays.asList(opts.split(" +"))); + } commands.add("-d"); commands.add(dest.toString()); Stream.of(sourceFiles).map(Object::toString).forEach(x -> commands.add(x)); @@ -282,6 +543,7 @@ public class Basic { String jar = JDKToolFinder.getJDKTool("jar"); List commands = new ArrayList<>(); commands.add(jar); + commands.addAll(Utils.getForwardVmOptions()); Stream.of(args).forEach(x -> commands.add(x)); ProcessBuilder p = new ProcessBuilder(commands); if (stdinSource != null) diff --git a/jdk/test/tools/jar/multiRelease/data/test04/v9/version/Version.java b/jdk/test/tools/jar/multiRelease/data/test04/v9/version/Version.java new file mode 100644 index 00000000000..c4731163178 --- /dev/null +++ b/jdk/test/tools/jar/multiRelease/data/test04/v9/version/Version.java @@ -0,0 +1,14 @@ +package version; + +public class Version { + public int getVersion() { + return 9; + } + + protected void doNothing() { + } + + // extra publc method + public void anyName() { + } +} diff --git a/jdk/test/tools/jar/multiRelease/data/test05/v9/version/Extra.java b/jdk/test/tools/jar/multiRelease/data/test05/v9/version/Extra.java new file mode 100644 index 00000000000..83ef985c410 --- /dev/null +++ b/jdk/test/tools/jar/multiRelease/data/test05/v9/version/Extra.java @@ -0,0 +1,13 @@ +package version; + +public class Extra { + public int getVersion() { + return 9; + } + + protected void doNothing() { + } + + private void anyName() { + } +} diff --git a/jdk/test/tools/jar/multiRelease/data/test06/v9/version/Extra.java b/jdk/test/tools/jar/multiRelease/data/test06/v9/version/Extra.java new file mode 100644 index 00000000000..f2d7862aad3 --- /dev/null +++ b/jdk/test/tools/jar/multiRelease/data/test06/v9/version/Extra.java @@ -0,0 +1,13 @@ +package version; + +class Extra { + public int getVersion() { + return 9; + } + + protected void doNothing() { + } + + private void anyName() { + } +} diff --git a/jdk/test/tools/jar/multiRelease/data/test10/base/version/Nested.java b/jdk/test/tools/jar/multiRelease/data/test10/base/version/Nested.java new file mode 100644 index 00000000000..88f3d93d19c --- /dev/null +++ b/jdk/test/tools/jar/multiRelease/data/test10/base/version/Nested.java @@ -0,0 +1,17 @@ +package version; + +public class Nested { + public int getVersion() { + return 9; + } + + protected void doNothing() { + } + + private void anyName() { + } + + class nested { + int save; + } +} diff --git a/jdk/test/tools/jar/multiRelease/data/test10/v9/version/Nested.java b/jdk/test/tools/jar/multiRelease/data/test10/v9/version/Nested.java new file mode 100644 index 00000000000..7e3d0c6b16e --- /dev/null +++ b/jdk/test/tools/jar/multiRelease/data/test10/v9/version/Nested.java @@ -0,0 +1,14 @@ +package version; + +public class Nested { + public int getVersion() { + return 9; + } + + protected void doNothing() { + } + + class nested { + int save = getVersion(); + } +}