Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 77cfbdf4 authored by William Escande's avatar William Escande
Browse files

RequiresPermissionChecker: change detection method 

Change the way the checker find the intent and the permission by using
the order instead of using the name of the variable. (since some target
have their parameters name being obfuscated prior to pass them to the
annotation processor)

Also fix when the intent is set in a chained pattern like:
new Intent(Foo).putExtra(...)

Add various test to make sure the new detection works for differently
ordered parameters

Fix checkstyle reported errors and move some check to be early return
instead of having a unneeded indentation level

Bug: 352610940
Fix: 352610940
Test: atest error_prone_android_framework_test | for unit test
Test: m Bluetooth and check the warning / errors
Flag: TEST_ONLY
Change-Id: I9308c08c7275708c5b1db3f35a9753563ba05dc9
parent e47fbe40

errorprone/java/com/google/errorprone/bugpatterns/android/RequiresPermissionChecker.java

+73 −71
Original line number Diff line number Diff line
@@ -55,9 +55,9 @@ import com.sun.source.util.TreeScanner; 
import com.sun.tools.javac.code.Symbol;

import com.sun.tools.javac.code.Symbol.ClassSymbol;

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.code.Type.ClassType;

import com.sun.tools.javac.tree.JCTree.JCNewClass;



import java.util.ArrayList;

import java.util.Arrays;
@@ -67,7 +67,6 @@ import java.util.List; 
import java.util.Optional;

import java.util.Set;

import java.util.concurrent.atomic.AtomicReference;

import java.util.function.Predicate;

import java.util.regex.Pattern;



import javax.lang.model.element.Name;
@@ -125,6 +124,12 @@ public final class RequiresPermissionChecker extends BugChecker 
            instanceMethod()

                    .onDescendantOf("android.content.Context")

                    .withNameMatching(Pattern.compile("^send(Ordered|Sticky)?Broadcast.*$")));

    private static final Matcher<ExpressionTree> SEND_BROADCAST_AS_USER =

            methodInvocation(

                    instanceMethod()

                            .onDescendantOf("android.content.Context")

                            .withNameMatching(

                                    Pattern.compile("^send(Ordered|Sticky)?Broadcast.*AsUser.*$")));

    private static final Matcher<ExpressionTree> SEND_PENDING_INTENT = methodInvocation(

            instanceMethod()

                    .onDescendantOf("android.app.PendingIntent")
@@ -306,18 +311,6 @@ public final class RequiresPermissionChecker extends BugChecker 
        }

    }



    private static ExpressionTree findArgumentByParameterName(MethodInvocationTree tree,

            Predicate<String> paramName) {

        final MethodSymbol sym = ASTHelpers.getSymbol(tree);

        final List<VarSymbol> params = sym.getParameters();

        for (int i = 0; i < params.size(); i++) {

            if (paramName.test(params.get(i).name.toString())) {

                return tree.getArguments().get(i);

            }

        }

        return null;

    }



    private static Name resolveName(ExpressionTree tree) {

        if (tree instanceof IdentifierTree) {

            return ((IdentifierTree) tree).getName();
@@ -345,24 +338,33 @@ public final class RequiresPermissionChecker extends BugChecker 


    private static ParsedRequiresPermission parseBroadcastSourceRequiresPermission(

            MethodInvocationTree methodTree, VisitorState state) {

        final ExpressionTree arg = findArgumentByParameterName(methodTree,

                (name) -> name.toLowerCase().contains("intent"));

        if (arg instanceof IdentifierTree) {

        if (methodTree.getArguments().size() < 1) {

            return null;

        }

        final ExpressionTree arg = methodTree.getArguments().get(0);

        if (!(arg instanceof IdentifierTree)) {

            return null;

        }

        final Name argName = ((IdentifierTree) arg).getName();

        final MethodTree method = state.findEnclosing(MethodTree.class);

        final AtomicReference<ParsedRequiresPermission> res = new AtomicReference<>();

        method.accept(new TreeScanner<Void, Void>() {

                private ParsedRequiresPermission last;

            private ParsedRequiresPermission mLast;



            @Override

            public Void visitMethodInvocation(MethodInvocationTree tree, Void param) {

                if (Objects.equal(methodTree, tree)) {

                        res.set(last);

                    res.set(mLast);

                } else {

                    final Name name = resolveName(tree.getMethodSelect());

                        if (Objects.equal(argName, name)

                                && INTENT_SET_ACTION.matches(tree, state)) {

                            last = parseIntentAction(tree);

                    if (Objects.equal(argName, name) && INTENT_SET_ACTION.matches(tree, state)) {

                        mLast = parseIntentAction(tree);

                    } else if (name == null && tree.getMethodSelect() instanceof MemberSelectTree) {

                        ExpressionTree innerTree =

                                ((MemberSelectTree) tree.getMethodSelect()).getExpression();

                        if (innerTree instanceof JCNewClass) {

                            mLast = parseIntentAction((NewClassTree) innerTree);

                        }

                    }

                }

                return super.visitMethodInvocation(tree, param);
@@ -372,9 +374,8 @@ public final class RequiresPermissionChecker extends BugChecker 
            public Void visitAssignment(AssignmentTree tree, Void param) {

                final Name name = resolveName(tree.getVariable());

                final Tree init = tree.getExpression();

                    if (Objects.equal(argName, name)

                            && init instanceof NewClassTree) {

                        last = parseIntentAction((NewClassTree) init);

                if (Objects.equal(argName, name) && init instanceof NewClassTree) {

                    mLast = parseIntentAction((NewClassTree) init);

                }

                return super.visitAssignment(tree, param);

            }
@@ -383,24 +384,26 @@ public final class RequiresPermissionChecker extends BugChecker 
            public Void visitVariable(VariableTree tree, Void param) {

                final Name name = tree.getName();

                final ExpressionTree init = tree.getInitializer();

                    if (Objects.equal(argName, name)

                            && init instanceof NewClassTree) {

                        last = parseIntentAction((NewClassTree) init);

                if (Objects.equal(argName, name) && init instanceof NewClassTree) {

                    mLast = parseIntentAction((NewClassTree) init);

                }

                return super.visitVariable(tree, param);

            }

        }, null);

        return res.get();

    }

        return null;

    }



    private static ParsedRequiresPermission parseBroadcastTargetRequiresPermission(

            MethodInvocationTree tree, VisitorState state) {

        final ExpressionTree arg = findArgumentByParameterName(tree,

                (name) -> name.toLowerCase().contains("permission"));

        final ParsedRequiresPermission res = new ParsedRequiresPermission();

        if (arg != null) {

        int permission_position = 1;

        if (SEND_BROADCAST_AS_USER.matches(tree, state)) {

            permission_position = 2;

        }

        if (tree.getArguments().size() < permission_position + 1) {

            return res;

        }

        final ExpressionTree arg = tree.getArguments().get(permission_position);

        arg.accept(new TreeScanner<Void, Void>() {

            @Override

            public Void visitIdentifier(IdentifierTree tree, Void param) {
@@ -414,7 +417,6 @@ public final class RequiresPermissionChecker extends BugChecker 
                return super.visitMemberSelect(tree, param);

            }

        }, null);

        }

        return res;

    }

errorprone/tests/java/com/google/errorprone/bugpatterns/android/RequiresPermissionCheckerTest.java

+13 −0
Original line number Diff line number Diff line
@@ -412,6 +412,19 @@ public class RequiresPermissionCheckerTest { 
                        "      context.sendBroadcast(intent);",

                        "    }",

                        "  }",

                        "  public void exampleWithChainedMethod(Context context) {",

                        "    Intent intent = new Intent(FooManager.ACTION_RED)",

                        "            .putExtra(\"foo\", 42);",

                        "    context.sendBroadcast(intent, FooManager.PERMISSION_RED);",

                        "    context.sendBroadcastWithMultiplePermissions(intent,",

                        "        new String[] { FooManager.PERMISSION_RED });",

                        "  }",

                        "  public void exampleWithAsUser(Context context) {",

                        "    Intent intent = new Intent(FooManager.ACTION_RED);",

                        "    context.sendBroadcastAsUser(intent, 42, FooManager.PERMISSION_RED);",

                        "    context.sendBroadcastAsUserMultiplePermissions(intent, 42,",

                        "        new String[] { FooManager.PERMISSION_RED });",

                        "  }",

                        "}")

                .doTest();

    }

errorprone/tests/res/android/content/Context.java

+11 −0
Original line number Diff line number Diff line
@@ -36,4 +36,15 @@ public class Context { 
    public void sendBroadcastWithMultiplePermissions(Intent intent, String[] receiverPermissions) {

        throw new UnsupportedOperationException();

    }



    /* Fake user type for test purposes */

    public void sendBroadcastAsUser(Intent intent, int user, String receiverPermission) {

        throw new UnsupportedOperationException();

    }



    /* Fake user type for test purposes */

    public void sendBroadcastAsUserMultiplePermissions(

            Intent intent, int user, String[] receiverPermissions) {

        throw new UnsupportedOperationException();

    }

}

errorprone/tests/res/android/content/Intent.java

+4 −0
Original line number Diff line number Diff line
@@ -24,4 +24,8 @@ public class Intent { 
    public Intent setAction(String action) {

        throw new UnsupportedOperationException();

    }



    public Intent putExtra(String extra, int value) {

        throw new UnsupportedOperationException();

    }

}