Merge "Pass valid file descriptor to binder"

1 files changed, 38 insertions, 5 deletions

diff --git a/core/java/android/os/BugreportManager.java b/core/java/android/os/BugreportManager.java
index f87abde361f3..0feed68163e2 100644
--- a/core/java/android/os/BugreportManager.java
+++ b/core/java/android/os/BugreportManager.java
@@ -25,12 +25,14 @@ import android.annotation.RequiresPermission;
 import android.annotation.SystemApi;
 import android.annotation.SystemService;
 import android.content.Context;
+import android.util.Log;
 
 import com.android.internal.util.Preconditions;
 
 import libcore.io.IoUtils;
 
-import java.io.FileDescriptor;
+import java.io.File;
+import java.io.IOException;
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.util.concurrent.Executor;
@@ -43,6 +45,9 @@ import java.util.concurrent.Executor;
 @SystemApi
 @SystemService(Context.BUGREPORT_SERVICE)
 public final class BugreportManager {
+
+    private static final String TAG = "BugreportManager";
+
     private final Context mContext;
     private final IDumpstate mBinder;
 
@@ -141,22 +146,35 @@ public final class BugreportManager {
             @NonNull BugreportParams params,
             @NonNull @CallbackExecutor Executor executor,
             @NonNull BugreportCallback callback) {
+        File tmpScreenshotFile = null;
         try {
             Preconditions.checkNotNull(bugreportFd);
             Preconditions.checkNotNull(params);
             Preconditions.checkNotNull(executor);
             Preconditions.checkNotNull(callback);
 
-            DumpstateListener dsListener = new DumpstateListener(executor, callback);
+            if (screenshotFd == null) {
+                // Binder needs a valid File Descriptor to be passed
+                tmpScreenshotFile = File.createTempFile("tmp", ".png");
+                screenshotFd = ParcelFileDescriptor.open(tmpScreenshotFile,
+                        ParcelFileDescriptor.MODE_READ_ONLY);
+            }
+            DumpstateListener dsListener = new DumpstateListener(executor,
+                    callback, tmpScreenshotFile);
+
             // Note: mBinder can get callingUid from the binder transaction.
             mBinder.startBugreport(-1 /* callingUid */,
                     mContext.getOpPackageName(),
                     bugreportFd.getFileDescriptor(),
-                    (screenshotFd != null
-                            ? screenshotFd.getFileDescriptor() : new FileDescriptor()),
+                    screenshotFd.getFileDescriptor(),
                     params.getMode(), dsListener);
         } catch (RemoteException e) {
+            deleteFile(tmpScreenshotFile);
             throw e.rethrowFromSystemServer();
+        } catch (IOException e) {
+            // Need to delete the file if it was created but failed while trying to get fd
+            deleteFile(tmpScreenshotFile);
+            Log.e(TAG, "Not able to create/open temporary screenshot file ", e);
         } finally {
             // We can close the file descriptors here because binder would have duped them.
             IoUtils.closeQuietly(bugreportFd);
@@ -178,13 +196,26 @@ public final class BugreportManager {
         }
     }
 
+    private void deleteFile(@Nullable File tmpScreenshotFile) {
+        try {
+            if (tmpScreenshotFile != null && tmpScreenshotFile.exists()) {
+                tmpScreenshotFile.delete();
+            }
+        } catch (SecurityException e) {
+            Log.e(TAG, "Not able to delete temporary screenshot file ", e);
+        }
+    }
+
     private final class DumpstateListener extends IDumpstateListener.Stub {
         private final Executor mExecutor;
         private final BugreportCallback mCallback;
+        private final File mTmpScreenshotFile;
 
-        DumpstateListener(Executor executor, BugreportCallback callback) {
+        DumpstateListener(Executor executor, BugreportCallback callback,
+                @Nullable File tmpScreenshotFile) {
             mExecutor = executor;
             mCallback = callback;
+            mTmpScreenshotFile = tmpScreenshotFile;
         }
 
         @Override
@@ -208,6 +239,7 @@ public final class BugreportManager {
                 });
             } finally {
                 Binder.restoreCallingIdentity(identity);
+                deleteFile(mTmpScreenshotFile);
             }
         }
 
@@ -220,6 +252,7 @@ public final class BugreportManager {
                 });
             } finally {
                 Binder.restoreCallingIdentity(identity);
+                deleteFile(mTmpScreenshotFile);
             }
         }