Tests for max retries/redirects, ETag switches.

Verify that servers responding with many retries or redirects result
in failed download, instead of spinning out of control.  Test to
verify that changed ETag results in download failing.

Also fix handling of HTTP 301 to update Uri in database.

Change-Id: Iff2948d79961a245b7900117d107edaa356618c9

7 files changed, 119 insertions, 26 deletions

diff --git a/src/com/android/providers/downloads/DownloadThread.java b/src/com/android/providers/downloads/DownloadThread.java
index bd347e42..0d427fdd 100644
--- a/src/com/android/providers/downloads/DownloadThread.java
+++ b/src/com/android/providers/downloads/DownloadThread.java
@@ -65,7 +65,6 @@ import java.net.URL;
 import java.net.URLConnection;
 
 import libcore.io.IoUtils;
-import libcore.net.http.HttpEngine;
 
 /**
  * Task which executes a given {@link DownloadInfo}: making network requests,
@@ -275,9 +274,7 @@ public class DownloadThread implements Runnable {
             return;
         }
 
-        // TODO: compare mInfo.mNumFailed against Constants.MAX_RETRIES
-
-        while (state.mRedirectionCount++ < HttpEngine.MAX_REDIRECTS) {
+        while (state.mRedirectionCount++ < Constants.MAX_REDIRECTS) {
             // Open connection and follow any redirects until we have a useful
             // response with body.
             HttpURLConnection conn = null;
@@ -315,18 +312,16 @@ public class DownloadThread implements Runnable {
                     case HTTP_TEMP_REDIRECT:
                         final String location = conn.getHeaderField("Location");
                         state.mUrl = new URL(state.mUrl, location);
+                        if (responseCode == HTTP_MOVED_PERM) {
+                            // Push updated URL back to database
+                            state.mRequestUri = state.mUrl.toString();
+                        }
                         continue;
 
                     case HTTP_REQUESTED_RANGE_NOT_SATISFIABLE:
                         throw new StopRequestException(
                                 STATUS_CANNOT_RESUME, "Requested range not satisfiable");
 
-                    case HTTP_PRECON_FAILED:
-                        // TODO: probably means our etag precondition was
-                        // changed; flush and retry again
-                        StopRequestException.throwUnhandledHttpError(
-                                responseCode, conn.getResponseMessage());
-
                     case HTTP_UNAVAILABLE:
                         parseRetryAfterHeaders(state, conn);
                         throw new StopRequestException(
@@ -645,7 +640,7 @@ public class DownloadThread implements Runnable {
                 state.mMimeType,
                 mInfo.mDestination,
                 state.mContentLength,
-                mInfo.mIsPublicApi, mStorageManager);
+                mStorageManager);
 
         updateDatabaseFromHeaders(state);
         // check connectivity again now that we know the total size
@@ -826,6 +821,10 @@ public class DownloadThread implements Runnable {
         values.put(Downloads.Impl.COLUMN_FAILED_CONNECTIONS, numFailed);
         values.put(Constants.RETRY_AFTER_X_REDIRECT_COUNT, state.mRetryAfter);
 
+        if (!TextUtils.equals(mInfo.mUri, state.mRequestUri)) {
+            values.put(Downloads.Impl.COLUMN_URI, state.mRequestUri);
+        }
+
         // save the error message. could be useful to developers.
         if (!TextUtils.isEmpty(errorMsg)) {
             values.put(Downloads.Impl.COLUMN_ERROR_MSG, errorMsg);
diff --git a/src/com/android/providers/downloads/Helpers.java b/src/com/android/providers/downloads/Helpers.java
index 059e9703..5c34ebe1 100644
--- a/src/com/android/providers/downloads/Helpers.java
+++ b/src/com/android/providers/downloads/Helpers.java
@@ -77,7 +77,7 @@ public class Helpers {
             String mimeType,
             int destination,
             long contentLength,
-            boolean isPublicApi, StorageManager storageManager) throws StopRequestException {
+            StorageManager storageManager) throws StopRequestException {
         if (contentLength < 0) {
             contentLength = 0;
         }
diff --git a/tests/src/com/android/providers/downloads/AbstractDownloadProviderFunctionalTest.java b/tests/src/com/android/providers/downloads/AbstractDownloadProviderFunctionalTest.java
index 48eb0b47..acfd4736 100644
--- a/tests/src/com/android/providers/downloads/AbstractDownloadProviderFunctionalTest.java
+++ b/tests/src/com/android/providers/downloads/AbstractDownloadProviderFunctionalTest.java
@@ -52,11 +52,9 @@ public abstract class AbstractDownloadProviderFunctionalTest extends
     protected static final String LOG_TAG = "DownloadProviderFunctionalTest";
     private static final String PROVIDER_AUTHORITY = "downloads";
     protected static final long RETRY_DELAY_MILLIS = 61 * 1000;
-    protected static final String FILE_CONTENT = "hello world hello world hello world hello world";
-    protected static final int HTTP_OK = 200;
-    protected static final int HTTP_PARTIAL_CONTENT = 206;
-    protected static final int HTTP_NOT_FOUND = 404;
-    protected static final int HTTP_SERVICE_UNAVAILABLE = 503;
+
+    protected static final String
+            FILE_CONTENT = "0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ";
 
     protected MockWebServer mServer;
     protected MockContentResolverWithNotify mResolver;
diff --git a/tests/src/com/android/providers/downloads/AbstractPublicApiTest.java b/tests/src/com/android/providers/downloads/AbstractPublicApiTest.java
index e7f08c90..bff9333a 100644
--- a/tests/src/com/android/providers/downloads/AbstractPublicApiTest.java
+++ b/tests/src/com/android/providers/downloads/AbstractPublicApiTest.java
@@ -27,7 +27,6 @@ import android.os.ParcelFileDescriptor;
 import android.os.SystemClock;
 import android.util.Log;
 
-import java.io.FileInputStream;
 import java.io.InputStream;
 import java.net.MalformedURLException;
 import java.net.UnknownHostException;
@@ -49,6 +48,10 @@ public abstract class AbstractPublicApiTest extends AbstractDownloadProviderFunc
             return (int) getLongField(DownloadManager.COLUMN_STATUS);
         }
 
+        public int getReason() {
+            return (int) getLongField(DownloadManager.COLUMN_REASON);
+        }
+
         public int getStatusIfExists() {
             Cursor cursor = mManager.query(new DownloadManager.Query().setFilterById(mId));
             try {
@@ -91,7 +94,8 @@ public abstract class AbstractPublicApiTest extends AbstractDownloadProviderFunc
             ParcelFileDescriptor downloadedFile = mManager.openDownloadedFile(mId);
             assertTrue("Invalid file descriptor: " + downloadedFile,
                        downloadedFile.getFileDescriptor().valid());
-            InputStream stream = new FileInputStream(downloadedFile.getFileDescriptor());
+            final InputStream stream = new ParcelFileDescriptor.AutoCloseInputStream(
+                    downloadedFile);
             try {
                 return readStream(stream);
             } finally {
diff --git a/tests/src/com/android/providers/downloads/DownloadProviderFunctionalTest.java b/tests/src/com/android/providers/downloads/DownloadProviderFunctionalTest.java
index d5249378..dbab203c 100644
--- a/tests/src/com/android/providers/downloads/DownloadProviderFunctionalTest.java
+++ b/tests/src/com/android/providers/downloads/DownloadProviderFunctionalTest.java
@@ -17,6 +17,7 @@
 package com.android.providers.downloads;
 
 import static android.text.format.DateUtils.SECOND_IN_MILLIS;
+import static java.net.HttpURLConnection.HTTP_OK;
 
 import android.content.ContentValues;
 import android.database.Cursor;
diff --git a/tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java b/tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java
index 09739c0d..84390aad 100644
--- a/tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java
+++ b/tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java
@@ -16,6 +16,9 @@
 
 package com.android.providers.downloads;
 
+import static android.app.DownloadManager.STATUS_FAILED;
+import static android.app.DownloadManager.STATUS_PAUSED;
+import static android.text.format.DateUtils.SECOND_IN_MILLIS;
 import static com.google.testing.littlemock.LittleMock.anyInt;
 import static com.google.testing.littlemock.LittleMock.anyString;
 import static com.google.testing.littlemock.LittleMock.atLeastOnce;
@@ -23,6 +26,12 @@ import static com.google.testing.littlemock.LittleMock.isA;
 import static com.google.testing.littlemock.LittleMock.never;
 import static com.google.testing.littlemock.LittleMock.times;
 import static com.google.testing.littlemock.LittleMock.verify;
+import static java.net.HttpURLConnection.HTTP_MOVED_TEMP;
+import static java.net.HttpURLConnection.HTTP_NOT_FOUND;
+import static java.net.HttpURLConnection.HTTP_OK;
+import static java.net.HttpURLConnection.HTTP_PARTIAL;
+import static java.net.HttpURLConnection.HTTP_PRECON_FAILED;
+import static java.net.HttpURLConnection.HTTP_UNAVAILABLE;
 
 import android.app.DownloadManager;
 import android.app.Notification;
@@ -33,6 +42,7 @@ import android.database.Cursor;
 import android.net.ConnectivityManager;
 import android.net.Uri;
 import android.os.Environment;
+import android.os.SystemClock;
 import android.provider.Downloads;
 import android.test.suitebuilder.annotation.LargeTest;
 
@@ -44,8 +54,8 @@ import java.io.FileInputStream;
 import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.InputStream;
-import java.net.MalformedURLException;
 import java.util.List;
+import java.util.concurrent.TimeoutException;
 
 @LargeTest
 public class PublicApiFunctionalTest extends AbstractPublicApiTest {
@@ -189,7 +199,7 @@ public class PublicApiFunctionalTest extends AbstractPublicApiTest {
     private MockResponse buildPartialResponse(int start, int end) {
         int totalLength = FILE_CONTENT.length();
         boolean isFirstResponse = (start == 0);
-        int status = isFirstResponse ? HTTP_OK : HTTP_PARTIAL_CONTENT;
+        int status = isFirstResponse ? HTTP_OK : HTTP_PARTIAL;
         MockResponse response = buildResponse(status, FILE_CONTENT.substring(start, end))
                 .setHeader("Content-length", totalLength)
                 .setHeader("Etag", ETAG);
@@ -383,11 +393,72 @@ public class PublicApiFunctionalTest extends AbstractPublicApiTest {
         assertEquals(REQUEST_PATH, lastRequest.getPath());
     }
 
+    public void testRunawayRedirect() throws Exception {
+        for (int i = 0; i < 16; i++) {
+            enqueueResponse(buildEmptyResponse(HTTP_MOVED_TEMP)
+                    .setHeader("Location", mServer.getUrl("/" + i).toString()));
+        }
+
+        final Download download = enqueueRequest(getRequest());
+
+        // Ensure that we arrive at failed download, instead of spinning forever
+        download.runUntilStatus(DownloadManager.STATUS_FAILED);
+        assertEquals(DownloadManager.ERROR_TOO_MANY_REDIRECTS, download.getReason());
+    }
+
+    public void testRunawayUnavailable() throws Exception {
+        final int RETRY_DELAY = 120;
+        for (int i = 0; i < 16; i++) {
+            enqueueResponse(
+                    buildEmptyResponse(HTTP_UNAVAILABLE).setHeader("Retry-after", RETRY_DELAY));
+        }
+
+        final Download download = enqueueRequest(getRequest());
+        for (int i = 0; i < Constants.MAX_RETRIES - 1; i++) {
+            download.runUntilStatus(DownloadManager.STATUS_PAUSED);
+            mSystemFacade.incrementTimeMillis((RETRY_DELAY + 60) * SECOND_IN_MILLIS);
+        }
+
+        // Ensure that we arrive at failed download, instead of spinning forever
+        download.runUntilStatus(DownloadManager.STATUS_FAILED);
+    }
+
     public void testNoEtag() throws Exception {
         enqueueResponse(buildPartialResponse(0, 5).removeHeader("Etag"));
         runSimpleFailureTest(DownloadManager.ERROR_CANNOT_RESUME);
     }
 
+    public void testEtagChanged() throws Exception {
+        final String A = "kittenz";
+        final String B = "puppiez";
+
+        // 1. Try downloading A, but partial result
+        enqueueResponse(buildResponse(HTTP_OK, A.substring(0, 2))
+                .setHeader("Content-length", A.length())
+                .setHeader("Etag", A));
+
+        // 2. Try resuming A, but fail ETag check
+        enqueueResponse(buildEmptyResponse(HTTP_PRECON_FAILED));
+
+        final Download download = enqueueRequest(getRequest());
+        RecordedRequest req;
+
+        // 1. Try downloading A, but partial result
+        download.runUntilStatus(STATUS_PAUSED);
+        assertEquals(DownloadManager.PAUSED_WAITING_TO_RETRY, download.getReason());
+        req = takeRequest();
+        assertNull(getHeaderValue(req, "Range"));
+        assertNull(getHeaderValue(req, "If-Match"));
+
+        // 2. Try resuming A, but fail ETag check
+        mSystemFacade.incrementTimeMillis(RETRY_DELAY_MILLIS);
+        download.runUntilStatus(STATUS_FAILED);
+        assertEquals(HTTP_PRECON_FAILED, download.getReason());
+        req = takeRequest();
+        assertEquals("bytes=2-", getHeaderValue(req, "Range"));
+        assertEquals(A, getHeaderValue(req, "If-Match"));
+    }
+
     public void testSanitizeMediaType() throws Exception {
         enqueueResponse(buildEmptyResponse(HTTP_OK)
                 .setHeader("Content-Type", "text/html; charset=ISO-8859-4"));
@@ -420,8 +491,14 @@ public class PublicApiFunctionalTest extends AbstractPublicApiTest {
         assertTrue(rslt);
         mManager.remove(download.mId);
 
-        // make sure the row is gone from the database
-        download.waitForStatus(-1, -1);
+        // Verify that row is removed from database
+        final long timeout = SystemClock.elapsedRealtime() + (15 * SECOND_IN_MILLIS);
+        while (download.getStatusIfExists() != -1) {
+            if (SystemClock.elapsedRealtime() > timeout) {
+                throw new TimeoutException("Row wasn't removed");
+            }
+            SystemClock.sleep(100);
+        }
     }
 
     public void testDownloadCompleteBroadcast() throws Exception {
@@ -550,7 +627,7 @@ public class PublicApiFunctionalTest extends AbstractPublicApiTest {
     public void testRetryAfter() throws Exception {
         final int delay = 120;
         enqueueResponse(
-                buildEmptyResponse(HTTP_SERVICE_UNAVAILABLE).setHeader("Retry-after", delay));
+                buildEmptyResponse(HTTP_UNAVAILABLE).setHeader("Retry-after", delay));
         enqueueResponse(buildEmptyResponse(HTTP_OK));
 
         Download download = enqueueRequest(getRequest());
@@ -634,8 +711,7 @@ public class PublicApiFunctionalTest extends AbstractPublicApiTest {
      * 3) Resume request to complete download
      * @return the last request sent to the server, resuming after the interruption
      */
-    private RecordedRequest runRedirectionTest(int status)
-            throws MalformedURLException, Exception {
+    private RecordedRequest runRedirectionTest(int status) throws Exception {
         enqueueResponse(buildEmptyResponse(status)
                 .setHeader("Location", mServer.getUrl(REDIRECTED_PATH).toString()));
         enqueueInterruptedDownloadResponses(5);
@@ -650,4 +726,17 @@ public class PublicApiFunctionalTest extends AbstractPublicApiTest {
 
         return takeRequest();
     }
+
+    /**
+     * Return value of requested HTTP header, if it exists.
+     */
+    private static String getHeaderValue(RecordedRequest req, String header) {
+        header = header.toLowerCase() + ":";
+        for (String h : req.getHeaders()) {
+            if (h.toLowerCase().startsWith(header)) {
+                return h.substring(header.length()).trim();
+            }
+        }
+        return null;
+    }
 }
diff --git a/tests/src/com/android/providers/downloads/ThreadingTest.java b/tests/src/com/android/providers/downloads/ThreadingTest.java
index e73bae2a..fcb33290 100644
--- a/tests/src/com/android/providers/downloads/ThreadingTest.java
+++ b/tests/src/com/android/providers/downloads/ThreadingTest.java
@@ -16,6 +16,8 @@
 
 package com.android.providers.downloads;
 
+import static java.net.HttpURLConnection.HTTP_OK;
+
 import android.app.DownloadManager;
 import android.test.suitebuilder.annotation.LargeTest;