Merge "[SettingsProvider] lint checks for non-user getter calls from system server"
diff --git a/tools/lint/checks/src/main/java/com/google/android/lint/AndroidFrameworkIssueRegistry.kt b/tools/lint/checks/src/main/java/com/google/android/lint/AndroidFrameworkIssueRegistry.kt
index 5736a80..900c214 100644
--- a/tools/lint/checks/src/main/java/com/google/android/lint/AndroidFrameworkIssueRegistry.kt
+++ b/tools/lint/checks/src/main/java/com/google/android/lint/AndroidFrameworkIssueRegistry.kt
@@ -30,7 +30,8 @@
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
diff --git a/tools/lint/checks/src/main/java/com/google/android/lint/CallingSettingsNonUserGetterMethodsDetector.kt b/tools/lint/checks/src/main/java/com/google/android/lint/CallingSettingsNonUserGetterMethodsDetector.kt
new file mode 100644
index 0000000..641f337
--- /dev/null
+++ b/tools/lint/checks/src/main/java/com/google/android/lint/CallingSettingsNonUserGetterMethodsDetector.kt
@@ -0,0 +1,82 @@
+/*
+ * 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. "
+ }
+}
diff --git a/tools/lint/checks/src/test/java/com/google/android/lint/CallingSettingsNonUserGetterMethodsIssueDetectorTest.kt b/tools/lint/checks/src/test/java/com/google/android/lint/CallingSettingsNonUserGetterMethodsIssueDetectorTest.kt
new file mode 100644
index 0000000..1034029
--- /dev/null
+++ b/tools/lint/checks/src/test/java/com/google/android/lint/CallingSettingsNonUserGetterMethodsIssueDetectorTest.kt
@@ -0,0 +1,217 @@
+/*
+ * 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", "")
+}