diff options
author | 2025-04-09 11:02:01 +1000 | |
---|---|---|
committer | 2025-09-18 12:17:18 +0200 | |
commit | d40c037bccbae7537698968c1650a633710c5b6e (patch) | |
tree | 29a85b8012c33bd81e9eb5c8de6d2f5a846cf4df | |
parent | b0f64324499a3f65aca817d3f221c716d953a800 (diff) |
[SP 2025-09-01] Trim the application name to make it safe for presentationbanksia-dev
The application name is presented in the ConfirmFragment and as such we
don't want to allow for any length. This follows a similar approach to
PackageManager using the TextUtils.makeSafeForPresentation with a total
available character length of 500.
This removes the unused getCallingAppName from the DirectoryFragment as
it was causing false positives from DirectoryFragment to avoid false
positives when trying to find who calls the Shared function.
On top of this, add some quotation marks around the app name to avoid
the app name being a contination of the existing text in the dialog,
e.g. 'This will let app name access current and future content storage
in Alarms' will now be 'This will let "app name" access current and
future content storage'.
Bug: 397216537
Test: atest com.android.documentsui.picker.ApplicationNameTest
Flag: EXEMPT bug fix
Change-Id: Iad0d03de09b1e4ad953bd6bd46a619cfcc56d384
(cherry picked from commit c8ef2db3bb4645704384226976e59583b9e8d3d3)
6 files changed, 102 insertions, 14 deletions
diff --git a/res/values/strings.xml b/res/values/strings.xml index c3f11bab4..4cd85344b 100644 --- a/res/values/strings.xml +++ b/res/values/strings.xml @@ -575,7 +575,7 @@ <!-- Confrim dialog title show on open document tree flow. [CHAR_LIMIT=80] --> <string name="open_tree_dialog_title">Allow <xliff:g id="appName" example="Drive">%1$s</xliff:g> to access files in <xliff:g id="directory" example="DCIM">%2$s</xliff:g>?</string> <!-- Confrim dialog message show on open document tree flow.--> - <string name="open_tree_dialog_message">This will let <xliff:g id="appName" example="Drive">%1$s</xliff:g> access current and future content stored in <xliff:g id="directory" example="DCIM">%2$s</xliff:g>.</string> + <string name="open_tree_dialog_message">This will let "<xliff:g id="appName" example="Drive">%1$s</xliff:g>" access current and future content stored in <xliff:g id="directory" example="DCIM">%2$s</xliff:g>.</string> <!-- Header message title show on open document tree flow when directory is blocked. [CHAR_LIMIT=48] --> <string name="directory_blocked_header_title">Can\u2019t use this folder</string> <!-- Header message subtitle show on open document tree flow when directory is blocked. [CHAR_LIMIT=90]--> diff --git a/src/com/android/documentsui/base/Shared.java b/src/com/android/documentsui/base/Shared.java index ac089999f..bb8a39393 100644 --- a/src/com/android/documentsui/base/Shared.java +++ b/src/com/android/documentsui/base/Shared.java @@ -16,6 +16,9 @@ package com.android.documentsui.base; +import static android.text.TextUtils.SAFE_STRING_FLAG_SINGLE_LINE; +import static android.text.TextUtils.SAFE_STRING_FLAG_TRIM; + import static com.android.documentsui.base.SharedMinimal.TAG; import static com.android.documentsui.ChangeIds.RESTRICT_STORAGE_ACCESS_FRAMEWORK; @@ -265,7 +268,7 @@ public final class Shared { * @return the calling app name or general anonymous name if not found */ @NonNull - public static String getCallingAppName(Activity activity) { + public static CharSequence getCallingAppName(Activity activity) { final String anonymous = activity.getString(R.string.anonymous_application); final String packageName = getCallingPackageName(activity); if (TextUtils.isEmpty(packageName)) { @@ -281,7 +284,12 @@ public final class Shared { } CharSequence result = pm.getApplicationLabel(ai); - return TextUtils.isEmpty(result) ? anonymous : result.toString(); + if (TextUtils.isEmpty(result)) { + return anonymous; + } + + return TextUtils.makeSafeForPresentation( + result.toString(), 500, 0, SAFE_STRING_FLAG_TRIM | SAFE_STRING_FLAG_SINGLE_LINE); } /** diff --git a/src/com/android/documentsui/dirlist/DirectoryFragment.java b/src/com/android/documentsui/dirlist/DirectoryFragment.java index 2911d04e9..ae728f616 100644 --- a/src/com/android/documentsui/dirlist/DirectoryFragment.java +++ b/src/com/android/documentsui/dirlist/DirectoryFragment.java @@ -1634,10 +1634,5 @@ public class DirectoryFragment extends Fragment implements SwipeRefreshLayout.On public ActionHandler getActionHandler() { return mActions; } - - @Override - public String getCallingAppName() { - return Shared.getCallingAppName(mActivity); - } } } diff --git a/src/com/android/documentsui/dirlist/DocumentsAdapter.java b/src/com/android/documentsui/dirlist/DocumentsAdapter.java index 41ce73c8c..b32c15335 100644 --- a/src/com/android/documentsui/dirlist/DocumentsAdapter.java +++ b/src/com/android/documentsui/dirlist/DocumentsAdapter.java @@ -90,7 +90,6 @@ public abstract class DocumentsAdapter extends RecyclerView.Adapter<DocumentHold boolean isInSearchMode(); boolean isSelected(String id); Model getModel(); - String getCallingAppName(); boolean isDocumentEnabled(String mimeType, int flags); void initDocumentHolder(DocumentHolder holder); void onBindDocumentHolder(DocumentHolder holder, Cursor cursor); diff --git a/tests/common/com/android/documentsui/dirlist/TestEnvironment.java b/tests/common/com/android/documentsui/dirlist/TestEnvironment.java index 2e91e0733..cf50f0780 100644 --- a/tests/common/com/android/documentsui/dirlist/TestEnvironment.java +++ b/tests/common/com/android/documentsui/dirlist/TestEnvironment.java @@ -88,9 +88,4 @@ public final class TestEnvironment implements DocumentsAdapter.Environment { @Override public void onBindDocumentHolder(DocumentHolder holder, Cursor cursor) { } - - @Override - public String getCallingAppName() { - return "unknown"; - } } diff --git a/tests/unit/com/android/documentsui/picker/ApplicationNameTest.kt b/tests/unit/com/android/documentsui/picker/ApplicationNameTest.kt new file mode 100644 index 000000000..7ee3b9aa6 --- /dev/null +++ b/tests/unit/com/android/documentsui/picker/ApplicationNameTest.kt @@ -0,0 +1,91 @@ +/* + * Copyright (C) 2025 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.documentsui.picker + +import android.app.Activity +import android.content.pm.ApplicationInfo +import android.content.pm.PackageManager +import android.content.res.Resources +import com.android.documentsui.R +import com.android.documentsui.base.Shared +import org.junit.Assert.assertEquals +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.Parameterized +import org.mockito.Mock +import org.mockito.Mockito.eq +import org.mockito.Mockito.`when` as whenever +import org.mockito.MockitoAnnotations + +/** + * When apps are requesting access to a folder using `ACTION_OPEN_DOCUMENT_TREE` the application + * label is shown in the UI. Let's ensure that the label is appropriately sanitised. + */ +@RunWith(Parameterized::class) +class ApplicationNameTest { + @Mock + private lateinit var mockActivity: Activity + + @Mock + private lateinit var resources: Resources + + @Mock + private lateinit var pm: PackageManager + + companion object { + const val PACKAGE_NAME = "com.package.test" + const val ANONYMOUS_PACKAGE = "anonymous" + + @Parameterized.Parameters(name = "{index}") + @JvmStatic + fun parameters() = + listOf( + // Normal labels return their values. + Pair("label", "label"), + + // Newlines are stripped. + Pair("label\nlabel", "label label"), + + // Labels are trimmed to 500 characters. + Pair("a".repeat(600), "a".repeat(500)), + + // An empty label returns the anonymous package name. + Pair("", ANONYMOUS_PACKAGE) + ) + } + + @Parameterized.Parameter(0) + lateinit var testData: Pair<String, String> + + @Before + fun setUp() { + MockitoAnnotations.openMocks(this) + whenever(mockActivity.resources).thenReturn(resources) + whenever(mockActivity.packageManager).thenReturn(pm) + whenever(resources.getString(R.string.anonymous_application)).thenReturn(ANONYMOUS_PACKAGE) + whenever<String>(mockActivity.callingPackage).thenReturn(PACKAGE_NAME) + } + + @Test + fun testNameIsSanitized() { + val info = ApplicationInfo() + whenever(pm.getApplicationInfo(PACKAGE_NAME, 0)).thenReturn(info) + + whenever(pm.getApplicationLabel(eq(info))).thenReturn(testData.first) + assertEquals(Shared.getCallingAppName(mockActivity), testData.second) + } +} |