diff --git a/src/main/java/com/mebigfatguy/fbcontrib/detect/BogusExceptionDeclaration.java b/src/main/java/com/mebigfatguy/fbcontrib/detect/BogusExceptionDeclaration.java index 75c19edc..9e03ff53 100755 --- a/src/main/java/com/mebigfatguy/fbcontrib/detect/BogusExceptionDeclaration.java +++ b/src/main/java/com/mebigfatguy/fbcontrib/detect/BogusExceptionDeclaration.java @@ -30,6 +30,7 @@ import com.mebigfatguy.fbcontrib.collect.MethodInfo; import com.mebigfatguy.fbcontrib.collect.Statistics; import com.mebigfatguy.fbcontrib.utils.BugType; +import com.mebigfatguy.fbcontrib.utils.FQMethod; import com.mebigfatguy.fbcontrib.utils.OpcodeUtils; import com.mebigfatguy.fbcontrib.utils.SignatureUtils; import com.mebigfatguy.fbcontrib.utils.StopOpcodeParsingException; @@ -43,15 +44,17 @@ import edu.umd.cs.findbugs.ba.ClassContext; /** - * looks for constructors, private methods or static methods that declare that they throw specific checked exceptions, but that do not. This just causes callers - * of these methods to do extra work to handle an exception that will never be thrown. also looks for throws clauses where two exceptions declared to be thrown - * are related through inheritance. + * looks for constructors, private methods or static methods that declare that + * they throw specific checked exceptions, but that do not. This just causes + * callers of these methods to do extra work to handle an exception that will + * never be thrown. also looks for throws clauses where two exceptions declared + * to be thrown are related through inheritance. */ public class BogusExceptionDeclaration extends BytecodeScanningDetector { private static final String IGNORE_INHERITED_METHODS_PROPERTY = "fb-contrib.bed.ignore_inherited"; - private static final Set safeClasses = UnmodifiableSet.create( + private static final Set SAFE_CLASSES = UnmodifiableSet.create( //@formatter:off Values.SLASHED_JAVA_LANG_OBJECT, Values.SLASHED_JAVA_LANG_STRING, @@ -62,9 +65,15 @@ public class BogusExceptionDeclaration extends BytecodeScanningDetector { Values.SLASHED_JAVA_LANG_SHORT, Values.SLASHED_JAVA_LANG_BYTE, Values.SLASHED_JAVA_LANG_BOOLEAN - //@formatter:on + //@formatter:on ); - + + private static final Set SAFE_EXCEPTION_METHODS = UnmodifiableSet.create( + //@formatter:off + new FQMethod(Values.SLASHED_JAVA_LANG_STRING, "getBytes", "(Ljava/lang/String;)[B") + //@formatter:on + ); + private static final boolean IGNORE_INHERITED_METHODS = Boolean.getBoolean(IGNORE_INHERITED_METHODS_PROPERTY); private final BugReporter bugReporter; @@ -93,8 +102,7 @@ public BogusExceptionDeclaration(BugReporter bugReporter) { /** * overrides the visitor to create the opcode stack * - * @param classContext - * the context object of the currently parsed class + * @param classContext the context object of the currently parsed class * */ @Override @@ -115,10 +123,10 @@ public void visitClassContext(ClassContext classContext) { } /** - * implements the visitor to see if the method declares that it throws any checked exceptions. + * implements the visitor to see if the method declares that it throws any + * checked exceptions. * - * @param obj - * the context object of the currently parsed code block + * @param obj the context object of the currently parsed code block */ @Override public void visitCode(Code obj) { @@ -127,21 +135,23 @@ public void visitCode(Code obj) { if (method.isSynthetic()) { return; } - + if (IGNORE_INHERITED_METHODS) { - MethodInfo mi = Statistics.getStatistics().getMethodStatistics(getClassName(), getMethodName(), getMethodSig()); - if (mi != null && mi.isDerived()) { - return; - } + MethodInfo mi = Statistics.getStatistics().getMethodStatistics(getClassName(), getMethodName(), + getMethodSig()); + if (mi != null && mi.isDerived()) { + return; + } } - + declaredCheckedExceptions.clear(); stack.resetForMethodEntry(this); ExceptionTable et = method.getExceptionTable(); if (et != null) { if (classIsFinal || classIsAnonymous || method.isStatic() || method.isPrivate() || method.isFinal() - || ((Values.CONSTRUCTOR.equals(method.getName()) && !isAnonymousInnerCtor(method, getThisClass())))) { + || ((Values.CONSTRUCTOR.equals(method.getName()) + && !isAnonymousInnerCtor(method, getThisClass())))) { String[] exNames = et.getExceptionNames(); for (String exName : exNames) { try { @@ -157,8 +167,8 @@ public void visitCode(Code obj) { try { super.visitCode(obj); if (!declaredCheckedExceptions.isEmpty()) { - BugInstance bi = new BugInstance(this, BugType.BED_BOGUS_EXCEPTION_DECLARATION.name(), NORMAL_PRIORITY).addClass(this) - .addMethod(this).addSourceLine(this, 0); + BugInstance bi = new BugInstance(this, BugType.BED_BOGUS_EXCEPTION_DECLARATION.name(), + NORMAL_PRIORITY).addClass(this).addMethod(this).addSourceLine(this, 0); for (String ex : declaredCheckedExceptions) { bi.addString(ex.replaceAll("/", ".")); } @@ -189,8 +199,10 @@ public void visitCode(Code obj) { } if (!parentEx.equals(exceptionClass)) { - bugReporter.reportBug(new BugInstance(this, BugType.BED_HIERARCHICAL_EXCEPTION_DECLARATION.name(), NORMAL_PRIORITY).addClass(this) - .addMethod(this).addString(childEx.getClassName() + " derives from " + parentEx.getClassName())); + bugReporter.reportBug(new BugInstance(this, + BugType.BED_HIERARCHICAL_EXCEPTION_DECLARATION.name(), NORMAL_PRIORITY) + .addClass(this).addMethod(this) + .addString(childEx.getClassName() + " derives from " + parentEx.getClassName())); return; } @@ -203,21 +215,23 @@ public void visitCode(Code obj) { } /** - * checks to see if this method is a constructor of an instance based inner class, the handling of the Exception table for this method is odd, -- doesn't + * checks to see if this method is a constructor of an instance based inner + * class, the handling of the Exception table for this method is odd, -- doesn't * seem correct, in some cases. So just ignore these cases * - * @param m - * the method to check - * @param cls - * the cls that owns the method - * @return whether this method is a ctor of an instance based anonymous inner class + * @param m the method to check + * @param cls the cls that owns the method + * @return whether this method is a ctor of an instance based anonymous inner + * class */ private static boolean isAnonymousInnerCtor(Method m, JavaClass cls) { - return Values.CONSTRUCTOR.equals(m.getName()) && (cls.getClassName().lastIndexOf(Values.INNER_CLASS_SEPARATOR) >= 0); + return Values.CONSTRUCTOR.equals(m.getName()) + && (cls.getClassName().lastIndexOf(Values.INNER_CLASS_SEPARATOR) >= 0); } /** - * implements the visitor to look for method calls that could throw the exceptions that are listed in the declaration. + * implements the visitor to look for method calls that could throw the + * exceptions that are listed in the declaration. */ @Override public void sawOpcode(int seen) { @@ -227,7 +241,8 @@ public void sawOpcode(int seen) { if (OpcodeUtils.isStandardInvoke(seen)) { String clsName = getClassConstantOperand(); - if (!safeClasses.contains(clsName)) { + if (!SAFE_CLASSES.contains(clsName) || SAFE_EXCEPTION_METHODS + .contains(new FQMethod(clsName, getNameConstantOperand(), getSigConstantOperand()))) { try { JavaClass cls = Repository.lookupClass(clsName); Method[] methods = cls.getMethods(); @@ -238,7 +253,8 @@ public void sawOpcode(int seen) { if (m.getName().equals(methodName) && m.getSignature().equals(signature)) { if (isAnonymousInnerCtor(m, cls)) { - // The java compiler doesn't properly attached an Exception Table to anonymous constructors, so just clear if so + // The java compiler doesn't properly attached an Exception Table to anonymous + // constructors, so just clear if so break; } @@ -280,16 +296,19 @@ public void sawOpcode(int seen) { } /** - * removes this thrown exception the list of declared thrown exceptions, including all exceptions in this exception's hierarchy. If an exception class is - * found that can't be loaded, then just clear the list of declared checked exceptions and get out. + * removes this thrown exception the list of declared thrown exceptions, + * including all exceptions in this exception's hierarchy. If an exception class + * is found that can't be loaded, then just clear the list of declared checked + * exceptions and get out. * - * @param thrownException - * the exception and it's hierarchy to remove + * @param thrownException the exception and it's hierarchy to remove */ private void removeThrownExceptionHierarchy(String thrownException) { try { - if (Values.DOTTED_JAVA_LANG_EXCEPTION.equals(thrownException) || Values.DOTTED_JAVA_LANG_THROWABLE.equals(thrownException)) { - // Exception/Throwable can be thrown even tho the method isn't declared to throw Exception/Throwable in the case of templated Exceptions + if (Values.DOTTED_JAVA_LANG_EXCEPTION.equals(thrownException) + || Values.DOTTED_JAVA_LANG_THROWABLE.equals(thrownException)) { + // Exception/Throwable can be thrown even tho the method isn't declared to throw + // Exception/Throwable in the case of templated Exceptions clearExceptions(); } else { removeException(thrownException); @@ -314,10 +333,10 @@ private void removeThrownExceptionHierarchy(String thrownException) { } /** - * removes the declared checked exception, and if that was the last declared exception, stops opcode parsing by throwing exception + * removes the declared checked exception, and if that was the last declared + * exception, stops opcode parsing by throwing exception * - * @param clsName - * the name of the exception to remove + * @param clsName the name of the exception to remove */ private void removeException(String clsName) { declaredCheckedExceptions.remove(clsName); @@ -327,7 +346,8 @@ private void removeException(String clsName) { } /** - * clears all declared checked exceptions and throws an exception to stop opcode parsing + * clears all declared checked exceptions and throws an exception to stop opcode + * parsing */ private void clearExceptions() { declaredCheckedExceptions.clear(); diff --git a/src/samples/java/ex/BED_Sample.java b/src/samples/java/ex/BED_Sample.java index f129d235..c293ea95 100755 --- a/src/samples/java/ex/BED_Sample.java +++ b/src/samples/java/ex/BED_Sample.java @@ -5,6 +5,7 @@ import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; +import java.io.UnsupportedEncodingException; import java.sql.SQLException; import java.util.Hashtable; import java.util.Optional; @@ -112,6 +113,10 @@ public static Process fpInterrupted(String command) throws IOException, Interrup return p; } + private void fp406(String p) throws UnsupportedEncodingException { + byte[] v = p.getBytes("UTF8"); + } + static class FPAnonBase { FPAnonBase(InputStream is) throws IOException { @@ -123,15 +128,16 @@ public static FPAnonBase makeAnon(InputStream is) throws IOException { }; } } - + static class BaseBED { - public void foo() throws IOException { - } + public void foo() throws IOException { + } } - + /* is caught based on the option fb-contrib.bed.ignore_inherited */ static class ChildBED extends BaseBED { - public void foo() throws IOException { - } + @Override + public void foo() throws IOException { + } } }