Navigation Menu

Skip to content

Commit

Permalink
Simplify signatures lookup by using only one map, not 3 (keys are uni…
Browse files Browse the repository at this point in the history 
…que anyways)
  • Loading branch information
@uschindler
uschindler committed Mar 31, 2018
1 parent b95a724 commit fca4915
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 49 deletions.
40 changes: 21 additions & 19 deletions src/main/java/de/thetaphi/forbiddenapis/Checker.java
View file
Expand Up @@ -75,22 +75,26 @@ public static enum Option {
final java.lang.reflect.Method method_Class_getModule, method_Module_getName;
final EnumSet<Option> options;

// key is the binary name (dotted):
/** Classes to check: key is the binary name (dotted) */
final Map<String,ClassSignature> classesToCheck = new HashMap<String,ClassSignature>();
// key is the binary name (dotted):
/** Cache of loaded classes: key is the binary name (dotted) */
final Map<String,ClassSignature> classpathClassCache = new HashMap<String,ClassSignature>();

// if enabled, the bundled signature to enable heuristics for detection of non-portable runtime calls is used:
private boolean forbidNonPortableRuntime = false;
// key is the internal name (slashed), followed by \000 and the field name:
final Map<String,String> forbiddenFields = new HashMap<String,String>();
// key is the internal name (slashed), followed by \000 and the method signature:
final Map<String,String> forbiddenMethods = new HashMap<String,String>();
// key is the internal name (slashed):
final Map<String,String> forbiddenClasses = new HashMap<String,String>();
// set of patterns of forbidden classes:
/** if enabled, the bundled signature to enable heuristics for detection of non-portable runtime calls is used */
private boolean forbidNonPortableRuntime = false;

/** Key is used to lookup forbidden signature in following formats:
* <ul>
* <li>methods: key is the internal name (slashed), followed by \000 and the method signature
* <li>fields: key is the internal name (slashed), followed by \000 and the field name
* <li>classes: key is the internal name (slashed)
* </ul>
*/
final Map<String,String> forbiddenSignatures = new HashMap<String,String>();

/** set of patterns of forbidden classes */
final Set<ClassPatternRule> forbiddenClassPatterns = new LinkedHashSet<ClassPatternRule>();
// descriptors (not internal names) of all annotations that suppress:
/** descriptors (not internal names) of all annotations that suppress */
final Set<String> suppressAnnotations = new LinkedHashSet<String>();

private static enum UnresolvableReporting {
Expand Down Expand Up @@ -423,7 +427,7 @@ private void addSignature(final String line, final String defaultMessage, final
for (final Method m : c.methods) {
if (m.getName().equals(method.getName()) && Arrays.equals(m.getArgumentTypes(), method.getArgumentTypes())) {
found = true;
forbiddenMethods.put(c.className + '\000' + m, printout);
forbiddenSignatures.put(c.className + '\000' + m, printout);
// don't break when found, as there may be more covariant overrides!
}
}
Expand All @@ -437,11 +441,11 @@ private void addSignature(final String line, final String defaultMessage, final
report.parseFailed(logger, "Field not found", signature);
return;
}
forbiddenFields.put(c.className + '\000' + field, printout);
forbiddenSignatures.put(c.className + '\000' + field, printout);
} else {
assert field == null && method == null;
// only add the signature as class name
forbiddenClasses.put(c.className, printout);
forbiddenSignatures.put(c.className, printout);
}
}
}
Expand Down Expand Up @@ -634,9 +638,7 @@ public void addClassesToCheck(File basedir, String... relativeNames) throws IOEx
}

public boolean hasNoSignatures() {
return 0 == forbiddenMethods.size() +
forbiddenFields.size() +
forbiddenClasses.size() +
return 0 == forbiddenSignatures.size() +
forbiddenClassPatterns.size() +
(forbidNonPortableRuntime ? 1 : 0);
}
Expand All @@ -654,7 +656,7 @@ public void addSuppressAnnotation(String annoName) {
/** Parses a class and checks for valid method invocations */
private int checkClass(final ClassReader reader, Pattern suppressAnnotationsPattern) {
final String className = Type.getObjectType(reader.getClassName()).getClassName();
final ClassScanner scanner = new ClassScanner(this, forbiddenClasses, forbiddenClassPatterns, forbiddenMethods, forbiddenFields, suppressAnnotationsPattern, forbidNonPortableRuntime);
final ClassScanner scanner = new ClassScanner(this, forbiddenSignatures, forbiddenClassPatterns, suppressAnnotationsPattern, forbidNonPortableRuntime);
reader.accept(scanner, ClassReader.SKIP_FRAMES);
final List<ForbiddenViolation> violations = scanner.getSortedViolations();
final Pattern splitter = Pattern.compile(Pattern.quote(ForbiddenViolation.SEPARATOR));
Expand Down
29 changes: 14 additions & 15 deletions src/main/java/de/thetaphi/forbiddenapis/ClassScanner.java
View file
Expand Up @@ -44,12 +44,14 @@ final class ClassScanner extends ClassVisitor implements Constants {
final RelatedClassLookup lookup;
final List<ForbiddenViolation> violations = new ArrayList<ForbiddenViolation>();

// key is the internal name (slashed), followed by \000 and the field name:
final Map<String,String> forbiddenFields;
// key is the internal name (slashed), followed by \000 and the method signature:
final Map<String,String> forbiddenMethods;
// key is the internal name (slashed):
final Map<String,String> forbiddenClasses;
/** Key is used to lookup forbidden signature in following formats:
* <ul>
* <li>methods: key is the internal name (slashed), followed by \000 and the method signature
* <li>fields: key is the internal name (slashed), followed by \000 and the field name
* <li>classes: key is the internal name (slashed)
* </ul>
*/
final Map<String,String> forbiddenSignatures;
// key is pattern to binary class name:
final Iterable<ClassPatternRule> forbiddenClassPatterns;
// pattern that matches binary (dotted) class name of all annotations that suppress:
Expand All @@ -69,16 +71,13 @@ final class ClassScanner extends ClassVisitor implements Constants {
boolean classSuppressed = false;

public ClassScanner(RelatedClassLookup lookup,
final Map<String,String> forbiddenClasses, final Iterable<ClassPatternRule> forbiddenClassPatterns,
final Map<String,String> forbiddenMethods, final Map<String,String> forbiddenFields,
final Map<String,String> forbiddenSignatures, final Iterable<ClassPatternRule> forbiddenClassPatterns,
final Pattern suppressAnnotations,
final boolean forbidNonPortableRuntime) {
super(Opcodes.ASM6);
this.lookup = lookup;
this.forbiddenClasses = forbiddenClasses;
this.forbiddenSignatures = forbiddenSignatures;
this.forbiddenClassPatterns = forbiddenClassPatterns;
this.forbiddenMethods = forbiddenMethods;
this.forbiddenFields = forbiddenFields;
this.suppressAnnotations = suppressAnnotations;
this.forbidNonPortableRuntime = forbidNonPortableRuntime;
}
Expand Down Expand Up @@ -106,7 +105,7 @@ String checkClassUse(Type type, String what, boolean deep) {
return null; // we don't know this type, just pass!
}
final String internalName = type.getInternalName();
final String printout = forbiddenClasses.get(internalName);
final String printout = forbiddenSignatures.get(internalName);
if (printout != null) {
return String.format(Locale.ENGLISH, "Forbidden %s use: %s", what, printout);
}
Expand Down Expand Up @@ -342,7 +341,7 @@ private String checkMethodAccess(String owner, Method method) {
}

private String checkMethodAccessRecursion(String owner, Method method, boolean checkClassUse) {
String printout = forbiddenMethods.get(owner + '\000' + method);
String printout = forbiddenSignatures.get(owner + '\000' + method);
if (printout != null) {
return "Forbidden method invocation: " + printout;
}
Expand All @@ -351,7 +350,7 @@ private String checkMethodAccessRecursion(String owner, Method method, boolean c
if (c.signaturePolymorphicMethods.contains(method.getName())) {
// convert the invoked descriptor to a signature polymorphic one for the lookup
final Method lookupMethod = new Method(method.getName(), SIGNATURE_POLYMORPHIC_DESCRIPTOR);
printout = forbiddenMethods.get(owner + '\000' + lookupMethod);
printout = forbiddenSignatures.get(owner + '\000' + lookupMethod);
if (printout != null) {
return "Forbidden method invocation: " + printout;
}
Expand Down Expand Up @@ -387,7 +386,7 @@ private String checkFieldAccess(String owner, String field) {
if (violation != null) {
return violation;
}
final String printout = forbiddenFields.get(owner + '\000' + field);
final String printout = forbiddenSignatures.get(owner + '\000' + field);
if (printout != null) {
return "Forbidden field access: " + printout;
}
Expand Down
20 changes: 5 additions & 15 deletions src/test/java/de/thetaphi/forbiddenapis/CheckerSetupTest.java
View file
Expand Up @@ -40,46 +40,36 @@ public void setUp() {

@Test
public void testEmpty() {
assertEquals(Collections.emptyMap(), checker.forbiddenClasses);
assertEquals(Collections.emptyMap(), checker.forbiddenSignatures);
assertEquals(Collections.emptySet(), checker.forbiddenClassPatterns);
assertEquals(Collections.emptyMap(), checker.forbiddenFields);
assertEquals(Collections.emptyMap(), checker.forbiddenMethods);
}

@Test
public void testClassSignature() throws Exception {
checker.parseSignaturesString("java.lang.Object @ Foobar");
assertEquals(Collections.singletonMap("java/lang/Object", "java.lang.Object [Foobar]"), checker.forbiddenClasses);
assertEquals(Collections.singletonMap("java/lang/Object", "java.lang.Object [Foobar]"), checker.forbiddenSignatures);
assertEquals(Collections.emptySet(), checker.forbiddenClassPatterns);
assertEquals(Collections.emptyMap(), checker.forbiddenFields);
assertEquals(Collections.emptyMap(), checker.forbiddenMethods);
}

@Test
public void testClassPatternSignature() throws Exception {
checker.parseSignaturesString("java.lang.** @ Foobar");
assertEquals(Collections.emptyMap(), checker.forbiddenClasses);
assertEquals(Collections.emptyMap(), checker.forbiddenSignatures);
assertEquals(Collections.singleton(new ClassPatternRule("java.lang.**", "Foobar")), checker.forbiddenClassPatterns);
assertEquals(Collections.emptyMap(), checker.forbiddenFields);
assertEquals(Collections.emptyMap(), checker.forbiddenMethods);
}

@Test
public void testFieldSignature() throws Exception {
checker.parseSignaturesString("java.lang.String#CASE_INSENSITIVE_ORDER @ Foobar");
assertEquals(Collections.emptyMap(), checker.forbiddenClasses);
assertEquals(Collections.singletonMap("java/lang/String\000CASE_INSENSITIVE_ORDER", "java.lang.String#CASE_INSENSITIVE_ORDER [Foobar]"), checker.forbiddenSignatures);
assertEquals(Collections.emptySet(), checker.forbiddenClassPatterns);
assertEquals(Collections.singletonMap("java/lang/String\000CASE_INSENSITIVE_ORDER", "java.lang.String#CASE_INSENSITIVE_ORDER [Foobar]"), checker.forbiddenFields);
assertEquals(Collections.emptyMap(), checker.forbiddenMethods);
}

@Test
public void testMethodSignature() throws Exception {
checker.parseSignaturesString("java.lang.Object#toString() @ Foobar");
assertEquals(Collections.emptyMap(), checker.forbiddenClasses);
assertEquals(Collections.singletonMap("java/lang/Object\000toString()Ljava/lang/String;", "java.lang.Object#toString() [Foobar]"), checker.forbiddenSignatures);
assertEquals(Collections.emptySet(), checker.forbiddenClassPatterns);
assertEquals(Collections.emptyMap(), checker.forbiddenFields);
assertEquals(Collections.singletonMap("java/lang/Object\000toString()Ljava/lang/String;", "java.lang.Object#toString() [Foobar]"), checker.forbiddenMethods);
}

@Test
Expand Down

0 comments on commit fca4915

Please sign in to comment.