Merge changes from topic "oct5"

* changes:
  Slight relaxing of Context.getUserId() checks.
  Exclude Telephony Binder interfaces.
This commit is contained in:
TreeHugger Robot
2020-10-05 20:52:35 +00:00
committed by Android (Google) Code Review
5 changed files with 147 additions and 12 deletions

View File

@@ -18,6 +18,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.anyOf;
import static com.google.errorprone.matchers.Matchers.enclosingClass;
import static com.google.errorprone.matchers.Matchers.hasAnnotation;
import static com.google.errorprone.matchers.Matchers.instanceMethod;
@@ -60,8 +61,13 @@ public final class ContextUserIdChecker extends BugChecker implements MethodInvo
private static final Matcher<ExpressionTree> BINDER_CALL = methodInvocation(
instanceMethod().onDescendantOf("android.os.IInterface").withAnyName());
private static final Matcher<ExpressionTree> GET_USER_ID_CALL = methodInvocation(
instanceMethod().onDescendantOf("android.content.Context").named("getUserId"));
private static final Matcher<ExpressionTree> GET_USER_ID_CALL = methodInvocation(anyOf(
instanceMethod().onExactClass("android.app.admin.DevicePolicyManager")
.named("myUserId"),
instanceMethod().onExactClass("android.content.pm.ShortcutManager")
.named("injectMyUserId"),
instanceMethod().onDescendantOf("android.content.Context")
.named("getUserId")));
private static final Matcher<ExpressionTree> USER_ID_FIELD = new Matcher<ExpressionTree>() {
@Override

View File

@@ -17,11 +17,14 @@
package com.google.errorprone.bugpatterns.android;
import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
import static com.google.errorprone.matchers.Matchers.allOf;
import static com.google.errorprone.matchers.Matchers.contains;
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.isSameType;
import static com.google.errorprone.matchers.Matchers.methodInvocation;
import static com.google.errorprone.matchers.Matchers.not;
import static com.google.errorprone.matchers.Matchers.throwStatement;
import static com.google.errorprone.matchers.Matchers.variableType;
@@ -29,13 +32,17 @@ 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.CatchTreeMatcher;
import com.google.errorprone.bugpatterns.BugChecker.TryTreeMatcher;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.predicates.TypePredicate;
import com.sun.source.tree.CatchTree;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.StatementTree;
import com.sun.source.tree.Tree;
import com.sun.source.tree.TryTree;
import com.sun.source.tree.VariableTree;
import com.sun.tools.javac.code.Type;
import java.util.List;
@@ -54,9 +61,17 @@ import java.util.List;
name = "AndroidFrameworkRethrowFromSystem",
summary = "Verifies that system_server calls use rethrowFromSystemServer()",
severity = WARNING)
public final class RethrowFromSystemChecker extends BugChecker implements CatchTreeMatcher {
public final class RethrowFromSystemChecker extends BugChecker implements TryTreeMatcher {
private static final Matcher<Tree> INSIDE_MANAGER =
enclosingClass(hasAnnotation("android.annotation.SystemService"));
// Purposefully exclude telephony Binder interfaces, since we know they
// always run under the separate AID_RADIO
private static final Matcher<ExpressionTree> SYSTEM_BINDER_CALL = methodInvocation(allOf(
instanceMethod().onDescendantOf("android.os.IInterface").withAnyName(),
not(instanceMethod().onClass(inPackage("com.android.internal.telephony"))),
not(instanceMethod().onClass(inPackage("com.android.internal.telecom")))));
private static final Matcher<VariableTree> REMOTE_EXCEPTION = variableType(
isSameType("android.os.RemoteException"));
private static final Matcher<StatementTree> RETHROW_FROM_SYSTEM = throwStatement(
@@ -64,17 +79,33 @@ public final class RethrowFromSystemChecker extends BugChecker implements CatchT
.named("rethrowFromSystemServer")));
@Override
public Description matchCatch(CatchTree tree, VisitorState state) {
public Description matchTry(TryTree tree, VisitorState state) {
if (INSIDE_MANAGER.matches(tree, state)
&& REMOTE_EXCEPTION.matches(tree.getParameter(), state)) {
final List<? extends StatementTree> statements = tree.getBlock().getStatements();
if (statements.size() != 1 || !RETHROW_FROM_SYSTEM.matches(statements.get(0), state)) {
return buildDescription(tree)
.setMessage("Must contain single "
+ "'throw e.rethrowFromSystemServer()' statement")
.build();
&& contains(ExpressionTree.class, SYSTEM_BINDER_CALL)
.matches(tree.getBlock(), state)) {
for (CatchTree catchTree : tree.getCatches()) {
if (REMOTE_EXCEPTION.matches(catchTree.getParameter(), state)) {
final List<? extends StatementTree> statements = catchTree.getBlock()
.getStatements();
if (statements.size() != 1
|| !RETHROW_FROM_SYSTEM.matches(statements.get(0), state)) {
return buildDescription(catchTree)
.setMessage("Must contain single "
+ "'throw e.rethrowFromSystemServer()' statement")
.build();
}
}
}
}
return Description.NO_MATCH;
}
private static TypePredicate inPackage(final String filter) {
return new TypePredicate() {
@Override
public boolean apply(Type type, VisitorState state) {
return type.tsym.packge().fullname.toString().startsWith(filter);
}
};
}
}

View File

@@ -92,4 +92,56 @@ public class ContextUserIdCheckerTest {
"}")
.doTest();
}
@Test
public void testDevicePolicyManager() {
compilationHelper
.addSourceFile("/android/annotation/SystemService.java")
.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("DevicePolicyManager.java",
"package android.app.admin;",
"import android.annotation.SystemService;",
"import android.content.Context;",
"import android.foo.IFooService;",
"import android.os.UserHandle;",
"import android.os.RemoteException;",
"@SystemService(\"dp\") public class DevicePolicyManager {",
" IFooService mService;",
" int myUserId() { return 0; }",
" void bar() throws RemoteException {",
" mService.baz(null, myUserId());",
" }",
"}")
.doTest();
}
@Test
public void testShortcutManager() {
compilationHelper
.addSourceFile("/android/annotation/SystemService.java")
.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("ShortcutManager.java",
"package android.content.pm;",
"import android.annotation.SystemService;",
"import android.content.Context;",
"import android.foo.IFooService;",
"import android.os.UserHandle;",
"import android.os.RemoteException;",
"@SystemService(\"shortcut\") public class ShortcutManager {",
" IFooService mService;",
" int injectMyUserId() { return 0; }",
" void bar() throws RemoteException {",
" mService.baz(null, injectMyUserId());",
" }",
"}")
.doTest();
}
}

View File

@@ -105,4 +105,27 @@ public class RethrowFromSystemCheckerTest {
"}")
.doTest();
}
@Test
public void testTelephony() {
compilationHelper
.addSourceFile("/android/annotation/SystemService.java")
.addSourceFile("/com/android/internal/telephony/ITelephony.java")
.addSourceFile("/android/os/IInterface.java")
.addSourceFile("/android/os/RemoteException.java")
.addSourceLines("TelephonyManager.java",
"import android.annotation.SystemService;",
"import com.android.internal.telephony.ITelephony;",
"import android.os.RemoteException;",
"@SystemService(\"telephony\") public class TelephonyManager {",
" ITelephony mService;",
" void bar() {",
" try {",
" mService.bar();",
" } catch (RemoteException ignored) {",
" }",
" }",
"}")
.doTest();
}
}

View File

@@ -0,0 +1,23 @@
/*
* 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.android.internal.telephony;
import android.os.RemoteException;
public interface ITelephony extends android.os.IInterface {
public void bar() throws RemoteException;
}