diff options
author | Jeff Sharkey <jsharkey@android.com> | 2020-06-24 17:52:55 -0600 |
---|---|---|
committer | Jeff Sharkey <jsharkey@android.com> | 2020-06-24 17:52:57 -0600 |
commit | a84ad2b349d69c2e159361cca6445c55e06b5a59 (patch) | |
tree | 0fe9fb08a34ea7a160e2fb94e5f8e388fcbe24df | |
parent | 439b86167748a3e65b473bc80b3229b045bee90b (diff) |
Slight relaxing of ContextUserIdChecker.
Several managers keep an "int mUserId" field which is assigned from
Context.getUserId(), so Binder calls referencing that field are okay.
Also shift to borrowing the "flavor" logic for detecting userId
parameters consistently.
Bug: 115654727, 159626156
Test: atest error_prone_android_framework_test
Change-Id: I9841fdf16f34c08b113e689e74b94f1ede839e2c
4 files changed, 40 insertions, 27 deletions
diff --git a/errorprone/java/com/google/errorprone/bugpatterns/android/ContextUserIdChecker.java b/errorprone/java/com/google/errorprone/bugpatterns/android/ContextUserIdChecker.java index 7f2cce6ea7f0..4f1af3e9bea2 100644 --- a/errorprone/java/com/google/errorprone/bugpatterns/android/ContextUserIdChecker.java +++ b/errorprone/java/com/google/errorprone/bugpatterns/android/ContextUserIdChecker.java @@ -17,6 +17,7 @@ package com.google.errorprone.bugpatterns.android; import static com.google.errorprone.BugPattern.SeverityLevel.WARNING; +import static com.google.errorprone.bugpatterns.android.UidChecker.getFlavor; import static com.google.errorprone.matchers.Matchers.enclosingClass; import static com.google.errorprone.matchers.Matchers.hasAnnotation; import static com.google.errorprone.matchers.Matchers.instanceMethod; @@ -27,17 +28,17 @@ import com.google.errorprone.BugPattern; import com.google.errorprone.VisitorState; import com.google.errorprone.bugpatterns.BugChecker; import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher; +import com.google.errorprone.bugpatterns.android.UidChecker.Flavor; import com.google.errorprone.matchers.Description; import com.google.errorprone.matchers.Matcher; import com.google.errorprone.util.ASTHelpers; import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.IdentifierTree; import com.sun.source.tree.MethodInvocationTree; import com.sun.source.tree.Tree; import com.sun.tools.javac.code.Symbol.VarSymbol; import java.util.List; -import java.util.Locale; -import java.util.function.Predicate; /** * To avoid an explosion of {@code startActivityForUser} style methods, we've @@ -62,39 +63,34 @@ public final class ContextUserIdChecker extends BugChecker implements MethodInvo private static final Matcher<ExpressionTree> GET_USER_ID_CALL = methodInvocation( instanceMethod().onDescendantOf("android.content.Context").named("getUserId")); + private static final Matcher<ExpressionTree> USER_ID_FIELD = new Matcher<ExpressionTree>() { + @Override + public boolean matches(ExpressionTree tree, VisitorState state) { + if (tree instanceof IdentifierTree) { + return "mUserId".equals((((IdentifierTree) tree).getName().toString())); + } + return false; + } + }; + @Override public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { if (INSIDE_MANAGER.matches(tree, state) && BINDER_CALL.matches(tree, state)) { final List<VarSymbol> vars = ASTHelpers.getSymbol(tree).params(); for (int i = 0; i < vars.size(); i++) { - if (USER_ID_VAR.test(vars.get(i)) && - !GET_USER_ID_CALL.matches(tree.getArguments().get(i), state)) { + final Flavor varFlavor = getFlavor(vars.get(i)); + final ExpressionTree arg = tree.getArguments().get(i); + if (varFlavor == Flavor.USER_ID && + !GET_USER_ID_CALL.matches(arg, state) && + !USER_ID_FIELD.matches(arg, state)) { return buildDescription(tree) .setMessage("Must pass Context.getUserId() as user ID" - + "to enable createContextAsUser()") + + " to enable createContextAsUser()") .build(); } } } return Description.NO_MATCH; } - - private static final UserIdMatcher USER_ID_VAR = new UserIdMatcher(); - - private static class UserIdMatcher implements Predicate<VarSymbol> { - @Override - public boolean test(VarSymbol t) { - if ("int".equals(t.type.toString())) { - switch (t.name.toString().toLowerCase(Locale.ROOT)) { - case "user": - case "userid": - case "userhandle": - case "user_id": - return true; - } - } - return false; - } - } } diff --git a/errorprone/java/com/google/errorprone/bugpatterns/android/UidChecker.java b/errorprone/java/com/google/errorprone/bugpatterns/android/UidChecker.java index 533586f65c61..3ffb8ea40789 100644 --- a/errorprone/java/com/google/errorprone/bugpatterns/android/UidChecker.java +++ b/errorprone/java/com/google/errorprone/bugpatterns/android/UidChecker.java @@ -63,7 +63,7 @@ public final class UidChecker extends BugChecker implements MethodInvocationTree return Description.NO_MATCH; } - private static enum Flavor { + public static enum Flavor { UNKNOWN(null), PID(Pattern.compile("(^pid$|Pid$)")), UID(Pattern.compile("(^uid$|Uid$)")), @@ -78,7 +78,7 @@ public final class UidChecker extends BugChecker implements MethodInvocationTree } } - private static Flavor getFlavor(String name) { + public static Flavor getFlavor(String name) { for (Flavor f : Flavor.values()) { if (f.matches(name)) { return f; @@ -87,7 +87,7 @@ public final class UidChecker extends BugChecker implements MethodInvocationTree return Flavor.UNKNOWN; } - private static Flavor getFlavor(VarSymbol symbol) { + public static Flavor getFlavor(VarSymbol symbol) { final String type = symbol.type.toString(); if ("int".equals(type)) { return getFlavor(symbol.name.toString()); @@ -95,7 +95,7 @@ public final class UidChecker extends BugChecker implements MethodInvocationTree return Flavor.UNKNOWN; } - private static Flavor getFlavor(ExpressionTree tree) { + public static Flavor getFlavor(ExpressionTree tree) { if (tree instanceof IdentifierTree) { return getFlavor(((IdentifierTree) tree).getName().toString()); } else if (tree instanceof MemberSelectTree) { diff --git a/errorprone/tests/java/com/google/errorprone/bugpatterns/android/ContextUserIdCheckerTest.java b/errorprone/tests/java/com/google/errorprone/bugpatterns/android/ContextUserIdCheckerTest.java index 46a24d16f35a..c0b8cd745afc 100644 --- a/errorprone/tests/java/com/google/errorprone/bugpatterns/android/ContextUserIdCheckerTest.java +++ b/errorprone/tests/java/com/google/errorprone/bugpatterns/android/ContextUserIdCheckerTest.java @@ -49,9 +49,16 @@ public class ContextUserIdCheckerTest { "@SystemService(\"foo\") public class FooManager {", " Context mContext;", " IFooService mService;", + " final int mUserId;", + " FooManager(Context context) {", + " mUserId = mContext.getUserId();", + " }", " void bar() throws RemoteException {", " mService.baz(null, mContext.getUserId());", " }", + " void baz() throws RemoteException {", + " mService.baz(null, mUserId);", + " }", "}") .doTest(); } @@ -63,11 +70,13 @@ public class ContextUserIdCheckerTest { .addSourceFile("/android/content/Context.java") .addSourceFile("/android/foo/IFooService.java") .addSourceFile("/android/os/IInterface.java") + .addSourceFile("/android/os/UserHandle.java") .addSourceFile("/android/os/RemoteException.java") .addSourceLines("FooManager.java", "import android.annotation.SystemService;", "import android.content.Context;", "import android.foo.IFooService;", + "import android.os.UserHandle;", "import android.os.RemoteException;", "@SystemService(\"foo\") public class FooManager {", " Context mContext;", @@ -76,6 +85,10 @@ public class ContextUserIdCheckerTest { " // BUG: Diagnostic contains:", " mService.baz(null, 0);", " }", + " void baz() throws RemoteException {", + " // BUG: Diagnostic contains:", + " mService.baz(null, UserHandle.myUserId());", + " }", "}") .doTest(); } diff --git a/errorprone/tests/res/android/os/UserHandle.java b/errorprone/tests/res/android/os/UserHandle.java index a05fb9e42efa..c58d661fad08 100644 --- a/errorprone/tests/res/android/os/UserHandle.java +++ b/errorprone/tests/res/android/os/UserHandle.java @@ -28,4 +28,8 @@ public class UserHandle { public static int getUid(int userId, int appId) { throw new UnsupportedOperationException(); } + + public static int myUserId() { + throw new UnsupportedOperationException(); + } } |