From b9b16eab3ce5efdca2619d1a7c9ed080f991526a Mon Sep 17 00:00:00 2001 From: Gray Mackall <34871572+gmackall@users.noreply.github.com> Date: Wed, 29 Nov 2023 08:04:20 -0800 Subject: [PATCH] Fix new lint from android 14 upgrade, and remove it from the baseline (flutter/engine#47817) See https://github.com/flutter/engine/pull/47609#discussion_r1385803721 for context. It isn't clear to me what the file descriptor here is actually doing, so I'm not actually too sure about this fix. Can it just be deleted? Update: it seems to me that the motivation here is that `ClipData.Item.coerceToText` [consumes a `SecurityException` here](https://cs.android.com/android/platform/superproject/main/+/main:frameworks/base/core/java/android/content/ClipData.java;l=411;bpv=0;bpt=1), with just a log line that isn't particularly descriptive. And basically we want to run into that same exception so we can [provide a more helpful log line](https://github.com/flutter/engine/blob/e5da8d9a87f37a2d726904a23c5ef92da48fa0cf/shell/platform/android/io/flutter/plugin/platform/PlatformPlugin.java#L525), so we do the same thing that the underlying `coerceToText` method does? [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style --- .../plugin/platform/PlatformPlugin.java | 16 ++++++++-- .../plugin/platform/PlatformPluginTest.java | 29 ++++++++++++++++++- .../flutter/tools/android_lint/baseline.xml | 11 ------- 3 files changed, 42 insertions(+), 14 deletions(-) diff --git a/engine/src/flutter/shell/platform/android/io/flutter/plugin/platform/PlatformPlugin.java b/engine/src/flutter/shell/platform/android/io/flutter/plugin/platform/PlatformPlugin.java index 41ffa23853c..c3c14dff252 100644 --- a/engine/src/flutter/shell/platform/android/io/flutter/plugin/platform/PlatformPlugin.java +++ b/engine/src/flutter/shell/platform/android/io/flutter/plugin/platform/PlatformPlugin.java @@ -11,6 +11,7 @@ import android.content.ClipData; import android.content.ClipDescription; import android.content.ClipboardManager; import android.content.Context; +import android.content.res.AssetFileDescriptor; import android.os.Build; import android.view.HapticFeedbackConstants; import android.view.SoundEffectConstants; @@ -25,6 +26,7 @@ import androidx.core.view.WindowInsetsControllerCompat; import io.flutter.Log; import io.flutter.embedding.engine.systemchannels.PlatformChannel; import java.io.FileNotFoundException; +import java.io.IOException; import java.util.List; /** Android implementation of the platform plugin. */ @@ -512,14 +514,21 @@ public class PlatformPlugin { if (!clipboard.hasPrimaryClip()) return null; + CharSequence charSequence = null; try { ClipData clip = clipboard.getPrimaryClip(); if (clip == null) return null; if (format == null || format == PlatformChannel.ClipboardContentFormat.PLAIN_TEXT) { ClipData.Item item = clip.getItemAt(0); + AssetFileDescriptor assetFileDescriptor = null; if (item.getUri() != null) - activity.getContentResolver().openTypedAssetFileDescriptor(item.getUri(), "text/*", null); - return item.coerceToText(activity); + assetFileDescriptor = + activity + .getContentResolver() + .openTypedAssetFileDescriptor(item.getUri(), "text/*", null); + charSequence = item.coerceToText(activity); + if (assetFileDescriptor != null) assetFileDescriptor.close(); + return charSequence; } } catch (SecurityException e) { Log.w( @@ -531,6 +540,9 @@ public class PlatformPlugin { return null; } catch (FileNotFoundException e) { return null; + } catch (IOException e) { + Log.w(TAG, "Failed to close AssetFileDescriptor while accessing clipboard data.", e); + return charSequence; } return null; diff --git a/engine/src/flutter/shell/platform/android/test/io/flutter/plugin/platform/PlatformPluginTest.java b/engine/src/flutter/shell/platform/android/test/io/flutter/plugin/platform/PlatformPluginTest.java index 23b7fbf5b6a..51e66c91db3 100644 --- a/engine/src/flutter/shell/platform/android/test/io/flutter/plugin/platform/PlatformPluginTest.java +++ b/engine/src/flutter/shell/platform/android/test/io/flutter/plugin/platform/PlatformPluginTest.java @@ -11,6 +11,10 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.anyBoolean; +import static org.mockito.Mockito.anyString; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; @@ -20,9 +24,11 @@ import static org.mockito.Mockito.when; import android.app.Activity; import android.content.ClipData; +import android.content.ClipDescription; import android.content.ClipboardManager; import android.content.ContentResolver; import android.content.Context; +import android.content.res.AssetFileDescriptor; import android.net.Uri; import android.os.Build; import android.view.View; @@ -84,18 +90,39 @@ public class PlatformPluginTest { PlatformChannel fakePlatformChannel = mock(PlatformChannel.class); PlatformPlugin platformPlugin = new PlatformPlugin(fakeActivity, fakePlatformChannel); + // Successfully get the contents of the primary clip when they contain text. ClipboardContentFormat clipboardFormat = ClipboardContentFormat.PLAIN_TEXT; assertNull(platformPlugin.mPlatformMessageHandler.getClipboardData(clipboardFormat)); ClipData clip = ClipData.newPlainText("label", "Text"); clipboardManager.setPrimaryClip(clip); assertNotNull(platformPlugin.mPlatformMessageHandler.getClipboardData(clipboardFormat)); - ContentResolver contentResolver = ctx.getContentResolver(); + // Return null when the primary clip contains non-text media. + ContentResolver contentResolver = spy(ctx.getContentResolver()); when(fakeActivity.getContentResolver()).thenReturn(contentResolver); Uri uri = Uri.parse("content://media/external_primary/images/media/"); clip = ClipData.newUri(contentResolver, "URI", uri); clipboardManager.setPrimaryClip(clip); assertNull(platformPlugin.mPlatformMessageHandler.getClipboardData(clipboardFormat)); + + // Still return text when the AssetFileDescriptor throws an IOException. + when(fakeActivity.getContentResolver()).thenReturn(contentResolver); + ClipDescription clipDescription = + new ClipDescription( + "label", + new String[] { + ClipDescription.MIMETYPE_TEXT_PLAIN, ClipDescription.MIMETYPE_TEXT_URILIST + }); + ClipData.Item clipDataItem = new ClipData.Item("Text", null, uri); + ClipData clipData = new ClipData(clipDescription, clipDataItem); + clipboardManager.setPrimaryClip(clipData); + AssetFileDescriptor fakeAssetFileDescriptor = mock(AssetFileDescriptor.class); + doReturn(fakeAssetFileDescriptor) + .when(contentResolver) + .openTypedAssetFileDescriptor(eq(uri), anyString(), eq(null)); + doThrow(new IOException()).when(fakeAssetFileDescriptor).close(); + assertNotNull(platformPlugin.mPlatformMessageHandler.getClipboardData(clipboardFormat)); + verify(fakeAssetFileDescriptor).close(); } @SuppressWarnings("deprecation") diff --git a/engine/src/flutter/tools/android_lint/baseline.xml b/engine/src/flutter/tools/android_lint/baseline.xml index b540917bd52..2fccb83d76c 100644 --- a/engine/src/flutter/tools/android_lint/baseline.xml +++ b/engine/src/flutter/tools/android_lint/baseline.xml @@ -67,17 +67,6 @@ column="82"/> - - - -