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

Commit a84ad2b3 authored by Jeff Sharkey's avatar Jeff Sharkey
Browse files

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
parent 439b8616

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

+19 −23
Original line number Diff line number Diff line
@@ -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,14 +63,27 @@ 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()")
@@ -79,22 +93,4 @@ public final class ContextUserIdChecker extends BugChecker implements MethodInvo 
        }

        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;

        }

    }

}

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

+4 −4
Original line number Diff line number Diff line
@@ -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) {

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

+13 −0
Original line number Diff line number Diff line
@@ -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();

    }

errorprone/tests/res/android/os/UserHandle.java

+4 −0
Original line number Diff line number Diff line
@@ -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();

    }

}