From f5121a9b802c6ddd3661ed5cae602380dbe67090 Mon Sep 17 00:00:00 2001 From: Kenny Root Date: Wed, 10 Aug 2011 16:23:32 -0700 Subject: Better errors from copyResource Copy resource would fail without a good error code when the file couldn't be found during copy. Also destroy the target container ID during move operations since it might exist. If the copy failed due to it existing, it would get destroyed anyway. This way the user has a chance to have a good outcome the first time. Bug: 3375299 Bug: 5113898 Change-Id: I00559833f0801bc50e7cc031b462495e37a6b4ab --- .../internal/app/IMediaContainerService.aidl | 2 +- .../defcontainer/DefaultContainerService.java | 106 ++++++++++----------- .../android/server/pm/PackageManagerService.java | 12 ++- 3 files changed, 62 insertions(+), 58 deletions(-) diff --git a/core/java/com/android/internal/app/IMediaContainerService.aidl b/core/java/com/android/internal/app/IMediaContainerService.aidl index dd22e25..d407080 100755 --- a/core/java/com/android/internal/app/IMediaContainerService.aidl +++ b/core/java/com/android/internal/app/IMediaContainerService.aidl @@ -25,7 +25,7 @@ interface IMediaContainerService { String copyResourceToContainer(in Uri packageURI, String containerId, String key, String resFileName); - boolean copyResource(in Uri packageURI, + int copyResource(in Uri packageURI, in ParcelFileDescriptor outStream); PackageInfoLite getMinimalPackageInfo(in Uri fileUri, in int flags, in long threshold); boolean checkInternalFreeStorage(in Uri fileUri, in long threshold); diff --git a/packages/DefaultContainerService/src/com/android/defcontainer/DefaultContainerService.java b/packages/DefaultContainerService/src/com/android/defcontainer/DefaultContainerService.java index eae6112..8c57595 100644 --- a/packages/DefaultContainerService/src/com/android/defcontainer/DefaultContainerService.java +++ b/packages/DefaultContainerService/src/com/android/defcontainer/DefaultContainerService.java @@ -20,6 +20,7 @@ import com.android.internal.app.IMediaContainerService; import com.android.internal.content.NativeLibraryHelper; import com.android.internal.content.PackageHelper; +import android.app.IntentService; import android.content.Intent; import android.content.pm.IPackageManager; import android.content.pm.PackageInfo; @@ -30,25 +31,24 @@ import android.content.res.ObbInfo; import android.content.res.ObbScanner; import android.net.Uri; import android.os.Environment; +import android.os.FileUtils; import android.os.IBinder; import android.os.ParcelFileDescriptor; import android.os.Process; import android.os.RemoteException; import android.os.ServiceManager; import android.os.StatFs; -import android.app.IntentService; +import android.provider.Settings; import android.util.DisplayMetrics; import android.util.Slog; +import java.io.BufferedInputStream; import java.io.File; import java.io.FileInputStream; import java.io.FileNotFoundException; -import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; - -import android.os.FileUtils; -import android.provider.Settings; +import java.io.OutputStream; /* * This service copies a downloaded apk to a file passed in as @@ -88,19 +88,32 @@ public class DefaultContainerService extends IntentService { /* * Copy specified resource to output stream - * @param packageURI the uri of resource to be copied. Should be a - * file uri + * @param packageURI the uri of resource to be copied. Should be a file + * uri * @param outStream Remote file descriptor to be used for copying - * @return Returns true if copy succeded or false otherwise. + * @return returns status code according to those in {@link + * PackageManager} */ - public boolean copyResource(final Uri packageURI, - ParcelFileDescriptor outStream) { - if (packageURI == null || outStream == null) { - return false; + public int copyResource(final Uri packageURI, ParcelFileDescriptor outStream) { + if (packageURI == null || outStream == null) { + return PackageManager.INSTALL_FAILED_INVALID_URI; + } + + ParcelFileDescriptor.AutoCloseOutputStream autoOut + = new ParcelFileDescriptor.AutoCloseOutputStream(outStream); + + try { + copyFile(packageURI, autoOut); + return PackageManager.INSTALL_SUCCEEDED; + } catch (FileNotFoundException e) { + Slog.e(TAG, "Could not copy URI " + packageURI.toString() + " FNF: " + + e.getMessage()); + return PackageManager.INSTALL_FAILED_INVALID_URI; + } catch (IOException e) { + Slog.e(TAG, "Could not copy URI " + packageURI.toString() + " IO: " + + e.getMessage()); + return PackageManager.INSTALL_FAILED_INSUFFICIENT_STORAGE; } - ParcelFileDescriptor.AutoCloseOutputStream - autoOut = new ParcelFileDescriptor.AutoCloseOutputStream(outStream); - return copyFile(packageURI, autoOut); } /* @@ -315,76 +328,63 @@ public class DefaultContainerService extends IntentService { return newCachePath; } - private static boolean copyToFile(InputStream inputStream, FileOutputStream out) { - try { - byte[] buffer = new byte[4096]; - int bytesRead; - while ((bytesRead = inputStream.read(buffer)) >= 0) { - out.write(buffer, 0, bytesRead); - } - return true; - } catch (IOException e) { - Slog.i(TAG, "Exception : " + e + " when copying file"); - return false; + private static void copyToFile(InputStream inputStream, OutputStream out) throws IOException { + byte[] buffer = new byte[16384]; + int bytesRead; + while ((bytesRead = inputStream.read(buffer)) >= 0) { + out.write(buffer, 0, bytesRead); } } - private static boolean copyToFile(File srcFile, FileOutputStream out) { - InputStream inputStream = null; + private static void copyToFile(File srcFile, OutputStream out) + throws FileNotFoundException, IOException { + InputStream inputStream = new BufferedInputStream(new FileInputStream(srcFile)); try { - inputStream = new FileInputStream(srcFile); - return copyToFile(inputStream, out); - } catch (IOException e) { - return false; + copyToFile(inputStream, out); } finally { - try { if (inputStream != null) inputStream.close(); } catch (IOException e) {} + try { inputStream.close(); } catch (IOException e) {} } } - private boolean copyFile(Uri pPackageURI, FileOutputStream outStream) { + private void copyFile(Uri pPackageURI, OutputStream outStream) throws FileNotFoundException, + IOException { String scheme = pPackageURI.getScheme(); if (scheme == null || scheme.equals("file")) { final File srcPackageFile = new File(pPackageURI.getPath()); // We copy the source package file to a temp file and then rename it to the // destination file in order to eliminate a window where the package directory // scanner notices the new package file but it's not completely copied yet. - if (!copyToFile(srcPackageFile, outStream)) { - Slog.e(TAG, "Couldn't copy file: " + srcPackageFile); - return false; - } + copyToFile(srcPackageFile, outStream); } else if (scheme.equals("content")) { ParcelFileDescriptor fd = null; try { fd = getContentResolver().openFileDescriptor(pPackageURI, "r"); } catch (FileNotFoundException e) { - Slog.e(TAG, - "Couldn't open file descriptor from download service. Failed with exception " - + e); - return false; + Slog.e(TAG, "Couldn't open file descriptor from download service. " + + "Failed with exception " + e); + throw e; } + if (fd == null) { - Slog.e(TAG, "Couldn't open file descriptor from download service (null)."); - return false; + Slog.e(TAG, "Provider returned no file descriptor for " + pPackageURI.toString()); + throw new FileNotFoundException("provider returned no file descriptor"); } else { if (localLOGV) { Slog.i(TAG, "Opened file descriptor from download service."); } - ParcelFileDescriptor.AutoCloseInputStream - dlStream = new ParcelFileDescriptor.AutoCloseInputStream(fd); + ParcelFileDescriptor.AutoCloseInputStream dlStream + = new ParcelFileDescriptor.AutoCloseInputStream(fd); + // We copy the source package file to a temp file and then rename it to the // destination file in order to eliminate a window where the package directory // scanner notices the new package file but it's not completely - // cop - if (!copyToFile(dlStream, outStream)) { - Slog.e(TAG, "Couldn't copy " + pPackageURI + " to temp file."); - return false; - } + // copied + copyToFile(dlStream, outStream); } } else { Slog.e(TAG, "Package URI is not 'file:' or 'content:' - " + pPackageURI); - return false; + throw new FileNotFoundException("Package URI is not 'file:' or 'content:'"); } - return true; } private static final int PREFER_INTERNAL = 1; diff --git a/services/java/com/android/server/pm/PackageManagerService.java b/services/java/com/android/server/pm/PackageManagerService.java index 6ddd7bf..a4eacaf 100644 --- a/services/java/com/android/server/pm/PackageManagerService.java +++ b/services/java/com/android/server/pm/PackageManagerService.java @@ -5348,7 +5348,7 @@ public class PackageManagerService extends IPackageManager.Stub { try { out = ParcelFileDescriptor.open(codeFile, ParcelFileDescriptor.MODE_READ_WRITE); } catch (FileNotFoundException e) { - Slog.e(TAG, "Failed to create file descritpor for : " + codeFileName); + Slog.e(TAG, "Failed to create file descriptor for : " + codeFileName); return PackageManager.INSTALL_FAILED_INSUFFICIENT_STORAGE; } // Copy the resource now @@ -5356,9 +5356,7 @@ public class PackageManagerService extends IPackageManager.Stub { try { mContext.grantUriPermission(DEFAULT_CONTAINER_PACKAGE, packageURI, Intent.FLAG_GRANT_READ_URI_PERMISSION); - if (imcs.copyResource(packageURI, out)) { - ret = PackageManager.INSTALL_SUCCEEDED; - } + ret = imcs.copyResource(packageURI, out); } finally { try { if (out != null) out.close(); } catch (IOException e) {} mContext.revokeUriPermission(packageURI, Intent.FLAG_GRANT_READ_URI_PERMISSION); @@ -5553,6 +5551,12 @@ public class PackageManagerService extends IPackageManager.Stub { int copyApk(IMediaContainerService imcs, boolean temp) throws RemoteException { if (temp) { createCopyFile(); + } else { + /* + * Pre-emptively destroy the container since it's destroyed if + * copying fails due to it existing anyway. + */ + PackageHelper.destroySdDir(cid); } final String newCachePath; -- cgit v1.1