From f58714496ab5454ea10c6d638c094bb1d21306d8 Mon Sep 17 00:00:00 2001 From: Grace Cheng Date: Mon, 12 Jun 2023 20:34:20 +0000 Subject: [PATCH 1/3] Fix NPE in updateAddPreference Update mAddFingerprintPreference to avoid NPE Fixes: 286495189 Test: Rotate screen during first fingerprint enrollment, complete enrollment, and observe no crash Change-Id: Id7edde492168b467360c6c99b326721cd883bba8 Merged-In: Id7edde492168b467360c6c99b326721cd883bba8 --- .../settings/biometrics/fingerprint/FingerprintSettings.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/com/android/settings/biometrics/fingerprint/FingerprintSettings.java b/src/com/android/settings/biometrics/fingerprint/FingerprintSettings.java index be090e33366..fb3319c3ff6 100644 --- a/src/com/android/settings/biometrics/fingerprint/FingerprintSettings.java +++ b/src/com/android/settings/biometrics/fingerprint/FingerprintSettings.java @@ -623,9 +623,9 @@ public class FingerprintSettings extends SubSettings { return; // Activity went away } - final Preference addPreference = findPreference(KEY_FINGERPRINT_ADD); + mAddFingerprintPreference = findPreference(KEY_FINGERPRINT_ADD); - if (addPreference == null) { + if (mAddFingerprintPreference == null) { return; // b/275519315 Skip if updateAddPreference() invoke before addPreference() } From 027f0a46aee333ba29b904cf366c9d81794cf088 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mat=C3=ADas=20Hern=C3=A1ndez?= Date: Thu, 15 Jun 2023 18:37:52 +0200 Subject: [PATCH 2/3] Settings: don't try to allow NLSes with too-long component names * NotificationAccessConfirmationActivity (triggered through CompanionDeviceManager) -> Don't show the dialog, bail out early similarly to other invalid inputs. * NotificationAccessSettings (from Special App Access) -> No changes, but use the canonical constant now. * ApprovalPreferenceController (used in NotificationAccessDetails) -> Disable the toggle, unless the NLS was previously approved (in which case it can still be removed). Fixes: 260570119 Fixes: 286043036 Test: atest + manually Change-Id: Ifc048311746c027e3683cdcf65f1079d04cf7c56 --- .../ApprovalPreferenceController.java | 5 +- ...otificationAccessConfirmationActivity.java | 4 +- .../NotificationAccessSettings.java | 4 +- ...icationAccessConfirmationActivityTest.java | 104 ++++++++++++++++++ .../ApprovalPreferenceControllerTest.java | 30 +++++ 5 files changed, 143 insertions(+), 4 deletions(-) create mode 100644 tests/robotests/src/com/android/settings/notification/NotificationAccessConfirmationActivityTest.java diff --git a/src/com/android/settings/applications/specialaccess/notificationaccess/ApprovalPreferenceController.java b/src/com/android/settings/applications/specialaccess/notificationaccess/ApprovalPreferenceController.java index 0767e65bbcb..6bee62cc688 100644 --- a/src/com/android/settings/applications/specialaccess/notificationaccess/ApprovalPreferenceController.java +++ b/src/com/android/settings/applications/specialaccess/notificationaccess/ApprovalPreferenceController.java @@ -81,6 +81,8 @@ public class ApprovalPreferenceController extends BasePreferenceController { final RestrictedSwitchPreference preference = (RestrictedSwitchPreference) pref; final CharSequence label = mPkgInfo.applicationInfo.loadLabel(mPm); + final boolean isAllowedCn = mCn.flattenToShortString().length() + <= NotificationManager.MAX_SERVICE_COMPONENT_NAME_LENGTH; final boolean isEnabled = isServiceEnabled(mCn); preference.setChecked(isEnabled); preference.setOnPreferenceChangeListener((p, newValue) -> { @@ -105,7 +107,8 @@ public class ApprovalPreferenceController extends BasePreferenceController { return false; } }); - preference.updateState(mCn.getPackageName(), mPkgInfo.applicationInfo.uid, isEnabled); + preference.updateState( + mCn.getPackageName(), mPkgInfo.applicationInfo.uid, isAllowedCn, isEnabled); } public void disable(final ComponentName cn) { diff --git a/src/com/android/settings/notification/NotificationAccessConfirmationActivity.java b/src/com/android/settings/notification/NotificationAccessConfirmationActivity.java index dfe6df2a5ca..a6b565ae6ba 100644 --- a/src/com/android/settings/notification/NotificationAccessConfirmationActivity.java +++ b/src/com/android/settings/notification/NotificationAccessConfirmationActivity.java @@ -67,7 +67,9 @@ public class NotificationAccessConfirmationActivity extends Activity mUserId = getIntent().getIntExtra(EXTRA_USER_ID, UserHandle.USER_NULL); CharSequence mAppLabel; - if (mComponentName == null || mComponentName.getPackageName() == null) { + if (mComponentName == null || mComponentName.getPackageName() == null + || mComponentName.flattenToString().length() + > NotificationManager.MAX_SERVICE_COMPONENT_NAME_LENGTH) { finish(); return; } diff --git a/src/com/android/settings/notification/NotificationAccessSettings.java b/src/com/android/settings/notification/NotificationAccessSettings.java index 369c4f6dfaf..e2ef0ddccb4 100644 --- a/src/com/android/settings/notification/NotificationAccessSettings.java +++ b/src/com/android/settings/notification/NotificationAccessSettings.java @@ -66,7 +66,6 @@ public class NotificationAccessSettings extends EmptyTextSettings { private static final String TAG = "NotifAccessSettings"; static final String ALLOWED_KEY = "allowed"; static final String NOT_ALLOWED_KEY = "not_allowed"; - private static final int MAX_CN_LENGTH = 500; private static final ManagedServiceSettings.Config CONFIG = new ManagedServiceSettings.Config.Builder() @@ -150,7 +149,8 @@ public class NotificationAccessSettings extends EmptyTextSettings { for (ServiceInfo service : services) { final ComponentName cn = new ComponentName(service.packageName, service.name); boolean isAllowed = mNm.isNotificationListenerAccessGranted(cn); - if (!isAllowed && cn.flattenToString().length() > MAX_CN_LENGTH) { + if (!isAllowed && cn.flattenToString().length() + > NotificationManager.MAX_SERVICE_COMPONENT_NAME_LENGTH) { continue; } diff --git a/tests/robotests/src/com/android/settings/notification/NotificationAccessConfirmationActivityTest.java b/tests/robotests/src/com/android/settings/notification/NotificationAccessConfirmationActivityTest.java new file mode 100644 index 00000000000..86631ffb2d4 --- /dev/null +++ b/tests/robotests/src/com/android/settings/notification/NotificationAccessConfirmationActivityTest.java @@ -0,0 +1,104 @@ +/* + * Copyright (C) 2023 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.settings.notification; + +import static com.android.internal.notification.NotificationAccessConfirmationActivityContract.EXTRA_COMPONENT_NAME; + +import static com.google.common.truth.Truth.assertThat; + +import static org.robolectric.Shadows.shadowOf; + +import android.annotation.Nullable; +import android.app.Activity; +import android.content.ComponentName; +import android.content.Intent; +import android.content.pm.ApplicationInfo; +import android.content.pm.PackageInfo; +import android.widget.TextView; + +import com.android.settings.R; + +import com.google.common.base.Strings; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.Robolectric; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.RuntimeEnvironment; + +@RunWith(RobolectricTestRunner.class) +public class NotificationAccessConfirmationActivityTest { + + @Test + public void start_showsDialog() { + ComponentName cn = new ComponentName("com.example", "com.example.SomeService"); + installPackage(cn.getPackageName(), "X"); + + NotificationAccessConfirmationActivity activity = startActivityWithIntent(cn); + + assertThat(activity.isFinishing()).isFalse(); + assertThat(getDialogText(activity)).isEqualTo( + activity.getString(R.string.notification_listener_security_warning_summary, "X")); + } + + @Test + public void start_withMissingPackage_finishes() { + ComponentName cn = new ComponentName("com.example", "com.example.SomeService"); + + NotificationAccessConfirmationActivity activity = startActivityWithIntent(cn); + + assertThat(getDialogText(activity)).isNull(); + assertThat(activity.isFinishing()).isTrue(); + } + + @Test + public void start_componentNameTooLong_finishes() { + ComponentName longCn = new ComponentName("com.example", Strings.repeat("Blah", 150)); + installPackage(longCn.getPackageName(), ""); + + NotificationAccessConfirmationActivity activity = startActivityWithIntent(longCn); + + assertThat(getDialogText(activity)).isNull(); + assertThat(activity.isFinishing()).isTrue(); + } + + private static NotificationAccessConfirmationActivity startActivityWithIntent( + ComponentName cn) { + return Robolectric.buildActivity( + NotificationAccessConfirmationActivity.class, + new Intent().putExtra(EXTRA_COMPONENT_NAME, cn)) + .setup() + .get(); + } + + private static void installPackage(String packageName, String appName) { + PackageInfo pi = new PackageInfo(); + pi.packageName = packageName; + pi.applicationInfo = new ApplicationInfo(); + pi.applicationInfo.packageName = packageName; + pi.applicationInfo.name = appName; + shadowOf(RuntimeEnvironment.application.getPackageManager()).installPackage(pi); + } + + @Nullable + private static String getDialogText(Activity activity) { + TextView tv = activity.getWindow().findViewById(android.R.id.message); + CharSequence text = (tv != null ? tv.getText() : null); + return text != null ? text.toString() : null; + } + +} diff --git a/tests/unit/src/com/android/settings/applications/specialaccess/notificationaccess/ApprovalPreferenceControllerTest.java b/tests/unit/src/com/android/settings/applications/specialaccess/notificationaccess/ApprovalPreferenceControllerTest.java index 249b713987c..4601a1cfbaa 100644 --- a/tests/unit/src/com/android/settings/applications/specialaccess/notificationaccess/ApprovalPreferenceControllerTest.java +++ b/tests/unit/src/com/android/settings/applications/specialaccess/notificationaccess/ApprovalPreferenceControllerTest.java @@ -83,6 +83,36 @@ public class ApprovalPreferenceControllerTest { } + @Test + public void updateState_enabled() { + when(mAppOpsManager.noteOpNoThrow(anyInt(), anyInt(), anyString())).thenReturn( + AppOpsManager.MODE_ALLOWED); + when(mNm.isNotificationListenerAccessGranted(mCn)).thenReturn(true); + RestrictedSwitchPreference pref = new RestrictedSwitchPreference( + mContext); + pref.setAppOps(mAppOpsManager); + + mController.updateState(pref); + + assertThat(pref.isEnabled()).isTrue(); + } + + @Test + public void updateState_invalidCn_disabled() { + ComponentName longCn = new ComponentName("com.example.package", + com.google.common.base.Strings.repeat("Blah", 150)); + mController.setCn(longCn); + when(mAppOpsManager.noteOpNoThrow(anyInt(), anyInt(), anyString())).thenReturn( + AppOpsManager.MODE_ALLOWED); + RestrictedSwitchPreference pref = new RestrictedSwitchPreference( + mContext); + pref.setAppOps(mAppOpsManager); + + mController.updateState(pref); + + assertThat(pref.isEnabled()).isFalse(); + } + @Test public void updateState_checked() { when(mAppOpsManager.noteOpNoThrow(anyInt(), anyInt(), anyString())).thenReturn( From dab8f53e9b78dba006de0adc40badd35ae57aa39 Mon Sep 17 00:00:00 2001 From: Zoey Chen Date: Mon, 26 Jun 2023 14:34:46 +0000 Subject: [PATCH 3/3] [Panlingual] Add metric in Languages Bug: 279915462 Test: manual Change-Id: I12dc7f8a0c594de88790e116c4b4f7ca9dc7253e --- .../localepicker/LocaleDialogFragment.java | 5 ++++- .../localepicker/LocaleDragAndDropAdapter.java | 16 +++++++++++++++- .../settings/localepicker/LocaleListEditor.java | 9 +++++++-- 3 files changed, 26 insertions(+), 4 deletions(-) diff --git a/src/com/android/settings/localepicker/LocaleDialogFragment.java b/src/com/android/settings/localepicker/LocaleDialogFragment.java index ad9e10f19ed..f54446af063 100644 --- a/src/com/android/settings/localepicker/LocaleDialogFragment.java +++ b/src/com/android/settings/localepicker/LocaleDialogFragment.java @@ -164,15 +164,18 @@ public class LocaleDialogFragment extends InstrumentedDialogFragment { public void onClick(DialogInterface dialog, int which) { if (mDialogType == DIALOG_CONFIRM_SYSTEM_DEFAULT) { int result = Activity.RESULT_CANCELED; + boolean changed = false; if (which == DialogInterface.BUTTON_POSITIVE) { result = Activity.RESULT_OK; + changed = true; } Intent intent = new Intent(); Bundle bundle = new Bundle(); bundle.putInt(ARG_DIALOG_TYPE, DIALOG_CONFIRM_SYSTEM_DEFAULT); intent.putExtras(bundle); mParent.onActivityResult(DIALOG_CONFIRM_SYSTEM_DEFAULT, result, intent); - mMetricsFeatureProvider.action(mContext, SettingsEnums.ACTION_CHANGE_LANGUAGE); + mMetricsFeatureProvider.action(mContext, SettingsEnums.ACTION_CHANGE_LANGUAGE, + changed); } mShouldKeepDialog = false; } diff --git a/src/com/android/settings/localepicker/LocaleDragAndDropAdapter.java b/src/com/android/settings/localepicker/LocaleDragAndDropAdapter.java index edd302645e2..6c254d954c1 100644 --- a/src/com/android/settings/localepicker/LocaleDragAndDropAdapter.java +++ b/src/com/android/settings/localepicker/LocaleDragAndDropAdapter.java @@ -16,6 +16,7 @@ package com.android.settings.localepicker; +import android.app.settings.SettingsEnums; import android.content.Context; import android.graphics.Canvas; import android.os.Bundle; @@ -37,6 +38,7 @@ import androidx.recyclerview.widget.RecyclerView; import com.android.internal.app.LocalePicker; import com.android.internal.app.LocaleStore; import com.android.settings.R; +import com.android.settings.overlay.FeatureFactory; import com.android.settings.shortcut.ShortcutsUpdateTask; import java.text.NumberFormat; @@ -210,6 +212,13 @@ class LocaleDragAndDropAdapter Log.e(TAG, String.format(Locale.US, "Negative position in onItemMove %d -> %d", fromPosition, toPosition)); } + + if (fromPosition != toPosition) { + FeatureFactory.getFactory(mContext).getMetricsFeatureProvider() + .action(mContext, SettingsEnums.ACTION_REORDER_LANGUAGE, + mDragLocale.getLocale().getDisplayName() + " move to " + toPosition); + } + notifyItemChanged(fromPosition); // to update the numbers notifyItemChanged(toPosition); notifyItemMoved(fromPosition, toPosition); @@ -244,8 +253,13 @@ class LocaleDragAndDropAdapter void removeChecked() { int itemCount = mFeedItemList.size(); + LocaleStore.LocaleInfo localeInfo; for (int i = itemCount - 1; i >= 0; i--) { - if (mFeedItemList.get(i).getChecked()) { + localeInfo = mFeedItemList.get(i); + if (localeInfo.getChecked()) { + FeatureFactory.getFactory(mContext).getMetricsFeatureProvider() + .action(mContext, SettingsEnums.ACTION_REMOVE_LANGUAGE, + localeInfo.getLocale().getDisplayName()); mFeedItemList.remove(i); } } diff --git a/src/com/android/settings/localepicker/LocaleListEditor.java b/src/com/android/settings/localepicker/LocaleListEditor.java index 55cff3be6b1..17d3a6aa996 100644 --- a/src/com/android/settings/localepicker/LocaleListEditor.java +++ b/src/com/android/settings/localepicker/LocaleListEditor.java @@ -199,9 +199,11 @@ public class LocaleListEditor extends RestrictedSettingsFragment implements View localeInfo = (LocaleStore.LocaleInfo) data.getSerializableExtra(INTENT_LOCALE_KEY); String preferencesTags = Settings.System.getString( getContext().getContentResolver(), Settings.System.LOCALE_PREFERENCES); - - mAdapter.addLocale(mayAppendUnicodeTags(localeInfo, preferencesTags)); + localeInfo = mayAppendUnicodeTags(localeInfo, preferencesTags); + mAdapter.addLocale(localeInfo); updateVisibilityOfRemoveMenu(); + mMetricsFeatureProvider.action(getContext(), SettingsEnums.ACTION_ADD_LANGUAGE, + localeInfo.getLocale().getDisplayName()); } else if (requestCode == DIALOG_CONFIRM_SYSTEM_DEFAULT) { localeInfo = mAdapter.getFeedItemList().get(0); if (resultCode == Activity.RESULT_OK) { @@ -214,6 +216,9 @@ public class LocaleListEditor extends RestrictedSettingsFragment implements View LocaleDialogFragment localeDialogFragment = LocaleDialogFragment.newInstance(); localeDialogFragment.setArguments(args); localeDialogFragment.show(mFragmentManager, TAG_DIALOG_NOT_AVAILABLE); + mMetricsFeatureProvider.action(getContext(), + SettingsEnums.ACTION_NOT_SUPPORTED_SYSTEM_LANGUAGE, + localeInfo.getLocale().getDisplayName()); } } else { mAdapter.notifyListChanged(localeInfo);