diff options
4 files changed, 170 insertions, 64 deletions
diff --git a/src/com/android/providers/downloads/DownloadProvider.java b/src/com/android/providers/downloads/DownloadProvider.java index d7c24f9a..7070f31c 100644 --- a/src/com/android/providers/downloads/DownloadProvider.java +++ b/src/com/android/providers/downloads/DownloadProvider.java @@ -16,6 +16,8 @@ package com.android.providers.downloads; +import com.google.common.annotations.VisibleForTesting; + import android.content.ContentProvider; import android.content.ContentValues; import android.content.Context; @@ -110,6 +112,9 @@ public final class DownloadProvider extends ContentProvider { private int mSystemUid = -1; private int mDefContainerUid = -1; + @VisibleForTesting + SystemFacade mSystemFacade; + /** * Creates and updated database on demand when opening it. * Helper class to create database the first time the provider is @@ -172,6 +177,10 @@ public final class DownloadProvider extends ContentProvider { */ @Override public boolean onCreate() { + if (mSystemFacade == null) { + mSystemFacade = new RealSystemFacade(); + } + mOpenHelper = new DatabaseHelper(getContext()); // Initialize the system uid mSystemUid = Process.SYSTEM_UID; @@ -294,9 +303,16 @@ public final class DownloadProvider extends ContentProvider { if (getContext().checkCallingPermission(Downloads.Impl.PERMISSION_ACCESS_ADVANCED) != PackageManager.PERMISSION_GRANTED && dest != Downloads.Impl.DESTINATION_EXTERNAL - && dest != Downloads.Impl.DESTINATION_CACHE_PARTITION_PURGEABLE) { + && dest != Downloads.Impl.DESTINATION_CACHE_PARTITION_PURGEABLE + && dest != Downloads.Impl.DESTINATION_FILE_URI) { throw new SecurityException("unauthorized destination code"); } + if (dest == Downloads.Impl.DESTINATION_FILE_URI) { + getContext().enforcePermission( + android.Manifest.permission.WRITE_EXTERNAL_STORAGE, + Binder.getCallingPid(), Binder.getCallingUid(), + "need WRITE_EXTERNAL_STORAGE permission to use DESTINATION_FILE_URI"); + } filteredValues.put(Downloads.Impl.COLUMN_DESTINATION, dest); } Integer vis = values.getAsInteger(Downloads.Impl.COLUMN_VISIBILITY); @@ -313,7 +329,8 @@ public final class DownloadProvider extends ContentProvider { } copyInteger(Downloads.Impl.COLUMN_CONTROL, values, filteredValues); filteredValues.put(Downloads.Impl.COLUMN_STATUS, Downloads.Impl.STATUS_PENDING); - filteredValues.put(Downloads.Impl.COLUMN_LAST_MODIFICATION, System.currentTimeMillis()); + filteredValues.put(Downloads.Impl.COLUMN_LAST_MODIFICATION, + mSystemFacade.currentTimeMillis()); String pckg = values.getAsString(Downloads.Impl.COLUMN_NOTIFICATION_PACKAGE); String clazz = values.getAsString(Downloads.Impl.COLUMN_NOTIFICATION_CLASS); if (pckg != null && clazz != null) { @@ -733,7 +750,7 @@ public final class DownloadProvider extends ContentProvider { throw new FileNotFoundException("couldn't open file"); } else { ContentValues values = new ContentValues(); - values.put(Downloads.Impl.COLUMN_LAST_MODIFICATION, System.currentTimeMillis()); + values.put(Downloads.Impl.COLUMN_LAST_MODIFICATION, mSystemFacade.currentTimeMillis()); update(uri, values, null, null); } return ret; diff --git a/src/com/android/providers/downloads/Helpers.java b/src/com/android/providers/downloads/Helpers.java index 6bb5093a..2705a7cb 100644 --- a/src/com/android/providers/downloads/Helpers.java +++ b/src/com/android/providers/downloads/Helpers.java @@ -35,7 +35,7 @@ import android.util.Config; import android.util.Log; import android.webkit.MimeTypeMap; -import java.io.File; +import java.io.File; import java.io.FileNotFoundException; import java.io.FileOutputStream; import java.util.Random; @@ -76,6 +76,17 @@ public class Helpers { } /** + * Exception thrown from methods called by generateSaveFile() for any fatal error. + */ + private static class GenerateSaveFileError extends Exception { + int mStatus; + + public GenerateSaveFileError(int status) { + mStatus = status; + } + } + + /** * Creates a filename (where the file should be saved) from a uri. */ public static DownloadFileInfo generateSaveFile( @@ -88,16 +99,80 @@ public class Helpers { int destination, int contentLength) throws FileNotFoundException { - /* - * Don't download files that we won't be able to handle - */ + if (!canHandleDownload(context, mimeType, destination)) { + return new DownloadFileInfo(null, null, Downloads.Impl.STATUS_NOT_ACCEPTABLE); + } + + String fullFilename; + try { + if (destination == Downloads.Impl.DESTINATION_FILE_URI) { + fullFilename = getPathForFileUri(hint); + } else { + fullFilename = chooseFullPath(context, url, hint, contentDisposition, + contentLocation, mimeType, destination, + contentLength); + } + } catch (GenerateSaveFileError exc) { + return new DownloadFileInfo(null, null, exc.mStatus); + } + + return new DownloadFileInfo(fullFilename, new FileOutputStream(fullFilename), 0); + } + + private static String getPathForFileUri(String hint) throws GenerateSaveFileError { + Uri uri = Uri.parse(hint); + if (!uri.getScheme().equals("file")) { + Log.d(Constants.TAG, "Not a file URI: " + hint); + throw new GenerateSaveFileError(Downloads.Impl.STATUS_FILE_ERROR); + } + + String path = uri.getSchemeSpecificPart(); + if (new File(path).exists()) { + Log.d(Constants.TAG, "File already exists: " + path); + throw new GenerateSaveFileError(Downloads.Impl.STATUS_FILE_ERROR); + } + + return path; + } + + private static String chooseFullPath(Context context, String url, String hint, + String contentDisposition, String contentLocation, + String mimeType, int destination, int contentLength) + throws GenerateSaveFileError { + File base = locateDestinationDirectory(context, mimeType, destination, contentLength); + String filename = chooseFilename(url, hint, contentDisposition, contentLocation, + destination); + + // Split filename between base and extension + // Add an extension if filename does not have one + String extension = null; + int dotIndex = filename.indexOf('.'); + if (dotIndex < 0) { + extension = chooseExtensionFromMimeType(mimeType, true); + } else { + extension = chooseExtensionFromFilename(mimeType, destination, filename, dotIndex); + filename = filename.substring(0, dotIndex); + } + + boolean recoveryDir = Constants.RECOVERY_DIRECTORY.equalsIgnoreCase(filename + extension); + + filename = base.getPath() + File.separator + filename; + + if (Constants.LOGVV) { + Log.v(Constants.TAG, "target file: " + filename + extension); + } + + return chooseUniqueFilename(destination, filename, extension, recoveryDir); + } + + private static boolean canHandleDownload(Context context, String mimeType, int destination) { if (destination == Downloads.Impl.DESTINATION_EXTERNAL || destination == Downloads.Impl.DESTINATION_CACHE_PARTITION_PURGEABLE) { if (mimeType == null) { if (Config.LOGD) { Log.d(Constants.TAG, "external download with no mime type not allowed"); } - return new DownloadFileInfo(null, null, Downloads.Impl.STATUS_NOT_ACCEPTABLE); + return false; } if (!DrmRawContent.DRM_MIMETYPE_MESSAGE_STRING.equalsIgnoreCase(mimeType)) { // Check to see if we are allowed to download this file. Only files @@ -121,32 +196,19 @@ public class Helpers { if (Config.LOGD) { Log.d(Constants.TAG, "no handler found for type " + mimeType); } - return new DownloadFileInfo(null, null, Downloads.Impl.STATUS_NOT_ACCEPTABLE); + return false; } } } - String filename = chooseFilename( - url, hint, contentDisposition, contentLocation, destination); - - // Split filename between base and extension - // Add an extension if filename does not have one - String extension = null; - int dotIndex = filename.indexOf('.'); - if (dotIndex < 0) { - extension = chooseExtensionFromMimeType(mimeType, true); - } else { - extension = chooseExtensionFromFilename( - mimeType, destination, filename, dotIndex); - filename = filename.substring(0, dotIndex); - } - - /* - * Locate the directory where the file will be saved - */ + return true; + } + private static File locateDestinationDirectory(Context context, String mimeType, + int destination, int contentLength) + throws GenerateSaveFileError { File base = null; StatFs stat = null; - // DRM messages should be temporarily stored internally and then passed to + // DRM messages should be temporarily stored internally and then passed to // the DRM content provider if (destination == Downloads.Impl.DESTINATION_CACHE_PARTITION || destination == Downloads.Impl.DESTINATION_CACHE_PARTITION_PURGEABLE @@ -170,8 +232,7 @@ public class Helpers { Log.d(Constants.TAG, "download aborted - not enough free space in internal storage"); } - return new DownloadFileInfo(null, null, - Downloads.Impl.STATUS_INSUFFICIENT_SPACE_ERROR); + throw new GenerateSaveFileError(Downloads.Impl.STATUS_INSUFFICIENT_SPACE_ERROR); } else { // Recalculate available space and try again. stat.restat(base.getPath()); @@ -192,8 +253,7 @@ public class Helpers { if (Config.LOGD) { Log.d(Constants.TAG, "download aborted - not enough free space"); } - return new DownloadFileInfo(null, null, - Downloads.Impl.STATUS_INSUFFICIENT_SPACE_ERROR); + throw new GenerateSaveFileError(Downloads.Impl.STATUS_INSUFFICIENT_SPACE_ERROR); } base = new File(root + Constants.DEFAULT_DL_SUBDIR); @@ -204,35 +264,17 @@ public class Helpers { Log.d(Constants.TAG, "download aborted - can't create base directory " + base.getPath()); } - return new DownloadFileInfo(null, null, Downloads.Impl.STATUS_FILE_ERROR); + throw new GenerateSaveFileError(Downloads.Impl.STATUS_FILE_ERROR); } } else { // No SD card found. if (Config.LOGD) { Log.d(Constants.TAG, "download aborted - no external storage"); } - return new DownloadFileInfo(null, null, - Downloads.Impl.STATUS_DEVICE_NOT_FOUND_ERROR); + throw new GenerateSaveFileError(Downloads.Impl.STATUS_DEVICE_NOT_FOUND_ERROR); } - boolean recoveryDir = Constants.RECOVERY_DIRECTORY.equalsIgnoreCase(filename + extension); - - filename = base.getPath() + File.separator + filename; - - /* - * Generate a unique filename, create the file, return it. - */ - if (Constants.LOGVV) { - Log.v(Constants.TAG, "target file: " + filename + extension); - } - - String fullFilename = chooseUniqueFilename( - destination, filename, extension, recoveryDir); - if (fullFilename != null) { - return new DownloadFileInfo(fullFilename, new FileOutputStream(fullFilename), 0); - } else { - return new DownloadFileInfo(null, null, Downloads.Impl.STATUS_FILE_ERROR); - } + return base; } private static String chooseFilename(String url, String hint, String contentDisposition, @@ -383,7 +425,7 @@ public class Helpers { } private static String chooseUniqueFilename(int destination, String filename, - String extension, boolean recoveryDir) { + String extension, boolean recoveryDir) throws GenerateSaveFileError { String fullFilename = filename + extension; if (!new File(fullFilename).exists() && (!recoveryDir || @@ -420,7 +462,7 @@ public class Helpers { sequence += sRandom.nextInt(magnitude) + 1; } } - return null; + throw new GenerateSaveFileError(Downloads.Impl.STATUS_FILE_ERROR); } /** diff --git a/tests/src/com/android/providers/downloads/AbstractDownloadManagerFunctionalTest.java b/tests/src/com/android/providers/downloads/AbstractDownloadManagerFunctionalTest.java index 1394a17d..06dd52af 100644 --- a/tests/src/com/android/providers/downloads/AbstractDownloadManagerFunctionalTest.java +++ b/tests/src/com/android/providers/downloads/AbstractDownloadManagerFunctionalTest.java @@ -17,7 +17,6 @@ package com.android.providers.downloads; import android.content.ComponentName; -import android.content.ContentProvider; import android.content.ContentResolver; import android.content.Context; import android.content.Intent; @@ -138,6 +137,7 @@ public abstract class AbstractDownloadManagerFunctionalTest extends protected void setUp() throws Exception { super.setUp(); + mSystemFacade = new FakeSystemFacade(); Context realContext = getContext(); mTestContext = new TestContext(realContext); setupProviderAndResolver(); @@ -146,7 +146,6 @@ public abstract class AbstractDownloadManagerFunctionalTest extends mTestContext.setResolver(mResolver); setContext(mTestContext); setupService(); - mSystemFacade = new FakeSystemFacade(); getService().mSystemFacade = mSystemFacade; mServer = new MockWebServer(); @@ -169,7 +168,8 @@ public abstract class AbstractDownloadManagerFunctionalTest extends } void setupProviderAndResolver() { - ContentProvider provider = new DownloadProvider(); + DownloadProvider provider = new DownloadProvider(); + provider.mSystemFacade = mSystemFacade; provider.attachInfo(mTestContext, null); mResolver = new MockContentResolver(); mResolver.addProvider(PROVIDER_AUTHORITY, provider); diff --git a/tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java b/tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java index 1927545f..e3b278bc 100644 --- a/tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java +++ b/tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java @@ -22,14 +22,12 @@ import android.net.Uri; import android.os.Environment; import tests.http.RecordedRequest; +import java.io.File; import java.io.FileInputStream; import java.io.InputStream; import java.net.MalformedURLException; public class PublicApiFunctionalTest extends AbstractDownloadManagerFunctionalTest { - /** - * - */ private static final String REQUEST_PATH = "/path"; class Download implements StatusReader { @@ -87,11 +85,34 @@ public class PublicApiFunctionalTest extends AbstractDownloadManagerFunctionalTe } private DownloadManager mManager; + private File mTestDirectory; @Override protected void setUp() throws Exception { super.setUp(); mManager = new DownloadManager(mResolver); + + mTestDirectory = new File(Environment.getExternalStorageDirectory() + File.separator + + "download_manager_functional_test"); + if (mTestDirectory.exists()) { + throw new RuntimeException( + "Test directory on external storage already exists, cannot run"); + } + if (!mTestDirectory.mkdir()) { + throw new RuntimeException("Couldn't create test directory: " + + mTestDirectory.getPath()); + } + } + + @Override + protected void tearDown() throws Exception { + if (mTestDirectory != null) { + for (File file : mTestDirectory.listFiles()) { + file.delete(); + } + mTestDirectory.delete(); + } + super.tearDown(); } public void testBasicRequest() throws Exception { @@ -103,20 +124,25 @@ public class PublicApiFunctionalTest extends AbstractDownloadManagerFunctionalTe assertEquals(getServerUri(REQUEST_PATH), download.getStringField(DownloadManager.COLUMN_URI)); assertEquals(download.mId, download.getLongField(DownloadManager.COLUMN_ID)); + assertEquals(mSystemFacade.currentTimeMillis(), + download.getLongField(DownloadManager.COLUMN_LAST_MODIFIED_TIMESTAMP)); + mSystemFacade.incrementTimeMillis(10); RecordedRequest request = download.runUntilStatus(DownloadManager.STATUS_SUCCESSFUL); assertEquals("GET", request.getMethod()); assertEquals(REQUEST_PATH, request.getPath()); Uri localUri = Uri.parse(download.getStringField(DownloadManager.COLUMN_LOCAL_URI)); assertEquals("file", localUri.getScheme()); - assertStartsWith(Environment.getDownloadCacheDirectory().getPath(), + assertStartsWith("//" + Environment.getDownloadCacheDirectory().getPath(), localUri.getSchemeSpecificPart()); assertEquals("text/plain", download.getStringField(DownloadManager.COLUMN_MEDIA_TYPE)); int size = FILE_CONTENT.length(); assertEquals(size, download.getLongField(DownloadManager.COLUMN_TOTAL_SIZE_BYTES)); assertEquals(size, download.getLongField(DownloadManager.COLUMN_BYTES_DOWNLOADED_SO_FAR)); + assertEquals(mSystemFacade.currentTimeMillis(), + download.getLongField(DownloadManager.COLUMN_LAST_MODIFIED_TIMESTAMP)); assertEquals(FILE_CONTENT, download.getContents()); } @@ -176,12 +202,16 @@ public class PublicApiFunctionalTest extends AbstractDownloadManagerFunctionalTe Download download1 = enqueueRequest(getRequest()); download1.runUntilStatus(DownloadManager.STATUS_SUCCESSFUL); enqueueEmptyResponse(HTTP_NOT_FOUND); + + mSystemFacade.incrementTimeMillis(1); // ensure downloads are correctly ordered by time Download download2 = enqueueRequest(getRequest()); download2.runUntilStatus(DownloadManager.STATUS_FAILED); + + mSystemFacade.incrementTimeMillis(1); Download download3 = enqueueRequest(getRequest()); Cursor cursor = mManager.query(new DownloadManager.Query()); - checkAndCloseCursor(cursor, download1, download2, download3); + checkAndCloseCursor(cursor, download3, download2, download1); cursor = mManager.query(new DownloadManager.Query().setFilterById(download2.mId)); checkAndCloseCursor(cursor, download2); @@ -193,7 +223,7 @@ public class PublicApiFunctionalTest extends AbstractDownloadManagerFunctionalTe cursor = mManager.query(new DownloadManager.Query() .setFilterByStatus(DownloadManager.STATUS_FAILED | DownloadManager.STATUS_SUCCESSFUL)); - checkAndCloseCursor(cursor, download1, download2); + checkAndCloseCursor(cursor, download2, download1); cursor = mManager.query(new DownloadManager.Query() .setFilterByStatus(DownloadManager.STATUS_RUNNING)); @@ -224,6 +254,23 @@ public class PublicApiFunctionalTest extends AbstractDownloadManagerFunctionalTe fail("No exception thrown for invalid URI"); } + public void testDestination() throws Exception { + enqueueResponse(HTTP_OK, FILE_CONTENT); + Uri destination = Uri.fromFile(mTestDirectory).buildUpon().appendPath("testfile").build(); + Download download = enqueueRequest(getRequest().setDestinationUri(destination)); + download.runUntilStatus(DownloadManager.STATUS_SUCCESSFUL); + + Uri localUri = Uri.parse(download.getStringField(DownloadManager.COLUMN_LOCAL_URI)); + assertEquals(destination, localUri); + + InputStream stream = new FileInputStream(destination.getSchemeSpecificPart()); + try { + assertEquals(FILE_CONTENT, readStream(stream)); + } finally { + stream.close(); + } + } + private DownloadManager.Request getRequest() throws MalformedURLException { return getRequest(getServerUri(REQUEST_PATH)); } |