diff --git a/errorprone/java/com/google/errorprone/bugpatterns/android/ContextUserIdChecker.java b/errorprone/java/com/google/errorprone/bugpatterns/android/ContextUserIdChecker.java new file mode 100644 index 0000000000000..7f2cce6ea7f05 --- /dev/null +++ b/errorprone/java/com/google/errorprone/bugpatterns/android/ContextUserIdChecker.java @@ -0,0 +1,100 @@ +/* + * Copyright (C) 2020 The Android Open Source Project + * + * 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.google.errorprone.bugpatterns.android; + +import static com.google.errorprone.BugPattern.SeverityLevel.WARNING; +import static com.google.errorprone.matchers.Matchers.enclosingClass; +import static com.google.errorprone.matchers.Matchers.hasAnnotation; +import static com.google.errorprone.matchers.Matchers.instanceMethod; +import static com.google.errorprone.matchers.Matchers.methodInvocation; + +import com.google.auto.service.AutoService; +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.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.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 + * converged on recommending the use of {@code Context.createContextAsUser()}, + * and then ensuring that all system services pass {@link Context.getUserId()} + * for any {@code int userId} arguments across Binder interfaces. + *

+ * This design allows developers to easily redirect all services obtained from a + * specific {@code Context} to a different user with no additional API surface. + */ +@AutoService(BugChecker.class) +@BugPattern( + name = "AndroidFrameworkContextUserId", + summary = "Verifies that system_server calls use Context.getUserId()", + severity = WARNING) +public final class ContextUserIdChecker extends BugChecker implements MethodInvocationTreeMatcher { + private static final Matcher INSIDE_MANAGER = + enclosingClass(hasAnnotation("android.annotation.SystemService")); + + private static final Matcher BINDER_CALL = methodInvocation( + instanceMethod().onDescendantOf("android.os.IInterface").withAnyName()); + private static final Matcher GET_USER_ID_CALL = methodInvocation( + instanceMethod().onDescendantOf("android.content.Context").named("getUserId")); + + @Override + public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { + if (INSIDE_MANAGER.matches(tree, state) + && BINDER_CALL.matches(tree, state)) { + final List 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)) { + return buildDescription(tree) + .setMessage("Must pass Context.getUserId() as user ID" + + "to enable createContextAsUser()") + .build(); + } + } + } + return Description.NO_MATCH; + } + + private static final UserIdMatcher USER_ID_VAR = new UserIdMatcher(); + + private static class UserIdMatcher implements Predicate { + @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/tests/java/com/google/errorprone/bugpatterns/android/ContextUserIdCheckerTest.java b/errorprone/tests/java/com/google/errorprone/bugpatterns/android/ContextUserIdCheckerTest.java new file mode 100644 index 0000000000000..46a24d16f35a4 --- /dev/null +++ b/errorprone/tests/java/com/google/errorprone/bugpatterns/android/ContextUserIdCheckerTest.java @@ -0,0 +1,82 @@ +/* + * Copyright (C) 2020 The Android Open Source Project + * + * 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.google.errorprone.bugpatterns.android; + +import com.google.errorprone.CompilationTestHelper; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class ContextUserIdCheckerTest { + private CompilationTestHelper compilationHelper; + + @Before + public void setUp() { + compilationHelper = CompilationTestHelper.newInstance( + ContextUserIdChecker.class, getClass()); + } + + @Test + public void testValid() { + compilationHelper + .addSourceFile("/android/annotation/SystemService.java") + .addSourceFile("/android/content/Context.java") + .addSourceFile("/android/foo/IFooService.java") + .addSourceFile("/android/os/IInterface.java") + .addSourceFile("/android/os/RemoteException.java") + .addSourceLines("FooManager.java", + "import android.annotation.SystemService;", + "import android.content.Context;", + "import android.foo.IFooService;", + "import android.os.RemoteException;", + "@SystemService(\"foo\") public class FooManager {", + " Context mContext;", + " IFooService mService;", + " void bar() throws RemoteException {", + " mService.baz(null, mContext.getUserId());", + " }", + "}") + .doTest(); + } + + @Test + public void testInvalid() { + compilationHelper + .addSourceFile("/android/annotation/SystemService.java") + .addSourceFile("/android/content/Context.java") + .addSourceFile("/android/foo/IFooService.java") + .addSourceFile("/android/os/IInterface.java") + .addSourceFile("/android/os/RemoteException.java") + .addSourceLines("FooManager.java", + "import android.annotation.SystemService;", + "import android.content.Context;", + "import android.foo.IFooService;", + "import android.os.RemoteException;", + "@SystemService(\"foo\") public class FooManager {", + " Context mContext;", + " IFooService mService;", + " void bar() throws RemoteException {", + " // BUG: Diagnostic contains:", + " mService.baz(null, 0);", + " }", + "}") + .doTest(); + } +}