Index: lucene/tools/src/java/org/apache/lucene/validation/ForbiddenApisCheckTask.java =================================================================== --- lucene/tools/src/java/org/apache/lucene/validation/ForbiddenApisCheckTask.java (revision 1359630) +++ lucene/tools/src/java/org/apache/lucene/validation/ForbiddenApisCheckTask.java (working copy) @@ -20,13 +20,11 @@ import org.objectweb.asm.ClassReader; import org.objectweb.asm.Label; import org.objectweb.asm.ClassVisitor; +import org.objectweb.asm.FieldVisitor; import org.objectweb.asm.MethodVisitor; import org.objectweb.asm.Opcodes; import org.objectweb.asm.Type; import org.objectweb.asm.commons.Method; -import org.objectweb.asm.tree.ClassNode; -import org.objectweb.asm.tree.FieldNode; -import org.objectweb.asm.tree.MethodNode; import org.apache.tools.ant.AntClassLoader; import org.apache.tools.ant.BuildException; @@ -50,12 +48,14 @@ import java.io.File; import java.io.StringReader; import java.util.Arrays; +import java.util.Collections; import java.util.Formatter; import java.util.HashMap; import java.util.Iterator; -import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.HashSet; +import java.util.Set; /** * Task to check if a set of class files contains calls to forbidden APIs @@ -69,32 +69,39 @@ private final Resources classFiles = new Resources(); private final Resources apiSignatures = new Resources(); private Path classpath = null; - - private final Map classCache = new HashMap(); - private final Map forbiddenFields = new HashMap(); - private final Map forbiddenMethods = new HashMap(); - private final Map forbiddenClasses = new HashMap(); - /** Reads a class (binary name) from the given {@link ClassLoader}. - */ - private ClassReader readClass(final ClassLoader loader, final String clazz) throws BuildException { - try { - final InputStream in = loader.getResourceAsStream(clazz.replace('.', '/') + ".class"); - if (in == null) { - throw new BuildException("Loading of class " + clazz + " failed: Not found"); - } + ClassLoader loader = null; + + final Map classesToCheck = new HashMap(); + final Map classpathClassCache = new HashMap(); + + final Map forbiddenFields = new HashMap(); + final Map forbiddenMethods = new HashMap(); + final Map forbiddenClasses = new HashMap(); + + /** Reads a class (binary name) from the given {@link ClassLoader}. */ + ClassSignatureLookup getClassFromClassLoader(final String clazz) throws BuildException { + ClassSignatureLookup c = classpathClassCache.get(clazz); + if (c == null) { try { - return new ClassReader(in); - } finally { - in.close(); + final InputStream in = loader.getResourceAsStream(clazz.replace('.', '/') + ".class"); + if (in == null) { + throw new BuildException("Loading of class " + clazz + " failed: Not found"); + } + try { + classpathClassCache.put(clazz, c = new ClassSignatureLookup(new ClassReader(in))); + } finally { + in.close(); + } + } catch (IOException ioe) { + throw new BuildException("Loading of class " + clazz + " failed.", ioe); } - } catch (IOException ioe) { - throw new BuildException("Loading of class " + clazz + " failed.", ioe); } + return c; } /** Adds the method signature to the list of disallowed methods. The Signature is checked against the given ClassLoader. */ - private void addSignature(final ClassLoader loader, final String signature) throws BuildException { + private void addSignature(final String signature) throws BuildException { final String clazz, field; final Method method; int p = signature.indexOf('#'); @@ -123,19 +130,15 @@ field = null; } // check class & method/field signature, if it is really existent (in classpath), but we don't really load the class into JVM: - ClassNode c = classCache.get(clazz); - if (c == null) { - readClass(loader, clazz).accept(c = new ClassNode(Opcodes.ASM4), ClassReader.SKIP_CODE | ClassReader.SKIP_DEBUG | ClassReader.SKIP_FRAMES); - classCache.put(clazz, c); - } + final ClassSignatureLookup c = getClassFromClassLoader(clazz); if (method != null) { assert field == null; // list all methods with this signature: boolean found = false; - for (final MethodNode mn : c.methods) { - if (mn.name.equals(method.getName()) && Arrays.equals(Type.getArgumentTypes(mn.desc), method.getArgumentTypes())) { + for (final Method m : c.methods) { + if (m.getName().equals(method.getName()) && Arrays.equals(m.getArgumentTypes(), method.getArgumentTypes())) { found = true; - forbiddenMethods.put(c.name + '\000' + new Method(mn.name, mn.desc), signature); + forbiddenMethods.put(c.reader.getClassName() + '\000' + m, signature); // don't break when found, as there may be more covariant overrides! } } @@ -144,27 +147,19 @@ } } else if (field != null) { assert method == null; - // list all fields to find the right one: - boolean found = false; - for (final FieldNode fld : c.fields) { - if (fld.name.equals(field)) { - found = true; - forbiddenFields.put(c.name + '\000' + fld.name, signature); - break; - } - } - if (!found) { + if (!c.fields.contains(field)) { throw new BuildException("No field found with following name: " + signature); } + forbiddenFields.put(c.reader.getClassName() + '\000' + field, signature); } else { assert field == null && method == null; // only add the signature as class name - forbiddenClasses.put(c.name, signature); + forbiddenClasses.put(c.reader.getClassName(), signature); } } /** Reads a list of API signatures. Closes the Reader when done (on Exception, too)! */ - private void parseApiFile(ClassLoader loader, Reader reader) throws IOException { + private void parseApiFile(Reader reader) throws IOException { final BufferedReader r = new BufferedReader(reader); try { String line; @@ -172,116 +167,170 @@ line = line.trim(); if (line.length() == 0 || line.startsWith("#")) continue; - addSignature(loader, line); + addSignature(line); } } finally { r.close(); } } + /** Parses a class given as (FileSet) Resource */ + private ClassReader loadClassFromResource(final Resource res) throws BuildException { + try { + final InputStream stream = res.getInputStream(); + try { + return new ClassReader(stream); + } finally { + stream.close(); + } + } catch (IOException ioe) { + throw new BuildException("IO problem while reading class file " + res, ioe); + } + } + /** Parses a class given as Resource and checks for valid method invocations */ - private int checkClass(final Resource res) throws IOException { - final InputStream stream = res.getInputStream(); - try { - final int[] violations = new int[1]; - new ClassReader(stream).accept(new ClassVisitor(Opcodes.ASM4) { - String className = null, source = null; - - @Override - public void visit(int version, int access, String name, String signature, String superName, String[] interfaces) { - // save class name in source code format: - this.className = Type.getObjectType(name).getClassName(); - } - - @Override - public void visitSource(String source, String debug) { - this.source = source; - } - - @Override - public MethodVisitor visitMethod(int access, String name, String desc, String signature, String[] exceptions) { - return new MethodVisitor(Opcodes.ASM4) { - private int lineNo = -1; - - private boolean checkClassUse(String owner) { - final String printout = forbiddenClasses.get(owner); - if (printout != null) { - log("Forbidden class use: " + printout, Project.MSG_ERR); + private int checkClass(final ClassReader reader) { + final int[] violations = new int[1]; + reader.accept(new ClassVisitor(Opcodes.ASM4) { + final String className = Type.getObjectType(reader.getClassName()).getClassName(); + String source = null; + + @Override + public void visitSource(String source, String debug) { + this.source = source; + } + + @Override + public MethodVisitor visitMethod(int access, String name, String desc, String signature, String[] exceptions) { + return new MethodVisitor(Opcodes.ASM4) { + private int lineNo = -1; + + private ClassSignatureLookup lookupRelatedClass(String internalName) { + ClassSignatureLookup c = classesToCheck.get(internalName); + if (c == null) try { + c = getClassFromClassLoader(internalName); + } catch (BuildException be) { + // we ignore lookup errors and simply ignore this related class + c = null; + } + return c; + } + + private boolean checkClassUse(String owner) { + final String printout = forbiddenClasses.get(owner); + if (printout != null) { + log("Forbidden class use: " + printout, Project.MSG_ERR); + return true; + } + return false; + } + + private boolean checkMethodAccess(String owner, Method method) { + if (checkClassUse(owner)) { + return true; + } + final String printout = forbiddenMethods.get(owner + '\000' + method); + if (printout != null) { + log("Forbidden method invocation: " + printout, Project.MSG_ERR); + return true; + } + final ClassSignatureLookup c = lookupRelatedClass(owner); + if (c != null && !c.methods.contains(method)) { + String superName = c.reader.getSuperName(); + if (superName != null && checkMethodAccess(superName, method)) { return true; } - return false; - } - - private void reportSourceAndLine() { - final StringBuilder sb = new StringBuilder(" in ").append(className); - if (source != null && lineNo >= 0) { - new Formatter(sb, Locale.ROOT).format(" (%s:%d)", source, lineNo).flush(); + final String[] interfaces = c.reader.getInterfaces(); + if (interfaces != null) { + for (String intf : interfaces) { + if (intf != null && checkMethodAccess(intf, method)) { + return true; + } + } } - log(sb.toString(), Project.MSG_ERR); } - - @Override - public void visitMethodInsn(int opcode, String owner, String name, String desc) { - boolean found = checkClassUse(owner); - if (!found) { - final String printout = forbiddenMethods.get(owner + '\000' + new Method(name, desc)); - if (printout != null) { - found = true; - log("Forbidden method invocation: " + printout, Project.MSG_ERR); - } + return false; + } + + private boolean checkFieldAccess(String owner, String field) { + if (checkClassUse(owner)) { + return true; + } + final String printout = forbiddenFields.get(owner + '\000' + field); + if (printout != null) { + log("Forbidden field access: " + printout, Project.MSG_ERR); + return true; + } + final ClassSignatureLookup c = lookupRelatedClass(owner); + if (c != null && !c.fields.contains(field)) { + String superName = c.reader.getSuperName(); + if (superName != null && checkFieldAccess(superName, field)) { + return true; } - if (found) { - violations[0]++; - reportSourceAndLine(); - } - } - - @Override - public void visitFieldInsn(int opcode, String owner, String name, String desc) { - boolean found = checkClassUse(owner); - if (!found) { - final String printout = forbiddenFields.get(owner + '\000' + name); - if (printout != null) { - found = true; - log("Forbidden field access: " + printout, Project.MSG_ERR); + final String[] interfaces = c.reader.getInterfaces(); + if (interfaces != null) { + for (String intf : interfaces) { + if (intf != null && checkFieldAccess(intf, field)) { + return true; + } } } - if (found) { - violations[0]++; - reportSourceAndLine(); - } } + return false; + } - @Override - public void visitLineNumber(int lineNo, Label start) { - this.lineNo = lineNo; + @Override + public void visitMethodInsn(int opcode, String owner, String name, String desc) { + if (checkMethodAccess(owner, new Method(name, desc))) { + violations[0]++; + reportSourceAndLine(); } - }; - } - }, ClassReader.SKIP_FRAMES); - return violations[0]; - } finally { - stream.close(); - } + } + + @Override + public void visitFieldInsn(int opcode, String owner, String name, String desc) { + if (checkFieldAccess(owner, name)) { + violations[0]++; + reportSourceAndLine(); + } + } + + private void reportSourceAndLine() { + final StringBuilder sb = new StringBuilder(" in ").append(className); + if (source != null && lineNo >= 0) { + new Formatter(sb, Locale.ROOT).format(" (%s:%d)", source, lineNo).flush(); + } + log(sb.toString(), Project.MSG_ERR); + } + + @Override + public void visitLineNumber(int lineNo, Label start) { + this.lineNo = lineNo; + } + }; + } + }, ClassReader.SKIP_FRAMES); + return violations[0]; } @Override public void execute() throws BuildException { AntClassLoader antLoader = null; try { - final ClassLoader loader; if (classpath != null) { classpath.setProject(getProject()); - loader = antLoader = getProject().createClassLoader(ClassLoader.getSystemClassLoader(), classpath); + this.loader = antLoader = getProject().createClassLoader(ClassLoader.getSystemClassLoader(), classpath); // force that loading from this class loader is done first, then parent is asked. // This violates spec, but prevents classes in any system classpath to be used if a local one is available: antLoader.setParentFirst(false); } else { - loader = ClassLoader.getSystemClassLoader(); + this.loader = ClassLoader.getSystemClassLoader(); } classFiles.setProject(getProject()); apiSignatures.setProject(getProject()); + final long start = System.currentTimeMillis(); + try { @SuppressWarnings("unchecked") Iterator iter = (Iterator) apiSignatures.iterator(); @@ -295,10 +344,10 @@ } if (r instanceof StringResource) { log("Reading inline API signatures...", Project.MSG_INFO); - parseApiFile(loader, new StringReader(((StringResource) r).getValue())); + parseApiFile(new StringReader(((StringResource) r).getValue())); } else { log("Reading API signatures: " + r, Project.MSG_INFO); - parseApiFile(loader, new InputStreamReader(r.getInputStream(), "UTF-8")); + parseApiFile(new InputStreamReader(r.getInputStream(), "UTF-8")); } } } catch (IOException ioe) { @@ -307,41 +356,48 @@ if (forbiddenMethods.isEmpty() && forbiddenClasses.isEmpty()) { throw new BuildException("No API signatures found; use apiFile=, , or inner text to define those!"); } - } finally { - if (antLoader != null) antLoader.cleanup(); - antLoader = null; - } - long start = System.currentTimeMillis(); - - int checked = 0; - int errors = 0; - @SuppressWarnings("unchecked") - Iterator iter = (Iterator) classFiles.iterator(); - if (!iter.hasNext()) { - throw new BuildException("There is no given or the fileset does not contain any class files to check."); - } - while (iter.hasNext()) { - final Resource r = iter.next(); - if (!r.isExists()) { - throw new BuildException("Class file does not exist: " + r); + log("Loading classes to check...", Project.MSG_INFO); + + @SuppressWarnings("unchecked") + Iterator iter = (Iterator) classFiles.iterator(); + if (!iter.hasNext()) { + throw new BuildException("There is no given or the fileset does not contain any class files to check."); } + while (iter.hasNext()) { + final Resource r = iter.next(); + if (!r.isExists()) { + throw new BuildException("Class file does not exist: " + r); + } - try { - errors += checkClass(r); - } catch (IOException ioe) { - throw new BuildException("IO problem while reading class file " + r, ioe); + ClassReader reader = loadClassFromResource(r); + classesToCheck.put(reader.getClassName(), new ClassSignatureLookup(reader)); } - checked++; - } - log(String.format(Locale.ROOT, - "Scanned %d class file(s) for forbidden API invocations (in %.2fs), %d error(s).", - checked, (System.currentTimeMillis() - start) / 1000.0, errors), - errors > 0 ? Project.MSG_ERR : Project.MSG_INFO); + log("Scanning for API signatures and dependencies...", Project.MSG_INFO); - if (errors > 0) { - throw new BuildException("Check for forbidden API calls failed, see log."); + int errors = 0; + for (final ClassSignatureLookup c : classesToCheck.values()) { + errors += checkClass(c.reader); + } + + log(String.format(Locale.ROOT, + "Scanned %d (and %d related) class file(s) for forbidden API invocations (in %.2fs), %d error(s).", + classesToCheck.size(), classpathClassCache.size(), (System.currentTimeMillis() - start) / 1000.0, errors), + errors > 0 ? Project.MSG_ERR : Project.MSG_INFO); + + if (errors > 0) { + throw new BuildException("Check for forbidden API calls failed, see log."); + } + } finally { + this.loader = null; + if (antLoader != null) antLoader.cleanup(); + antLoader = null; + classesToCheck.clear(); + classpathClassCache.clear(); + forbiddenFields.clear(); + forbiddenMethods.clear(); + forbiddenClasses.clear(); } } @@ -386,4 +442,32 @@ return this.classpath.createPath(); } + static final class ClassSignatureLookup { + public final ClassReader reader; + public final Set methods; + public final Set fields; + + public ClassSignatureLookup(final ClassReader reader) { + this.reader = reader; + final Set methods = new HashSet(); + final Set fields = new HashSet(); + reader.accept(new ClassVisitor(Opcodes.ASM4) { + @Override + public MethodVisitor visitMethod(int access, String name, String desc, String signature, String[] exceptions) { + final Method m = new Method(name, desc); + methods.add(m); + return null; + } + + @Override + public FieldVisitor visitField(int access, String name, String desc, String signature, Object value) { + fields.add(name); + return null; + } + }, ClassReader.SKIP_CODE | ClassReader.SKIP_DEBUG | ClassReader.SKIP_FRAMES); + this.methods = Collections.unmodifiableSet(methods); + this.fields = Collections.unmodifiableSet(fields); + } + } + }