Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ private Description matchClassOrInterface(ClassTree classTree, ClassSymbol class
return matchBasedOnToString(classTree, classSymbol, state);
}

@SuppressWarnings("checkstyle:CyclomaticComplexity")
private Description matchImmutables(ClassTree classTree, ClassSymbol classSymbol, VisitorState state) {
Safety existingClassSafety = SafetyAnnotations.getSafety(classTree, state);
Safety safety = getTypeSafetyFromAncestors(classTree, state);
Expand All @@ -220,9 +221,11 @@ private Description matchImmutables(ClassTree classTree, ClassSymbol classSymbol
// The redaction check allows us to add @DoNotLog to redacted fields in the same sweep as
// adding class-level safety annotations. Otherwise, we would have to run the automatic
// fixes twice.
Safety getterSafety = redacted
? Safety.DO_NOT_LOG
: SafetyAnnotations.getSafety(methodMember.getReturnType(), state);
Safety getterSafety = SafetyAnnotations.getSafety(methodMember.getReturnType(), state);
if (redacted && (getterSafety == Safety.UNKNOWN || getterSafety == Safety.SAFE)) {
// unsafe data may be redacted, however we assume redaction means do-not-log by default
getterSafety = Safety.DO_NOT_LOG;
}
if (getterSafety != Safety.UNKNOWN) {
hasKnownGetter = true;
}
Expand Down Expand Up @@ -295,6 +298,7 @@ private Description annotate(Tree tree, ModifiersTree treeModifiers, VisitorStat
}

@Override
@SuppressWarnings("checkstyle:CyclomaticComplexity")
public Description matchMethod(MethodTree method, VisitorState state) {
if (METHOD_RETURNS_VOID.matches(method, state) || method.getReturnType() == null) {
return Description.NO_MATCH;
Expand All @@ -307,7 +311,9 @@ public Description matchMethod(MethodTree method, VisitorState state) {
Safety methodDeclaredSafety = Safety.mergeAssumingUnknownIsSame(
SafetyAnnotations.getSafety(methodSymbol, state),
SafetyAnnotations.getSafety(method.getReturnType(), state));
// Redacted is do-not-log by default, but may be unsafe.
if (methodDeclaredSafety != Safety.DO_NOT_LOG
&& methodDeclaredSafety != Safety.UNSAFE
&& ASTHelpers.hasAnnotation(methodSymbol, "org.immutables.value.Value.Redacted", state)) {
return handleSafety(method, method.getModifiers(), state, methodDeclaredSafety, Safety.DO_NOT_LOG);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -566,6 +566,36 @@ void testRedactedIsDoNotLog() {
.doTest();
}

@Test
void testRedactedMayBeUnsafe() {
fix().addInputLines(
"Test.java",
"import com.palantir.logsafe.*;",
"import com.fasterxml.jackson.annotation.*;",
"import org.immutables.value.Value;",
"@Value.Immutable",
"interface Test {",
" @Unsafe",
" @JsonValue",
" @Value.Redacted",
" String token();",
"}")
.addOutputLines(
"Test.java",
"import com.palantir.logsafe.*;",
"import com.fasterxml.jackson.annotation.*;",
"import org.immutables.value.Value;",
"@Unsafe",
"@Value.Immutable",
"interface Test {",
" @Unsafe",
" @JsonValue",
" @Value.Redacted",
" String token();",
"}")
.doTest();
}

@Test
void testRedactedIsDoNotLog_jsonSerializable() {
fix().addInputLines(
Expand Down
5 changes: 5 additions & 0 deletions changelog/@unreleased/pr-2277.v2.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
type: improvement
improvement:
description: Immutables redactions may be unsafe (previously forced do-not-log)
links:
- https://github.com/palantir/gradle-baseline/pull/2277