diff --git a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/PreventTokenLogging.java b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/PreventTokenLogging.java index 5b26962ec..0804c68dd 100644 --- a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/PreventTokenLogging.java +++ b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/PreventTokenLogging.java @@ -46,15 +46,18 @@ public final class PreventTokenLogging extends BugChecker implements BugChecker. .onClassAny("com.palantir.logsafe.SafeArg", "com.palantir.logsafe.UnsafeArg") .named("of")); - private static final Matcher AUTH_MATCHER = - Matchers.anyOf( - Matchers.isSubtypeOf("com.palantir.tokens.auth.AuthHeader"), - Matchers.isSubtypeOf("com.palantir.tokens.auth.BearerToken")); + private static final Matcher AUTH_MATCHER = + Matchers.allOf( + Matchers.anyOf( + Matchers.isSubtypeOf("com.palantir.tokens.auth.AuthHeader"), + Matchers.isSubtypeOf("com.palantir.tokens.auth.BearerToken")), + Matchers.not( + Matchers.kindIs(Tree.Kind.NULL_LITERAL))); @Override public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { if (METHOD_MATCHER.matches(tree, state)) { - for (Tree arg : tree.getArguments()) { + for (ExpressionTree arg : tree.getArguments()) { if (AUTH_MATCHER.matches(arg, state)) { return buildDescription(arg) .setMessage("Authentication information is not allowed to be logged.") diff --git a/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/PreventTokenLoggingTests.java b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/PreventTokenLoggingTests.java index 3fdc47894..ec054e270 100644 --- a/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/PreventTokenLoggingTests.java +++ b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/PreventTokenLoggingTests.java @@ -144,6 +144,16 @@ public void testSlf4jTraceNoArgs() { passSlf4j("log.trace(message);"); } + @Test + public void testSlf4jTraceNullMessageNoArgs() { + passSlf4j("log.trace(null);"); + } + + @Test + public void testSlf4jTraceNullArg() { + passSlf4j("log.trace(message, arg1, null);"); + } + @Test public void testSlf4jDebug() { passSlf4j("log.debug(message, arg1);"); @@ -159,6 +169,16 @@ public void testSlf4jDebugNoArgs() { passSlf4j("log.debug(message);"); } + @Test + public void testSlf4jDebugNullMessageNoArgs() { + passSlf4j("log.debug(null);"); + } + + @Test + public void testSlf4jDebugNullArg() { + passSlf4j("log.debug(message, arg1, null);"); + } + @Test public void testSlf4jInfo() { passSlf4j("log.info(message, arg1);"); @@ -174,6 +194,16 @@ public void testSlf4jInfoNoArgs() { passSlf4j("log.info(message);"); } + @Test + public void testSlf4jInfoNullMessageNoArgs() { + passSlf4j("log.info(null);"); + } + + @Test + public void testSlf4jInfoNullArg() { + passSlf4j("log.info(message, arg1, null);"); + } + @Test public void testSlf4jWarn() { passSlf4j("log.warn(message, arg1);"); @@ -189,6 +219,16 @@ public void testSlf4jWarnNoArgs() { passSlf4j("log.warn(message);"); } + @Test + public void testSlf4jWarnNullMessageNoArgs() { + passSlf4j("log.warn(null);"); + } + + @Test + public void testSlf4jWarnNullArg() { + passSlf4j("log.warn(message, arg1, null);"); + } + @Test public void testSlf4jError() { passSlf4j("log.error(message, arg1);"); @@ -204,6 +244,16 @@ public void testSlf4jErrorNoArgs() { passSlf4j("log.error(message);"); } + @Test + public void testSlf4jErrorNullMessageNoArgs() { + passSlf4j("log.error(null);"); + } + + @Test + public void testSlf4jErrorNullArg() { + passSlf4j("log.error(message, arg1, null);"); + } + @Test public void testSafeArgAuthHeader() { failLogSafe("SafeArg.of(name, authHeader);"); @@ -229,11 +279,31 @@ public void testSafeArg() { passLogSafe("SafeArg.of(name, value);"); } + @Test + public void testSafeArgNullName() { + passLogSafe("SafeArg.of(null, value);"); + } + + @Test + public void testSafeArgNullValue() { + passLogSafe("SafeArg.of(name, null);"); + } + @Test public void testUnsafeArg() { passLogSafe("UnsafeArg.of(name, value);"); } + @Test + public void testUnsafeArgNullName() { + passLogSafe("UnsafeArg.of(null, value);"); + } + + @Test + public void testUnsafeArgNullValue() { + passLogSafe("UnsafeArg.of(name, null);"); + } + private void passSlf4j(String statement) { compilationHelper.addSourceLines( "Test.java", diff --git a/changelog/@unreleased/pr-674.v2.yml b/changelog/@unreleased/pr-674.v2.yml new file mode 100644 index 000000000..71322c87f --- /dev/null +++ b/changelog/@unreleased/pr-674.v2.yml @@ -0,0 +1,6 @@ +type: fix +fix: + description: The PreventTokenLogging error-prone check will now correctly handle + null use in SLF4J and Safe/Unsafe Arg functions. + links: + - https://github.com/palantir/gradle-baseline/pull/674