Permalink
Browse files

Merge branch 'findbugs' into spotbugs

Conflicts:
	.classpath
	yank.xls
  • Loading branch information...
mebigfatguy committed Sep 28, 2017
2 parents 290cb21 + cc078a3 commit 9f6d336e579ffc6355028ef035e5b1fa930f81d5
View
@@ -32,6 +32,7 @@
<classpathentry kind="lib" path="lib/javax.ws.rs-api-2.0.1.jar"/>
<classpathentry kind="lib" path="lib/mockito-core-2.8.47.jar" sourcepath="/home/dave/.m2/repository/org/mockito/mockito-core/2.8.47/mockito-core-2.8.47-sources.jar"/>
<classpathentry kind="lib" path="lib/hamcrest-core-1.3.jar"/>
<classpathentry kind="lib" path="lib/log4j-api-2.9.1.jar"/>
<classpathentry kind="lib" path="lib/bcel-6.0.jar"/>
<classpathentry kind="lib" path="lib/spotbugs-3.1.0-RC3.jar"/>
<classpathentry kind="lib" path="lib/spotbugs-annotations-3.1.0-RC3.jar"/>
View
@@ -121,6 +121,7 @@
<pathelement location="${lib.dir}/testng-${testng.version}.jar" />
<pathelement location="${lib.dir}/javax.servlet-api-${javax.servlet-api.version}.jar" />
<pathelement location="${lib.dir}/log4j-${log4j.version}.jar" />
<pathelement location="${lib.dir}/log4j-api-${log4j-api.version}.jar" />
<pathelement location="${lib.dir}/commons-lang3-${commons-lang3.version}.jar" />
<pathelement location="${lib.dir}/commons-io-${commons-io.version}.jar" />
<pathelement location="${lib.dir}/backport-util-concurrent-${backport-util-concurrent.version}.jar" />
View
@@ -909,7 +909,7 @@ public void test(int i)
<Detector class="com.mebigfatguy.fbcontrib.detect.LoggerOddities">
<Details>
<![CDATA[
<p>Looks for odd patterns of use of Logger classes from either log4j, SLF4J or Commons Logging.</p>
<p>Looks for odd patterns of use of Logger classes from either Log4j, Log4j2, SLF4J or Commons Logging.</p>
<p>It is a fast detector.</p>
]]>
</Details>
@@ -4030,8 +4030,8 @@ return new String[0];<br/>
<LongDescription>Method {1} attempts to log using numbered formatting anchors</LongDescription>
<Details>
<![CDATA[
<p>This method attempts to use an SLF4J logger to log a parameterized expression using formatting anchors.
However, SLF4J uses simple non numbered anchors such as {}, rather than anchors with digits in them as the
<p>This method attempts to use an SLF4J or Log4j2 logger to log a parameterized expression using formatting anchors.
However, SLF4J and Log4j2 use simple non numbered anchors such as {}, rather than anchors with digits in them as the
code uses. Thus no parameter replacement will occur.</p>
<p>This pattern is invalid:
<code>LOGGER.error("{0} is broken", theThing);</code>
@@ -4047,8 +4047,8 @@ return new String[0];<br/>
<LongDescription>Method {1} attempts to log using String.format notation</LongDescription>
<Details>
<![CDATA[
<p>This method attempts to use an SLF4J logger to log a parameterized expression using String.format notation.
However, SLF4J uses simple non numbered anchors such as {}, rather than anchors with percent signs in them as the
<p>This method attempts to use an SLF4J or Log4j2 logger to log a parameterized expression using String.format notation.
However, SLF4J and Log4j2 uses simple non numbered anchors such as {}, rather than anchors with percent signs in them as the
code uses. Thus no parameter replacement will occur.</p>
<p>This pattern is invalid:
<code>LOGGER.error("%s is broken", theThing);</code>
@@ -4060,11 +4060,11 @@ return new String[0];<br/>
</BugPattern>
<BugPattern type="LO_INCORRECT_NUMBER_OF_ANCHOR_PARAMETERS">
<ShortDescription>Method passes an incorrect number of parameters to an SLF4J logging statement</ShortDescription>
<LongDescription>Method {1} passes an incorrect number of parameters to an SLF4J logging statement</LongDescription>
<ShortDescription>Method passes an incorrect number of parameters to an SLF4J or Log4j2 logging statement</ShortDescription>
<LongDescription>Method {1} passes an incorrect number of parameters to an SLF4J or Log4j2 logging statement</LongDescription>
<Details>
<![CDATA[
<p>This method passes the wrong number of parameters to an SLF4J logging method (error, warn, info, debug) based on the number of anchors {} in the
<p>This method passes the wrong number of parameters to an SLF4J or Log4j2 logging method (error, warn, info, debug) based on the number of anchors {} in the
format string. An additional exception argument is allowed if found.</p>
]]>
</Details>
@@ -4076,19 +4076,19 @@ return new String[0];<br/>
<Details>
<![CDATA[
<p>This method passes a standard exception as a logger parameter, and expects this exception to be substituted in
an SLF4J style parameter marker '{}'. This marker will not be translated as SLF4J doesn't process the Exception
an SLF4J or Log4j style parameter marker '{}'. This marker will not be translated as SLF4J or Log4j2 doesn't process the Exception
class for markers.
</p>
]]>
</Details>
</BugPattern>
<BugPattern type="LO_APPENDED_STRING_IN_FORMAT_STRING">
<ShortDescription>Method passes a concatenated string to SLF4J's format string</ShortDescription>
<LongDescription>Method {1} passes a concatenated string to SLF4J's format string</LongDescription>
<ShortDescription>Method passes a concatenated string to SLF4J's or Log4j2's format string</ShortDescription>
<LongDescription>Method {1} passes a concatenated string to SLF4J's or Log4j2's format string</LongDescription>
<Details>
<![CDATA[
<p>This method uses an SLF4J logger to log a string, where the first (format) string is created using concatenation.
<p>This method uses an SLF4J or Log4j2 logger to log a string, where the first (format) string is created using concatenation.
You should use {} markers to inject dynamic content into the string, so that String building is delayed until the
actual log string is needed. If the log level is high enough that this log statement isn't used, then the appends
will never be executed.</p>
@@ -4098,13 +4098,13 @@ return new String[0];<br/>
<BugPattern type="LO_EMBEDDED_SIMPLE_STRING_FORMAT_IN_FORMAT_STRING">
<ShortDescription>Method passes a simple String.format result to an SLF4J's format string</ShortDescription>
<LongDescription>Method {1} passes a simple String.format result to an SLF4J's format string</LongDescription>
<ShortDescription>Method passes a simple String.format result to an SLF4J's or Log4j2's format string</ShortDescription>
<LongDescription>Method {1} passes a simple String.format result to an SLF4J's or Log4j2's format string</LongDescription>
<Details>
<![CDATA[
<p>This method uses an SLF4J logger to log a string, which was produced through a call to String.format, where
<p>This method uses an SLF4J or Log4j2 logger to log a string, which was produced through a call to String.format, where
the format string passed was a constant string containing only simple format markers that could be directly handled
by slf4j. Rather than doing
by slf4j or Log4j. Rather than doing
<pre>
logger.error("String.format("This %s is an error", s));
<pre>
View
@@ -1,30 +1,31 @@
<Project projectName="sample">
<Jar>../target/classes/samples</Jar>
<AuxClasspathEntry>../lib/asm-debug-all-5.0.2.jar</AuxClasspathEntry>
<AuxClasspathEntry>../lib/backport-util-concurrent-3.1.jar</AuxClasspathEntry>
<AuxClasspathEntry>../lib/commons-codec-1.10.jar</AuxClasspathEntry>
<AuxClasspathEntry>../lib/commons-collections-3.2.2.jar</AuxClasspathEntry>
<AuxClasspathEntry>../lib/commons-lang3-3.4.jar</AuxClasspathEntry>
<AuxClasspathEntry>../lib/guava-19.0.jar</AuxClasspathEntry>
<AuxClasspathEntry>../lib/httpclient-4.5.2.jar</AuxClasspathEntry>
<AuxClasspathEntry>../lib/httpclient-cache-4.5.2.jar</AuxClasspathEntry>
<AuxClasspathEntry>../lib/httpcore-4.4.5.jar</AuxClasspathEntry>
<AuxClasspathEntry>../lib/javax.persistence-2.1.1.jar</AuxClasspathEntry>
<AuxClasspathEntry>../lib/javax.servlet-api-3.1.0.jar</AuxClasspathEntry>
<AuxClasspathEntry>../lib/javax.servlet.jsp-api-2.3.1.jar</AuxClasspathEntry>
<AuxClasspathEntry>../lib/junit-4.12.jar</AuxClasspathEntry>
<AuxClasspathEntry>../lib/log4j-1.2.17.jar</AuxClasspathEntry>
<AuxClasspathEntry>../lib/slf4j-api-1.7.21.jar</AuxClasspathEntry>
<AuxClasspathEntry>../lib/spring-beans-4.3.3.RELEASE.jar</AuxClasspathEntry>
<AuxClasspathEntry>../lib/spring-context-4.3.3.RELEASE.jar</AuxClasspathEntry>
<AuxClasspathEntry>../lib/spring-tx-4.3.3.RELEASE.jar</AuxClasspathEntry>
<AuxClasspathEntry>../lib/threetenbp-1.3.2.jar</AuxClasspathEntry>
<AuxClasspathEntry>../lib/jena-shaded-guava-3.1.0.jar</AuxClasspathEntry>
<AuxClasspathEntry>../lib/commons-io-2.4.jar</AuxClasspathEntry>
<AuxClasspathEntry>../lib/testng-6.9.10.jar</AuxClasspathEntry>
<AuxClasspathEntry>../lib/javax.ws.rs-api-2.0.1.jar</AuxClasspathEntry>
<AuxClasspathEntry>../lib/jersey-media-multipart-2.25.1.jar</AuxClasspathEntry>
<AuxClasspathEntry>../lib/mockito-core-2.8.47.jar</AuxClasspathEntry>
<AuxClasspathEntry>../lib/hamcrest-core-1.3.jar</AuxClasspathEntry>
<SrcDir>../src/samples/java</SrcDir>
<Jar>./../target/classes/samples</Jar>
<AuxClasspathEntry>./../lib/asm-debug-all-5.0.2.jar</AuxClasspathEntry>
<AuxClasspathEntry>./../lib/backport-util-concurrent-3.1.jar</AuxClasspathEntry>
<AuxClasspathEntry>./../lib/commons-codec-1.10.jar</AuxClasspathEntry>
<AuxClasspathEntry>./../lib/commons-collections-3.2.2.jar</AuxClasspathEntry>
<AuxClasspathEntry>./../lib/commons-lang3-3.4.jar</AuxClasspathEntry>
<AuxClasspathEntry>./../lib/guava-19.0.jar</AuxClasspathEntry>
<AuxClasspathEntry>./../lib/httpclient-4.5.2.jar</AuxClasspathEntry>
<AuxClasspathEntry>./../lib/httpclient-cache-4.5.2.jar</AuxClasspathEntry>
<AuxClasspathEntry>./../lib/httpcore-4.4.5.jar</AuxClasspathEntry>
<AuxClasspathEntry>./../lib/javax.persistence-2.1.1.jar</AuxClasspathEntry>
<AuxClasspathEntry>./../lib/javax.servlet-api-3.1.0.jar</AuxClasspathEntry>
<AuxClasspathEntry>./../lib/javax.servlet.jsp-api-2.3.1.jar</AuxClasspathEntry>
<AuxClasspathEntry>./../lib/junit-4.12.jar</AuxClasspathEntry>
<AuxClasspathEntry>./../lib/log4j-1.2.17.jar</AuxClasspathEntry>
<AuxClasspathEntry>./../lib/slf4j-api-1.7.21.jar</AuxClasspathEntry>
<AuxClasspathEntry>./../lib/spring-beans-4.3.3.RELEASE.jar</AuxClasspathEntry>
<AuxClasspathEntry>./../lib/spring-context-4.3.3.RELEASE.jar</AuxClasspathEntry>
<AuxClasspathEntry>./../lib/spring-tx-4.3.3.RELEASE.jar</AuxClasspathEntry>
<AuxClasspathEntry>./../lib/threetenbp-1.3.2.jar</AuxClasspathEntry>
<AuxClasspathEntry>./../lib/jena-shaded-guava-3.1.0.jar</AuxClasspathEntry>
<AuxClasspathEntry>./../lib/commons-io-2.4.jar</AuxClasspathEntry>
<AuxClasspathEntry>./../lib/testng-6.9.10.jar</AuxClasspathEntry>
<AuxClasspathEntry>./../lib/javax.ws.rs-api-2.0.1.jar</AuxClasspathEntry>
<AuxClasspathEntry>./../lib/jersey-media-multipart-2.25.1.jar</AuxClasspathEntry>
<AuxClasspathEntry>./../lib/mockito-core-2.8.47.jar</AuxClasspathEntry>
<AuxClasspathEntry>./../lib/hamcrest-core-1.3.jar</AuxClasspathEntry>
<AuxClasspathEntry>../lib/log4j-api-2.9.1.jar</AuxClasspathEntry>
<SrcDir>./../src/samples/java</SrcDir>
</Project>
@@ -60,6 +60,8 @@
private static final Set<String> 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();
@@ -73,12 +75,16 @@
.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()
@@ -142,7 +148,7 @@ public void visitCode(Code obj) {
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(COMMONS_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));
}
}
@@ -312,7 +318,7 @@ private void checkForProblemsWithLoggerMethods() throws ClassNotFoundException {
.addMethod(this).addSourceLine(this));
}
}
} else if (SLF4J_LOGGER.equals(callingClsName)) {
} else if (SLF4J_LOGGER.equals(callingClsName) || (LOG4J2_LOGGER.equals(callingClsName))) {
String signature = getSigConstantOperand();
if (SignatureBuilder.SIG_STRING_TO_VOID.equals(signature) || SignatureBuilder.SIG_STRING_AND_OBJECT_TO_VOID.equals(signature)
|| SIG_STRING_AND_TWO_OBJECTS_TO_VOID.equals(signature) || SIG_STRING_AND_OBJECT_ARRAY_TO_VOID.equals(signature)) {
@@ -331,7 +337,7 @@ private void checkForProblemsWithLoggerMethods() throws ClassNotFoundException {
bugReporter.reportBug(new BugInstance(this, BugType.LO_INVALID_STRING_FORMAT_NOTATION.name(), NORMAL_PRIORITY)
.addClass(this).addMethod(this).addSourceLine(this));
} else {
int actualParms = getSLF4JParmCount(signature);
int actualParms = getVarArgsParmCount(signature);
if (actualParms != -1) {
int expectedParms = countAnchors((String) con);
boolean hasEx = hasExceptionOnStack();
@@ -464,6 +470,32 @@ private void lookForSuspectClasses() {
loggingClassName = (String) item.getConstant();
loggingPriority = LOW_PRIORITY;
}
} else if (LOG4J2_LOGMANAGER.equals(callingClsName) && "getLogger".equals(mthName)) {
String signature = getSigConstantOperand();
if (SIG_CLASS_TO_LOG4J2_LOGGER.equals(signature)) {
loggingClassName = getLoggingClassNameFromStackValue();
} else if (SIG_STRING_TO_LOG4J2_LOGGER.equals(signature)) {
if (stack.getStackDepth() > 0) {
OpcodeStack.Item item = stack.getStackItem(0);
loggingClassName = (String) item.getConstant();
LOUserValue<String> uv = (LOUserValue<String>) 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 ("org/apache/commons/logging/LogFactory".equals(callingClsName) && "getLog".equals(mthName)) {
String signature = getSigConstantOperand();
@@ -516,14 +548,14 @@ private static int countAnchors(String formatString) {
}
/**
* returns the number of parameters slf4j is expecting to inject into the format string
* 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 getSLF4JParmCount(String signature) {
private int getVarArgsParmCount(String signature) {
if (SignatureBuilder.SIG_STRING_AND_OBJECT_TO_VOID.equals(signature)) {
return 1;
}
@@ -246,8 +246,7 @@ public void sawOpcode(int seen) {
}
private void handleTernary(int seen) {
if (((seen == GETFIELD) || OpcodeUtils.isALoad(seen)) && (stack.getStackDepth() > 0)) {
&& (stack.getStackDepth() > 0)) {
if (((seen == GETFIELD) || OpcodeUtils.isALoad(seen)) && (stack.getStackDepth() > 0)) {
OpcodeStack.Item item = stack.getStackItem(0);
clearUserValue(item);
}
Oops, something went wrong.

0 comments on commit 9f6d336

Please sign in to comment.