Skip to content

Commit

Permalink
Allow to set severity per signature
Browse files Browse the repository at this point in the history 
This closes policeman-tools#252 and policeman-tools#219
  • Loading branch information
@kwin
kwin committed Jan 4, 2025
1 parent 535ecf7 commit 5773478
Show file tree
Hide file tree
Showing 5 changed files with 177 additions and 64 deletions.
31 changes: 28 additions & 3 deletions src/main/java/de/thetaphi/forbiddenapis/Checker.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ public static enum Option {
DISABLE_CLASSLOADING_CACHE
}

public enum ViolationSeverity {
ERROR, WARNING, INFO, DEBUG, SUPPRESS
}

public final boolean isSupportedJDK;

private final long start;
Expand Down Expand Up @@ -360,6 +364,10 @@ public boolean noSignaturesFilesParsed() {
return forbiddenSignatures.noSignaturesFilesParsed();
}

public void setSignatureSeverity(String signature, ViolationSeverity severity) {
forbiddenSignatures.setSignatureSeverity(signature, severity);
}

/** Parses and adds a class from the given stream to the list of classes to check. Closes the stream when parsed (on Exception, too)! Does not log anything. */
public void addClassToCheck(final InputStream in, String name) throws IOException {
final ClassReader reader;
Expand Down Expand Up @@ -417,7 +425,7 @@ public void addSuppressAnnotation(String annoName) {
/** Parses a class and checks for valid method invocations */
private int checkClass(ClassMetadata c, Pattern suppressAnnotationsPattern) throws ForbiddenApiException {
final String className = c.getBinaryClassName();
final ClassScanner scanner = new ClassScanner(c, this, forbiddenSignatures, suppressAnnotationsPattern);
final ClassScanner scanner = new ClassScanner(c, this, forbiddenSignatures, suppressAnnotationsPattern, options.contains(Option.FAIL_ON_VIOLATION));
try {
c.getReader().accept(scanner, ClassReader.SKIP_FRAMES);
} catch (RelatedClassLoadingException rcle) {
Expand Down Expand Up @@ -452,12 +460,29 @@ private int checkClass(ClassMetadata c, Pattern suppressAnnotationsPattern) thro
}
final List<ForbiddenViolation> violations = scanner.getSortedViolations();
final Pattern splitter = Pattern.compile(Pattern.quote(ForbiddenViolation.SEPARATOR));
int numErrors = 0;
for (final ForbiddenViolation v : violations) {
for (final String line : splitter.split(v.format(className, scanner.getSourceFile()))) {
logger.error(line);
switch (v.severity) {
case DEBUG:
logger.debug(line);
break;
case INFO:
logger.info(line);
break;
case WARNING:
logger.warn(line);
break;
case ERROR:
logger.error(line);
numErrors++;
break;
default:
break;
}
}
}
return violations.size();
return numErrors;
}

public void run() throws ForbiddenApiException {
Expand Down
101 changes: 55 additions & 46 deletions src/main/java/de/thetaphi/forbiddenapis/ClassScanner.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@
import org.objectweb.asm.TypePath;
import org.objectweb.asm.commons.Method;

import de.thetaphi.forbiddenapis.Checker.ViolationSeverity;
import de.thetaphi.forbiddenapis.Signatures.ViolationResult;

public final class ClassScanner extends ClassVisitor implements Constants {
private final boolean forbidNonPortableRuntime;
final ClassMetadata metadata;
Expand All @@ -63,14 +66,16 @@ public final class ClassScanner extends ClassVisitor implements Constants {
// all groups that were disabled due to suppressing annotation
final BitSet suppressedGroups = new BitSet();
boolean classSuppressed = false;
private final boolean failOnViolation;

public ClassScanner(ClassMetadata metadata, RelatedClassLookup lookup, Signatures forbiddenSignatures, final Pattern suppressAnnotations) {
public ClassScanner(ClassMetadata metadata, RelatedClassLookup lookup, Signatures forbiddenSignatures, final Pattern suppressAnnotations, boolean failOnViolation) {
super(Opcodes.ASM9);
this.metadata = metadata;
this.lookup = lookup;
this.forbiddenSignatures = forbiddenSignatures;
this.suppressAnnotations = suppressAnnotations;
this.forbidNonPortableRuntime = forbiddenSignatures.isNonPortableRuntimeForbidden();
this.failOnViolation = failOnViolation;
}

private void checkDone() {
Expand All @@ -87,14 +92,14 @@ public String getSourceFile() {
return source;
}

String checkClassUse(Type type, String what, boolean isAnnotation, String origInternalName) {
ViolationResult checkClassUse(Type type, String what, boolean isAnnotation, String origInternalName) {
while (type.getSort() == Type.ARRAY) {
type = type.getElementType(); // unwrap array
}
if (type.getSort() != Type.OBJECT) {
return null; // we don't know this type, just pass!
}
final String violation = forbiddenSignatures.checkType(type, what);
final ViolationResult violation = forbiddenSignatures.checkType(type, what);
if (violation != null) {
return violation;
}
Expand All @@ -103,10 +108,9 @@ String checkClassUse(Type type, String what, boolean isAnnotation, String origIn
final String binaryClassName = type.getClassName();
final ClassMetadata c = lookup.lookupRelatedClass(type.getInternalName(), origInternalName);
if (c != null && c.isNonPortableRuntime) {
return String.format(Locale.ENGLISH,
return new ViolationResult(String.format(Locale.ENGLISH,
"Forbidden %s use: %s [non-portable or internal runtime class]",
what, binaryClassName
);
what, binaryClassName), failOnViolation ? ViolationSeverity.ERROR : ViolationSeverity.WARNING);
}
} catch (RelatedClassLoadingException e) {
// only throw exception if it is not an annotation
Expand All @@ -115,20 +119,20 @@ String checkClassUse(Type type, String what, boolean isAnnotation, String origIn
return null;
}

String checkClassUse(String internalName, String what, String origInternalName) {
ViolationResult checkClassUse(String internalName, String what, String origInternalName) {
return checkClassUse(Type.getObjectType(internalName), what, false, origInternalName);
}

// TODO: @FunctionalInterface from Java 8 on
static interface AncestorVisitor {
final String STOP = new String("STOP");
final ViolationResult STOP = new ViolationResult("STOP", null);

String visit(ClassMetadata c, String origName, boolean isInterfaceOfAncestor, boolean previousInRuntime);
ViolationResult visit(ClassMetadata c, String origName, boolean isInterfaceOfAncestor, boolean previousInRuntime);
}

String visitAncestors(ClassMetadata cls, AncestorVisitor visitor, boolean visitSelf, boolean visitInterfacesFirst) {
ViolationResult visitAncestors(ClassMetadata cls, AncestorVisitor visitor, boolean visitSelf, boolean visitInterfacesFirst) {
if (visitSelf) {
final String result = visitor.visit(cls, cls.className, cls.isInterface, cls.isRuntimeClass);
final ViolationResult result = visitor.visit(cls, cls.className, cls.isInterface, cls.isRuntimeClass);
if (result == AncestorVisitor.STOP) {
return null;
}
Expand All @@ -139,11 +143,11 @@ String visitAncestors(ClassMetadata cls, AncestorVisitor visitor, boolean visitS
return visitAncestorsRecursive(cls, cls.className, visitor, cls.isRuntimeClass, visitInterfacesFirst);
}

private String visitSuperclassRecursive(ClassMetadata cls, String origName, AncestorVisitor visitor, boolean previousInRuntime, boolean visitInterfacesFirst) {
private ViolationResult visitSuperclassRecursive(ClassMetadata cls, String origName, AncestorVisitor visitor, boolean previousInRuntime, boolean visitInterfacesFirst) {
if (cls.superName != null) {
final ClassMetadata c = lookup.lookupRelatedClass(cls.superName, origName);
if (c != null) {
String result = visitor.visit(c, origName, false, previousInRuntime);
ViolationResult result = visitor.visit(c, origName, false, previousInRuntime);
if (result != AncestorVisitor.STOP) {
if (result != null) {
return result;
Expand All @@ -158,12 +162,12 @@ private String visitSuperclassRecursive(ClassMetadata cls, String origName, Ance
return null;
}

private String visitInterfacesRecursive(ClassMetadata cls, String origName, AncestorVisitor visitor, boolean previousInRuntime, boolean visitInterfacesFirst) {
private ViolationResult visitInterfacesRecursive(ClassMetadata cls, String origName, AncestorVisitor visitor, boolean previousInRuntime, boolean visitInterfacesFirst) {
if (cls.interfaces != null) {
for (String intf : cls.interfaces) {
final ClassMetadata c = lookup.lookupRelatedClass(intf, origName);
if (c == null) continue;
String result = visitor.visit(c, origName, true, previousInRuntime);
ViolationResult result = visitor.visit(c, origName, true, previousInRuntime);
if (result != AncestorVisitor.STOP) {
if (result != null) {
return result;
Expand All @@ -178,8 +182,8 @@ private String visitInterfacesRecursive(ClassMetadata cls, String origName, Ance
return null;
}

private String visitAncestorsRecursive(ClassMetadata cls, String origName, AncestorVisitor visitor, boolean previousInRuntime, boolean visitInterfacesFirst) {
String result;
private ViolationResult visitAncestorsRecursive(ClassMetadata cls, String origName, AncestorVisitor visitor, boolean previousInRuntime, boolean visitInterfacesFirst) {
ViolationResult result;
if (visitInterfacesFirst) {
result = visitInterfacesRecursive(cls, origName, visitor, previousInRuntime, visitInterfacesFirst);
if (result != null) {
Expand All @@ -202,17 +206,17 @@ private String visitAncestorsRecursive(ClassMetadata cls, String origName, Ances
// TODO: convert to lambda method with method reference
private final AncestorVisitor classRelationAncestorVisitor = new AncestorVisitor() {
@Override
public String visit(ClassMetadata c, String origName, boolean isInterfaceOfAncestor, boolean previousInRuntime) {
public ViolationResult visit(ClassMetadata c, String origName, boolean isInterfaceOfAncestor, boolean previousInRuntime) {
if (previousInRuntime && c.isNonPortableRuntime) {
return null; // something inside the JVM is extending internal class/interface
}
return checkClassUse(c.className, isInterfaceOfAncestor ? "interface" : "class", origName);
}
};

String checkType(Type type) {
ViolationResult checkType(Type type) {
while (type != null) {
String violation;
ViolationResult violation;
switch (type.getSort()) {
case Type.OBJECT:
final String internalName = type.getInternalName();
Expand All @@ -226,7 +230,7 @@ String checkType(Type type) {
type = type.getElementType();
break;
case Type.METHOD:
final ArrayList<String> violations = new ArrayList<>();
final ArrayList<ViolationResult> violations = new ArrayList<>();
violation = checkType(type.getReturnType());
if (violation != null) {
violations.add(violation);
Expand All @@ -244,12 +248,17 @@ String checkType(Type type) {
} else {
final StringBuilder sb = new StringBuilder();
boolean nl = false;
for (final String v : violations) {
ViolationSeverity severity = null;
for (final ViolationResult v : violations) {
if (nl) sb.append(ForbiddenViolation.SEPARATOR);
sb.append(v);
nl = true;
// use the highest severity reported on this method
if (severity == null || v.severity.ordinal() > severity.ordinal()) {
severity = v.severity;
}
}
return sb.toString();
return new ViolationResult(sb.toString(), severity);
}
default:
return null;
Expand All @@ -258,11 +267,11 @@ String checkType(Type type) {
return null;
}

String checkDescriptor(String desc) {
ViolationResult checkDescriptor(String desc) {
return checkType(Type.getType(desc));
}

String checkAnnotationDescriptor(Type type, boolean visible) {
ViolationResult checkAnnotationDescriptor(Type type, boolean visible) {
// for annotations, we don't need to look into super-classes, interfaces,...
return checkClassUse(type, "annotation", true, type.getInternalName());
}
Expand All @@ -273,9 +282,9 @@ void maybeSuppressCurrentGroup(Type annotation) {
}
}

private void reportClassViolation(String violation, String where) {
private void reportClassViolation(ViolationResult violation, String where) {
if (violation != null) {
violations.add(new ForbiddenViolation(currentGroupId, violation, where, -1));
violations.add(new ForbiddenViolation(currentGroupId, violation.message, where, -1, violation.severity));
}
}

Expand Down Expand Up @@ -352,9 +361,9 @@ public AnnotationVisitor visitTypeAnnotation(int typeRef, TypePath typePath, Str
return null;
}

private void reportFieldViolation(String violation, String where) {
if (violation != null) {
violations.add(new ForbiddenViolation(currentGroupId, violation, String.format(Locale.ENGLISH, "%s of '%s'", where, name), -1));
private void reportFieldViolation(ViolationResult violationResult, String where) {
if (violationResult != null) {
violations.add(new ForbiddenViolation(currentGroupId, violationResult.message, String.format(Locale.ENGLISH, "%s of '%s'", where, name), -1, violationResult.severity));
}
}
};
Expand Down Expand Up @@ -382,12 +391,12 @@ public MethodVisitor visitMethod(final int access, final String name, final Stri
}
}

private String checkMethodAccess(String owner, final Method method, final boolean callIsVirtual) {
private ViolationResult checkMethodAccess(String owner, final Method method, final boolean callIsVirtual) {
if (CLASS_CONSTRUCTOR_METHOD_NAME.equals(method.getName())) {
// we don't check for violations on class constructors
return null;
}
String violation = checkClassUse(owner, "class/interface", owner);
ViolationResult violation = checkClassUse(owner, "class/interface", owner);
if (violation != null) {
return violation;
}
Expand All @@ -405,7 +414,7 @@ private String checkMethodAccess(String owner, final Method method, final boolea
}
return visitAncestors(c, new AncestorVisitor() {
@Override
public String visit(ClassMetadata c, String origName, boolean isInterfaceOfAncestor, boolean previousInRuntime) {
public ViolationResult visit(ClassMetadata c, String origName, boolean isInterfaceOfAncestor, boolean previousInRuntime) {
final Method lookupMethod;
if (c.signaturePolymorphicMethods.contains(method.getName())) {
// convert the invoked descriptor to a signature polymorphic one for the lookup
Expand All @@ -417,11 +426,11 @@ public String visit(ClassMetadata c, String origName, boolean isInterfaceOfAnces
return null;
}
// is we have a virtual call, look into superclasses, otherwise stop:
final String notFoundRet = callIsVirtual ? null : AncestorVisitor.STOP;
final ViolationResult notFoundRet = callIsVirtual ? null : AncestorVisitor.STOP;
if (previousInRuntime && c.isNonPortableRuntime) {
return notFoundRet; // something inside the JVM is extending internal class/interface
}
String violation = forbiddenSignatures.checkMethod(c.className, lookupMethod);
ViolationResult violation = forbiddenSignatures.checkMethod(c.className, lookupMethod);
if (violation != null) {
return violation;
}
Expand All @@ -437,8 +446,8 @@ public String visit(ClassMetadata c, String origName, boolean isInterfaceOfAnces
}, true, false /* JVM spec says: interfaces after superclasses */);
}

private String checkFieldAccess(String owner, final String field) {
String violation = checkClassUse(owner, "class/interface", owner);
private ViolationResult checkFieldAccess(String owner, final String field) {
ViolationResult violation = checkClassUse(owner, "class/interface", owner);
if (violation != null) {
return violation;
}
Expand All @@ -453,15 +462,15 @@ private String checkFieldAccess(String owner, final String field) {
}
return visitAncestors(c, new AncestorVisitor() {
@Override
public String visit(ClassMetadata c, String origName, boolean isInterfaceOfAncestor, boolean previousInRuntime) {
public ViolationResult visit(ClassMetadata c, String origName, boolean isInterfaceOfAncestor, boolean previousInRuntime) {
if (!c.fields.contains(field)) {
return null;
}
// we found the field: from now on we use STOP to exit, because fields are not virtual!
if (previousInRuntime && c.isNonPortableRuntime) {
return STOP; // something inside the JVM is extending internal class/interface
}
String violation = forbiddenSignatures.checkField(c.className, field);
ViolationResult violation = forbiddenSignatures.checkField(c.className, field);
if (violation != null) {
return violation;
}
Expand All @@ -478,7 +487,7 @@ public String visit(ClassMetadata c, String origName, boolean isInterfaceOfAnces
}, true, true /* JVM spec says: superclasses after interfaces */);
}

private String checkHandle(Handle handle, boolean checkLambdaHandle) {
private ViolationResult checkHandle(Handle handle, boolean checkLambdaHandle) {
switch (handle.getTag()) {
case Opcodes.H_GETFIELD:
case Opcodes.H_PUTFIELD:
Expand All @@ -503,7 +512,7 @@ private String checkHandle(Handle handle, boolean checkLambdaHandle) {
return null;
}

private String checkConstant(Object cst, boolean checkLambdaHandle) {
private ViolationResult checkConstant(Object cst, boolean checkLambdaHandle) {
if (cst instanceof Type) {
return checkType((Type) cst);
} else if (cst instanceof Handle) {
Expand Down Expand Up @@ -604,9 +613,9 @@ private String getHumanReadableMethodSignature() {
return sb.toString();
}

private void reportMethodViolation(String violation, String where) {
private void reportMethodViolation(ViolationResult violation, String where) {
if (violation != null) {
violations.add(new ForbiddenViolation(currentGroupId, myself, violation, String.format(Locale.ENGLISH, "%s of '%s'", where, getHumanReadableMethodSignature()), lineNo));
violations.add(new ForbiddenViolation(currentGroupId, myself, violation.message, String.format(Locale.ENGLISH, "%s of '%s'", where, getHumanReadableMethodSignature()), lineNo, violation.severity));
}
}

Expand Down Expand Up @@ -642,9 +651,9 @@ public AnnotationVisitor visitTypeAnnotation(int typeRef, TypePath typePath, Str
return null;
}

private void reportRecordComponentViolation(String violation, String where) {
if (violation != null) {
violations.add(new ForbiddenViolation(currentGroupId, violation, String.format(Locale.ENGLISH, "%s of '%s'", where, name), -1));
private void reportRecordComponentViolation(ViolationResult violationResult, String where) {
if (violationResult != null) {
violations.add(new ForbiddenViolation(currentGroupId, violationResult.message, String.format(Locale.ENGLISH, "%s of '%s'", where, name), -1, violationResult.severity));
}
}
};
Expand Down
Loading

0 comments on commit 5773478

Please sign in to comment.