diff options
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; |