From c61c428a0c7c9e844ad337e27dc11d042f1ec4ea Mon Sep 17 00:00:00 2001 From: Dave Brosius Date: Sat, 28 Sep 2024 19:11:39 -0400 Subject: [PATCH] add fix for LO with log4j, with Object parameter #465 --- .../fbcontrib/detect/LoggerOddities.java | 1399 +++++++++-------- src/samples/java/ex/LO_Sample.java | 844 +++++----- 2 files changed, 1129 insertions(+), 1114 deletions(-) diff --git a/src/main/java/com/mebigfatguy/fbcontrib/detect/LoggerOddities.java b/src/main/java/com/mebigfatguy/fbcontrib/detect/LoggerOddities.java index 6ff99340..8c92a4e4 100755 --- a/src/main/java/com/mebigfatguy/fbcontrib/detect/LoggerOddities.java +++ b/src/main/java/com/mebigfatguy/fbcontrib/detect/LoggerOddities.java @@ -65,702 +65,705 @@ @CustomUserValue public class LoggerOddities extends BytecodeScanningDetector { - private static final Set LOGGER_METHODS = UnmodifiableSet.create("trace", "debug", "info", "warn", "error", - "fatal"); - private static final String COMMONS_LOGGER = "org/apache/commons/logging/Log"; - private static final String LOG4J_LOGGER = "org/apache/log4j/Logger"; - private static final String LOG4J2_LOGGER = "org/apache/logging/log4j/Logger"; - private static final String LOG4J2_LOGMANAGER = "org/apache/logging/log4j/LogManager"; - private static final String SLF4J_LOGGER = "org/slf4j/Logger"; - private static final String SIG_STRING_AND_TWO_OBJECTS_TO_VOID = new SignatureBuilder() - .withParamTypes(Values.SLASHED_JAVA_LANG_STRING, Values.SLASHED_JAVA_LANG_OBJECT, - Values.SLASHED_JAVA_LANG_OBJECT) - .toString(); - private static final String SIG_STRING_AND_OBJECT_ARRAY_TO_VOID = new SignatureBuilder() - .withParamTypes(Values.SLASHED_JAVA_LANG_STRING, SignatureBuilder.SIG_OBJECT_ARRAY).toString(); - private static final String SIG_OBJECT_AND_THROWABLE_TO_VOID = new SignatureBuilder() - .withParamTypes(Values.SLASHED_JAVA_LANG_OBJECT, Values.SLASHED_JAVA_LANG_THROWABLE).toString(); - private static final String SIG_STRING_AND_THROWABLE_TO_VOID = new SignatureBuilder() - .withParamTypes(Values.SLASHED_JAVA_LANG_STRING, Values.SLASHED_JAVA_LANG_THROWABLE).toString(); - private static final String SIG_CLASS_TO_COMMONS_LOGGER = new SignatureBuilder() - .withParamTypes(Values.SLASHED_JAVA_LANG_CLASS).withReturnType(COMMONS_LOGGER).toString(); - private static final String SIG_CLASS_TO_LOG4J_LOGGER = new SignatureBuilder() - .withParamTypes(Values.SLASHED_JAVA_LANG_CLASS).withReturnType(LOG4J_LOGGER).toString(); - private static final String SIG_CLASS_TO_LOG4J2_LOGGER = new SignatureBuilder() - .withParamTypes(Values.SLASHED_JAVA_LANG_CLASS).withReturnType(LOG4J2_LOGGER).toString(); - private static final String SIG_CLASS_TO_SLF4J_LOGGER = new SignatureBuilder() - .withParamTypes(Values.SLASHED_JAVA_LANG_CLASS).withReturnType(SLF4J_LOGGER).toString(); - private static final String SIG_STRING_TO_COMMONS_LOGGER = new SignatureBuilder() - .withParamTypes(Values.SLASHED_JAVA_LANG_STRING).withReturnType(COMMONS_LOGGER).toString(); - private static final String SIG_STRING_TO_LOG4J_LOGGER = new SignatureBuilder() - .withParamTypes(Values.SLASHED_JAVA_LANG_STRING).withReturnType(LOG4J_LOGGER).toString(); - private static final String SIG_STRING_TO_LOG4J2_LOGGER = new SignatureBuilder() - .withParamTypes(Values.SLASHED_JAVA_LANG_STRING).withReturnType(LOG4J2_LOGGER).toString(); - private static final String SIG_STRING_TO_SLF4J_LOGGER = new SignatureBuilder() - .withParamTypes(Values.SLASHED_JAVA_LANG_STRING).withReturnType(SLF4J_LOGGER).toString(); - private static final String SIG_STRING_AND_FACTORY_TO_LOG4J_LOGGER = new SignatureBuilder() - .withParamTypes(Values.SLASHED_JAVA_LANG_STRING, "org/apache/log4j/spi/LoggerFactory") - .withReturnType(LOG4J_LOGGER).toString(); - - private static final Pattern BAD_FORMATTING_ANCHOR = Pattern.compile("\\{[0-9]\\}"); - private static final Pattern BAD_STRING_FORMAT_PATTERN = Pattern - .compile("(? formatterLoggers; - private JavaClass throwableClass; - private OpcodeStack stack; - private String nameOfThisClass; - private boolean isStaticInitializer; - - /** - * constructs a LO detector given the reporter to report bugs on. - * - * @param bugReporter the sync of bug reports - */ - public LoggerOddities(final BugReporter bugReporter) { - this.bugReporter = bugReporter; - - try { - throwableClass = Repository.lookupClass(Values.SLASHED_JAVA_LANG_THROWABLE); - } catch (ClassNotFoundException cnfe) { - bugReporter.reportMissingClass(cnfe); - } - - } - - /** - * implements the visitor to discover what the class name is if it is a normal - * class, or the owning class, if the class is an anonymous class. - * - * @param classContext the context object of the currently parsed class - */ - @Override - public void visitClassContext(ClassContext classContext) { - try { - stack = new OpcodeStack(); - nameOfThisClass = SignatureUtils.getNonAnonymousPortion(classContext.getJavaClass().getClassName()); - formatterLoggers = new HashSet<>(); - super.visitClassContext(classContext); - } finally { - formatterLoggers = null; - stack = null; - } - } - - /** - * implements the visitor to reset the stack - * - * @param obj the context object of the currently parsed code block - */ - @Override - public void visitCode(Code obj) { - stack.resetForMethodEntry(this); - Method m = getMethod(); - if (Values.CONSTRUCTOR.equals(m.getName())) { - for (String parmSig : SignatureUtils.getParameterSignatures(m.getSignature())) { - if (SignatureUtils.classToSignature(SLF4J_LOGGER).equals(parmSig) - || SignatureUtils.classToSignature(LOG4J_LOGGER).equals(parmSig) - || SignatureUtils.classToSignature(LOG4J2_LOGGER).equals(parmSig) - || SignatureUtils.classToSignature(COMMONS_LOGGER).equals(parmSig)) { - bugReporter - .reportBug(new BugInstance(this, BugType.LO_SUSPECT_LOG_PARAMETER.name(), NORMAL_PRIORITY) - .addClass(this).addMethod(this)); - } - } - } - - isStaticInitializer = Values.STATIC_INITIALIZER.equals(m.getName()); - super.visitCode(obj); - } - - /** - * implements the visitor to look for calls to Logger.getLogger with the wrong - * class name - * - * @param seen the opcode of the currently parsed instruction - */ - @Override - @SuppressWarnings("unchecked") - public void sawOpcode(int seen) { - String ldcClassName = null; - String seenMethodName = null; - String callingClsName = null; - boolean seenToString = false; - boolean seenFormatterLogger = false; - int exMessageReg = -1; - Integer arraySize = null; - boolean simpleFormat = false; - - try { - stack.precomputation(this); - - if ((seen == Const.LDC) || (seen == Const.LDC_W)) { - Constant c = getConstantRefOperand(); - if (c instanceof ConstantClass) { - ConstantPool pool = getConstantPool(); - ldcClassName = ((ConstantUtf8) pool.getConstant(((ConstantClass) c).getNameIndex())).getBytes(); - } - } else if (seen == Const.INVOKESTATIC) { - lookForSuspectClasses(); - - String clsName = getClassConstantOperand(); - String methodName = getNameConstantOperand(); - if (Values.SLASHED_JAVA_LANG_STRING.equals(clsName) && "format".equals(methodName) - && (stack.getStackDepth() >= 2)) { - String format = (String) stack.getStackItem(1).getConstant(); - if (format != null) { - Matcher m = NON_SIMPLE_FORMAT.matcher(format); - if (!m.matches()) { - simpleFormat = true; - } - } - } else if ("getFormatterLogger".equals(methodName) && LOG4J2_LOGMANAGER.equals(clsName)) { - seenFormatterLogger = true; - } - } else if (((seen == Const.INVOKEVIRTUAL) || (seen == Const.INVOKEINTERFACE)) && (throwableClass != null)) { - String mthName = getNameConstantOperand(); - if ("getName".equals(mthName)) { - if (stack.getStackDepth() >= 1) { - // Foo.class.getName() is being called, so we pass the - // name of the class to the current top of the stack - // (the name of the class is currently on the top of the - // stack, but won't be on the stack at all next opcode) - Item stackItem = stack.getStackItem(0); - LOUserValue uv = (LOUserValue) stackItem.getUserValue(); - if ((uv != null) && (uv.getType() == LOUserValue.LOType.CLASS_NAME)) { - ldcClassName = uv.getValue(); - } - } - } else if ("getMessage".equals(mthName)) { - callingClsName = getClassConstantOperand(); - JavaClass cls = Repository.lookupClass(callingClsName); - if (cls.instanceOf(throwableClass) && (stack.getStackDepth() > 0)) { - OpcodeStack.Item exItem = stack.getStackItem(0); - exMessageReg = exItem.getRegisterNumber(); - } - } else if (LOGGER_METHODS.contains(mthName)) { - checkForProblemsWithLoggerMethods(); - } else if (Values.TOSTRING.equals(mthName) - && SignatureBuilder.SIG_VOID_TO_STRING.equals(getSigConstantOperand())) { - callingClsName = getClassConstantOperand(); - if (SignatureUtils.isPlainStringConvertableClass(callingClsName) && (stack.getStackDepth() > 0)) { - OpcodeStack.Item item = stack.getStackItem(0); - // if the stringbuilder was previously stored, don't report it - if (item.getRegisterNumber() < 0) { - seenMethodName = mthName; - } - } - - if (seenMethodName == null) { - seenToString = true; - } - } - } else if (seen == Const.INVOKESPECIAL) { - checkForLoggerParam(); - } else if (seen == Const.ANEWARRAY) { - if (stack.getStackDepth() > 0) { - OpcodeStack.Item sizeItem = stack.getStackItem(0); - Object con = sizeItem.getConstant(); - if (con instanceof Integer) { - arraySize = (Integer) con; - } - } - } else if (seen == Const.AASTORE) { - if (stack.getStackDepth() >= 3) { - OpcodeStack.Item arrayItem = stack.getStackItem(2); - LOUserValue uv = (LOUserValue) arrayItem.getUserValue(); - if ((uv != null) && (uv.getType() == LOUserValue.LOType.ARRAY_SIZE)) { - Integer size = uv.getValue(); - if ((size != null) && (size.intValue() > 0) && hasExceptionOnStack()) { - arrayItem.setUserValue(new LOUserValue<>(LOUserValue.LOType.ARRAY_SIZE, - Integer.valueOf(-size.intValue()))); - } - } - } - } else if (seen == Const.PUTSTATIC) { - OpcodeStack.Item itm = stack.getStackItem(0); - if (isStaticInitializer && isNonPrivateLogField(getClassConstantOperand(), getNameConstantOperand(), - getSigConstantOperand())) { - XMethod m = itm.getReturnValueOf(); - if ((m != null) && isLoggerWithClassParm(m)) { - bugReporter.reportBug( - new BugInstance(this, BugType.LO_NON_PRIVATE_STATIC_LOGGER.name(), NORMAL_PRIORITY) - .addClass(this).addMethod(this).addSourceLine(this)); - } - } - - LOUserValue loggerUV = (LOUserValue) itm.getUserValue(); - if ((loggerUV != null) && (loggerUV.getType() == LOUserValue.LOType.FORMATTER_LOGGER)) { - formatterLoggers.add(getNameConstantOperand()); - } - } else if (seen == Const.GETSTATIC) { - if (formatterLoggers.contains(getNameConstantOperand())) { - seenFormatterLogger = true; - } - } else if (OpcodeUtils.isAStore(seen) && (stack.getStackDepth() > 0)) { - OpcodeStack.Item item = stack.getStackItem(0); - LOUserValue uv = (LOUserValue) item.getUserValue(); - if (uv != null) { - if (((uv.getType() == LOUserValue.LOType.METHOD_NAME) && Values.TOSTRING.equals(uv.getValue())) - || (uv.getType() == LOUserValue.LOType.SIMPLE_FORMAT) - || (uv.getType() == LOUserValue.LOType.TOSTRING)) { - item.setUserValue(new LOUserValue<>(LOUserValue.LOType.NULL, null)); - } - } - } - } catch (ClassNotFoundException cnfe) { - bugReporter.reportMissingClass(cnfe); - } finally { - TernaryPatcher.pre(stack, seen); - stack.sawOpcode(this, seen); - TernaryPatcher.post(stack, seen); - - if (stack.getStackDepth() > 0) { - OpcodeStack.Item item = stack.getStackItem(0); - if (ldcClassName != null) { - item.setUserValue(new LOUserValue<>(LOUserValue.LOType.CLASS_NAME, ldcClassName)); - } else if (seenMethodName != null) { - item.setUserValue(new LOUserValue<>(LOUserValue.LOType.METHOD_NAME, seenMethodName)); - } else if (exMessageReg >= 0) { - item.setUserValue(new LOUserValue<>(LOUserValue.LOType.MESSAGE_REG, Integer.valueOf(exMessageReg))); - } else if (arraySize != null) { - item.setUserValue(new LOUserValue<>(LOUserValue.LOType.ARRAY_SIZE, arraySize)); - } else if (simpleFormat) { - item.setUserValue(new LOUserValue<>(LOUserValue.LOType.SIMPLE_FORMAT, Boolean.TRUE)); - } else if (seenToString) { - item.setUserValue(new LOUserValue<>(LOUserValue.LOType.TOSTRING, callingClsName)); - } else if (seenFormatterLogger) { - item.setUserValue(new LOUserValue<>(LOUserValue.LOType.FORMATTER_LOGGER, null)); - } - } - } - } - - /** - * looks to see if this field is a logger, and declared non privately - * - * @param fieldClsName the owning class type of the field - * @param fieldName the name of the field - * @param fieldSig the signature of the field - * @return if the field is a logger and not private - */ - private boolean isNonPrivateLogField(@SlashedClassName String fieldClsName, String fieldName, String fieldSig) { - - String fieldType = SignatureUtils.trimSignature(fieldSig); - if (!SLF4J_LOGGER.equals(fieldType) && !COMMONS_LOGGER.equals(fieldType) && !LOG4J_LOGGER.equals(fieldType) - && !LOG4J2_LOGGER.equals(fieldType)) { - return false; - } - - JavaClass cls = getClassContext().getJavaClass(); - if (!cls.getClassName().equals(fieldClsName.replace('/', '.'))) { - return false; - } - - for (Field f : getClassContext().getJavaClass().getFields()) { - if (f.getName().equals(fieldName)) { - return !f.isPrivate(); - } - } - - return true; - } - - /** - * returns whether this method class is a standard logger instantiation that - * takes a java/lang/Class parameter - * - * @param m the method to check - * @return if the method is a logger factory method that takes a Class object - */ - private boolean isLoggerWithClassParm(XMethod m) { - String signature = m.getSignature(); - return SIG_CLASS_TO_SLF4J_LOGGER.equals(signature) || SIG_CLASS_TO_LOG4J_LOGGER.equals(signature) - || SIG_CLASS_TO_LOG4J2_LOGGER.equals(signature) || SIG_CLASS_TO_COMMONS_LOGGER.equals(signature); - // otherwise check the calling class? - } - - /** - * looks for a variety of logging issues with log statements - * - * @throws ClassNotFoundException if the exception class, or a parent class - * can't be found - */ - @SuppressWarnings("unchecked") - private void checkForProblemsWithLoggerMethods() throws ClassNotFoundException { - String callingClsName = getClassConstantOperand(); - if (!(callingClsName.endsWith("Log") || (callingClsName.endsWith("Logger"))) || (stack.getStackDepth() == 0)) { - return; - } - String sig = getSigConstantOperand(); - if (SIG_STRING_AND_THROWABLE_TO_VOID.equals(sig) || SIG_OBJECT_AND_THROWABLE_TO_VOID.equals(sig)) { - checkForProblemsWithLoggerThrowableMethods(); - } else if (SignatureBuilder.SIG_OBJECT_TO_VOID.equals(sig)) { - checkForProblemsWithLoggerSingleArgumentMethod(); - } else if ((SLF4J_LOGGER.equals(callingClsName) || LOG4J2_LOGGER.equals(callingClsName)) - && (SignatureBuilder.SIG_STRING_TO_VOID.equals(sig) - || SignatureBuilder.SIG_STRING_AND_OBJECT_TO_VOID.equals(sig) - || SIG_STRING_AND_TWO_OBJECTS_TO_VOID.equals(sig) - || SIG_STRING_AND_OBJECT_ARRAY_TO_VOID.equals(sig))) { - checkForProblemsWithLoggerParameterisedMethods(sig); - } - } - - private void checkForProblemsWithLoggerThrowableMethods() { - if (stack.getStackDepth() < 2) { - return; - } - OpcodeStack.Item exItem = stack.getStackItem(0); - OpcodeStack.Item msgItem = stack.getStackItem(1); - - LOUserValue uv = (LOUserValue) msgItem.getUserValue(); - if ((uv != null) && (uv.getType() == LOUserValue.LOType.MESSAGE_REG) - && (uv.getValue().intValue() == exItem.getRegisterNumber())) { - bugReporter.reportBug(new BugInstance(this, BugType.LO_STUTTERED_MESSAGE.name(), NORMAL_PRIORITY) - .addClass(this).addMethod(this).addSourceLine(this)); - } else { - Object cons = msgItem.getConstant(); - if ((cons instanceof String) && ((String) cons).contains("{}")) { - bugReporter.reportBug( - new BugInstance(this, BugType.LO_INCORRECT_NUMBER_OF_ANCHOR_PARAMETERS.name(), NORMAL_PRIORITY) - .addClass(this).addMethod(this).addSourceLine(this)); - } - } - } - - private void checkForProblemsWithLoggerSingleArgumentMethod() throws ClassNotFoundException { - final JavaClass clazz = stack.getStackItem(0).getJavaClass(); - if ((clazz != null) && clazz.instanceOf(throwableClass)) { - bugReporter.reportBug( - new BugInstance(this, BugType.LO_LOGGER_LOST_EXCEPTION_STACK_TRACE.name(), NORMAL_PRIORITY) - .addClass(this).addMethod(this).addSourceLine(this)); - } - } - - private void checkForProblemsWithLoggerParameterisedMethods(String sig) { - int numParms = SignatureUtils.getNumParameters(sig); - if (stack.getStackDepth() < numParms) { - return; - } - OpcodeStack.Item formatItem = stack.getStackItem(numParms - 1); - Object con = formatItem.getConstant(); - if (con instanceof String) { - Matcher m = BAD_FORMATTING_ANCHOR.matcher((String) con); - if (m.find()) { - bugReporter - .reportBug(new BugInstance(this, BugType.LO_INVALID_FORMATTING_ANCHOR.name(), NORMAL_PRIORITY) - .addClass(this).addMethod(this).addSourceLine(this)); - } else { - m = BAD_STRING_FORMAT_PATTERN.matcher((String) con); - if (m.find()) { - OpcodeStack.Item loggerItem = stack.getStackItem(numParms); - LOUserValue loggerUV = (LOUserValue) loggerItem.getUserValue(); - if ((loggerUV == null) || (loggerUV.getType() != LOUserValue.LOType.FORMATTER_LOGGER)) { - bugReporter.reportBug( - new BugInstance(this, BugType.LO_INVALID_STRING_FORMAT_NOTATION.name(), NORMAL_PRIORITY) - .addClass(this).addMethod(this).addSourceLine(this)); - } - } else { - int actualParms = getVarArgsParmCount(sig); - if (actualParms != -1) { - int expectedParms = countAnchors((String) con); - boolean hasEx = hasExceptionOnStack(); - if ((!hasEx && (expectedParms != actualParms)) || (hasEx - && ((expectedParms != (actualParms - 1)) && (expectedParms != actualParms)))) { - bugReporter.reportBug(new BugInstance(this, - BugType.LO_INCORRECT_NUMBER_OF_ANCHOR_PARAMETERS.name(), NORMAL_PRIORITY) - .addClass(this).addMethod(this).addSourceLine(this) - .addString("Expected: " + expectedParms).addString("Actual: " + actualParms)); - } - } - } - } - } else { - LOUserValue uv = (LOUserValue) formatItem.getUserValue(); - if ((uv != null) && (uv.getType() == LOUserValue.LOType.METHOD_NAME) - && Values.TOSTRING.equals(uv.getValue())) { - - bugReporter.reportBug( - new BugInstance(this, BugType.LO_APPENDED_STRING_IN_FORMAT_STRING.name(), NORMAL_PRIORITY) - .addClass(this).addMethod(this).addSourceLine(this)); - } else { - - if ((uv != null) && (uv.getType() == LOUserValue.LOType.SIMPLE_FORMAT)) { - bugReporter.reportBug( - new BugInstance(this, BugType.LO_EMBEDDED_SIMPLE_STRING_FORMAT_IN_FORMAT_STRING.name(), - NORMAL_PRIORITY).addClass(this).addMethod(this).addSourceLine(this)); - } - } - } - - boolean foundToString = false; - for (int i = 0; i < (numParms - 1); i++) { - OpcodeStack.Item itm = stack.getStackItem(i); - LOUserValue uv = (LOUserValue) itm.getUserValue(); - foundToString = ((uv != null) && ((uv.getType() == LOUserValue.LOType.TOSTRING) - || (uv.getType() == LOUserValue.LOType.METHOD_NAME))); - if (foundToString) { - String callingClassName = (String) uv.getValue(); - if (callingClassName != null) { - try { - JavaClass cls = Repository.lookupClass(callingClassName); - if (Values.DOTTED_JAVA_LANG_EXCEPTION.equals(cls.getClassName())) { - foundToString = false; - } else { - JavaClass[] supers = cls.getSuperClasses(); - - for (JavaClass spr : supers) { - if (Values.DOTTED_JAVA_LANG_EXCEPTION.equals(spr.getClassName())) { - foundToString = false; - break; - } - } - } - } catch (ClassNotFoundException cnfe) { - bugReporter.reportMissingClass(cnfe); - } - } - if (foundToString) { - break; - } - } - } - - if (foundToString) { - bugReporter.reportBug(new BugInstance(this, BugType.LO_TOSTRING_PARAMETER.name(), NORMAL_PRIORITY) - .addClass(this).addMethod(this).addSourceLine(this)); - } - } - - /** - * looks for slf4j calls where an exception is passed as a logger parameter, - * expecting to be substituted for a {} marker. As slf4j just passes the - * exception down to the message generation itself, the {} marker will go - * unpopulated. - */ - private void checkForLoggerParam() { - if (Values.CONSTRUCTOR.equals(getNameConstantOperand())) { - String cls = getClassConstantOperand(); - if ((cls.startsWith("java/") || cls.startsWith("javax/")) && cls.endsWith("Exception")) { - String sig = getSigConstantOperand(); - List types = SignatureUtils.getParameterSignatures(sig); - if (types.size() <= stack.getStackDepth()) { - for (int i = 0; i < types.size(); i++) { - String parmSig = types.get(i); - if (Values.SIG_JAVA_LANG_STRING.equals(parmSig)) { - OpcodeStack.Item item = stack.getStackItem(types.size() - i - 1); - String cons = (String) item.getConstant(); - if ((cons != null) && cons.contains("{}")) { - bugReporter - .reportBug(new BugInstance(this, BugType.LO_EXCEPTION_WITH_LOGGER_PARMS.name(), - NORMAL_PRIORITY).addClass(this).addMethod(this).addSourceLine(this)); - break; - } - } - } - } - } - } - } - - /** - * looks for instantiation of a logger with what looks like a class name that - * isn't the same as the class in which it exists. There are some cases where a - * 'classname-like' string is presented purposely different than this class, and - * an attempt is made to ignore those. - */ - @SuppressWarnings("unchecked") - private void lookForSuspectClasses() { - String callingClsName = getClassConstantOperand(); - String mthName = getNameConstantOperand(); - - String loggingClassName = null; - int loggingPriority = NORMAL_PRIORITY; - - if ("org/slf4j/LoggerFactory".equals(callingClsName) && "getLogger".equals(mthName)) { - String signature = getSigConstantOperand(); - - if (SIG_CLASS_TO_SLF4J_LOGGER.equals(signature)) { - loggingClassName = getLoggingClassNameFromStackValue(); - } else if (SIG_STRING_TO_SLF4J_LOGGER.equals(signature) && (stack.getStackDepth() > 0)) { - OpcodeStack.Item item = stack.getStackItem(0); - loggingClassName = (String) item.getConstant(); - loggingPriority = LOW_PRIORITY; - } - } else if ((LOG4J_LOGGER.equals(callingClsName) || LOG4J2_LOGMANAGER.equals(callingClsName)) - && "getLogger".equals(mthName)) { - String signature = getSigConstantOperand(); - - if (SIG_CLASS_TO_LOG4J_LOGGER.equals(signature) || SIG_CLASS_TO_LOG4J2_LOGGER.equals(signature)) { - loggingClassName = getLoggingClassNameFromStackValue(); - } else if (SIG_STRING_TO_LOG4J_LOGGER.equals(signature) || SIG_STRING_TO_LOG4J2_LOGGER.equals(signature)) { - if (stack.getStackDepth() > 0) { - OpcodeStack.Item item = stack.getStackItem(0); - loggingClassName = (String) item.getConstant(); - LOUserValue uv = (LOUserValue) item.getUserValue(); - if (uv != null) { - Object userValue = uv.getValue(); - - if (loggingClassName != null) { - // first look at the constant passed in - loggingPriority = LOW_PRIORITY; - } else if (userValue instanceof String) { - // try the user value, which may have been set by a call - // to Foo.class.getName() - loggingClassName = (String) userValue; - } - } else { - return; - } - } - } else if (SIG_STRING_AND_FACTORY_TO_LOG4J_LOGGER.equals(signature) && (stack.getStackDepth() > 1)) { - OpcodeStack.Item item = stack.getStackItem(1); - loggingClassName = (String) item.getConstant(); - loggingPriority = LOW_PRIORITY; - } - } else if ("org/apache/commons/logging/LogFactory".equals(callingClsName) && "getLog".equals(mthName)) { - String signature = getSigConstantOperand(); - - if (SIG_CLASS_TO_COMMONS_LOGGER.equals(signature)) { - loggingClassName = getLoggingClassNameFromStackValue(); - } else if (SIG_STRING_TO_COMMONS_LOGGER.equals(signature) && (stack.getStackDepth() > 0)) { - OpcodeStack.Item item = stack.getStackItem(0); - loggingClassName = (String) item.getConstant(); - loggingPriority = LOW_PRIORITY; - } - } - - if (loggingClassName != null) { - loggingClassName = loggingClassName.replace('/', '.'); - if ((stack.getStackDepth() > 0) - && !loggingClassName.equals(SignatureUtils.getNonAnonymousPortion(nameOfThisClass))) { - bugReporter.reportBug(new BugInstance(this, BugType.LO_SUSPECT_LOG_CLASS.name(), loggingPriority) - .addClass(this).addMethod(this).addSourceLine(this).addString(loggingClassName) - .addString(nameOfThisClass)); - } - } - } - - @Nullable - private String getLoggingClassNameFromStackValue() { - if (stack.getStackDepth() > 0) { - OpcodeStack.Item item = stack.getStackItem(0); - LOUserValue uv = (LOUserValue) item.getUserValue(); - if ((uv != null) && (uv.getType() == LOUserValue.LOType.CLASS_NAME)) { - return uv.getValue(); - } - } - return null; - } - - /** - * returns the number of anchors {} in a string - * - * @param formatString the format string - * @return the number of anchors - */ - private static int countAnchors(String formatString) { - Matcher m = FORMATTER_ANCHOR.matcher(formatString); - int count = 0; - int start = 0; - while (m.find(start)) { - ++count; - start = m.end(); - } - - return count; - } - - /** - * returns the number of parameters slf4j or log4j2 is expecting to inject into - * the format string - * - * @param signature the method signature of the error, warn, info, debug - * statement - * @return the number of expected parameters - */ - @SuppressWarnings("unchecked") - private int getVarArgsParmCount(String signature) { - if (SignatureBuilder.SIG_STRING_AND_OBJECT_TO_VOID.equals(signature)) { - return 1; - } - if (SIG_STRING_AND_TWO_OBJECTS_TO_VOID.equals(signature)) { - return 2; - } - - OpcodeStack.Item item = stack.getStackItem(0); - LOUserValue uv = (LOUserValue) item.getUserValue(); - if ((uv != null) && (uv.getType() == LOUserValue.LOType.ARRAY_SIZE)) { - Integer size = uv.getValue(); - if (size != null) { - return Math.abs(size.intValue()); - } - } - return -1; - } - - /** - * returns whether an exception object is on the stack slf4j will find this, and - * not include it in the parm list so i we find one, just don't report - * - * @return whether or not an exception i present - */ - @SuppressWarnings("unchecked") - private boolean hasExceptionOnStack() { - try { - for (int i = 0; i < (stack.getStackDepth() - 1); i++) { - OpcodeStack.Item item = stack.getStackItem(i); - String sig = item.getSignature(); - if (sig.startsWith(Values.SIG_QUALIFIED_CLASS_PREFIX)) { - String name = SignatureUtils.stripSignature(sig); - JavaClass cls = Repository.lookupClass(name); - if (cls.instanceOf(throwableClass)) { - return true; - } - } else if (sig.startsWith(Values.SIG_ARRAY_PREFIX)) { - LOUserValue uv = (LOUserValue) item.getUserValue(); - if ((uv != null) && (uv.getType() == LOUserValue.LOType.ARRAY_SIZE)) { - Integer sz = uv.getValue(); - if ((sz != null) && (sz.intValue() < 0)) { - return true; - } - } - } - } - return false; - } catch (ClassNotFoundException cnfe) { - bugReporter.reportMissingClass(cnfe); - return true; - } - } - - static class LOUserValue { - enum LOType { - CLASS_NAME, METHOD_NAME, MESSAGE_REG, ARRAY_SIZE, SIMPLE_FORMAT, TOSTRING, FORMATTER_LOGGER, NULL - }; - - LOType type; - T value; - - public LOUserValue(LOType type, T value) { - this.type = type; - this.value = value; - } - - public LOType getType() { - return type; - } - - public T getValue() { - return value; - } - - @Override - public String toString() { - return ToString.build(this); - } - } + private static final Set LOGGER_METHODS = UnmodifiableSet.create("trace", "debug", "info", "warn", "error", + "fatal"); + private static final String COMMONS_LOGGER = "org/apache/commons/logging/Log"; + private static final String LOG4J_LOGGER = "org/apache/log4j/Logger"; + private static final String LOG4J2_LOGGER = "org/apache/logging/log4j/Logger"; + private static final String LOG4J2_LOGMANAGER = "org/apache/logging/log4j/LogManager"; + private static final String SLF4J_LOGGER = "org/slf4j/Logger"; + private static final String SIG_STRING_AND_TWO_OBJECTS_TO_VOID = new SignatureBuilder() + .withParamTypes(Values.SLASHED_JAVA_LANG_STRING, Values.SLASHED_JAVA_LANG_OBJECT, + Values.SLASHED_JAVA_LANG_OBJECT) + .toString(); + private static final String SIG_STRING_AND_OBJECT_ARRAY_TO_VOID = new SignatureBuilder() + .withParamTypes(Values.SLASHED_JAVA_LANG_STRING, SignatureBuilder.SIG_OBJECT_ARRAY).toString(); + private static final String SIG_OBJECT_AND_THROWABLE_TO_VOID = new SignatureBuilder() + .withParamTypes(Values.SLASHED_JAVA_LANG_OBJECT, Values.SLASHED_JAVA_LANG_THROWABLE).toString(); + private static final String SIG_STRING_AND_THROWABLE_TO_VOID = new SignatureBuilder() + .withParamTypes(Values.SLASHED_JAVA_LANG_STRING, Values.SLASHED_JAVA_LANG_THROWABLE).toString(); + private static final String SIG_CLASS_TO_COMMONS_LOGGER = new SignatureBuilder() + .withParamTypes(Values.SLASHED_JAVA_LANG_CLASS).withReturnType(COMMONS_LOGGER).toString(); + private static final String SIG_CLASS_TO_LOG4J_LOGGER = new SignatureBuilder() + .withParamTypes(Values.SLASHED_JAVA_LANG_CLASS).withReturnType(LOG4J_LOGGER).toString(); + private static final String SIG_CLASS_TO_LOG4J2_LOGGER = new SignatureBuilder() + .withParamTypes(Values.SLASHED_JAVA_LANG_CLASS).withReturnType(LOG4J2_LOGGER).toString(); + private static final String SIG_CLASS_TO_SLF4J_LOGGER = new SignatureBuilder() + .withParamTypes(Values.SLASHED_JAVA_LANG_CLASS).withReturnType(SLF4J_LOGGER).toString(); + private static final String SIG_STRING_TO_COMMONS_LOGGER = new SignatureBuilder() + .withParamTypes(Values.SLASHED_JAVA_LANG_STRING).withReturnType(COMMONS_LOGGER).toString(); + private static final String SIG_STRING_TO_LOG4J_LOGGER = new SignatureBuilder() + .withParamTypes(Values.SLASHED_JAVA_LANG_STRING).withReturnType(LOG4J_LOGGER).toString(); + private static final String SIG_STRING_TO_LOG4J2_LOGGER = new SignatureBuilder() + .withParamTypes(Values.SLASHED_JAVA_LANG_STRING).withReturnType(LOG4J2_LOGGER).toString(); + private static final String SIG_STRING_TO_SLF4J_LOGGER = new SignatureBuilder() + .withParamTypes(Values.SLASHED_JAVA_LANG_STRING).withReturnType(SLF4J_LOGGER).toString(); + private static final String SIG_STRING_AND_FACTORY_TO_LOG4J_LOGGER = new SignatureBuilder() + .withParamTypes(Values.SLASHED_JAVA_LANG_STRING, "org/apache/log4j/spi/LoggerFactory") + .withReturnType(LOG4J_LOGGER).toString(); + + private static final Pattern BAD_FORMATTING_ANCHOR = Pattern.compile("\\{[0-9]\\}"); + private static final Pattern BAD_STRING_FORMAT_PATTERN = Pattern + .compile("(? formatterLoggers; + private JavaClass throwableClass; + private OpcodeStack stack; + private String nameOfThisClass; + private boolean isStaticInitializer; + + /** + * constructs a LO detector given the reporter to report bugs on. + * + * @param bugReporter the sync of bug reports + */ + public LoggerOddities(final BugReporter bugReporter) { + this.bugReporter = bugReporter; + + try { + throwableClass = Repository.lookupClass(Values.SLASHED_JAVA_LANG_THROWABLE); + } catch (ClassNotFoundException cnfe) { + bugReporter.reportMissingClass(cnfe); + } + + } + + /** + * implements the visitor to discover what the class name is if it is a normal + * class, or the owning class, if the class is an anonymous class. + * + * @param classContext the context object of the currently parsed class + */ + @Override + public void visitClassContext(ClassContext classContext) { + try { + stack = new OpcodeStack(); + nameOfThisClass = SignatureUtils.getNonAnonymousPortion(classContext.getJavaClass().getClassName()); + formatterLoggers = new HashSet<>(); + super.visitClassContext(classContext); + } finally { + formatterLoggers = null; + stack = null; + } + } + + /** + * implements the visitor to reset the stack + * + * @param obj the context object of the currently parsed code block + */ + @Override + public void visitCode(Code obj) { + stack.resetForMethodEntry(this); + Method m = getMethod(); + if (Values.CONSTRUCTOR.equals(m.getName())) { + for (String parmSig : SignatureUtils.getParameterSignatures(m.getSignature())) { + if (SignatureUtils.classToSignature(SLF4J_LOGGER).equals(parmSig) + || SignatureUtils.classToSignature(LOG4J_LOGGER).equals(parmSig) + || SignatureUtils.classToSignature(LOG4J2_LOGGER).equals(parmSig) + || SignatureUtils.classToSignature(COMMONS_LOGGER).equals(parmSig)) { + bugReporter + .reportBug(new BugInstance(this, BugType.LO_SUSPECT_LOG_PARAMETER.name(), NORMAL_PRIORITY) + .addClass(this).addMethod(this)); + } + } + } + + isStaticInitializer = Values.STATIC_INITIALIZER.equals(m.getName()); + super.visitCode(obj); + } + + /** + * implements the visitor to look for calls to Logger.getLogger with the wrong + * class name + * + * @param seen the opcode of the currently parsed instruction + */ + @Override + @SuppressWarnings("unchecked") + public void sawOpcode(int seen) { + String ldcClassName = null; + String seenMethodName = null; + String callingClsName = null; + boolean seenToString = false; + boolean seenFormatterLogger = false; + int exMessageReg = -1; + Integer arraySize = null; + boolean simpleFormat = false; + + try { + stack.precomputation(this); + + if ((seen == Const.LDC) || (seen == Const.LDC_W)) { + Constant c = getConstantRefOperand(); + if (c instanceof ConstantClass) { + ConstantPool pool = getConstantPool(); + ldcClassName = ((ConstantUtf8) pool.getConstant(((ConstantClass) c).getNameIndex())).getBytes(); + } + } else if (seen == Const.INVOKESTATIC) { + lookForSuspectClasses(); + + String clsName = getClassConstantOperand(); + String methodName = getNameConstantOperand(); + if (Values.SLASHED_JAVA_LANG_STRING.equals(clsName) && "format".equals(methodName) + && (stack.getStackDepth() >= 2)) { + String format = (String) stack.getStackItem(1).getConstant(); + if (format != null) { + Matcher m = NON_SIMPLE_FORMAT.matcher(format); + if (!m.matches()) { + simpleFormat = true; + } + } + } else if ("getFormatterLogger".equals(methodName) && LOG4J2_LOGMANAGER.equals(clsName)) { + seenFormatterLogger = true; + } + } else if (((seen == Const.INVOKEVIRTUAL) || (seen == Const.INVOKEINTERFACE)) && (throwableClass != null)) { + String mthName = getNameConstantOperand(); + if ("getName".equals(mthName)) { + if (stack.getStackDepth() >= 1) { + // Foo.class.getName() is being called, so we pass the + // name of the class to the current top of the stack + // (the name of the class is currently on the top of the + // stack, but won't be on the stack at all next opcode) + Item stackItem = stack.getStackItem(0); + LOUserValue uv = (LOUserValue) stackItem.getUserValue(); + if ((uv != null) && (uv.getType() == LOUserValue.LOType.CLASS_NAME)) { + ldcClassName = uv.getValue(); + } + } + } else if ("getMessage".equals(mthName)) { + callingClsName = getClassConstantOperand(); + JavaClass cls = Repository.lookupClass(callingClsName); + if (cls.instanceOf(throwableClass) && (stack.getStackDepth() > 0)) { + OpcodeStack.Item exItem = stack.getStackItem(0); + exMessageReg = exItem.getRegisterNumber(); + } + } else if (LOGGER_METHODS.contains(mthName)) { + checkForProblemsWithLoggerMethods(); + } else if (Values.TOSTRING.equals(mthName) + && SignatureBuilder.SIG_VOID_TO_STRING.equals(getSigConstantOperand())) { + callingClsName = getClassConstantOperand(); + if (SignatureUtils.isPlainStringConvertableClass(callingClsName) && (stack.getStackDepth() > 0)) { + OpcodeStack.Item item = stack.getStackItem(0); + // if the stringbuilder was previously stored, don't report it + if (item.getRegisterNumber() < 0) { + seenMethodName = mthName; + } + } + + if (seenMethodName == null) { + seenToString = true; + } + } + } else if (seen == Const.INVOKESPECIAL) { + checkForLoggerParam(); + } else if (seen == Const.ANEWARRAY) { + if (stack.getStackDepth() > 0) { + OpcodeStack.Item sizeItem = stack.getStackItem(0); + Object con = sizeItem.getConstant(); + if (con instanceof Integer) { + arraySize = (Integer) con; + } + } + } else if (seen == Const.AASTORE) { + if (stack.getStackDepth() >= 3) { + OpcodeStack.Item arrayItem = stack.getStackItem(2); + LOUserValue uv = (LOUserValue) arrayItem.getUserValue(); + if ((uv != null) && (uv.getType() == LOUserValue.LOType.ARRAY_SIZE)) { + Integer size = uv.getValue(); + if ((size != null) && (size.intValue() > 0) && hasExceptionOnStack()) { + arrayItem.setUserValue(new LOUserValue<>(LOUserValue.LOType.ARRAY_SIZE, + Integer.valueOf(-size.intValue()))); + } + } + } + } else if (seen == Const.PUTSTATIC) { + OpcodeStack.Item itm = stack.getStackItem(0); + if (isStaticInitializer && isNonPrivateLogField(getClassConstantOperand(), getNameConstantOperand(), + getSigConstantOperand())) { + XMethod m = itm.getReturnValueOf(); + if ((m != null) && isLoggerWithClassParm(m)) { + bugReporter.reportBug( + new BugInstance(this, BugType.LO_NON_PRIVATE_STATIC_LOGGER.name(), NORMAL_PRIORITY) + .addClass(this).addMethod(this).addSourceLine(this)); + } + } + + LOUserValue loggerUV = (LOUserValue) itm.getUserValue(); + if ((loggerUV != null) && (loggerUV.getType() == LOUserValue.LOType.FORMATTER_LOGGER)) { + formatterLoggers.add(getNameConstantOperand()); + } + } else if (seen == Const.GETSTATIC) { + if (formatterLoggers.contains(getNameConstantOperand())) { + seenFormatterLogger = true; + } + } else if (OpcodeUtils.isAStore(seen) && (stack.getStackDepth() > 0)) { + OpcodeStack.Item item = stack.getStackItem(0); + LOUserValue uv = (LOUserValue) item.getUserValue(); + if (uv != null) { + if (((uv.getType() == LOUserValue.LOType.METHOD_NAME) && Values.TOSTRING.equals(uv.getValue())) + || (uv.getType() == LOUserValue.LOType.SIMPLE_FORMAT) + || (uv.getType() == LOUserValue.LOType.TOSTRING)) { + item.setUserValue(new LOUserValue<>(LOUserValue.LOType.NULL, null)); + } + } + } + } catch (ClassNotFoundException cnfe) { + bugReporter.reportMissingClass(cnfe); + } finally { + TernaryPatcher.pre(stack, seen); + stack.sawOpcode(this, seen); + TernaryPatcher.post(stack, seen); + + if (stack.getStackDepth() > 0) { + OpcodeStack.Item item = stack.getStackItem(0); + if (ldcClassName != null) { + item.setUserValue(new LOUserValue<>(LOUserValue.LOType.CLASS_NAME, ldcClassName)); + } else if (seenMethodName != null) { + item.setUserValue(new LOUserValue<>(LOUserValue.LOType.METHOD_NAME, seenMethodName)); + } else if (exMessageReg >= 0) { + item.setUserValue(new LOUserValue<>(LOUserValue.LOType.MESSAGE_REG, Integer.valueOf(exMessageReg))); + } else if (arraySize != null) { + item.setUserValue(new LOUserValue<>(LOUserValue.LOType.ARRAY_SIZE, arraySize)); + } else if (simpleFormat) { + item.setUserValue(new LOUserValue<>(LOUserValue.LOType.SIMPLE_FORMAT, Boolean.TRUE)); + } else if (seenToString) { + item.setUserValue(new LOUserValue<>(LOUserValue.LOType.TOSTRING, callingClsName)); + } else if (seenFormatterLogger) { + item.setUserValue(new LOUserValue<>(LOUserValue.LOType.FORMATTER_LOGGER, null)); + } + } + } + } + + /** + * looks to see if this field is a logger, and declared non privately + * + * @param fieldClsName the owning class type of the field + * @param fieldName the name of the field + * @param fieldSig the signature of the field + * @return if the field is a logger and not private + */ + private boolean isNonPrivateLogField(@SlashedClassName String fieldClsName, String fieldName, String fieldSig) { + + String fieldType = SignatureUtils.trimSignature(fieldSig); + if (!SLF4J_LOGGER.equals(fieldType) && !COMMONS_LOGGER.equals(fieldType) && !LOG4J_LOGGER.equals(fieldType) + && !LOG4J2_LOGGER.equals(fieldType)) { + return false; + } + + JavaClass cls = getClassContext().getJavaClass(); + if (!cls.getClassName().equals(fieldClsName.replace('/', '.'))) { + return false; + } + + for (Field f : getClassContext().getJavaClass().getFields()) { + if (f.getName().equals(fieldName)) { + return !f.isPrivate(); + } + } + + return true; + } + + /** + * returns whether this method class is a standard logger instantiation that + * takes a java/lang/Class parameter + * + * @param m the method to check + * @return if the method is a logger factory method that takes a Class object + */ + private boolean isLoggerWithClassParm(XMethod m) { + String signature = m.getSignature(); + return SIG_CLASS_TO_SLF4J_LOGGER.equals(signature) || SIG_CLASS_TO_LOG4J_LOGGER.equals(signature) + || SIG_CLASS_TO_LOG4J2_LOGGER.equals(signature) || SIG_CLASS_TO_COMMONS_LOGGER.equals(signature); + // otherwise check the calling class? + } + + /** + * looks for a variety of logging issues with log statements + * + * @throws ClassNotFoundException if the exception class, or a parent class + * can't be found + */ + @SuppressWarnings("unchecked") + private void checkForProblemsWithLoggerMethods() throws ClassNotFoundException { + String callingClsName = getClassConstantOperand(); + if (!(callingClsName.endsWith("Log") || (callingClsName.endsWith("Logger"))) || (stack.getStackDepth() == 0)) { + return; + } + String sig = getSigConstantOperand(); + if (SIG_STRING_AND_THROWABLE_TO_VOID.equals(sig) || SIG_OBJECT_AND_THROWABLE_TO_VOID.equals(sig)) { + checkForProblemsWithLoggerThrowableMethods(); + } else if (SignatureBuilder.SIG_OBJECT_TO_VOID.equals(sig)) { + checkForProblemsWithLoggerSingleArgumentMethod(); + if (SLF4J_LOGGER.equals(callingClsName) || LOG4J2_LOGGER.equals(callingClsName)) { + checkForProblemsWithLoggerParameterisedMethods(sig); + } + } else if ((SLF4J_LOGGER.equals(callingClsName) || LOG4J2_LOGGER.equals(callingClsName)) + && (SignatureBuilder.SIG_STRING_TO_VOID.equals(sig) + || SignatureBuilder.SIG_STRING_AND_OBJECT_TO_VOID.equals(sig) + || SIG_STRING_AND_TWO_OBJECTS_TO_VOID.equals(sig) + || SIG_STRING_AND_OBJECT_ARRAY_TO_VOID.equals(sig))) { + checkForProblemsWithLoggerParameterisedMethods(sig); + } + } + + private void checkForProblemsWithLoggerThrowableMethods() { + if (stack.getStackDepth() < 2) { + return; + } + OpcodeStack.Item exItem = stack.getStackItem(0); + OpcodeStack.Item msgItem = stack.getStackItem(1); + + LOUserValue uv = (LOUserValue) msgItem.getUserValue(); + if ((uv != null) && (uv.getType() == LOUserValue.LOType.MESSAGE_REG) + && (uv.getValue().intValue() == exItem.getRegisterNumber())) { + bugReporter.reportBug(new BugInstance(this, BugType.LO_STUTTERED_MESSAGE.name(), NORMAL_PRIORITY) + .addClass(this).addMethod(this).addSourceLine(this)); + } else { + Object cons = msgItem.getConstant(); + if ((cons instanceof String) && ((String) cons).contains("{}")) { + bugReporter.reportBug( + new BugInstance(this, BugType.LO_INCORRECT_NUMBER_OF_ANCHOR_PARAMETERS.name(), NORMAL_PRIORITY) + .addClass(this).addMethod(this).addSourceLine(this)); + } + } + } + + private void checkForProblemsWithLoggerSingleArgumentMethod() throws ClassNotFoundException { + final JavaClass clazz = stack.getStackItem(0).getJavaClass(); + if ((clazz != null) && clazz.instanceOf(throwableClass)) { + bugReporter.reportBug( + new BugInstance(this, BugType.LO_LOGGER_LOST_EXCEPTION_STACK_TRACE.name(), NORMAL_PRIORITY) + .addClass(this).addMethod(this).addSourceLine(this)); + } + } + + private void checkForProblemsWithLoggerParameterisedMethods(String sig) { + int numParms = SignatureUtils.getNumParameters(sig); + if (stack.getStackDepth() < numParms) { + return; + } + OpcodeStack.Item formatItem = stack.getStackItem(numParms - 1); + Object con = formatItem.getConstant(); + if (con instanceof String) { + Matcher m = BAD_FORMATTING_ANCHOR.matcher((String) con); + if (m.find()) { + bugReporter + .reportBug(new BugInstance(this, BugType.LO_INVALID_FORMATTING_ANCHOR.name(), NORMAL_PRIORITY) + .addClass(this).addMethod(this).addSourceLine(this)); + } else { + m = BAD_STRING_FORMAT_PATTERN.matcher((String) con); + if (m.find()) { + OpcodeStack.Item loggerItem = stack.getStackItem(numParms); + LOUserValue loggerUV = (LOUserValue) loggerItem.getUserValue(); + if ((loggerUV == null) || (loggerUV.getType() != LOUserValue.LOType.FORMATTER_LOGGER)) { + bugReporter.reportBug( + new BugInstance(this, BugType.LO_INVALID_STRING_FORMAT_NOTATION.name(), NORMAL_PRIORITY) + .addClass(this).addMethod(this).addSourceLine(this)); + } + } else { + int actualParms = getVarArgsParmCount(sig); + if (actualParms != -1) { + int expectedParms = countAnchors((String) con); + boolean hasEx = hasExceptionOnStack(); + if ((!hasEx && (expectedParms != actualParms)) || (hasEx + && ((expectedParms != (actualParms - 1)) && (expectedParms != actualParms)))) { + bugReporter.reportBug(new BugInstance(this, + BugType.LO_INCORRECT_NUMBER_OF_ANCHOR_PARAMETERS.name(), NORMAL_PRIORITY) + .addClass(this).addMethod(this).addSourceLine(this) + .addString("Expected: " + expectedParms).addString("Actual: " + actualParms)); + } + } + } + } + } else { + LOUserValue uv = (LOUserValue) formatItem.getUserValue(); + if ((uv != null) && (uv.getType() == LOUserValue.LOType.METHOD_NAME) + && Values.TOSTRING.equals(uv.getValue())) { + + bugReporter.reportBug( + new BugInstance(this, BugType.LO_APPENDED_STRING_IN_FORMAT_STRING.name(), NORMAL_PRIORITY) + .addClass(this).addMethod(this).addSourceLine(this)); + } else { + + if ((uv != null) && (uv.getType() == LOUserValue.LOType.SIMPLE_FORMAT)) { + bugReporter.reportBug( + new BugInstance(this, BugType.LO_EMBEDDED_SIMPLE_STRING_FORMAT_IN_FORMAT_STRING.name(), + NORMAL_PRIORITY).addClass(this).addMethod(this).addSourceLine(this)); + } + } + } + + boolean foundToString = false; + for (int i = 0; i < (numParms - 1); i++) { + OpcodeStack.Item itm = stack.getStackItem(i); + LOUserValue uv = (LOUserValue) itm.getUserValue(); + foundToString = ((uv != null) && ((uv.getType() == LOUserValue.LOType.TOSTRING) + || (uv.getType() == LOUserValue.LOType.METHOD_NAME))); + if (foundToString) { + String callingClassName = (String) uv.getValue(); + if (callingClassName != null) { + try { + JavaClass cls = Repository.lookupClass(callingClassName); + if (Values.DOTTED_JAVA_LANG_EXCEPTION.equals(cls.getClassName())) { + foundToString = false; + } else { + JavaClass[] supers = cls.getSuperClasses(); + + for (JavaClass spr : supers) { + if (Values.DOTTED_JAVA_LANG_EXCEPTION.equals(spr.getClassName())) { + foundToString = false; + break; + } + } + } + } catch (ClassNotFoundException cnfe) { + bugReporter.reportMissingClass(cnfe); + } + } + if (foundToString) { + break; + } + } + } + + if (foundToString) { + bugReporter.reportBug(new BugInstance(this, BugType.LO_TOSTRING_PARAMETER.name(), NORMAL_PRIORITY) + .addClass(this).addMethod(this).addSourceLine(this)); + } + } + + /** + * looks for slf4j calls where an exception is passed as a logger parameter, + * expecting to be substituted for a {} marker. As slf4j just passes the + * exception down to the message generation itself, the {} marker will go + * unpopulated. + */ + private void checkForLoggerParam() { + if (Values.CONSTRUCTOR.equals(getNameConstantOperand())) { + String cls = getClassConstantOperand(); + if ((cls.startsWith("java/") || cls.startsWith("javax/")) && cls.endsWith("Exception")) { + String sig = getSigConstantOperand(); + List types = SignatureUtils.getParameterSignatures(sig); + if (types.size() <= stack.getStackDepth()) { + for (int i = 0; i < types.size(); i++) { + String parmSig = types.get(i); + if (Values.SIG_JAVA_LANG_STRING.equals(parmSig)) { + OpcodeStack.Item item = stack.getStackItem(types.size() - i - 1); + String cons = (String) item.getConstant(); + if ((cons != null) && cons.contains("{}")) { + bugReporter + .reportBug(new BugInstance(this, BugType.LO_EXCEPTION_WITH_LOGGER_PARMS.name(), + NORMAL_PRIORITY).addClass(this).addMethod(this).addSourceLine(this)); + break; + } + } + } + } + } + } + } + + /** + * looks for instantiation of a logger with what looks like a class name that + * isn't the same as the class in which it exists. There are some cases where a + * 'classname-like' string is presented purposely different than this class, and + * an attempt is made to ignore those. + */ + @SuppressWarnings("unchecked") + private void lookForSuspectClasses() { + String callingClsName = getClassConstantOperand(); + String mthName = getNameConstantOperand(); + + String loggingClassName = null; + int loggingPriority = NORMAL_PRIORITY; + + if ("org/slf4j/LoggerFactory".equals(callingClsName) && "getLogger".equals(mthName)) { + String signature = getSigConstantOperand(); + + if (SIG_CLASS_TO_SLF4J_LOGGER.equals(signature)) { + loggingClassName = getLoggingClassNameFromStackValue(); + } else if (SIG_STRING_TO_SLF4J_LOGGER.equals(signature) && (stack.getStackDepth() > 0)) { + OpcodeStack.Item item = stack.getStackItem(0); + loggingClassName = (String) item.getConstant(); + loggingPriority = LOW_PRIORITY; + } + } else if ((LOG4J_LOGGER.equals(callingClsName) || LOG4J2_LOGMANAGER.equals(callingClsName)) + && "getLogger".equals(mthName)) { + String signature = getSigConstantOperand(); + + if (SIG_CLASS_TO_LOG4J_LOGGER.equals(signature) || SIG_CLASS_TO_LOG4J2_LOGGER.equals(signature)) { + loggingClassName = getLoggingClassNameFromStackValue(); + } else if (SIG_STRING_TO_LOG4J_LOGGER.equals(signature) || SIG_STRING_TO_LOG4J2_LOGGER.equals(signature)) { + if (stack.getStackDepth() > 0) { + OpcodeStack.Item item = stack.getStackItem(0); + loggingClassName = (String) item.getConstant(); + LOUserValue uv = (LOUserValue) item.getUserValue(); + if (uv != null) { + Object userValue = uv.getValue(); + + if (loggingClassName != null) { + // first look at the constant passed in + loggingPriority = LOW_PRIORITY; + } else if (userValue instanceof String) { + // try the user value, which may have been set by a call + // to Foo.class.getName() + loggingClassName = (String) userValue; + } + } else { + return; + } + } + } else if (SIG_STRING_AND_FACTORY_TO_LOG4J_LOGGER.equals(signature) && (stack.getStackDepth() > 1)) { + OpcodeStack.Item item = stack.getStackItem(1); + loggingClassName = (String) item.getConstant(); + loggingPriority = LOW_PRIORITY; + } + } else if ("org/apache/commons/logging/LogFactory".equals(callingClsName) && "getLog".equals(mthName)) { + String signature = getSigConstantOperand(); + + if (SIG_CLASS_TO_COMMONS_LOGGER.equals(signature)) { + loggingClassName = getLoggingClassNameFromStackValue(); + } else if (SIG_STRING_TO_COMMONS_LOGGER.equals(signature) && (stack.getStackDepth() > 0)) { + OpcodeStack.Item item = stack.getStackItem(0); + loggingClassName = (String) item.getConstant(); + loggingPriority = LOW_PRIORITY; + } + } + + if (loggingClassName != null) { + loggingClassName = loggingClassName.replace('/', '.'); + if ((stack.getStackDepth() > 0) + && !loggingClassName.equals(SignatureUtils.getNonAnonymousPortion(nameOfThisClass))) { + bugReporter.reportBug(new BugInstance(this, BugType.LO_SUSPECT_LOG_CLASS.name(), loggingPriority) + .addClass(this).addMethod(this).addSourceLine(this).addString(loggingClassName) + .addString(nameOfThisClass)); + } + } + } + + @Nullable + private String getLoggingClassNameFromStackValue() { + if (stack.getStackDepth() > 0) { + OpcodeStack.Item item = stack.getStackItem(0); + LOUserValue uv = (LOUserValue) item.getUserValue(); + if ((uv != null) && (uv.getType() == LOUserValue.LOType.CLASS_NAME)) { + return uv.getValue(); + } + } + return null; + } + + /** + * returns the number of anchors {} in a string + * + * @param formatString the format string + * @return the number of anchors + */ + private static int countAnchors(String formatString) { + Matcher m = FORMATTER_ANCHOR.matcher(formatString); + int count = 0; + int start = 0; + while (m.find(start)) { + ++count; + start = m.end(); + } + + return count; + } + + /** + * returns the number of parameters slf4j or log4j2 is expecting to inject into + * the format string + * + * @param signature the method signature of the error, warn, info, debug + * statement + * @return the number of expected parameters + */ + @SuppressWarnings("unchecked") + private int getVarArgsParmCount(String signature) { + if (SignatureBuilder.SIG_STRING_AND_OBJECT_TO_VOID.equals(signature)) { + return 1; + } + if (SIG_STRING_AND_TWO_OBJECTS_TO_VOID.equals(signature)) { + return 2; + } + + OpcodeStack.Item item = stack.getStackItem(0); + LOUserValue uv = (LOUserValue) item.getUserValue(); + if ((uv != null) && (uv.getType() == LOUserValue.LOType.ARRAY_SIZE)) { + Integer size = uv.getValue(); + if (size != null) { + return Math.abs(size.intValue()); + } + } + return -1; + } + + /** + * returns whether an exception object is on the stack slf4j will find this, and + * not include it in the parm list so i we find one, just don't report + * + * @return whether or not an exception i present + */ + @SuppressWarnings("unchecked") + private boolean hasExceptionOnStack() { + try { + for (int i = 0; i < (stack.getStackDepth() - 1); i++) { + OpcodeStack.Item item = stack.getStackItem(i); + String sig = item.getSignature(); + if (sig.startsWith(Values.SIG_QUALIFIED_CLASS_PREFIX)) { + String name = SignatureUtils.stripSignature(sig); + JavaClass cls = Repository.lookupClass(name); + if (cls.instanceOf(throwableClass)) { + return true; + } + } else if (sig.startsWith(Values.SIG_ARRAY_PREFIX)) { + LOUserValue uv = (LOUserValue) item.getUserValue(); + if ((uv != null) && (uv.getType() == LOUserValue.LOType.ARRAY_SIZE)) { + Integer sz = uv.getValue(); + if ((sz != null) && (sz.intValue() < 0)) { + return true; + } + } + } + } + return false; + } catch (ClassNotFoundException cnfe) { + bugReporter.reportMissingClass(cnfe); + return true; + } + } + + static class LOUserValue { + enum LOType { + CLASS_NAME, METHOD_NAME, MESSAGE_REG, ARRAY_SIZE, SIMPLE_FORMAT, TOSTRING, FORMATTER_LOGGER, NULL + }; + + LOType type; + T value; + + public LOUserValue(LOType type, T value) { + this.type = type; + this.value = value; + } + + public LOType getType() { + return type; + } + + public T getValue() { + return value; + } + + @Override + public String toString() { + return ToString.build(this); + } + } } diff --git a/src/samples/java/ex/LO_Sample.java b/src/samples/java/ex/LO_Sample.java index 7f4a23b9..1eaec741 100755 --- a/src/samples/java/ex/LO_Sample.java +++ b/src/samples/java/ex/LO_Sample.java @@ -10,423 +10,435 @@ import javax.swing.text.DateFormatter; +import ex.LO_Sample.Log4j.FunnyToString; + @SuppressWarnings("all") public class LO_Sample { - public static class Slf4j { - // tag LO_SUSPECT_LOG_CLASS - private static org.slf4j.Logger l1 = org.slf4j.LoggerFactory.getLogger(String.class); - // no tag - private static org.slf4j.Logger l2 = org.slf4j.LoggerFactory.getLogger("com.foo.LO_Sample"); - // no tag - private static final org.slf4j.Logger l3 = org.slf4j.LoggerFactory.getLogger(LO_Sample.class); - // tag LO_SUSPECT_LOG_CLASS - private static org.slf4j.Logger l4 = org.slf4j.LoggerFactory.getLogger(ActionEvent.class.getName()); - // no tag - private static org.slf4j.Logger l5 = org.slf4j.LoggerFactory.getLogger(LO_Sample.class.getName()); - // no tag - private static org.slf4j.Logger l6 = org.slf4j.LoggerFactory.getLogger("my.nasty.logger.LOGGER"); - - public static org.slf4j.Logger l7 = org.slf4j.LoggerFactory.getLogger(Slf4j.class); - - // no tag - private org.slf4j.Logger someLocalLogger; - - // no tag - public Slf4j() { - someLocalLogger.info("Why am I using a local logger?"); - } - - // tag LO_SUSPECT_LOG_PARAMETER - public Slf4j(org.slf4j.Logger someLogger) { - this.someLocalLogger = someLogger; - } - - public void testStutter() throws IOException { - InputStream is = null; - try { - File f = new File("Foo"); - is = new FileInputStream(f); - } catch (Exception e) { - // tag LO_STUTTERED_MESSAGE - l1.error(e.getMessage(), e); - } finally { - is.close(); - } - } - - public void testParmInExMessage() throws Exception { - try { - InputStream is = new FileInputStream("foo/bar"); - } catch (IOException e) { - // tag LO_EXCEPTION_WITH_LOGGER_PARMS - throw new Exception("Failed to parse {}", e); - } - } - - public void testInvalidSLF4jParm() { - // tag LO_INVALID_FORMATTING_ANCHOR - l3.error("This is a problem {0}", "hello"); - } - - public void testInvalidSLF4jParm2() { - // tag LO_INVALID_FORMATTING_ANCHOR - l3.error("This is a problem %s", "hello"); - l3.error("This is a problem %3$-2.3e", 1); - l3.error("This is a problem %5d", 3.1); - } - - public void testLogAppending(String s) { - try { - // tag LO_APPENDED_STRING_IN_FORMAT_STRING - l3.info("Got an error with: " + s); - } catch (Exception e) { - l3.warn("Go a bad error with: " + s, e); - } - } - - public void testWrongNumberOfParms() { - // tag LO_INCORRECT_NUMBER_OF_ANCHOR_PARAMETERS - l3.error("This is a problem {}", "hello", "hello"); - // tag LO_INCORRECT_NUMBER_OF_ANCHOR_PARAMETERS - l3.error("This is a problem {} and this {}", "hello"); - // tag LO_INCORRECT_NUMBER_OF_ANCHOR_PARAMETERS - l3.error("This is a problem {} and this {} and this {}", "hello", "world"); - // tag LO_INCORRECT_NUMBER_OF_ANCHOR_PARAMETERS - l3.error("This is a problem {} and this {} and this {} and this {}", "hello", "hello", "hello"); - - // no tag - l3.error("This is a problem {} and this {} and this {} and this {}", "hello", "hello", "hello", "hello"); - } - - public void testSimpleFormatInLogger(String poo) { - l3.error(String.format("The error was %s", poo)); - } - - public void testToStringInParm(List data) { - l3.info("This is a parm: {}", data.toString()); - } - - public String testReuseToStringInParm(List data) { - String info = data.toString(); - l3.info("This is a parm: {}", info); - return info; - } - - public void testParmInNonParmError246(Exception e) { - l6.error("Lookie an error->{}", e); - } - - public void testFPReuseofSimpleFormatter(String poo) { - String s = String.format("The error was %s", poo); - l3.error(s); - throw new RuntimeException(s); - } - - public void testFPWrongNumberOfParms() { - // no tag An additional exception argument is allowed if found - l3.error("This is a problem {}", "hello", new IOException("Yikes")); - // no tag An additional exception argument is allowed if found - l3.error("This is a problem {} and this {} and this {} and this {}", "hello", "hello", "hello", "hello", - new RuntimeException("yikes")); - // no tag - l3.error("This is a problem {} and this {}", "hello", new RuntimeException("yikes")); - } - - public void testFPRealStringBuilderUser(List l) { - StringBuilder sb = new StringBuilder(); - for (String s : l) { - sb.append(s).append(":"); - } - - l3.warn(sb.toString()); - } - - public void fpOKPattern(File f) { - l3.error("Specify the path to {} with %TEMP% or using system property", f); - l3.info("Copy {}% complete", 1); - } - - public void fpFalseToString(FunnyToString fts) { - l3.error("Whoops", fts.toString(null)); - } - - public void fp419IgnoreToStringOnException(RuntimeException e) { - l3.error("Whoops {}", e.toString()); - } - - public class Inner { - public void fpUseAnon() { - ActionListener l = new ActionListener() { - @Override - public void actionPerformed(ActionEvent ae) { - // no tag - org.slf4j.LoggerFactory.getLogger(Inner.class).error("fp"); - // tag LO_SUSPECT_LOG_CLASS - org.slf4j.LoggerFactory.getLogger(LO_Sample.class).error("not fp"); - } - }; - } - } - } - - public static class Log4j { - // tag LO_SUSPECT_LOG_CLASS - private static org.apache.log4j.Logger l1 = org.apache.log4j.Logger.getLogger(String.class); - // no tag - private static org.apache.log4j.Logger l2 = org.apache.log4j.Logger.getLogger("com.foo.LO_Sample"); - // no tag - private static final org.apache.log4j.Logger l3 = org.apache.log4j.Logger.getLogger(LO_Sample.class); - // tag LO_SUSPECT_LOG_CLASS - private static org.apache.log4j.Logger l4 = org.apache.log4j.Logger.getLogger(ActionEvent.class.getName()); - // no tag - private static org.apache.log4j.Logger l5 = org.apache.log4j.Logger.getLogger(LO_Sample.class.getName()); - // no tag - private static org.apache.log4j.Logger l6 = org.apache.log4j.Logger.getLogger("my.nasty.logger.LOGGER"); - - public static org.apache.log4j.Logger l7 = org.apache.log4j.Logger.getLogger(Log4j.class); - - // no tag - private org.apache.log4j.Logger someLocalLogger; - - // no tag - public Log4j() { - this(org.apache.log4j.Logger.getRootLogger()); - - someLocalLogger.info("Why am I using a local logger?"); - } - - // tag LO_SUSPECT_LOG_PARAMETER - public Log4j(org.apache.log4j.Logger someLogger) { - this.someLocalLogger = someLogger; - } - - public void testStutter() throws IOException { - InputStream is = null; - try { - File f = new File("Foo"); - is = new FileInputStream(f); - } catch (Exception e) { - // tag LO_STUTTERED_MESSAGE - l1.error(e.getMessage(), e); - } finally { - is.close(); - } - } - - public void testParmInExMessage() throws Exception { - try { - InputStream is = new FileInputStream("foo/bar"); - } catch (IOException e) { - // tag LO_EXCEPTION_WITH_LOGGER_PARMS - throw new Exception("Failed to parse {}", e); - } - } - - public void testLogAppending(String s) { - try { - // tag LO_APPENDED_STRING_IN_FORMAT_STRING - l3.info("Got an error with: " + s); - } catch (Exception e) { - l3.warn("Go a bad error with: " + s, e); - } - } - - public void testSimpleFormatInLogger(String poo) { - l3.error(String.format("The error was %s", poo)); - } - - public void testFPReuseofSimpleFormatter(String poo) { - String s = String.format("The error was %s", poo); - l3.error(s); - throw new RuntimeException(s); - } - - public void testFPRealStringBuilderUser(List l) { - StringBuilder sb = new StringBuilder(); - for (String s : l) { - sb.append(s).append(":"); - } - - l3.warn(sb.toString()); - } - - public class Inner { - public void fpUseAnon() { - ActionListener l = new ActionListener() { - @Override - public void actionPerformed(ActionEvent ae) { - // no tag - org.apache.log4j.Logger.getLogger(Inner.class).error("fp"); - // tag LO_SUSPECT_LOG_CLASS - org.apache.log4j.Logger.getLogger(LO_Sample.class).error("not fp"); - } - }; - } - } - } - - public static class Log4j2 { - // tag LO_SUSPECT_LOG_CLASS - private static org.apache.logging.log4j.Logger l1 = org.apache.logging.log4j.LogManager.getLogger(String.class); - // no tag - private static org.apache.logging.log4j.Logger l2 = org.apache.logging.log4j.LogManager - .getLogger("com.foo.LO_Sample"); - // no tag - private static final org.apache.logging.log4j.Logger l3 = org.apache.logging.log4j.LogManager - .getLogger(LO_Sample.class); - // tag LO_SUSPECT_LOG_CLASS - private static org.apache.logging.log4j.Logger l4 = org.apache.logging.log4j.LogManager - .getLogger(ActionEvent.class.getName()); - // no tag - private static org.apache.logging.log4j.Logger l5 = org.apache.logging.log4j.LogManager - .getLogger(LO_Sample.class.getName()); - // no tag - private static org.apache.logging.log4j.Logger l6 = org.apache.logging.log4j.LogManager - .getLogger("my.nasty.logger.LOGGER"); - - public static org.apache.logging.log4j.Logger l7 = org.apache.logging.log4j.LogManager.getLogger(Log4j2.class); - - private static org.apache.logging.log4j.Logger l8 = org.apache.logging.log4j.LogManager - .getFormatterLogger(Log4j2.class); - - // no tag - private org.apache.logging.log4j.Logger someLocalLogger; - - // no tag - public Log4j2() { - this(org.apache.logging.log4j.LogManager.getRootLogger()); - - someLocalLogger.info("Why am I using a local logger?"); - } - - // tag LO_SUSPECT_LOG_PARAMETER - public Log4j2(org.apache.logging.log4j.Logger someLogger) { - this.someLocalLogger = someLogger; - } - - public void testStutter() throws IOException { - InputStream is = null; - try { - File f = new File("Foo"); - is = new FileInputStream(f); - } catch (Exception e) { - // tag LO_STUTTERED_MESSAGE - l1.error(e.getMessage(), e); - } finally { - is.close(); - } - } - - public void testParmInExMessage() throws Exception { - try { - InputStream is = new FileInputStream("foo/bar"); - } catch (IOException e) { - // tag LO_EXCEPTION_WITH_LOGGER_PARMS - throw new Exception("Failed to parse {}", e); - } - } - - public void testInvalidLog4j2Parm() { - // tag LO_INVALID_FORMATTING_ANCHOR - l3.error("This is a problem {0}", "hello"); - } - - public void testInvalidLog4j2Parm2() { - // tag LO_INVALID_FORMATTING_ANCHOR - l3.error("This is a problem %s", "hello"); - l3.error("This is a problem %3$-2.3e", 1); - l3.error("This is a problem %5d", 3.1); - } - - public void testLogAppending(String s) { - try { - // tag LO_APPENDED_STRING_IN_FORMAT_STRING - l3.info("Got an error with: " + s); - } catch (Exception e) { - l3.warn("Go a bad error with: " + s, e); - } - } - - public void testWrongNumberOfParms() { - // tag LO_INCORRECT_NUMBER_OF_ANCHOR_PARAMETERS - l3.error("This is a problem {}", "hello", "hello"); - // tag LO_INCORRECT_NUMBER_OF_ANCHOR_PARAMETERS - l3.error("This is a problem {} and this {}", "hello"); - // tag LO_INCORRECT_NUMBER_OF_ANCHOR_PARAMETERS - l3.error("This is a problem {} and this {} and this {}", "hello", "world"); - // tag LO_INCORRECT_NUMBER_OF_ANCHOR_PARAMETERS - l3.error("This is a problem {} and this {} and this {} and this {}", "hello", "hello", "hello"); - - // no tag - l3.error("This is a problem {} and this {} and this {} and this {}", "hello", "hello", "hello", "hello"); - } - - public void testSimpleFormatInLogger(String poo) { - l3.error(String.format("The error was %s", poo)); - } - - public void testToStringInParm(List data) { - l3.info("This is a parm: {}", data.toString()); - } - - public String testReuseToStringInParm(List data) { - String info = data.toString(); - l3.info("This is a parm: {}", info); - return info; - } - - public void testFPReuseofSimpleFormatter(String poo) { - String s = String.format("The error was %s", poo); - l3.error(s); - throw new RuntimeException(s); - } - - public void testFPWrongNumberOfParms() { - // no tag An additional exception argument is allowed if found - l3.error("This is a problem {}", "hello", new IOException("Yikes")); - // no tag An additional exception argument is allowed if found - l3.error("This is a problem {} and this {} and this {} and this {}", "hello", "hello", "hello", "hello", - new RuntimeException("yikes")); - // no tag - l3.error("This is a problem {} and this {}", "hello", new RuntimeException("yikes")); - } - - public void testFPRealStringBuilderUser(List l) { - StringBuilder sb = new StringBuilder(); - for (String s : l) { - sb.append(s).append(":"); - } - - l3.warn(sb.toString()); - } - - public void fpOKPattern(File f) { - l3.error("Specify the path to {} with %TEMP% or using system property", f); - } - - public void fpFormatterPattern() { - l8.error("%d", 5); - } - - public class Inner { - public void fpUseAnon() { - ActionListener l = new ActionListener() { - @Override - public void actionPerformed(ActionEvent ae) { - // no tag - org.apache.logging.log4j.LogManager.getLogger(Inner.class).error("fp"); - // tag LO_SUSPECT_LOG_CLASS - org.apache.logging.log4j.LogManager.getLogger(LO_Sample.class).error("not fp"); - } - }; - } - } - } - - class FunnyToString { - public String toString(DateFormatter df) { - return ""; - } - } + public static class Slf4j { + // tag LO_SUSPECT_LOG_CLASS + private static org.slf4j.Logger l1 = org.slf4j.LoggerFactory.getLogger(String.class); + // no tag + private static org.slf4j.Logger l2 = org.slf4j.LoggerFactory.getLogger("com.foo.LO_Sample"); + // no tag + private static final org.slf4j.Logger l3 = org.slf4j.LoggerFactory.getLogger(LO_Sample.class); + // tag LO_SUSPECT_LOG_CLASS + private static org.slf4j.Logger l4 = org.slf4j.LoggerFactory.getLogger(ActionEvent.class.getName()); + // no tag + private static org.slf4j.Logger l5 = org.slf4j.LoggerFactory.getLogger(LO_Sample.class.getName()); + // no tag + private static org.slf4j.Logger l6 = org.slf4j.LoggerFactory.getLogger("my.nasty.logger.LOGGER"); + + public static org.slf4j.Logger l7 = org.slf4j.LoggerFactory.getLogger(Slf4j.class); + + // no tag + private org.slf4j.Logger someLocalLogger; + + // no tag + public Slf4j() { + someLocalLogger.info("Why am I using a local logger?"); + } + + // tag LO_SUSPECT_LOG_PARAMETER + public Slf4j(org.slf4j.Logger someLogger) { + this.someLocalLogger = someLogger; + } + + public void testStutter() throws IOException { + InputStream is = null; + try { + File f = new File("Foo"); + is = new FileInputStream(f); + } catch (Exception e) { + // tag LO_STUTTERED_MESSAGE + l1.error(e.getMessage(), e); + } finally { + is.close(); + } + } + + public void testParmInExMessage() throws Exception { + try { + InputStream is = new FileInputStream("foo/bar"); + } catch (IOException e) { + // tag LO_EXCEPTION_WITH_LOGGER_PARMS + throw new Exception("Failed to parse {}", e); + } + } + + public void testInvalidSLF4jParm() { + // tag LO_INVALID_FORMATTING_ANCHOR + l3.error("This is a problem {0}", "hello"); + } + + public void testInvalidSLF4jParm2() { + // tag LO_INVALID_FORMATTING_ANCHOR + l3.error("This is a problem %s", "hello"); + l3.error("This is a problem %3$-2.3e", 1); + l3.error("This is a problem %5d", 3.1); + } + + public void testLogAppending(String s) { + try { + // tag LO_APPENDED_STRING_IN_FORMAT_STRING + l3.info("Got an error with: " + s); + } catch (Exception e) { + l3.warn("Go a bad error with: " + s, e); + } + } + + public void testWrongNumberOfParms() { + // tag LO_INCORRECT_NUMBER_OF_ANCHOR_PARAMETERS + l3.error("This is a problem {}", "hello", "hello"); + // tag LO_INCORRECT_NUMBER_OF_ANCHOR_PARAMETERS + l3.error("This is a problem {} and this {}", "hello"); + // tag LO_INCORRECT_NUMBER_OF_ANCHOR_PARAMETERS + l3.error("This is a problem {} and this {} and this {}", "hello", "world"); + // tag LO_INCORRECT_NUMBER_OF_ANCHOR_PARAMETERS + l3.error("This is a problem {} and this {} and this {} and this {}", "hello", "hello", "hello"); + + // no tag + l3.error("This is a problem {} and this {} and this {} and this {}", "hello", "hello", "hello", "hello"); + } + + public void testSimpleFormatInLogger(String poo) { + l3.error(String.format("The error was %s", poo)); + } + + public void testToStringInParm(List data) { + l3.info("This is a parm: {}", data.toString()); + } + + public String testReuseToStringInParm(List data) { + String info = data.toString(); + l3.info("This is a parm: {}", info); + return info; + } + + public void testParmInNonParmError246(Exception e) { + l6.error("Lookie an error->{}", e); + } + + public void testFPReuseofSimpleFormatter(String poo) { + String s = String.format("The error was %s", poo); + l3.error(s); + throw new RuntimeException(s); + } + + public void testFPWrongNumberOfParms() { + // no tag An additional exception argument is allowed if found + l3.error("This is a problem {}", "hello", new IOException("Yikes")); + // no tag An additional exception argument is allowed if found + l3.error("This is a problem {} and this {} and this {} and this {}", "hello", "hello", "hello", "hello", + new RuntimeException("yikes")); + // no tag + l3.error("This is a problem {} and this {}", "hello", new RuntimeException("yikes")); + } + + public void testFPRealStringBuilderUser(List l) { + StringBuilder sb = new StringBuilder(); + for (String s : l) { + sb.append(s).append(":"); + } + + l3.warn(sb.toString()); + } + + public void fpOKPattern(File f) { + l3.error("Specify the path to {} with %TEMP% or using system property", f); + l3.info("Copy {}% complete", 1); + } + + public void fpFalseToString(FunnyToString fts) { + l3.error("Whoops", fts.toString(null)); + } + + public void fp419IgnoreToStringOnException(RuntimeException e) { + l3.error("Whoops {}", e.toString()); + } + + public class Inner { + public void fpUseAnon() { + ActionListener l = new ActionListener() { + @Override + public void actionPerformed(ActionEvent ae) { + // no tag + org.slf4j.LoggerFactory.getLogger(Inner.class).error("fp"); + // tag LO_SUSPECT_LOG_CLASS + org.slf4j.LoggerFactory.getLogger(LO_Sample.class).error("not fp"); + } + }; + } + } + } + + public static class Log4j { + // tag LO_SUSPECT_LOG_CLASS + private static org.apache.log4j.Logger l1 = org.apache.log4j.Logger.getLogger(String.class); + // no tag + private static org.apache.log4j.Logger l2 = org.apache.log4j.Logger.getLogger("com.foo.LO_Sample"); + // no tag + private static final org.apache.log4j.Logger l3 = org.apache.log4j.Logger.getLogger(LO_Sample.class); + // tag LO_SUSPECT_LOG_CLASS + private static org.apache.log4j.Logger l4 = org.apache.log4j.Logger.getLogger(ActionEvent.class.getName()); + // no tag + private static org.apache.log4j.Logger l5 = org.apache.log4j.Logger.getLogger(LO_Sample.class.getName()); + // no tag + private static org.apache.log4j.Logger l6 = org.apache.log4j.Logger.getLogger("my.nasty.logger.LOGGER"); + + public static org.apache.log4j.Logger l7 = org.apache.log4j.Logger.getLogger(Log4j.class); + + // no tag + private org.apache.log4j.Logger someLocalLogger; + + // no tag + public Log4j() { + this(org.apache.log4j.Logger.getRootLogger()); + + someLocalLogger.info("Why am I using a local logger?"); + } + + // tag LO_SUSPECT_LOG_PARAMETER + public Log4j(org.apache.log4j.Logger someLogger) { + this.someLocalLogger = someLogger; + } + + public void testStutter() throws IOException { + InputStream is = null; + try { + File f = new File("Foo"); + is = new FileInputStream(f); + } catch (Exception e) { + // tag LO_STUTTERED_MESSAGE + l1.error(e.getMessage(), e); + } finally { + is.close(); + } + } + + public void testParmInExMessage() throws Exception { + try { + InputStream is = new FileInputStream("foo/bar"); + } catch (IOException e) { + // tag LO_EXCEPTION_WITH_LOGGER_PARMS + throw new Exception("Failed to parse {}", e); + } + } + + public void testLogAppending(String s) { + try { + // tag LO_APPENDED_STRING_IN_FORMAT_STRING + l3.info("Got an error with: " + s); + } catch (Exception e) { + l3.warn("Go a bad error with: " + s, e); + } + } + + public void testSimpleFormatInLogger(String poo) { + l3.error(String.format("The error was %s", poo)); + } + + public void testFPReuseofSimpleFormatter(String poo) { + String s = String.format("The error was %s", poo); + l3.error(s); + throw new RuntimeException(s); + } + + public void testFPRealStringBuilderUser(List l) { + StringBuilder sb = new StringBuilder(); + for (String s : l) { + sb.append(s).append(":"); + } + + l3.warn(sb.toString()); + } + + public class Inner { + public void fpUseAnon() { + ActionListener l = new ActionListener() { + @Override + public void actionPerformed(ActionEvent ae) { + // no tag + org.apache.log4j.Logger.getLogger(Inner.class).error("fp"); + // tag LO_SUSPECT_LOG_CLASS + org.apache.log4j.Logger.getLogger(LO_Sample.class).error("not fp"); + } + }; + } + } + + public static class Log4j2 { + // tag LO_SUSPECT_LOG_CLASS + private static org.apache.logging.log4j.Logger l1 = org.apache.logging.log4j.LogManager + .getLogger(String.class); + // no tag + private static org.apache.logging.log4j.Logger l2 = org.apache.logging.log4j.LogManager + .getLogger("com.foo.LO_Sample"); + // no tag + private static final org.apache.logging.log4j.Logger l3 = org.apache.logging.log4j.LogManager + .getLogger(LO_Sample.class); + // tag LO_SUSPECT_LOG_CLASS + private static org.apache.logging.log4j.Logger l4 = org.apache.logging.log4j.LogManager + .getLogger(ActionEvent.class.getName()); + // no tag + private static org.apache.logging.log4j.Logger l5 = org.apache.logging.log4j.LogManager + .getLogger(LO_Sample.class.getName()); + // no tag + private static org.apache.logging.log4j.Logger l6 = org.apache.logging.log4j.LogManager + .getLogger("my.nasty.logger.LOGGER"); + + public static org.apache.logging.log4j.Logger l7 = org.apache.logging.log4j.LogManager + .getLogger(Log4j2.class); + + private static org.apache.logging.log4j.Logger l8 = org.apache.logging.log4j.LogManager + .getFormatterLogger(Log4j2.class); + + // no tag + private org.apache.logging.log4j.Logger someLocalLogger; + + // no tag + public Log4j2() { + this(org.apache.logging.log4j.LogManager.getRootLogger()); + + someLocalLogger.info("Why am I using a local logger?"); + } + + // tag LO_SUSPECT_LOG_PARAMETER + public Log4j2(org.apache.logging.log4j.Logger someLogger) { + this.someLocalLogger = someLogger; + } + + public void testStutter() throws IOException { + InputStream is = null; + try { + File f = new File("Foo"); + is = new FileInputStream(f); + } catch (Exception e) { + // tag LO_STUTTERED_MESSAGE + l1.error(e.getMessage(), e); + } finally { + is.close(); + } + } + + public void testParmInExMessage() throws Exception { + try { + InputStream is = new FileInputStream("foo/bar"); + } catch (IOException e) { + // tag LO_EXCEPTION_WITH_LOGGER_PARMS + throw new Exception("Failed to parse {}", e); + } + } + + public void testInvalidLog4j2Parm() { + // tag LO_INVALID_FORMATTING_ANCHOR + l3.error("This is a problem {0}", "hello"); + } + + public void testInvalidLog4j2Parm2() { + // tag LO_INVALID_FORMATTING_ANCHOR + l3.error("This is a problem %s", "hello"); + l3.error("This is a problem %3$-2.3e", 1); + l3.error("This is a problem %5d", 3.1); + } + + public void testLogAppending(String s) { + try { + // tag LO_APPENDED_STRING_IN_FORMAT_STRING + l3.info("Got an error with: " + s); + } catch (Exception e) { + l3.warn("Go a bad error with: " + s, e); + } + } + + public void testWrongNumberOfParms() { + // tag LO_INCORRECT_NUMBER_OF_ANCHOR_PARAMETERS + l3.error("This is a problem {}", "hello", "hello"); + // tag LO_INCORRECT_NUMBER_OF_ANCHOR_PARAMETERS + l3.error("This is a problem {} and this {}", "hello"); + // tag LO_INCORRECT_NUMBER_OF_ANCHOR_PARAMETERS + l3.error("This is a problem {} and this {} and this {}", "hello", "world"); + // tag LO_INCORRECT_NUMBER_OF_ANCHOR_PARAMETERS + l3.error("This is a problem {} and this {} and this {} and this {}", "hello", "hello", "hello"); + + // no tag + l3.error("This is a problem {} and this {} and this {} and this {}", "hello", "hello", "hello", + "hello"); + } + + public void testSimpleFormatInLogger(String poo) { + l3.error(String.format("The error was %s", poo)); + } + + public void testToStringInParm(List data) { + l3.info("This is a parm: {}", data.toString()); + } + + public String testReuseToStringInParm(List data) { + String info = data.toString(); + l3.info("This is a parm: {}", info); + return info; + } + + public void testFPReuseofSimpleFormatter(String poo) { + String s = String.format("The error was %s", poo); + l3.error(s); + throw new RuntimeException(s); + } + + public void testFPWrongNumberOfParms() { + // no tag An additional exception argument is allowed if found + l3.error("This is a problem {}", "hello", new IOException("Yikes")); + // no tag An additional exception argument is allowed if found + l3.error("This is a problem {} and this {} and this {} and this {}", "hello", "hello", "hello", "hello", + new RuntimeException("yikes")); + // no tag + l3.error("This is a problem {} and this {}", "hello", new RuntimeException("yikes")); + } + + public void testFPRealStringBuilderUser(List l) { + StringBuilder sb = new StringBuilder(); + for (String s : l) { + sb.append(s).append(":"); + } + + l3.warn(sb.toString()); + } + + public void fpOKPattern(File f) { + l3.error("Specify the path to {} with %TEMP% or using system property", f); + } + + public void fpFormatterPattern() { + l8.error("%d", 5); + } + + public void test465(String... args) { + l1.error("main args " + args.length); // should raise LO_APPENDED_STRING_IN_FORMAT_STRING bug warning + l1.error("main args {}" + args.length, args); // should raise LO_APPENDED_STRING_IN_FORMAT_STRING bug + // warning + } + + public class Inner { + public void fpUseAnon() { + ActionListener l = new ActionListener() { + @Override + public void actionPerformed(ActionEvent ae) { + // no tag + org.apache.logging.log4j.LogManager.getLogger(Inner.class).error("fp"); + // tag LO_SUSPECT_LOG_CLASS + org.apache.logging.log4j.LogManager.getLogger(LO_Sample.class).error("not fp"); + } + }; + } + } + + } + + class FunnyToString { + public String toString(DateFormatter df) { + return ""; + } + } + } }