diff --git a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/IllegalSafeLoggingArgument.java b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/IllegalSafeLoggingArgument.java index b0a842b8b..ec9da9268 100644 --- a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/IllegalSafeLoggingArgument.java +++ b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/IllegalSafeLoggingArgument.java @@ -26,14 +26,17 @@ import com.google.errorprone.matchers.Matcher; import com.google.errorprone.matchers.method.MethodMatchers; import com.google.errorprone.util.ASTHelpers; +import com.palantir.baseline.errorprone.safety.Safety; +import com.palantir.baseline.errorprone.safety.SafetyAnalysis; +import com.palantir.baseline.errorprone.safety.SafetyAnnotations; import com.sun.source.tree.ExpressionTree; import com.sun.source.tree.MethodInvocationTree; -import com.sun.source.tree.Tree; -import com.sun.source.tree.TypeCastTree; -import com.sun.tools.javac.code.Symbol; +import com.sun.source.tree.MethodTree; +import com.sun.source.tree.ReturnTree; +import com.sun.source.tree.StatementTree; +import com.sun.source.util.TreePath; import com.sun.tools.javac.code.Symbol.MethodSymbol; import com.sun.tools.javac.code.Symbol.VarSymbol; -import com.sun.tools.javac.code.Type; import java.util.List; /** @@ -52,19 +55,14 @@ linkType = BugPattern.LinkType.CUSTOM, severity = BugPattern.SeverityLevel.ERROR, summary = "safe-logging annotations must agree between args and method parameters") -public final class IllegalSafeLoggingArgument extends BugChecker implements BugChecker.MethodInvocationTreeMatcher { - private static final String SAFE = "com.palantir.logsafe.Safe"; - private static final String UNSAFE = "com.palantir.logsafe.Unsafe"; - private static final String DO_NOT_LOG = "com.palantir.logsafe.DoNotLog"; +public final class IllegalSafeLoggingArgument extends BugChecker + implements BugChecker.MethodInvocationTreeMatcher, BugChecker.ReturnTreeMatcher { private static final String UNSAFE_ARG = "com.palantir.logsafe.UnsafeArg"; private static final Matcher SAFE_ARG_OF_METHOD_MATCHER = MethodMatchers.staticMethod() .onClass("com.palantir.logsafe.SafeArg") .named("of"); - private static final Matcher TO_STRING = - MethodMatchers.instanceMethod().anyClass().named("toString").withNoParameters(); - @Override public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { List arguments = tree.getArguments(); @@ -78,16 +76,18 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState List parameters = methodSymbol.getParameters(); for (int i = 0; i < parameters.size(); i++) { VarSymbol parameter = parameters.get(i); - Safety parameterSafety = getSafety(parameter, state); - if (parameterSafety == Safety.UNKNOWN) { - // Fast path, avoid analysis when the value isn't provided to a safety-aware consumer + Safety parameterSafety = SafetyAnnotations.getSafety(parameter, state); + if (parameterSafety.allowsAll()) { + // Fast path: all types are accepted, there's no reason to do further analysis. continue; } int limit = methodSymbol.isVarArgs() && i == parameters.size() - 1 ? arguments.size() : i + 1; for (int j = i; j < limit; j++) { ExpressionTree argument = arguments.get(j); - Safety argumentSafety = getSafety(argument, state); + + Safety argumentSafety = SafetyAnalysis.of(state.withPath(new TreePath(state.getPath(), argument))); + if (!parameterSafety.allowsValueWith(argumentSafety)) { // use state.reportMatch to report all failing arguments if multiple are invalid state.reportMatch(buildDescription(argument) @@ -102,6 +102,36 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState return Description.NO_MATCH; } + @Override + public Description matchReturn(ReturnTree tree, VisitorState state) { + if (tree.getExpression() == null) { + return Description.NO_MATCH; + } + TreePath path = state.getPath(); + while (path != null && path.getLeaf() instanceof StatementTree) { + path = path.getParentPath(); + } + if (path == null || !(path.getLeaf() instanceof MethodTree)) { + return Description.NO_MATCH; + } + MethodTree method = (MethodTree) path.getLeaf(); + Safety methodDeclaredSafety = SafetyAnnotations.getSafety(ASTHelpers.getSymbol(method), state); + if (methodDeclaredSafety.allowsAll()) { + // Fast path, all types are accepted, there's no reason to do further analysis. + return Description.NO_MATCH; + } + Safety returnValueSafety = + SafetyAnalysis.of(state.withPath(new TreePath(state.getPath(), tree.getExpression()))); + if (methodDeclaredSafety.allowsValueWith(returnValueSafety)) { + return Description.NO_MATCH; + } + return buildDescription(tree) + .setMessage(String.format( + "Dangerous return value: result is '%s' but the method is annotated '%s'.", + returnValueSafety, methodDeclaredSafety)) + .build(); + } + private static SuggestedFix getSuggestedFix(MethodInvocationTree tree, VisitorState state, Safety argumentSafety) { if (SAFE_ARG_OF_METHOD_MATCHER.matches(tree, state) && Safety.UNSAFE.allowsValueWith(argumentSafety)) { SuggestedFix.Builder fix = SuggestedFix.builder(); @@ -112,83 +142,4 @@ private static SuggestedFix getSuggestedFix(MethodInvocationTree tree, VisitorSt return SuggestedFix.emptyFix(); } - - private static Safety getSafety(ExpressionTree tree, VisitorState state) { - Tree argument = ASTHelpers.stripParentheses(tree); - // Check annotations on the result type - Type resultType = ASTHelpers.getResultType(tree); - if (resultType != null) { - Safety resultTypeSafety = getSafety(resultType.tsym, state); - if (resultTypeSafety != Safety.UNKNOWN) { - return resultTypeSafety; - } - } - // Unwrap type-casts: 'Object value = (Object) unsafeType;' is still unsafe. - if (argument instanceof TypeCastTree) { - TypeCastTree typeCastTree = (TypeCastTree) argument; - return getSafety(typeCastTree.getExpression(), state); - } - // If the argument is a method invocation, check the method for safety annotations - if (argument instanceof MethodInvocationTree) { - MethodInvocationTree argumentInvocation = (MethodInvocationTree) argument; - MethodSymbol methodSymbol = ASTHelpers.getSymbol(argumentInvocation); - if (methodSymbol != null) { - Safety methodSafety = getSafety(methodSymbol, state); - // non-annotated toString inherits type-level safety. - if (methodSafety == Safety.UNKNOWN && TO_STRING.matches(argumentInvocation, state)) { - return getSafety(ASTHelpers.getReceiver(argumentInvocation), state); - } - return methodSafety; - } - } - // Check the argument symbol itself: - Symbol argumentSymbol = ASTHelpers.getSymbol(argument); - return argumentSymbol != null ? getSafety(argumentSymbol, state) : Safety.UNKNOWN; - } - - private static Safety getSafety(Symbol symbol, VisitorState state) { - if (ASTHelpers.hasAnnotation(symbol, DO_NOT_LOG, state)) { - return Safety.DO_NOT_LOG; - } - if (ASTHelpers.hasAnnotation(symbol, UNSAFE, state)) { - return Safety.UNSAFE; - } - if (ASTHelpers.hasAnnotation(symbol, SAFE, state)) { - return Safety.SAFE; - } - return Safety.UNKNOWN; - } - - private enum Safety { - UNKNOWN() { - @Override - boolean allowsValueWith(Safety _valueSafety) { - // No constraints when safety isn't specified - return true; - } - }, - DO_NOT_LOG() { - @Override - boolean allowsValueWith(Safety _valueSafety) { - // do-not-log on a parameter isn't meaningful for callers, only for the implementation - return true; - } - }, - UNSAFE() { - @Override - boolean allowsValueWith(Safety valueSafety) { - // We allow safe data to be provided to an unsafe annotated parameter because that's safe, however - // we should separately flag and prompt migration of such UnsafeArgs to SafeArg. - return valueSafety != Safety.DO_NOT_LOG; - } - }, - SAFE() { - @Override - boolean allowsValueWith(Safety valueSafety) { - return valueSafety == Safety.UNKNOWN || valueSafety == Safety.SAFE; - } - }; - - abstract boolean allowsValueWith(Safety valueSafety); - } } diff --git a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/safety/Safety.java b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/safety/Safety.java new file mode 100644 index 000000000..ca59147de --- /dev/null +++ b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/safety/Safety.java @@ -0,0 +1,96 @@ +/* + * (c) Copyright 2022 Palantir Technologies Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.palantir.baseline.errorprone.safety; + +import org.checkerframework.errorprone.dataflow.analysis.AbstractValue; + +public enum Safety implements AbstractValue { + UNKNOWN() { + @Override + public Safety leastUpperBound(Safety other) { + return other == SAFE ? this : other; + } + + @Override + public boolean allowsValueWith(Safety _valueSafety) { + // No constraints when safety isn't specified + return true; + } + }, + DO_NOT_LOG() { + @Override + public Safety leastUpperBound(Safety _other) { + return this; + } + + @Override + public boolean allowsValueWith(Safety _valueSafety) { + // do-not-log on a parameter isn't meaningful for callers, only for the implementation + return true; + } + }, + UNSAFE() { + @Override + public Safety leastUpperBound(Safety other) { + return other == DO_NOT_LOG ? other : this; + } + + @Override + public boolean allowsValueWith(Safety valueSafety) { + // We allow safe data to be provided to an unsafe annotated parameter because that's safe, however + // we should separately flag and prompt migration of such UnsafeArgs to SafeArg. + return valueSafety != Safety.DO_NOT_LOG; + } + }, + SAFE() { + @Override + public Safety leastUpperBound(Safety other) { + return other; + } + + @Override + public boolean allowsValueWith(Safety valueSafety) { + return valueSafety == Safety.UNKNOWN || valueSafety == Safety.SAFE; + } + }; + + public abstract boolean allowsValueWith(Safety valueSafety); + + public boolean allowsAll() { + return this == UNKNOWN || this == DO_NOT_LOG; + } + + /** + * Merge Safety using {@link Safety#leastUpperBound(AbstractValue)} except that {@link Safety#UNKNOWN} assumes + * no confidence, preferring the other type if data is available. + * For example, casting from {@link Object} to a known-safe type should result in {@link Safety#SAFE}. + */ + public static Safety mergeAssumingUnknownIsSame(Safety one, Safety two) { + if (one == UNKNOWN) { + return two; + } + if (two == UNKNOWN) { + return one; + } + return one.leastUpperBound(two); + } + + public static Safety mergeAssumingUnknownIsSame(Safety one, Safety two, Safety three) { + Safety result = mergeAssumingUnknownIsSame(one, two); + return mergeAssumingUnknownIsSame(result, three); + } +} diff --git a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/safety/SafetyAnalysis.java b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/safety/SafetyAnalysis.java new file mode 100644 index 000000000..a5ab716ce --- /dev/null +++ b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/safety/SafetyAnalysis.java @@ -0,0 +1,49 @@ +/* + * (c) Copyright 2022 Palantir Technologies Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.palantir.baseline.errorprone.safety; + +import com.google.errorprone.VisitorState; +import com.google.errorprone.dataflow.DataFlow; +import com.palantir.baseline.errorprone.safety.SafetyPropagationTransfer.ClearVisitorState; +import com.sun.source.util.TreePath; +import com.sun.tools.javac.util.Context; + +public final class SafetyAnalysis { + private static final Context.Key SAFETY_PROPAGATION = new Context.Key<>(); + + /** + * Returns the safety of the item at the current path. + * Callers may need to use {@link VisitorState#withPath(TreePath)} to provide a more specific path. + */ + public static Safety of(VisitorState state) { + SafetyPropagationTransfer propagation = instance(state.context); + try (ClearVisitorState ignored = propagation.setVisitorState(state)) { + return DataFlow.expressionDataflow(state.getPath(), state.context, propagation); + } + } + + private static SafetyPropagationTransfer instance(Context context) { + SafetyPropagationTransfer instance = context.get(SAFETY_PROPAGATION); + if (instance == null) { + instance = new SafetyPropagationTransfer(); + context.put(SAFETY_PROPAGATION, instance); + } + return instance; + } + + private SafetyAnalysis() {} +} diff --git a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/safety/SafetyAnnotations.java b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/safety/SafetyAnnotations.java new file mode 100644 index 000000000..014cbda50 --- /dev/null +++ b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/safety/SafetyAnnotations.java @@ -0,0 +1,108 @@ +/* + * (c) Copyright 2022 Palantir Technologies Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.palantir.baseline.errorprone.safety; + +import com.google.errorprone.VisitorState; +import com.google.errorprone.util.ASTHelpers; +import com.sun.source.tree.ExpressionTree; +import com.sun.tools.javac.code.Symbol; +import com.sun.tools.javac.code.Symbol.MethodSymbol; +import com.sun.tools.javac.code.Symbol.VarSymbol; +import com.sun.tools.javac.code.Type; +import com.sun.tools.javac.util.List; +import java.util.Objects; +import javax.annotation.Nullable; + +public final class SafetyAnnotations { + private static final String SAFE = "com.palantir.logsafe.Safe"; + private static final String UNSAFE = "com.palantir.logsafe.Unsafe"; + private static final String DO_NOT_LOG = "com.palantir.logsafe.DoNotLog"; + + public static Safety getSafety(ExpressionTree input, VisitorState state) { + // Check the result type + ExpressionTree tree = ASTHelpers.stripParentheses(input); + Safety resultTypeSafet = getResultTypeSafety(tree, state); + + // Check the argument symbol itself: + Symbol argumentSymbol = ASTHelpers.getSymbol(tree); + Safety symbolSafety = argumentSymbol != null ? getSafety(argumentSymbol, state) : Safety.UNKNOWN; + return Safety.mergeAssumingUnknownIsSame(resultTypeSafet, symbolSafety); + } + + public static Safety getSafety(@Nullable Symbol symbol, VisitorState state) { + if (symbol != null) { + if (ASTHelpers.hasAnnotation(symbol, DO_NOT_LOG, state)) { + return Safety.DO_NOT_LOG; + } + if (ASTHelpers.hasAnnotation(symbol, UNSAFE, state)) { + return Safety.UNSAFE; + } + if (ASTHelpers.hasAnnotation(symbol, SAFE, state)) { + return Safety.SAFE; + } + // Check super-methods + if (symbol instanceof MethodSymbol) { + return getSuperMethodSafety((MethodSymbol) symbol, state); + } + if (symbol instanceof VarSymbol) { + VarSymbol varSymbol = (VarSymbol) symbol; + return getSuperMethodParameterSafety(varSymbol, state); + } + } + return Safety.UNKNOWN; + } + + private static Safety getSuperMethodSafety(MethodSymbol method, VisitorState state) { + Safety safety = Safety.UNKNOWN; + if (!method.isStaticOrInstanceInit()) { + for (MethodSymbol superMethod : ASTHelpers.findSuperMethods(method, state.getTypes())) { + safety = Safety.mergeAssumingUnknownIsSame(safety, getSafety(superMethod, state)); + } + } + return safety; + } + + private static Safety getSuperMethodParameterSafety(VarSymbol varSymbol, VisitorState state) { + Safety safety = Safety.UNKNOWN; + if (varSymbol.owner instanceof MethodSymbol) { + // If the owner is a MethodSymbol, this variable is a method parameter + MethodSymbol method = (MethodSymbol) varSymbol.owner; + if (!method.isStaticOrInstanceInit()) { + List methodParameters = method.getParameters(); + for (int i = 0; i < methodParameters.size(); i++) { + VarSymbol current = methodParameters.get(i); + if (Objects.equals(current, varSymbol)) { + for (MethodSymbol superMethod : ASTHelpers.findSuperMethods(method, state.getTypes())) { + safety = Safety.mergeAssumingUnknownIsSame( + safety, + getSafety(superMethod.getParameters().get(i), state)); + } + return safety; + } + } + } + } + return safety; + } + + public static Safety getResultTypeSafety(ExpressionTree expression, VisitorState state) { + Type resultType = ASTHelpers.getResultType(expression); + return resultType == null ? Safety.UNKNOWN : getSafety(resultType.tsym, state); + } + + private SafetyAnnotations() {} +} diff --git a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/safety/SafetyPropagationTransfer.java b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/safety/SafetyPropagationTransfer.java new file mode 100644 index 000000000..c6ecd5880 --- /dev/null +++ b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/safety/SafetyPropagationTransfer.java @@ -0,0 +1,853 @@ +/* + * (c) Copyright 2022 Palantir Technologies Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.palantir.baseline.errorprone.safety; + +import com.google.errorprone.VisitorState; +import com.google.errorprone.annotations.CheckReturnValue; +import com.google.errorprone.dataflow.AccessPath; +import com.google.errorprone.dataflow.AccessPathStore; +import com.google.errorprone.dataflow.AccessPathValues; +import com.google.errorprone.matchers.Matcher; +import com.google.errorprone.matchers.method.MethodMatchers; +import com.google.errorprone.util.ASTHelpers; +import com.sun.source.tree.ClassTree; +import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.Tree; +import com.sun.source.tree.TypeCastTree; +import com.sun.source.tree.VariableTree; +import com.sun.source.util.TreePath; +import com.sun.source.util.Trees; +import com.sun.tools.javac.code.Flags; +import com.sun.tools.javac.code.Symbol; +import com.sun.tools.javac.code.Symbol.MethodSymbol; +import com.sun.tools.javac.code.Symbol.VarSymbol; +import com.sun.tools.javac.code.Type; +import com.sun.tools.javac.processing.JavacProcessingEnvironment; +import java.io.Closeable; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import javax.annotation.Nullable; +import javax.lang.model.element.VariableElement; +import org.checkerframework.errorprone.dataflow.analysis.Analysis; +import org.checkerframework.errorprone.dataflow.analysis.ForwardAnalysisImpl; +import org.checkerframework.errorprone.dataflow.analysis.ForwardTransferFunction; +import org.checkerframework.errorprone.dataflow.analysis.RegularTransferResult; +import org.checkerframework.errorprone.dataflow.analysis.TransferInput; +import org.checkerframework.errorprone.dataflow.analysis.TransferResult; +import org.checkerframework.errorprone.dataflow.cfg.ControlFlowGraph; +import org.checkerframework.errorprone.dataflow.cfg.UnderlyingAST; +import org.checkerframework.errorprone.dataflow.cfg.builder.CFGBuilder; +import org.checkerframework.errorprone.dataflow.cfg.node.ArrayAccessNode; +import org.checkerframework.errorprone.dataflow.cfg.node.ArrayCreationNode; +import org.checkerframework.errorprone.dataflow.cfg.node.ArrayTypeNode; +import org.checkerframework.errorprone.dataflow.cfg.node.AssertionErrorNode; +import org.checkerframework.errorprone.dataflow.cfg.node.AssignmentNode; +import org.checkerframework.errorprone.dataflow.cfg.node.BinaryOperationNode; +import org.checkerframework.errorprone.dataflow.cfg.node.BitwiseAndNode; +import org.checkerframework.errorprone.dataflow.cfg.node.BitwiseComplementNode; +import org.checkerframework.errorprone.dataflow.cfg.node.BitwiseOrNode; +import org.checkerframework.errorprone.dataflow.cfg.node.BitwiseXorNode; +import org.checkerframework.errorprone.dataflow.cfg.node.BooleanLiteralNode; +import org.checkerframework.errorprone.dataflow.cfg.node.CaseNode; +import org.checkerframework.errorprone.dataflow.cfg.node.CharacterLiteralNode; +import org.checkerframework.errorprone.dataflow.cfg.node.ClassDeclarationNode; +import org.checkerframework.errorprone.dataflow.cfg.node.ClassNameNode; +import org.checkerframework.errorprone.dataflow.cfg.node.ConditionalAndNode; +import org.checkerframework.errorprone.dataflow.cfg.node.ConditionalNotNode; +import org.checkerframework.errorprone.dataflow.cfg.node.ConditionalOrNode; +import org.checkerframework.errorprone.dataflow.cfg.node.DoubleLiteralNode; +import org.checkerframework.errorprone.dataflow.cfg.node.EqualToNode; +import org.checkerframework.errorprone.dataflow.cfg.node.ExplicitThisNode; +import org.checkerframework.errorprone.dataflow.cfg.node.FieldAccessNode; +import org.checkerframework.errorprone.dataflow.cfg.node.FloatLiteralNode; +import org.checkerframework.errorprone.dataflow.cfg.node.FloatingDivisionNode; +import org.checkerframework.errorprone.dataflow.cfg.node.FloatingRemainderNode; +import org.checkerframework.errorprone.dataflow.cfg.node.FunctionalInterfaceNode; +import org.checkerframework.errorprone.dataflow.cfg.node.GreaterThanNode; +import org.checkerframework.errorprone.dataflow.cfg.node.GreaterThanOrEqualNode; +import org.checkerframework.errorprone.dataflow.cfg.node.ImplicitThisNode; +import org.checkerframework.errorprone.dataflow.cfg.node.InstanceOfNode; +import org.checkerframework.errorprone.dataflow.cfg.node.IntegerDivisionNode; +import org.checkerframework.errorprone.dataflow.cfg.node.IntegerLiteralNode; +import org.checkerframework.errorprone.dataflow.cfg.node.IntegerRemainderNode; +import org.checkerframework.errorprone.dataflow.cfg.node.LambdaResultExpressionNode; +import org.checkerframework.errorprone.dataflow.cfg.node.LeftShiftNode; +import org.checkerframework.errorprone.dataflow.cfg.node.LessThanNode; +import org.checkerframework.errorprone.dataflow.cfg.node.LessThanOrEqualNode; +import org.checkerframework.errorprone.dataflow.cfg.node.LocalVariableNode; +import org.checkerframework.errorprone.dataflow.cfg.node.LongLiteralNode; +import org.checkerframework.errorprone.dataflow.cfg.node.MarkerNode; +import org.checkerframework.errorprone.dataflow.cfg.node.MethodAccessNode; +import org.checkerframework.errorprone.dataflow.cfg.node.MethodInvocationNode; +import org.checkerframework.errorprone.dataflow.cfg.node.NarrowingConversionNode; +import org.checkerframework.errorprone.dataflow.cfg.node.Node; +import org.checkerframework.errorprone.dataflow.cfg.node.NotEqualNode; +import org.checkerframework.errorprone.dataflow.cfg.node.NullChkNode; +import org.checkerframework.errorprone.dataflow.cfg.node.NullLiteralNode; +import org.checkerframework.errorprone.dataflow.cfg.node.NumericalAdditionNode; +import org.checkerframework.errorprone.dataflow.cfg.node.NumericalMinusNode; +import org.checkerframework.errorprone.dataflow.cfg.node.NumericalMultiplicationNode; +import org.checkerframework.errorprone.dataflow.cfg.node.NumericalPlusNode; +import org.checkerframework.errorprone.dataflow.cfg.node.NumericalSubtractionNode; +import org.checkerframework.errorprone.dataflow.cfg.node.ObjectCreationNode; +import org.checkerframework.errorprone.dataflow.cfg.node.PackageNameNode; +import org.checkerframework.errorprone.dataflow.cfg.node.ParameterizedTypeNode; +import org.checkerframework.errorprone.dataflow.cfg.node.PrimitiveTypeNode; +import org.checkerframework.errorprone.dataflow.cfg.node.ReturnNode; +import org.checkerframework.errorprone.dataflow.cfg.node.ShortLiteralNode; +import org.checkerframework.errorprone.dataflow.cfg.node.SignedRightShiftNode; +import org.checkerframework.errorprone.dataflow.cfg.node.StringConcatenateAssignmentNode; +import org.checkerframework.errorprone.dataflow.cfg.node.StringConcatenateNode; +import org.checkerframework.errorprone.dataflow.cfg.node.StringConversionNode; +import org.checkerframework.errorprone.dataflow.cfg.node.StringLiteralNode; +import org.checkerframework.errorprone.dataflow.cfg.node.SuperNode; +import org.checkerframework.errorprone.dataflow.cfg.node.SynchronizedNode; +import org.checkerframework.errorprone.dataflow.cfg.node.TernaryExpressionNode; +import org.checkerframework.errorprone.dataflow.cfg.node.ThrowNode; +import org.checkerframework.errorprone.dataflow.cfg.node.TypeCastNode; +import org.checkerframework.errorprone.dataflow.cfg.node.UnaryOperationNode; +import org.checkerframework.errorprone.dataflow.cfg.node.UnsignedRightShiftNode; +import org.checkerframework.errorprone.dataflow.cfg.node.VariableDeclarationNode; +import org.checkerframework.errorprone.dataflow.cfg.node.WideningConversionNode; + +/** + * Heavily modified fork from error-prone NullnessPropagationTransfer (apache 2). + * @see NullnessPropagationTransfer + */ +public final class SafetyPropagationTransfer implements ForwardTransferFunction> { + + private static final Matcher TO_STRING = + MethodMatchers.instanceMethod().anyClass().named("toString").withNoParameters(); + + private VisitorState state; + private final Set traversed = new HashSet<>(); + + @Override + public AccessPathStore initialStore(UnderlyingAST _underlyingAst, List parameters) { + if (parameters == null) { + return AccessPathStore.empty(); + } + AccessPathStore.Builder result = AccessPathStore.empty().toBuilder(); + for (LocalVariableNode param : parameters) { + Safety declared = SafetyAnnotations.getSafety((Symbol) param.getElement(), state); + result.setInformation(AccessPath.fromLocalVariable(param), declared); + } + return result.build(); + } + + public ClearVisitorState setVisitorState(VisitorState value) { + this.state = Objects.requireNonNull(value, "VisitorState"); + traversed.clear(); + return new ClearVisitorState(); + } + + public final class ClearVisitorState implements Closeable { + @Override + public void close() { + SafetyPropagationTransfer.this.state = null; + traversed.clear(); + } + } + + private static TransferResult> noStoreChanges( + Safety value, TransferInput> input) { + return new RegularTransferResult<>(value, input.getRegularStore()); + } + + @CheckReturnValue + private static TransferResult> updateRegularStore( + Safety value, TransferInput> input, ReadableUpdates updates) { + ResultingStore newStore = updateStore(input.getRegularStore(), updates); + return new RegularTransferResult<>(value, newStore.store, newStore.storeChanged); + } + + @CheckReturnValue + private static ResultingStore updateStore(AccessPathStore oldStore, ReadableUpdates update) { + AccessPathStore.Builder builder = oldStore.toBuilder(); + update.values.forEach(builder::setInformation); + AccessPathStore newStore = builder.build(); + return new ResultingStore(newStore, !newStore.equals(oldStore)); + } + + @SuppressWarnings("CheckStyle") + private static final class ResultingStore { + private final AccessPathStore store; + private final boolean storeChanged; + + ResultingStore(AccessPathStore store, boolean storeChanged) { + this.store = store; + this.storeChanged = storeChanged; + } + } + + interface Updates { + void set(LocalVariableNode node, Safety value); + + void set(VariableDeclarationNode node, Safety value); + + void set(FieldAccessNode node, Safety value); + + void set(AccessPath path, Safety value); + + default void trySet(Node node, Safety value) { + if (node instanceof LocalVariableNode) { + set((LocalVariableNode) node, value); + } else if (node instanceof FieldAccessNode) { + set((FieldAccessNode) node, value); + } else if (node instanceof VariableDeclarationNode) { + set((VariableDeclarationNode) node, value); + } + } + + boolean isEmpty(); + } + + private static final class ReadableUpdates implements Updates { + private final Map values = new HashMap<>(); + + @Override + public void set(LocalVariableNode node, Safety value) { + values.put(AccessPath.fromLocalVariable(node), Objects.requireNonNull(value)); + } + + @Override + public void set(VariableDeclarationNode node, Safety value) { + values.put(AccessPath.fromVariableDecl(node), Objects.requireNonNull(value)); + } + + @Override + public void set(FieldAccessNode node, Safety value) { + AccessPath path = AccessPath.fromFieldAccess(node); + if (path != null) { + values.put(path, Objects.requireNonNull(value)); + } + } + + @Override + public void set(AccessPath path, Safety value) { + values.put(Objects.requireNonNull(path), Objects.requireNonNull(value)); + } + + @Override + public boolean isEmpty() { + return values.isEmpty(); + } + } + + private static TransferResult> unknown( + TransferInput> input) { + return noStoreChanges(Safety.UNKNOWN, input); + } + + private TransferResult> literal( + TransferInput> input) { + // Compile-time data (literal) is guaranteed to be safe. + return noStoreChanges(Safety.SAFE, input); + } + + private TransferResult> unary( + UnaryOperationNode node, TransferInput> input) { + Safety safety = input.getValueOfSubNode(node.getOperand()); + return noStoreChanges(safety, input); + } + + private TransferResult> binary( + BinaryOperationNode node, TransferInput> input) { + Safety safety = input.getValueOfSubNode(node.getLeftOperand()) + .leastUpperBound(input.getValueOfSubNode(node.getRightOperand())); + return noStoreChanges(safety, input); + } + + // Visitor methods + + @Override + public TransferResult> visitShortLiteral( + ShortLiteralNode _node, TransferInput> input) { + return literal(input); + } + + @Override + public TransferResult> visitIntegerLiteral( + IntegerLiteralNode _node, TransferInput> input) { + return literal(input); + } + + @Override + public TransferResult> visitLongLiteral( + LongLiteralNode _node, TransferInput> input) { + return literal(input); + } + + @Override + public TransferResult> visitFloatLiteral( + FloatLiteralNode _node, TransferInput> input) { + return literal(input); + } + + @Override + public TransferResult> visitDoubleLiteral( + DoubleLiteralNode _node, TransferInput> input) { + return literal(input); + } + + @Override + public TransferResult> visitBooleanLiteral( + BooleanLiteralNode _node, TransferInput> input) { + return literal(input); + } + + @Override + public TransferResult> visitCharacterLiteral( + CharacterLiteralNode _node, TransferInput> input) { + return literal(input); + } + + @Override + public TransferResult> visitStringLiteral( + StringLiteralNode _node, TransferInput> input) { + return literal(input); + } + + @Override + public TransferResult> visitNullLiteral( + NullLiteralNode _node, TransferInput> input) { + return literal(input); + } + + @Override + public TransferResult> visitNumericalMinus( + NumericalMinusNode node, TransferInput> input) { + return unary(node, input); + } + + @Override + public TransferResult> visitNumericalPlus( + NumericalPlusNode node, TransferInput> input) { + return unary(node, input); + } + + @Override + public TransferResult> visitBitwiseComplement( + BitwiseComplementNode node, TransferInput> input) { + return unary(node, input); + } + + @Override + public TransferResult> visitNullChk( + NullChkNode node, TransferInput> input) { + // null values are safe, and null check boolean results are safe + return noStoreChanges(Safety.SAFE, input); + } + + @Override + public TransferResult> visitStringConcatenate( + StringConcatenateNode node, TransferInput> input) { + return binary(node, input); + } + + @Override + public TransferResult> visitNumericalAddition( + NumericalAdditionNode node, TransferInput> input) { + return binary(node, input); + } + + @Override + public TransferResult> visitNumericalSubtraction( + NumericalSubtractionNode node, TransferInput> input) { + return binary(node, input); + } + + @Override + public TransferResult> visitNumericalMultiplication( + NumericalMultiplicationNode node, TransferInput> input) { + return binary(node, input); + } + + @Override + public TransferResult> visitIntegerDivision( + IntegerDivisionNode node, TransferInput> input) { + return binary(node, input); + } + + @Override + public TransferResult> visitFloatingDivision( + FloatingDivisionNode node, TransferInput> input) { + return binary(node, input); + } + + @Override + public TransferResult> visitIntegerRemainder( + IntegerRemainderNode node, TransferInput> input) { + return binary(node, input); + } + + @Override + public TransferResult> visitFloatingRemainder( + FloatingRemainderNode node, TransferInput> input) { + return binary(node, input); + } + + @Override + public TransferResult> visitLeftShift( + LeftShiftNode node, TransferInput> input) { + return binary(node, input); + } + + @Override + public TransferResult> visitSignedRightShift( + SignedRightShiftNode node, TransferInput> input) { + return binary(node, input); + } + + @Override + public TransferResult> visitUnsignedRightShift( + UnsignedRightShiftNode node, TransferInput> input) { + return binary(node, input); + } + + @Override + public TransferResult> visitBitwiseAnd( + BitwiseAndNode node, TransferInput> input) { + return binary(node, input); + } + + @Override + public TransferResult> visitBitwiseOr( + BitwiseOrNode node, TransferInput> input) { + return binary(node, input); + } + + @Override + public TransferResult> visitBitwiseXor( + BitwiseXorNode node, TransferInput> input) { + return binary(node, input); + } + + @Override + public TransferResult> visitStringConcatenateAssignment( + StringConcatenateAssignmentNode node, TransferInput> input) { + Safety safety = input.getValueOfSubNode(node.getLeftOperand()) + .leastUpperBound(input.getValueOfSubNode(node.getRightOperand())); + ReadableUpdates updates = new ReadableUpdates(); + updates.trySet(node.getLeftOperand(), safety); + return updateRegularStore(safety, input, updates); + } + + @Override + public TransferResult> visitLessThan( + LessThanNode node, TransferInput> input) { + return binary(node, input); + } + + @Override + public TransferResult> visitLessThanOrEqual( + LessThanOrEqualNode node, TransferInput> input) { + return binary(node, input); + } + + @Override + public TransferResult> visitGreaterThan( + GreaterThanNode node, TransferInput> input) { + return binary(node, input); + } + + @Override + public TransferResult> visitGreaterThanOrEqual( + GreaterThanOrEqualNode node, TransferInput> input) { + return binary(node, input); + } + + @Override + public TransferResult> visitEqualTo( + EqualToNode node, TransferInput> input) { + return binary(node, input); + } + + @Override + public TransferResult> visitNotEqual( + NotEqualNode node, TransferInput> input) { + return binary(node, input); + } + + @Override + public TransferResult> visitConditionalAnd( + ConditionalAndNode node, TransferInput> input) { + return binary(node, input); + } + + @Override + public TransferResult> visitConditionalOr( + ConditionalOrNode node, TransferInput> input) { + return binary(node, input); + } + + @Override + public TransferResult> visitConditionalNot( + ConditionalNotNode node, TransferInput> input) { + return unary(node, input); + } + + @Override + public TransferResult> visitTernaryExpression( + TernaryExpressionNode node, TransferInput> input) { + Safety safety = input.getValueOfSubNode(node.getThenOperand()) + .leastUpperBound(input.getValueOfSubNode(node.getElseOperand())); + return noStoreChanges(safety, input); + } + + @Override + public TransferResult> visitAssignment( + AssignmentNode node, TransferInput> input) { + ReadableUpdates updates = new ReadableUpdates(); + Safety safety = input.getValueOfSubNode(node.getExpression()); + Node target = node.getTarget(); + if (target instanceof LocalVariableNode) { + updates.trySet(target, safety); + } else if (target instanceof ArrayAccessNode) { + updates.trySet(((ArrayAccessNode) target).getArray(), safety); + } else if (target instanceof FieldAccessNode) { + FieldAccessNode fieldAccess = (FieldAccessNode) target; + updates.set(fieldAccess, safety); + } else { + throw new UnsupportedOperationException( + "Safety analysis bug, unknown target type: " + target.getClass() + " with value: " + target); + } + return updateRegularStore(safety, input, updates); + } + + @Override + public TransferResult> visitLocalVariable( + LocalVariableNode node, TransferInput> input) { + Safety safety = hasNonNullConstantValue(node) + ? Safety.SAFE + : input.getRegularStore().valueOfAccessPath(AccessPath.fromLocalVariable(node), Safety.UNKNOWN); + return noStoreChanges(safety, input); + } + + private static boolean hasNonNullConstantValue(LocalVariableNode node) { + if (node.getElement() instanceof VariableElement) { + VariableElement element = (VariableElement) node.getElement(); + return (element.getConstantValue() != null); + } + return false; + } + + @Override + public TransferResult> visitVariableDeclaration( + VariableDeclarationNode node, TransferInput> input) { + Safety safety = + SafetyAnnotations.getSafety(ASTHelpers.getSymbol(node.getTree().getType()), state); + return noStoreChanges(safety, input); + } + + @Override + public TransferResult> visitFieldAccess( + FieldAccessNode node, TransferInput> input) { + Safety fieldSafety = SafetyAnnotations.getSafety(ASTHelpers.getSymbol(node.getTree()), state); + Type fieldType = ASTHelpers.getType(node.getTree()); + Safety typeSafety = fieldType == null ? Safety.UNKNOWN : SafetyAnnotations.getSafety(fieldType.tsym, state); + VarSymbol symbol = (VarSymbol) ASTHelpers.getSymbol(node.getTree()); + AccessPath maybeAccessPath = AccessPath.fromFieldAccess(node); + Safety flowSafety = fieldSafety(symbol, maybeAccessPath, input.getRegularStore()); + Safety safety = Safety.mergeAssumingUnknownIsSame(fieldSafety, typeSafety, flowSafety); + return noStoreChanges(safety, input); + } + + @SuppressWarnings("checkstyle:CyclomaticComplexity") + private Safety fieldSafety( + @Nullable VarSymbol accessed, @Nullable AccessPath path, AccessPathValues store) { + if (accessed == null) { + return Safety.UNKNOWN; + } + Safety maybeFlowComputedSafety = (path == null) ? null : store.valueOfAccessPath(path, null); + Safety flowSafety = maybeFlowComputedSafety == null ? Safety.UNKNOWN : maybeFlowComputedSafety; + Safety symbolSafety = SafetyAnnotations.getSafety(accessed, state); + Safety typeSafety = SafetyAnnotations.getSafety(accessed.type.tsym, state); + Safety symbolAndTypeSafety = Safety.mergeAssumingUnknownIsSame(flowSafety, symbolSafety, typeSafety); + if (accessed.getSimpleName().contentEquals("class")) { + // compile-time constant + return Safety.mergeAssumingUnknownIsSame(symbolAndTypeSafety, Safety.SAFE); + } + if (accessed.isEnum()) { + // compile-time enum constant + return Safety.mergeAssumingUnknownIsSame(symbolAndTypeSafety, Safety.SAFE); + } + if (accessed.getConstValue() != null) { + // compile-time constant value + return Safety.mergeAssumingUnknownIsSame(symbolAndTypeSafety, Safety.SAFE); + } + // If a computed value was found, we can avoid expensive initializer interactions + if (maybeFlowComputedSafety == null) { + Safety initializer = fieldInitializerSafetyIfAvailable(accessed); + // If the field is final, we trust the result of the initializer. + // Non-final fields can be "tainted" by initializers (to unsafe/do-not-log) but not + // upgraded to safe. + if ((accessed.flags_field & Flags.FINAL) != 0 + || initializer == Safety.DO_NOT_LOG + || initializer == Safety.UNSAFE) { + return Safety.mergeAssumingUnknownIsSame(symbolAndTypeSafety, initializer); + } + } + return symbolAndTypeSafety; + } + + private Safety fieldInitializerSafetyIfAvailable(VarSymbol accessed) { + if (!traversed.add(accessed)) { + // Avoid infinite recursion between initializers with circular references. We recommend against + // writing such initializers, but handle it gracefully. + return Safety.UNKNOWN; + } + + try { + JavacProcessingEnvironment javacEnv = JavacProcessingEnvironment.instance(state.context); + TreePath fieldDeclPath = Trees.instance(javacEnv).getPath(accessed); + // Skip initializers in other compilation units as analysis of such nodes can fail due to + // missing types. + if (fieldDeclPath == null + || fieldDeclPath.getCompilationUnit() != state.getPath().getCompilationUnit() + || !(fieldDeclPath.getLeaf() instanceof VariableTree)) { + return Safety.UNKNOWN; + } + + ExpressionTree initializer = ((VariableTree) fieldDeclPath.getLeaf()).getInitializer(); + if (initializer == null) { + return Safety.UNKNOWN; + } + + ClassTree classTree = (ClassTree) fieldDeclPath.getParentPath().getLeaf(); + + // Run flow analysis on field initializer. This is inefficient compared to just walking + // the initializer expression tree but it avoids duplicating the logic from this transfer + // function into a method that operates on Javac Nodes. + TreePath initializerPath = TreePath.getPath(fieldDeclPath, initializer); + UnderlyingAST ast = new UnderlyingAST.CFGStatement(initializerPath.getLeaf(), classTree); + ControlFlowGraph cfg = CFGBuilder.build(initializerPath, ast, false, false, javacEnv); + Analysis, SafetyPropagationTransfer> analysis = + new ForwardAnalysisImpl<>(this); + analysis.performAnalysis(cfg); + Safety maybeResult = analysis.getValue(initializerPath.getLeaf()); + return maybeResult == null ? Safety.UNKNOWN : maybeResult; + } finally { + traversed.remove(accessed); + } + } + + @Override + public TransferResult> visitMethodAccess( + MethodAccessNode node, TransferInput> input) { + return unknown(input); + } + + @Override + public TransferResult> visitArrayAccess( + ArrayAccessNode node, TransferInput> input) { + return unknown(input); + } + + @Override + public TransferResult> visitImplicitThis( + ImplicitThisNode node, TransferInput> input) { + Symbol symbol = ASTHelpers.getSymbol(node.getTree()); + Safety safety = symbol == null ? Safety.UNKNOWN : SafetyAnnotations.getSafety(symbol.owner, state); + return noStoreChanges(safety, input); + } + + @Override + public TransferResult> visitExplicitThis( + ExplicitThisNode node, TransferInput> input) { + Symbol symbol = ASTHelpers.getSymbol(node.getTree()); + Safety safety = symbol == null ? Safety.UNKNOWN : SafetyAnnotations.getSafety(symbol.owner, state); + return noStoreChanges(safety, input); + } + + @Override + public TransferResult> visitSuper( + SuperNode node, TransferInput> input) { + Symbol symbol = ASTHelpers.getSymbol(node.getTree()); + Safety safety = symbol == null ? Safety.UNKNOWN : SafetyAnnotations.getSafety(symbol.owner, state); + return noStoreChanges(safety, input); + } + + @Override + public TransferResult> visitReturn( + ReturnNode node, TransferInput> input) { + Node result = node.getResult(); + Safety safety = result == null ? Safety.SAFE : input.getValueOfSubNode(result); + return noStoreChanges(safety, input); + } + + @Override + public TransferResult> visitLambdaResultExpression( + LambdaResultExpressionNode node, TransferInput> input) { + return unknown(input); + } + + @Override + public TransferResult> visitStringConversion( + StringConversionNode node, TransferInput> input) { + Safety safety = input.getValueOfSubNode(node.getOperand()); + return noStoreChanges(safety, input); + } + + @Override + public TransferResult> visitWideningConversion( + WideningConversionNode node, TransferInput> input) { + return handleTypeConversion(node.getTree(), node.getOperand(), input); + } + + @Override + public TransferResult> visitNarrowingConversion( + NarrowingConversionNode node, TransferInput> input) { + return handleTypeConversion(node.getTree(), node.getOperand(), input); + } + + @Override + public TransferResult> visitTypeCast( + TypeCastNode node, TransferInput> input) { + TypeCastTree castTree = (TypeCastTree) node.getTree(); + return handleTypeConversion(castTree.getType(), node.getOperand(), input); + } + + /** Handles type changes (widen, narrow, and cast). */ + private TransferResult> handleTypeConversion( + Tree newType, Node original, TransferInput> input) { + Safety valueSafety = input.getValueOfSubNode(original); + Type targetType = ASTHelpers.getType(newType); + Safety narrowTargetSafety = + targetType == null ? Safety.UNKNOWN : SafetyAnnotations.getSafety(targetType.tsym, state); + Safety resultSafety = Safety.mergeAssumingUnknownIsSame(valueSafety, narrowTargetSafety); + return noStoreChanges(resultSafety, input); + } + + @Override + public TransferResult> visitInstanceOf( + InstanceOfNode node, TransferInput> input) { + return unknown(input); + } + + @Override + public TransferResult> visitSynchronized( + SynchronizedNode node, TransferInput> input) { + return unknown(input); + } + + @Override + public TransferResult> visitAssertionError( + AssertionErrorNode node, TransferInput> input) { + return unknown(input); + } + + @Override + public TransferResult> visitThrow( + ThrowNode node, TransferInput> input) { + return unknown(input); + } + + @Override + public TransferResult> visitCase( + CaseNode node, TransferInput> input) { + return unknown(input); + } + + @Override + public TransferResult> visitMethodInvocation( + MethodInvocationNode node, TransferInput> input) { + Safety result = getMethodSymbolSafety(node, input); + return noStoreChanges(result, input); + } + + private Safety getMethodSymbolSafety( + MethodInvocationNode node, TransferInput> input) { + MethodSymbol methodSymbol = ASTHelpers.getSymbol(node.getTree()); + if (methodSymbol != null) { + Safety methodSafety = Safety.mergeAssumingUnknownIsSame( + SafetyAnnotations.getSafety(methodSymbol, state), + SafetyAnnotations.getResultTypeSafety(node.getTree(), state)); + // non-annotated toString inherits type-level safety. + if (methodSafety == Safety.UNKNOWN && TO_STRING.matches(node.getTree(), state)) { + return input.getValueOfSubNode(node.getTarget().getReceiver()); + } + return methodSafety; + } + return SafetyAnnotations.getResultTypeSafety(node.getTree(), state); + } + + @Override + public TransferResult> visitObjectCreation( + ObjectCreationNode node, TransferInput> input) { + Safety result = SafetyAnnotations.getSafety(node.getTree(), state); + return noStoreChanges(result, input); + } + + @Override + public TransferResult> visitMemberReference( + FunctionalInterfaceNode node, TransferInput> input) { + return unknown(input); + } + + @Override + public TransferResult> visitArrayCreation( + ArrayCreationNode node, TransferInput> input) { + return unknown(input); + } + + @Override + public TransferResult> visitArrayType( + ArrayTypeNode node, TransferInput> input) { + return unknown(input); + } + + @Override + public TransferResult> visitPrimitiveType( + PrimitiveTypeNode node, TransferInput> input) { + return unknown(input); + } + + @Override + public TransferResult> visitClassName( + ClassNameNode node, TransferInput> input) { + return unknown(input); + } + + @Override + public TransferResult> visitPackageName( + PackageNameNode node, TransferInput> input) { + return unknown(input); + } + + @Override + public TransferResult> visitParameterizedType( + ParameterizedTypeNode node, TransferInput> input) { + return unknown(input); + } + + @Override + public TransferResult> visitMarker( + MarkerNode node, TransferInput> input) { + return unknown(input); + } + + @Override + public TransferResult> visitClassDeclaration( + ClassDeclarationNode node, TransferInput> input) { + return unknown(input); + } +} diff --git a/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/IllegalSafeLoggingArgumentTest.java b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/IllegalSafeLoggingArgumentTest.java index 914c92f3d..91d91a3d8 100644 --- a/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/IllegalSafeLoggingArgumentTest.java +++ b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/IllegalSafeLoggingArgumentTest.java @@ -203,6 +203,58 @@ public void testSafeReturn() { .doTest(); } + @Test + public void testSafeReturnIndirect() { + helper().addSourceLines( + "Test.java", + "import com.palantir.logsafe.*;", + "class Test {", + " @DoNotLog static Object doNotLogMethod() { return null; }", + " @Safe static Object safeMethod() { return null; }", + " @Unsafe static Object unsafeMethod() { return null; }", + " void f() {", + " Object one = doNotLogMethod();", + " Object two = safeMethod();", + " Object three = unsafeMethod();", + " // BUG: Diagnostic contains: Dangerous argument value: arg is 'DO_NOT_LOG' " + + "but the parameter requires 'SAFE'.", + " fun(one);", + " fun(two);", + " // BUG: Diagnostic contains: Dangerous argument value: arg is 'UNSAFE' " + + "but the parameter requires 'SAFE'.", + " fun(three);", + " }", + " private static void fun(@Safe Object obj) {}", + "}") + .doTest(); + } + + @Test + public void testStringConcatenationWithUnsafe() { + helper().addSourceLines( + "Test.java", + "import com.palantir.logsafe.*;", + "class Test {", + " @DoNotLog static Object doNotLogMethod() { return null; }", + " @Safe static Object safeMethod() { return null; }", + " @Unsafe static Object unsafeMethod() { return null; }", + " void f() {", + " Object one = doNotLogMethod();", + " Object two = safeMethod();", + " Object three = unsafeMethod();", + " // BUG: Diagnostic contains: Dangerous argument value: arg is 'DO_NOT_LOG' " + + "but the parameter requires 'SAFE'.", + " fun(\"test\" + one);", + " fun(\"test\" + two);", + " // BUG: Diagnostic contains: Dangerous argument value: arg is 'UNSAFE' " + + "but the parameter requires 'SAFE'.", + " fun(\"test\" + three);", + " }", + " private static void fun(@Safe Object obj) {}", + "}") + .doTest(); + } + @Test public void testVarArgReturn() { helper().addSourceLines( @@ -267,6 +319,132 @@ public void testTypes_toStringUsesObjectSafety() { .doTest(); } + @Test + public void testUnsafeFinalFieldInitializer() { + helper().addSourceLines( + "Test.java", + "import com.palantir.logsafe.*;", + "class Test {", + " @DoNotLog static Object doNotLogMethod() { return null; }", + " @Safe static Object safeMethod() { return null; }", + " @Unsafe static Object unsafeMethod() { return null; }", + " public final Object one = doNotLogMethod();", + " public final Object two = safeMethod();", + " public final Object three = unsafeMethod();", + " void f() {", + " fun(two);", + " // BUG: Diagnostic contains: Dangerous argument value: arg is 'UNSAFE'", + " fun(three);", + " // BUG: Diagnostic contains: Dangerous argument value: arg is 'DO_NOT_LOG'", + " fun(one);", + " }", + " private static void fun(@Safe Object obj) {}", + "}") + .doTest(); + } + + @Test + public void testUnsafeFieldInitializer() { + helper().addSourceLines( + "Test.java", + "import com.palantir.logsafe.*;", + "class Test {", + " @DoNotLog static Object doNotLogMethod() { return null; }", + " @Safe static Object safeMethod() { return null; }", + " @Unsafe static Object unsafeMethod() { return null; }", + " public Object one = doNotLogMethod();", + " public Object two = safeMethod();", + " public Object three = unsafeMethod();", + " void f() {", + " fun(two);", + " // BUG: Diagnostic contains: Dangerous argument value: arg is 'UNSAFE'", + " fun(three);", + " // BUG: Diagnostic contains: Dangerous argument value: arg is 'DO_NOT_LOG'", + " fun(one);", + " }", + " private static void fun(@Safe Object obj) {}", + "}") + .doTest(); + } + + @Test + public void testUnsafeFieldType() { + helper().addSourceLines( + "Test.java", + "import com.palantir.logsafe.*;", + "class Test {", + " @DoNotLog static class DoNotLogClass {}", + " @Safe static class SafeClass {}", + " @Unsafe static class UnsafeClass {}", + " public DoNotLogClass one;", + " public SafeClass two;", + " public UnsafeClass three;", + " void f() {", + " fun(two);", + " // BUG: Diagnostic contains: Dangerous argument value: arg is 'UNSAFE'", + " fun(three);", + " // BUG: Diagnostic contains: Dangerous argument value: arg is 'DO_NOT_LOG'", + " fun(one);", + " }", + " private static void fun(@Safe Object obj) {}", + "}") + .doTest(); + } + + @Test + public void testUnsafeMethodReturnType_retainsUnsafeType() { + helper().addSourceLines( + "Test.java", + "import com.palantir.logsafe.*;", + "class Test {", + " @DoNotLog static class DoNotLogClass {}", + " @Safe static class SafeClass {}", + " @Unsafe static class UnsafeClass {}", + " static DoNotLogClass doNotLogMethod() { return new DoNotLogClass(); }", + " static SafeClass safeMethod() { return new SafeClass(); }", + " static UnsafeClass unsafeMethod() { return new UnsafeClass(); }", + " void f() {", + " DoNotLogClass one = doNotLogMethod();", + " SafeClass two = safeMethod();", + " UnsafeClass three = unsafeMethod();", + " fun(two);", + " // BUG: Diagnostic contains: Dangerous argument value: arg is 'UNSAFE'", + " fun(three);", + " // BUG: Diagnostic contains: Dangerous argument value: arg is 'DO_NOT_LOG'", + " fun(one);", + " }", + " private static void fun(@Safe Object obj) {}", + "}") + .doTest(); + } + + @Test + public void testUnsafeMethodReturnType_widenedType() { + helper().addSourceLines( + "Test.java", + "import com.palantir.logsafe.*;", + "class Test {", + " @DoNotLog static class DoNotLogClass {}", + " @Safe static class SafeClass {}", + " @Unsafe static class UnsafeClass {}", + " static DoNotLogClass doNotLogMethod() { return new DoNotLogClass(); }", + " static SafeClass safeMethod() { return new SafeClass(); }", + " static UnsafeClass unsafeMethod() { return new UnsafeClass(); }", + " void f() {", + " Object one = doNotLogMethod();", + " Object two = safeMethod();", + " Object three = unsafeMethod();", + " fun(two);", + " // BUG: Diagnostic contains: Dangerous argument value: arg is 'UNSAFE'", + " fun(three);", + " // BUG: Diagnostic contains: Dangerous argument value: arg is 'DO_NOT_LOG'", + " fun(one);", + " }", + " private static void fun(@Safe Object obj) {}", + "}") + .doTest(); + } + @Test public void testIncomingParameter_toStringUsesObjectSafety() { helper().addSourceLines( @@ -288,6 +466,175 @@ public void testIncomingParameter_toStringUsesObjectSafety() { .doTest(); } + @Test + public void testStringConcatAssignmentSafety() { + helper().addSourceLines( + "Test.java", + "import com.palantir.logsafe.*;", + "class Test {", + " void f(", + " @Safe String safeParam,", + " @Unsafe String unsafeParam,", + " @DoNotLog String doNotLogParam) {", + " String foo = safeParam;", + " fun(foo);", + " foo += unsafeParam;", + " // BUG: Diagnostic contains: Dangerous argument value: arg is 'UNSAFE'", + " fun(foo);", + " foo += doNotLogParam;", + " // BUG: Diagnostic contains: Dangerous argument value: arg is 'DO_NOT_LOG'", + " fun(foo);", + " }", + " private static void fun(@Safe Object obj) {}", + "}") + .doTest(); + } + + @Test + public void testIntegerCompound() { + helper().addSourceLines( + "Test.java", + "import com.palantir.logsafe.*;", + "class Test {", + " void f(", + " @Safe int safeParam,", + " @Unsafe int unsafeParam,", + " @DoNotLog int doNotLogParam) {", + " int foo = safeParam;", + " fun(foo);", + " foo += unsafeParam;", + " // BUG: Diagnostic contains: Dangerous argument value: arg is 'UNSAFE'", + " fun(foo);", + " foo += doNotLogParam;", + " // BUG: Diagnostic contains: Dangerous argument value: arg is 'DO_NOT_LOG'", + " fun(foo);", + " }", + " private static void fun(@Safe int obj) {}", + "}") + .doTest(); + } + + @Test + public void testSafeOfThisUnsafe() { + helper().addSourceLines( + "Test.java", + "import com.palantir.logsafe.*;", + "@Unsafe", + "class Test {", + " void f() {", + " // BUG: Diagnostic contains: Dangerous argument value: arg is 'UNSAFE'", + " fun(this);", + " }", + " private static void fun(@Safe Object obj) {}", + "}") + .doTest(); + } + + @Test + public void testSafeOfSuperToStringUnsafe() { + helper().addSourceLines( + "Test.java", + "import com.palantir.logsafe.*;", + "@Unsafe", + "class Test {", + " void f() {", + " // BUG: Diagnostic contains: Dangerous argument value: arg is 'UNSAFE'", + " fun(super.toString());", + " }", + " private static void fun(@Safe Object obj) {}", + "}") + .doTest(); + } + + @Test + public void testInvalidReturnValue() { + helper().addSourceLines( + "Test.java", + "import com.palantir.logsafe.*;", + "class Test {", + " @DoNotLog static class DoNotLogClass {}", + " @Safe static class SafeClass {}", + " @Unsafe static class UnsafeClass {}", + " @Safe Object safe() {", + " return new SafeClass();", + " }", + " @Safe Object unsafe() {", + " // BUG: Diagnostic contains: Dangerous return value: result is 'UNSAFE' " + + "but the method is annotated 'SAFE'.", + " return new UnsafeClass();", + " }", + " @Safe Object doNotLog() {", + " // BUG: Diagnostic contains: Dangerous return value: result is 'DO_NOT_LOG' " + + "but the method is annotated 'SAFE'.", + " return new DoNotLogClass();", + " }", + "}") + .doTest(); + } + + @Test + public void testInvalidReturnValue_AnnotatedSupertype() { + helper().addSourceLines( + "Annotated.java", + "import com.palantir.logsafe.*;", + "interface Annotated {", + " @Safe Object safe();", + " @Safe Object unsafe();", + " @Safe Object doNotLog();", + "}") + .addSourceLines( + "Test.java", + "import com.palantir.logsafe.*;", + "class Test implements Annotated {", + " @DoNotLog static class DoNotLogClass {}", + " @Safe static class SafeClass {}", + " @Unsafe static class UnsafeClass {}", + " @Override public Object safe() {", + " return new SafeClass();", + " }", + " @Override public Object unsafe() {", + " // BUG: Diagnostic contains: Dangerous return value: result is 'UNSAFE' " + + "but the method is annotated 'SAFE'.", + " return new UnsafeClass();", + " }", + " @Override public Object doNotLog() {", + " // BUG: Diagnostic contains: Dangerous return value: result is 'DO_NOT_LOG' " + + "but the method is annotated 'SAFE'.", + " return new DoNotLogClass();", + " }", + "}") + .doTest(); + } + + @Test + public void testInvalidValue_AnnotatedSupertype() { + helper().addSourceLines( + "Annotated.java", + "import com.palantir.logsafe.*;", + "interface Annotated {", + " void safe(@Safe Object value);", + "}") + .addSourceLines( + "Test.java", + "import com.palantir.logsafe.*;", + "class Test implements Annotated {", + " @DoNotLog static class DoNotLogClass {}", + " @Safe static class SafeClass {}", + " @Unsafe static class UnsafeClass {}", + " void f() {", + " // BUG: Diagnostic contains: Dangerous argument value: arg is 'DO_NOT_LOG' " + + "but the parameter requires 'SAFE'.", + " safe(new DoNotLogClass());", + " safe(new SafeClass());", + " // BUG: Diagnostic contains: Dangerous argument value: arg is 'UNSAFE' " + + "but the parameter requires 'SAFE'.", + " safe(new UnsafeClass());", + " }", + " @Override public void safe(Object value) {}", + "}") + .doTest(); + } + @Test public void testSafeArgOfUnsafe_recommendsUnsafeArgOf() { refactoringHelper() diff --git a/changelog/@unreleased/pr-2143.v2.yml b/changelog/@unreleased/pr-2143.v2.yml new file mode 100644 index 000000000..4256fb916 --- /dev/null +++ b/changelog/@unreleased/pr-2143.v2.yml @@ -0,0 +1,5 @@ +type: improvement +improvement: + description: Implement Safety flow checks + links: + - https://github.com/palantir/gradle-baseline/pull/2143