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

Commit fb211e75 authored by Songchun Fan's avatar Songchun Fan
Browse files

[SettingsProvider] lint checks for non-user getter calls from system server

Detect method calls like Settings.Secure.getInt().

System server code should always use get*ForUser() instead of get*()
methods.

BUG: 166312046
Test: atest com.google.android.lint.CallingSettingsNonUserGetterMethodsIssueDetectorTest
Change-Id: If54c867f58ffd92a1dd1759f806a280e5b5c04c6
parent 6e336465
Loading
Loading
Loading
Loading
+2 −1
Original line number Diff line number Diff line
@@ -30,7 +30,8 @@ class AndroidFrameworkIssueRegistry : IssueRegistry() {
            CallingIdentityTokenDetector.ISSUE_NESTED_CLEAR_IDENTITY_CALLS,
            CallingIdentityTokenDetector.ISSUE_RESTORE_IDENTITY_CALL_NOT_IN_FINALLY_BLOCK,
            CallingIdentityTokenDetector.ISSUE_USE_OF_CALLER_AWARE_METHODS_WITH_CLEARED_IDENTITY,
            CallingIdentityTokenDetector.ISSUE_CLEAR_IDENTITY_CALL_NOT_FOLLOWED_BY_TRY_FINALLY
            CallingIdentityTokenDetector.ISSUE_CLEAR_IDENTITY_CALL_NOT_FOLLOWED_BY_TRY_FINALLY,
            CallingSettingsNonUserGetterMethodsDetector.ISSUE_NON_USER_GETTER_CALLED
    )

    override val api: Int
+82 −0
Original line number Diff line number Diff line
/*
 * Copyright (C) 2021 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.android.lint

import com.android.tools.lint.detector.api.Category
import com.android.tools.lint.detector.api.Detector
import com.android.tools.lint.detector.api.Implementation
import com.android.tools.lint.detector.api.Issue
import com.android.tools.lint.detector.api.JavaContext
import com.android.tools.lint.detector.api.Scope
import com.android.tools.lint.detector.api.Severity
import com.android.tools.lint.detector.api.SourceCodeScanner
import com.intellij.psi.PsiMethod
import org.jetbrains.uast.UCallExpression

/**
 * Lint Detector that finds issues with improper usages of the non-user getter methods of Settings
 */
@Suppress("UnstableApiUsage")
class CallingSettingsNonUserGetterMethodsDetector : Detector(), SourceCodeScanner {
    override fun getApplicableMethodNames(): List<String> = listOf(
            "getString",
            "getInt",
            "getLong",
            "getFloat"
    )

    override fun visitMethodCall(context: JavaContext, node: UCallExpression, method: PsiMethod) {
        val evaluator = context.evaluator
        if (evaluator.isMemberInClass(method, "android.provider.Settings.Secure") ||
                evaluator.isMemberInClass(method, "android.provider.Settings.System")
        ) {
            val message = getIncidentMessageNonUserGetterMethods(getMethodSignature(method))
            context.report(ISSUE_NON_USER_GETTER_CALLED, node, context.getLocation(node), message)
        }
    }

    private fun getMethodSignature(method: PsiMethod) =
            method.containingClass
                    ?.qualifiedName
                    ?.let { "$it#${method.name}" }
                    ?: method.name

    companion object {
        @JvmField
        val ISSUE_NON_USER_GETTER_CALLED: Issue = Issue.create(
                id = "NonUserGetterCalled",
                briefDescription = "Non-ForUser Getter Method called to Settings",
                explanation = """
                    System process should not call the non-ForUser getter methods of \
                    `Settings.Secure` or `Settings.System`. For example, instead of \
                    `Settings.Secure.getInt()`, use `Settings.Secure.getIntForUser()` instead. \
                    This will make sure that the correct Settings value is retrieved.
                    """,
                category = Category.CORRECTNESS,
                priority = 6,
                severity = Severity.WARNING,
                implementation = Implementation(
                        CallingSettingsNonUserGetterMethodsDetector::class.java,
                        Scope.JAVA_FILE_SCOPE
                )
        )

        fun getIncidentMessageNonUserGetterMethods(methodSignature: String) =
                "`$methodSignature()` called from system process. " +
                        "Please call `${methodSignature}ForUser()` instead. "
    }
}
+217 −0
Original line number Diff line number Diff line
/*
 * Copyright (C) 2021 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.android.lint

import com.android.tools.lint.checks.infrastructure.LintDetectorTest
import com.android.tools.lint.checks.infrastructure.TestFile
import com.android.tools.lint.checks.infrastructure.TestLintTask
import com.android.tools.lint.detector.api.Detector
import com.android.tools.lint.detector.api.Issue

@Suppress("UnstableApiUsage")
class CallingSettingsNonUserGetterMethodsIssueDetectorTest : LintDetectorTest() {
    override fun getDetector(): Detector = CallingSettingsNonUserGetterMethodsDetector()

    override fun getIssues(): List<Issue> = listOf(
            CallingSettingsNonUserGetterMethodsDetector.ISSUE_NON_USER_GETTER_CALLED
    )

    override fun lint(): TestLintTask = super.lint().allowMissingSdk(true)

    fun testDoesNotDetectIssues() {
        lint().files(
                java(
                        """
                    package test.pkg;
                    import android.provider.Settings.Secure;
                    public class TestClass1 {
                        private void testMethod(Context context) {
                            final int value = Secure.getIntForUser(context.getContentResolver(),
                                Settings.Secure.KEY1, 0, 0);
                        }
                    }
                    """
                ).indented(),
                *stubs
        )
                .run()
                .expectClean()
    }

    fun testDetectsNonUserGetterCalledFromSecure() {
        lint().files(
                java(
                        """
                    package test.pkg;
                    import android.provider.Settings.Secure;
                    public class TestClass1 {
                        private void testMethod(Context context) {
                            final int value = Secure.getInt(context.getContentResolver(),
                                Settings.Secure.KEY1);
                        }
                    }
                    """
                ).indented(),
                *stubs
        )
                .run()
                .expect(
                        """
                        src/test/pkg/TestClass1.java:5: Warning: \
                        android.provider.Settings.Secure#getInt() called from system process. \
                        Please call android.provider.Settings.Secure#getIntForUser() instead.  \
                        [NonUserGetterCalled]
                                final int value = Secure.getInt(context.getContentResolver(),
                                                  ^
                        0 errors, 1 warnings
                        """.addLineContinuation()
                )
    }
    fun testDetectsNonUserGetterCalledFromSystem() {
        lint().files(
                java(
                        """
                    package test.pkg;
                    import android.provider.Settings.System;
                    public class TestClass1 {
                        private void testMethod(Context context) {
                            final float value = System.getFloat(context.getContentResolver(),
                                Settings.System.KEY1);
                        }
                    }
                    """
                ).indented(),
                *stubs
        )
                .run()
                .expect(
                        """
                        src/test/pkg/TestClass1.java:5: Warning: \
                        android.provider.Settings.System#getFloat() called from system process. \
                        Please call android.provider.Settings.System#getFloatForUser() instead.  \
                        [NonUserGetterCalled]
                                final float value = System.getFloat(context.getContentResolver(),
                                                    ^
                        0 errors, 1 warnings
                        """.addLineContinuation()
                )
    }

    fun testDetectsNonUserGetterCalledFromSettings() {
        lint().files(
                java(
                        """
                    package test.pkg;
                    import android.provider.Settings;
                    public class TestClass1 {
                        private void testMethod(Context context) {
                            float value = Settings.System.getFloat(context.getContentResolver(),
                                Settings.System.KEY1);
                        }
                    }
                    """
                ).indented(),
                *stubs
        )
                .run()
                .expect(
                        """
                        src/test/pkg/TestClass1.java:5: Warning: \
                        android.provider.Settings.System#getFloat() called from system process. \
                        Please call android.provider.Settings.System#getFloatForUser() instead.  \
                        [NonUserGetterCalled]
                                float value = Settings.System.getFloat(context.getContentResolver(),
                                              ^
                        0 errors, 1 warnings
                        """.addLineContinuation()
                )
    }

    fun testDetectsNonUserGettersCalledFromSystemAndSecure() {
        lint().files(
                java(
                        """
                    package test.pkg;
                    import android.provider.Settings.Secure;
                    import android.provider.Settings.System;
                    public class TestClass1 {
                        private void testMethod(Context context) {
                            final long value1 = Secure.getLong(context.getContentResolver(),
                                Settings.Secure.KEY1, 0);
                            final String value2 = System.getString(context.getContentResolver(),
                                Settings.System.KEY2);
                        }
                    }
                    """
                ).indented(),
                *stubs
        )
                .run()
                .expect(
                        """
                        src/test/pkg/TestClass1.java:6: Warning: \
                        android.provider.Settings.Secure#getLong() called from system process. \
                        Please call android.provider.Settings.Secure#getLongForUser() instead.  \
                        [NonUserGetterCalled]
                                final long value1 = Secure.getLong(context.getContentResolver(),
                                                    ^
                        src/test/pkg/TestClass1.java:8: Warning: \
                        android.provider.Settings.System#getString() called from system process. \
                        Please call android.provider.Settings.System#getStringForUser() instead.  \
                        [NonUserGetterCalled]
                                final String value2 = System.getString(context.getContentResolver(),
                                                      ^
                        0 errors, 2 warnings
                        """.addLineContinuation()
                )
    }

    private val SettingsStub: TestFile = java(
            """
            package android.provider;
            public class Settings {
                public class Secure {
                    float getFloat(ContentResolver cr, String key) {
                        return 0.0f;
                    }
                    long getLong(ContentResolver cr, String key) {
                        return 0l;
                    }
                    int getInt(ContentResolver cr, String key) {
                        return 0;
                    }
                }
                public class System {
                    float getFloat(ContentResolver cr, String key) {
                        return 0.0f;
                    }
                    long getLong(ContentResolver cr, String key) {
                        return 0l;
                    }
                    String getString(ContentResolver cr, String key) {
                        return null;
                    }
                }
            }
            """
    ).indented()

    private val stubs = arrayOf(SettingsStub)

    // Substitutes "backslash + new line" with an empty string to imitate line continuation
    private fun String.addLineContinuation(): String = this.trimIndent().replace("\\\n", "")
}