From d80f760516c293d011a8a211c076558a0328959c Mon Sep 17 00:00:00 2001 From: Biswarup Pal Date: Mon, 1 Mar 2021 13:12:30 +0000 Subject: [PATCH] Refactor FileUtils#convertToModernFd Replace implementation of FileUtils#convertToModernFd with MediaStore#getOriginalMediaFormatFileDescriptor. Fix fd leak by returning ParcelFileDescriptor instead of FileDescriptor. Test: atest CtsProviderTestCases Bug: 170488060 Change-Id: I7d6ecf4549e8ed5134d20977c2089240be804827 --- core/java/android/os/FileUtils.java | 41 ++++--------------- media/java/android/media/ExifInterface.java | 30 ++++++++------ .../android/media/MediaMetadataRetriever.java | 5 ++- media/java/android/media/MediaPlayer.java | 6 +-- 4 files changed, 30 insertions(+), 52 deletions(-) diff --git a/core/java/android/os/FileUtils.java b/core/java/android/os/FileUtils.java index b12dad038ce3e..40c658f01e288 100644 --- a/core/java/android/os/FileUtils.java +++ b/core/java/android/os/FileUtils.java @@ -46,8 +46,6 @@ import android.content.ContentResolver; import android.content.Context; import android.content.pm.PackageManager; import android.content.pm.ProviderInfo; -import android.content.res.AssetFileDescriptor; -import android.net.Uri; import android.provider.DocumentsContract.Document; import android.provider.MediaStore; import android.system.ErrnoException; @@ -83,7 +81,6 @@ import java.security.NoSuchAlgorithmException; import java.util.Arrays; import java.util.Collection; import java.util.Comparator; -import java.util.Locale; import java.util.Objects; import java.util.concurrent.Executor; import java.util.concurrent.TimeUnit; @@ -1446,46 +1443,22 @@ public final class FileUtils { } } - // TODO(b/170488060): Consider better approach /** {@hide} */ @VisibleForTesting - public static FileDescriptor convertToModernFd(FileDescriptor fd) { + public static ParcelFileDescriptor convertToModernFd(FileDescriptor fd) { try { Context context = AppGlobals.getInitialApplication(); - // /mnt/user paths are not accessible directly so convert to a /storage path - String filePath = Os.readlink("/proc/self/fd/" + fd.getInt$()).replace( - "/mnt/user/" + UserHandle.myUserId(), "/storage"); - File realFile = new File(filePath); - String fileName = realFile.getName(); - boolean isCameraVideo = !fileName.startsWith(".") && fileName.endsWith(".mp4") - && contains(CAMERA_DIR_LOWER_CASE, filePath.toLowerCase(Locale.ROOT)); - - if (!SystemProperties.getBoolean("sys.fuse.transcode_enabled", false) - || UserHandle.getAppId(Process.myUid()) == getMediaProviderAppId(context) - || !isCameraVideo) { - // 1. If transcode is enabled we optimize by default, unless explicitly disabled. - // 2. Never convert modern fd for MediaProvider, because this requires + if (UserHandle.getAppId(Process.myUid()) == getMediaProviderAppId(context)) { + // Never convert modern fd for MediaProvider, because this requires // MediaStore#scanFile and can cause infinite loops when MediaProvider scans - // 3. Only convert published mp4 videos in the DCIM/Camera dir return null; } - Log.i(TAG, "Changing to modern format dataSource for: " + realFile); - ContentResolver resolver = context.getContentResolver(); - - Uri uri = MediaStore.scanFile(resolver, realFile); - if (uri != null) { - Bundle opts = new Bundle(); - opts.putBoolean(MediaStore.EXTRA_ACCEPT_ORIGINAL_MEDIA_FORMAT, true); - AssetFileDescriptor afd = resolver.openTypedAssetFileDescriptor(uri, "*/*", opts); - Log.i(TAG, "Changed to modern format dataSource for: " + realFile); - return afd.getFileDescriptor(); - } else { - Log.i(TAG, "Failed to change to modern format dataSource for: " + realFile); - } + return MediaStore.getOriginalMediaFormatFileDescriptor(context, + ParcelFileDescriptor.dup(fd)); } catch (Exception e) { - Log.w(TAG, "Failed to change to modern format dataSource", e); + Log.w(TAG, "Failed to convert to modern format file descriptor", e); + return null; } - return null; } private static int getMediaProviderAppId(Context context) { diff --git a/media/java/android/media/ExifInterface.java b/media/java/android/media/ExifInterface.java index 21f8623b19530..d746c850a0181 100644 --- a/media/java/android/media/ExifInterface.java +++ b/media/java/android/media/ExifInterface.java @@ -32,6 +32,7 @@ import android.content.res.AssetManager; import android.graphics.Bitmap; import android.graphics.BitmapFactory; import android.os.FileUtils; +import android.os.ParcelFileDescriptor; import android.system.ErrnoException; import android.system.Os; import android.system.OsConstants; @@ -1531,16 +1532,24 @@ public class ExifInterface { if (fileDescriptor == null) { throw new NullPointerException("fileDescriptor cannot be null"); } - FileDescriptor modernFd = FileUtils.convertToModernFd(fileDescriptor); + // If a file descriptor has a modern file descriptor, this means that the file can be + // transcoded and not using the modern file descriptor will trigger the transcoding + // operation. Thus, to avoid unnecessary transcoding, need to convert to modern file + // descriptor if it exists. As of Android S, transcoding is not supported for image files, + // so this is for protecting against non-image files sent to ExifInterface, but support may + // be added in the future. + ParcelFileDescriptor modernFd = FileUtils.convertToModernFd(fileDescriptor); if (modernFd != null) { - fileDescriptor = modernFd; + fileDescriptor = modernFd.getFileDescriptor(); } mAssetInputStream = null; mFilename = null; boolean isFdDuped = false; - if (isSeekableFD(fileDescriptor)) { + // Can't save attributes to files with transcoding because apps get a different copy of + // that file when they're not using it through framework libraries like ExifInterface. + if (isSeekableFD(fileDescriptor) && modernFd == null) { mSeekableFileDescriptor = fileDescriptor; // Keep the original file descriptor in order to save attributes when it's seekable. // Otherwise, just close the given file descriptor after reading it because the save @@ -2545,27 +2554,22 @@ public class ExifInterface { private void initForFilename(String filename) throws IOException { FileInputStream in = null; - FileInputStream legacyInputStream = null; mAssetInputStream = null; mFilename = filename; mIsInputStream = false; try { in = new FileInputStream(filename); - FileDescriptor modernFd = FileUtils.convertToModernFd(in.getFD()); + ParcelFileDescriptor modernFd = FileUtils.convertToModernFd(in.getFD()); if (modernFd != null) { - legacyInputStream = in; - in = new FileInputStream(modernFd); - } - - if (isSeekableFD(in.getFD())) { - mSeekableFileDescriptor = in.getFD(); - } else { + closeQuietly(in); + in = new FileInputStream(modernFd.getFileDescriptor()); mSeekableFileDescriptor = null; + } else if (isSeekableFD(in.getFD())) { + mSeekableFileDescriptor = in.getFD(); } loadAttributes(in); } finally { closeQuietly(in); - closeQuietly(legacyInputStream); } } diff --git a/media/java/android/media/MediaMetadataRetriever.java b/media/java/android/media/MediaMetadataRetriever.java index c13f610a908c8..2943eee5b1da4 100644 --- a/media/java/android/media/MediaMetadataRetriever.java +++ b/media/java/android/media/MediaMetadataRetriever.java @@ -33,6 +33,7 @@ import android.os.Build; import android.os.Bundle; import android.os.FileUtils; import android.os.IBinder; +import android.os.ParcelFileDescriptor; import android.os.SystemProperties; import android.text.TextUtils; @@ -300,11 +301,11 @@ public class MediaMetadataRetriever implements AutoCloseable { */ public void setDataSource(FileDescriptor fd, long offset, long length) throws IllegalArgumentException { - FileDescriptor modernFd = FileUtils.convertToModernFd(fd); + ParcelFileDescriptor modernFd = FileUtils.convertToModernFd(fd); if (modernFd == null) { _setDataSource(fd, offset, length); } else { - _setDataSource(modernFd, offset, length); + _setDataSource(modernFd.getFileDescriptor(), offset, length); } } diff --git a/media/java/android/media/MediaPlayer.java b/media/java/android/media/MediaPlayer.java index 644afb79814f6..2d8babdc9f947 100644 --- a/media/java/android/media/MediaPlayer.java +++ b/media/java/android/media/MediaPlayer.java @@ -46,6 +46,7 @@ import android.os.IBinder; import android.os.Looper; import android.os.Message; import android.os.Parcel; +import android.os.ParcelFileDescriptor; import android.os.Parcelable; import android.os.PersistableBundle; import android.os.PowerManager; @@ -97,7 +98,6 @@ import java.util.UUID; import java.util.Vector; import java.util.concurrent.Executor; - /** * MediaPlayer class can be used to control playback of audio/video files and streams. * @@ -1268,11 +1268,11 @@ public class MediaPlayer extends PlayerBase */ public void setDataSource(FileDescriptor fd, long offset, long length) throws IOException, IllegalArgumentException, IllegalStateException { - FileDescriptor modernFd = FileUtils.convertToModernFd(fd); + ParcelFileDescriptor modernFd = FileUtils.convertToModernFd(fd); if (modernFd == null) { _setDataSource(fd, offset, length); } else { - _setDataSource(modernFd, offset, length); + _setDataSource(modernFd.getFileDescriptor(), offset, length); } }