Fix bug with closing output stream for external downloads.
Steve Howard [Wed, 28 Jul 2010 00:02:14 +0000 (17:02 -0700)]
I added a unit test to cover this, and it caught another issue with
disallowing external destinations outside of the default downloads
directory (which are now allowed with the new API).

Change-Id: I4df6442bebb06458ad28c85f6bc8cbcbf3ce67a1

src/com/android/providers/downloads/DownloadThread.java
src/com/android/providers/downloads/Helpers.java
tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java

index 6cb409e..56894fe 100644 (file)
@@ -318,6 +318,7 @@ public class DownloadThread extends Thread {
             // close the file
             if (state.mStream != null) {
                 state.mStream.close();
+                state.mStream = null;
             }
         } catch (IOException ex) {
             if (Constants.LOGV) {
index 58ab578..f298895 100644 (file)
@@ -513,10 +513,9 @@ public class Helpers {
      * Checks whether the filename looks legitimate
      */
     public static boolean isFilenameValid(String filename) {
-        File dir = new File(filename).getParentFile();
-        return dir.equals(Environment.getDownloadCacheDirectory())
-                || dir.equals(new File(Environment.getExternalStorageDirectory()
-                        + Constants.DEFAULT_DL_SUBDIR));
+        filename = filename.replaceFirst("/+", "/"); // normalize leading slashes
+        return filename.startsWith(Environment.getDownloadCacheDirectory().toString())
+                || filename.startsWith(Environment.getExternalStorageDirectory().toString());
     }
 
     /**
index ba3059b..b02844f 100644 (file)
@@ -148,6 +148,16 @@ public class PublicApiFunctionalTest extends AbstractPublicApiTest {
         assertTrue("No ETag header: " + headers, headers.contains("If-Match: " + ETAG));
     }
 
+    public void testInterruptedExternalDownload() throws Exception {
+        enqueueInterruptedDownloadResponses(5);
+        Uri destination = getExternalUri();
+        Download download = enqueueRequest(getRequest().setDestinationUri(destination));
+        download.runUntilStatus(DownloadManager.STATUS_PAUSED);
+        mSystemFacade.incrementTimeMillis(RETRY_DELAY_MILLIS);
+        download.runUntilStatus(DownloadManager.STATUS_SUCCESSFUL);
+        assertEquals(FILE_CONTENT, download.getContents());
+    }
+
     private void enqueueInterruptedDownloadResponses(int initialLength) {
         int totalLength = FILE_CONTENT.length();
         // the first response has normal headers but unexpectedly closes after initialLength bytes
@@ -225,7 +235,7 @@ public class PublicApiFunctionalTest extends AbstractPublicApiTest {
 
     public void testDestination() throws Exception {
         enqueueResponse(HTTP_OK, FILE_CONTENT);
-        Uri destination = Uri.fromFile(mTestDirectory).buildUpon().appendPath("testfile").build();
+        Uri destination = getExternalUri();
         Download download = enqueueRequest(getRequest().setDestinationUri(destination));
         download.runUntilStatus(DownloadManager.STATUS_SUCCESSFUL);
 
@@ -240,6 +250,10 @@ public class PublicApiFunctionalTest extends AbstractPublicApiTest {
         }
     }
 
+    private Uri getExternalUri() {
+        return Uri.fromFile(mTestDirectory).buildUpon().appendPath("testfile").build();
+    }
+
     public void testRequestHeaders() throws Exception {
         enqueueEmptyResponse(HTTP_OK);
         Download download = enqueueRequest(getRequest().setRequestHeader("Header1", "value1")