You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by us...@apache.org on 2012/07/10 20:34:16 UTC
svn commit: r1359828 - in /lucene/dev/branches/branch_4x: ./ lucene/
lucene/CHANGES.txt lucene/tools/
lucene/tools/src/java/org/apache/lucene/validation/ForbiddenApisCheckTask.java
Author: uschindler
Date: Tue Jul 10 18:34:15 2012
New Revision: 1359828
URL: http://svn.apache.org/viewvc?rev=1359828&view=rev
Log:
Merged revision(s) 1359827 from lucene/dev/trunk:
LUCENE-4206: Allow check-forbidden-apis to also investigate calls to subclasses of forbidden APIs
Modified:
lucene/dev/branches/branch_4x/ (props changed)
lucene/dev/branches/branch_4x/lucene/ (props changed)
lucene/dev/branches/branch_4x/lucene/CHANGES.txt (contents, props changed)
lucene/dev/branches/branch_4x/lucene/tools/ (props changed)
lucene/dev/branches/branch_4x/lucene/tools/src/java/org/apache/lucene/validation/ForbiddenApisCheckTask.java
Modified: lucene/dev/branches/branch_4x/lucene/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/CHANGES.txt?rev=1359828&r1=1359827&r2=1359828&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/CHANGES.txt (original)
+++ lucene/dev/branches/branch_4x/lucene/CHANGES.txt Tue Jul 10 18:34:15 2012
@@ -64,11 +64,11 @@ Build
* LUCENE-4115: JAR resolution/ cleanup should be done automatically for ant
clean/ eclipse/ resolve (Dawid Weiss)
-* LUCENE-4199, LUCENE-4202: Add a new target "check-forbidden-apis", that
- parses all generated .class files for use of APIs that use default charset,
- default locale, or default timezone and fail build if violations found. This
- ensures, that Lucene / Solr is independent on local configuration options.
- (Uwe Schindler, Robert Muir, Dawid Weiss)
+* LUCENE-4199, LUCENE-4202, LUCENE-4206: Add a new target "check-forbidden-apis"
+ that parses all generated .class files for use of APIs that use default
+ charset, default locale, or default timezone and fail build if violations
+ found. This ensures, that Lucene / Solr is independent on local configuration
+ options. (Uwe Schindler, Robert Muir, Dawid Weiss)
Documentation
Modified: lucene/dev/branches/branch_4x/lucene/tools/src/java/org/apache/lucene/validation/ForbiddenApisCheckTask.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/tools/src/java/org/apache/lucene/validation/ForbiddenApisCheckTask.java?rev=1359828&r1=1359827&r2=1359828&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/tools/src/java/org/apache/lucene/validation/ForbiddenApisCheckTask.java (original)
+++ lucene/dev/branches/branch_4x/lucene/tools/src/java/org/apache/lucene/validation/ForbiddenApisCheckTask.java Tue Jul 10 18:34:15 2012
@@ -20,13 +20,11 @@ package org.apache.lucene.validation;
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.Reader;
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 @@ public class ForbiddenApisCheckTask exte
private final Resources classFiles = new Resources();
private final Resources apiSignatures = new Resources();
private Path classpath = null;
-
- private final Map<String,ClassNode> classCache = new HashMap<String,ClassNode>();
- private final Map<String,String> forbiddenFields = new HashMap<String,String>();
- private final Map<String,String> forbiddenMethods = new HashMap<String,String>();
- private final Map<String,String> forbiddenClasses = new HashMap<String,String>();
- /** 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<String,ClassSignatureLookup> classesToCheck = new HashMap<String,ClassSignatureLookup>();
+ final Map<String,ClassSignatureLookup> classpathClassCache = new HashMap<String,ClassSignatureLookup>();
+
+ final Map<String,String> forbiddenFields = new HashMap<String,String>();
+ final Map<String,String> forbiddenMethods = new HashMap<String,String>();
+ final Map<String,String> forbiddenClasses = new HashMap<String,String>();
+
+ /** 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 @@ public class ForbiddenApisCheckTask exte
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 @@ public class ForbiddenApisCheckTask exte
}
} 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 @@ public class ForbiddenApisCheckTask exte
line = line.trim();
if (line.length() == 0 || line.startsWith("#"))
continue;
- addSignature(loader, line);
+ addSignature(line);
}
} finally {
r.close();
}
}
- /** 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();
+ /** Parses a class given as (FileSet) Resource */
+ private ClassReader loadClassFromResource(final Resource res) throws BuildException {
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);
- return true;
- }
- return false;
+ 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 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;
}
-
- 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);
+ 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;
}
-
- @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 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)) {
+ final String superName = c.reader.getSuperName();
+ if (superName != null && checkMethodAccess(superName, method)) {
+ return true;
}
- if (found) {
- violations[0]++;
- reportSourceAndLine();
+ final String[] interfaces = c.reader.getInterfaces();
+ if (interfaces != null) {
+ for (String intf : interfaces) {
+ if (intf != null && checkMethodAccess(intf, method)) {
+ return true;
+ }
+ }
}
}
-
- @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);
- }
+ 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)) {
+ final String superName = c.reader.getSuperName();
+ if (superName != null && checkFieldAccess(superName, field)) {
+ return true;
}
- if (found) {
- violations[0]++;
- reportSourceAndLine();
+ final String[] interfaces = c.reader.getInterfaces();
+ if (interfaces != null) {
+ for (String intf : interfaces) {
+ if (intf != null && checkFieldAccess(intf, field)) {
+ return true;
+ }
+ }
}
}
+ 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<Resource> iter = (Iterator<Resource>) apiSignatures.iterator();
@@ -295,10 +344,10 @@ public class ForbiddenApisCheckTask exte
}
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 @@ public class ForbiddenApisCheckTask exte
if (forbiddenMethods.isEmpty() && forbiddenClasses.isEmpty()) {
throw new BuildException("No API signatures found; use apiFile=, <apiFileSet/>, 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<Resource> iter = (Iterator<Resource>) classFiles.iterator();
- if (!iter.hasNext()) {
- throw new BuildException("There is no <fileset/> 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<Resource> iter = (Iterator<Resource>) classFiles.iterator();
+ if (!iter.hasNext()) {
+ throw new BuildException("There is no <fileset/> 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);
+ }
+
+ ClassReader reader = loadClassFromResource(r);
+ classesToCheck.put(reader.getClassName(), new ClassSignatureLookup(reader));
}
- try {
- errors += checkClass(r);
- } catch (IOException ioe) {
- throw new BuildException("IO problem while reading class file " + r, ioe);
+ log("Scanning for API signatures and dependencies...", Project.MSG_INFO);
+
+ int errors = 0;
+ for (final ClassSignatureLookup c : classesToCheck.values()) {
+ errors += checkClass(c.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(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.");
+ 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 @@ public class ForbiddenApisCheckTask exte
return this.classpath.createPath();
}
+ static final class ClassSignatureLookup {
+ public final ClassReader reader;
+ public final Set<Method> methods;
+ public final Set<String> fields;
+
+ public ClassSignatureLookup(final ClassReader reader) {
+ this.reader = reader;
+ final Set<Method> methods = new HashSet<Method>();
+ final Set<String> fields = new HashSet<String>();
+ 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);
+ }
+ }
+
}