diff options
6 files changed, 165 insertions, 110 deletions
diff --git a/src/com/android/providers/downloads/DownloadInfo.java b/src/com/android/providers/downloads/DownloadInfo.java index e64db289..eb4ef0a5 100644 --- a/src/com/android/providers/downloads/DownloadInfo.java +++ b/src/com/android/providers/downloads/DownloadInfo.java @@ -45,6 +45,9 @@ import java.util.List; * Stores information about an individual download. */ public class DownloadInfo { + // TODO: move towards these in-memory objects being sources of truth, and + // periodically pushing to provider. + public static class Reader { private ContentResolver mResolver; private Cursor mCursor; diff --git a/src/com/android/providers/downloads/DownloadThread.java b/src/com/android/providers/downloads/DownloadThread.java index ae279260..dc2ef571 100644 --- a/src/com/android/providers/downloads/DownloadThread.java +++ b/src/com/android/providers/downloads/DownloadThread.java @@ -16,10 +16,21 @@ package com.android.providers.downloads; +import static android.provider.Downloads.Impl.STATUS_BAD_REQUEST; +import static android.provider.Downloads.Impl.STATUS_CANNOT_RESUME; +import static android.provider.Downloads.Impl.STATUS_FILE_ERROR; +import static android.provider.Downloads.Impl.STATUS_HTTP_DATA_ERROR; +import static android.provider.Downloads.Impl.STATUS_TOO_MANY_REDIRECTS; +import static android.provider.Downloads.Impl.STATUS_WAITING_TO_RETRY; import static android.text.format.DateUtils.MINUTE_IN_MILLIS; import static com.android.providers.downloads.Constants.TAG; +import static java.net.HttpURLConnection.HTTP_INTERNAL_ERROR; +import static java.net.HttpURLConnection.HTTP_MOVED_PERM; +import static java.net.HttpURLConnection.HTTP_MOVED_TEMP; 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_SEE_OTHER; import static java.net.HttpURLConnection.HTTP_UNAVAILABLE; import android.content.ContentValues; @@ -47,10 +58,12 @@ import java.io.InputStream; import java.io.OutputStream; import java.io.RandomAccessFile; import java.net.HttpURLConnection; +import java.net.MalformedURLException; import java.net.URL; import java.net.URLConnection; import libcore.io.IoUtils; +import libcore.net.http.HttpEngine; /** * Thread which executes a given {@link DownloadInfo}: making network requests, @@ -59,6 +72,7 @@ import libcore.io.IoUtils; public class DownloadThread extends Thread { private static final int HTTP_REQUESTED_RANGE_NOT_SATISFIABLE = 416; + private static final int HTTP_TEMP_REDIRECT = 307; private static final int DEFAULT_TIMEOUT = (int) MINUTE_IN_MILLIS; @@ -116,6 +130,9 @@ public class DownloadThread extends Thread { public String mContentDisposition; public String mContentLocation; + public int mRedirectionCount; + public URL mUrl; + public State(DownloadInfo info) { mMimeType = Intent.normalizeMimeType(info.mMimeType); mRequestUri = info.mUri; @@ -129,6 +146,7 @@ public class DownloadThread extends Thread { mContentLength = -1; mContentDisposition = null; mContentLocation = null; + mRedirectionCount = 0; } } @@ -169,31 +187,22 @@ public class DownloadThread extends Thread { // while performing download, register for rules updates netPolicy.registerListener(mPolicyListener); - if (Constants.LOGV) { - Log.v(Constants.TAG, "initiating download for " + mInfo.mUri); - } + Log.i(Constants.TAG, "Initiating download " + mInfo.mId); - // network traffic on this thread should be counted against the - // requesting uid, and is tagged with well-known value. + // Network traffic on this thread should be counted against the + // requesting UID, and is tagged with well-known value. TrafficStats.setThreadStatsTag(TrafficStats.TAG_SYSTEM_DOWNLOAD); TrafficStats.setThreadStatsUid(mInfo.mUid); - boolean finished = false; - while (!finished) { - Log.i(Constants.TAG, "Initiating request for download " + mInfo.mId); - - final URL url = new URL(state.mRequestUri); - final HttpURLConnection conn = (HttpURLConnection) url.openConnection(); - conn.setConnectTimeout(DEFAULT_TIMEOUT); - conn.setReadTimeout(DEFAULT_TIMEOUT); - try { - executeDownload(state, conn); - finished = true; - } finally { - conn.disconnect(); - } + try { + // TODO: migrate URL sanity checking into client side of API + state.mUrl = new URL(state.mRequestUri); + } catch (MalformedURLException e) { + throw new StopRequestException(STATUS_BAD_REQUEST, e); } + executeDownload(state); + if (Constants.LOGV) { Log.v(Constants.TAG, "download completed for " + mInfo.mUri); } @@ -207,7 +216,7 @@ public class DownloadThread extends Thread { if (Constants.LOGV) { Log.w(Constants.TAG, msg, error); } - finalStatus = error.mFinalStatus; + finalStatus = error.getFinalStatus(); // fall through to finally block } catch (Throwable ex) { errorMsg = ex.getMessage(); @@ -234,14 +243,12 @@ public class DownloadThread extends Thread { } /** - * Fully execute a single download request - setup and send the request, handle the response, - * and transfer the data to the destination file. + * Fully execute a single download request. Setup and send the request, + * handle the response, and transfer the data to the destination file. */ - private void executeDownload(State state, HttpURLConnection conn) throws StopRequestException { + private void executeDownload(State state) throws StopRequestException { state.resetBeforeExecute(); - setupDestinationFile(state); - addRequestHeaders(state, conn); // skip when already finished; remove after fixing race in 5217390 if (state.mCurrentBytes == state.mTotalBytes) { @@ -250,23 +257,96 @@ public class DownloadThread extends Thread { return; } - // check just before sending the request to avoid using an invalid connection at all - checkConnectivity(); + // TODO: compare mInfo.mNumFailed against Constants.MAX_RETRIES + while (state.mRedirectionCount++ < HttpEngine.MAX_REDIRECTS) { + // Open connection and follow any redirects until we have a useful + // response with body. + HttpURLConnection conn = null; + try { + checkConnectivity(); + conn = (HttpURLConnection) state.mUrl.openConnection(); + conn.setInstanceFollowRedirects(false); + conn.setConnectTimeout(DEFAULT_TIMEOUT); + conn.setReadTimeout(DEFAULT_TIMEOUT); + + addRequestHeaders(state, conn); + + final int responseCode = conn.getResponseCode(); + switch (responseCode) { + case HTTP_OK: + if (state.mContinuingDownload) { + throw new StopRequestException( + STATUS_CANNOT_RESUME, "Expected partial, but received OK"); + } + processResponseHeaders(state, conn); + transferData(state, conn); + return; + + case HTTP_PARTIAL: + if (!state.mContinuingDownload) { + throw new StopRequestException( + STATUS_CANNOT_RESUME, "Expected OK, but received partial"); + } + transferData(state, conn); + return; + + case HTTP_MOVED_PERM: + case HTTP_MOVED_TEMP: + case HTTP_SEE_OTHER: + case HTTP_TEMP_REDIRECT: + final String location = conn.getHeaderField("Location"); + state.mUrl = new URL(state.mUrl, location); + 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); + + case HTTP_UNAVAILABLE: + parseRetryAfterHeaders(state, conn); + if (mInfo.mNumFailed < Constants.MAX_RETRIES) { + throw new StopRequestException(STATUS_WAITING_TO_RETRY, "Unavailable"); + } else { + throw new StopRequestException(STATUS_CANNOT_RESUME, "Unavailable"); + } + + case HTTP_INTERNAL_ERROR: + throw new StopRequestException(STATUS_WAITING_TO_RETRY, "Internal error"); + + default: + StopRequestException.throwUnhandledHttpError(responseCode); + } + } catch (IOException e) { + // Trouble with low-level sockets + throw new StopRequestException(STATUS_WAITING_TO_RETRY, e); + + } finally { + if (conn != null) conn.disconnect(); + } + } + + throw new StopRequestException(STATUS_TOO_MANY_REDIRECTS, "Too many redirects"); + } + + /** + * Transfer data from the given connection to the destination file. + */ + private void transferData(State state, HttpURLConnection conn) throws StopRequestException { DrmManagerClient drmClient = null; InputStream in = null; OutputStream out = null; FileDescriptor outFd = null; try { try { - // Asking for response code will execute the request - final int statusCode = conn.getResponseCode(); - handleExceptionalStatus(state, conn, statusCode); - processResponseHeaders(state, conn); in = conn.getInputStream(); } catch (IOException e) { - throw new StopRequestException( - getFinalStatusForHttpError(state), "Request failed: " + e, e); + throw new StopRequestException(STATUS_HTTP_DATA_ERROR, e); } try { @@ -281,10 +361,11 @@ public class DownloadThread extends Thread { outFd = ((FileOutputStream) out).getFD(); } } catch (IOException e) { - throw new StopRequestException( - Downloads.Impl.STATUS_FILE_ERROR, "Failed to open destination: " + e, e); + throw new StopRequestException(STATUS_FILE_ERROR, e); } + // Start streaming data, periodically watch for pause/cancel + // commands and checking disk space as needed. transferData(state, in, out); try { @@ -292,8 +373,7 @@ public class DownloadThread extends Thread { ((DrmOutputStream) out).finish(); } } catch (IOException e) { - throw new StopRequestException( - Downloads.Impl.STATUS_FILE_ERROR, "Failed to finish: " + e, e); + throw new StopRequestException(STATUS_FILE_ERROR, e); } } finally { @@ -530,15 +610,12 @@ public class DownloadThread extends Thread { } /** - * Read HTTP response headers and take appropriate action, including setting up the destination - * file and updating the database. + * Prepare target file based on given network response. Derives filename and + * target size as needed. */ private void processResponseHeaders(State state, HttpURLConnection conn) throws StopRequestException { - if (state.mContinuingDownload) { - // ignore response headers on resume requests - return; - } + // TODO: fallocate the entire file if header gave us specific length readResponseHeaders(state, conn); @@ -608,53 +685,7 @@ public class DownloadThread extends Thread { } } - /** - * Check the HTTP response status and handle anything unusual (e.g. not 200/206). - */ - private void handleExceptionalStatus(State state, HttpURLConnection conn, int statusCode) - throws StopRequestException { - if (statusCode == HTTP_UNAVAILABLE && mInfo.mNumFailed < Constants.MAX_RETRIES) { - handleServiceUnavailable(state, conn); - } - - if (Constants.LOGV) { - Log.i(Constants.TAG, "recevd_status = " + statusCode + - ", mContinuingDownload = " + state.mContinuingDownload); - } - int expectedStatus = state.mContinuingDownload ? HTTP_PARTIAL : HTTP_OK; - if (statusCode != expectedStatus) { - handleOtherStatus(state, statusCode); - } - } - - /** - * Handle a status that we don't know how to deal with properly. - */ - private void handleOtherStatus(State state, int statusCode) throws StopRequestException { - if (statusCode == HTTP_REQUESTED_RANGE_NOT_SATISFIABLE) { - // range request failed. it should never fail. - throw new IllegalStateException("Http Range request failure: totalBytes = " + - state.mTotalBytes + ", bytes recvd so far: " + state.mCurrentBytes); - } - int finalStatus; - if (statusCode >= 400 && statusCode < 600) { - finalStatus = statusCode; - } else if (statusCode >= 300 && statusCode < 400) { - finalStatus = Downloads.Impl.STATUS_UNHANDLED_REDIRECT; - } else if (state.mContinuingDownload && statusCode == HTTP_OK) { - finalStatus = Downloads.Impl.STATUS_CANNOT_RESUME; - } else { - finalStatus = Downloads.Impl.STATUS_UNHANDLED_HTTP_CODE; - } - throw new StopRequestException(finalStatus, "http error " + - statusCode + ", mContinuingDownload: " + state.mContinuingDownload); - } - - /** - * Handle a 503 Service Unavailable status by processing the Retry-After header. - */ - private void handleServiceUnavailable(State state, HttpURLConnection conn) - throws StopRequestException { + private void parseRetryAfterHeaders(State state, HttpURLConnection conn) { state.mCountRetry = true; state.mRetryAfter = conn.getHeaderFieldInt("Retry-After", -1); if (state.mRetryAfter < 0) { @@ -668,9 +699,6 @@ public class DownloadThread extends Thread { state.mRetryAfter += Helpers.sRandom.nextInt(Constants.MIN_RETRY_AFTER + 1); state.mRetryAfter *= 1000; } - - throw new StopRequestException(Downloads.Impl.STATUS_WAITING_TO_RETRY, - "got 503 Service Unavailable, will retry later"); } private int getFinalStatusForHttpError(State state) { @@ -774,11 +802,6 @@ public class DownloadThread extends Thread { conn.addRequestProperty("If-Match", state.mHeaderETag); } conn.addRequestProperty("Range", "bytes=" + state.mCurrentBytes + "-"); - if (Constants.LOGV) { - Log.i(Constants.TAG, "Adding Range header: " + - "bytes=" + state.mCurrentBytes + "-"); - Log.i(Constants.TAG, " totalBytes = " + state.mTotalBytes); - } } } diff --git a/src/com/android/providers/downloads/Helpers.java b/src/com/android/providers/downloads/Helpers.java index 225b8d49..059e9703 100644 --- a/src/com/android/providers/downloads/Helpers.java +++ b/src/com/android/providers/downloads/Helpers.java @@ -284,6 +284,9 @@ public class Helpers { private static String chooseUniqueFilename(int destination, String filename, String extension, boolean recoveryDir) throws StopRequestException { + // TODO: switch to actually creating the file here, otherwise we expose + // ourselves to race conditions. + String fullFilename = filename + extension; if (!new File(fullFilename).exists() && (!recoveryDir || diff --git a/src/com/android/providers/downloads/StopRequestException.java b/src/com/android/providers/downloads/StopRequestException.java index 0ccf53cb..6df61dce 100644 --- a/src/com/android/providers/downloads/StopRequestException.java +++ b/src/com/android/providers/downloads/StopRequestException.java @@ -15,6 +15,9 @@ */ package com.android.providers.downloads; +import static android.provider.Downloads.Impl.STATUS_UNHANDLED_HTTP_CODE; +import static android.provider.Downloads.Impl.STATUS_UNHANDLED_REDIRECT; + /** * Raised to indicate that the current request should be stopped immediately. * @@ -23,15 +26,35 @@ package com.android.providers.downloads; * URI, headers, or destination filename. */ class StopRequestException extends Exception { - public int mFinalStatus; + private final int mFinalStatus; public StopRequestException(int finalStatus, String message) { super(message); mFinalStatus = finalStatus; } - public StopRequestException(int finalStatus, String message, Throwable throwable) { - super(message, throwable); + public StopRequestException(int finalStatus, Throwable t) { + super(t); + mFinalStatus = finalStatus; + } + + public StopRequestException(int finalStatus, String message, Throwable t) { + super(message, t); mFinalStatus = finalStatus; } + + public int getFinalStatus() { + return mFinalStatus; + } + + public static StopRequestException throwUnhandledHttpError(int responseCode) + throws StopRequestException { + if (responseCode >= 400 && responseCode < 600) { + throw new StopRequestException(responseCode, "Unhandled HTTP response"); + } else if (responseCode >= 300 && responseCode < 400) { + throw new StopRequestException(STATUS_UNHANDLED_REDIRECT, "Unhandled HTTP response"); + } else { + throw new StopRequestException(STATUS_UNHANDLED_HTTP_CODE, "Unhandled HTTP response"); + } + } } diff --git a/tests/src/com/android/providers/downloads/AbstractPublicApiTest.java b/tests/src/com/android/providers/downloads/AbstractPublicApiTest.java index 12c04f6a..e7f08c90 100644 --- a/tests/src/com/android/providers/downloads/AbstractPublicApiTest.java +++ b/tests/src/com/android/providers/downloads/AbstractPublicApiTest.java @@ -100,21 +100,24 @@ public abstract class AbstractPublicApiTest extends AbstractDownloadProviderFunc } void runUntilStatus(int status) throws TimeoutException { + final long startMillis = mSystemFacade.currentTimeMillis(); startService(null); - waitForStatus(status); + waitForStatus(status, startMillis); } - void waitForStatus(int expected) throws TimeoutException { + void waitForStatus(int expected, long afterMillis) throws TimeoutException { int actual = -1; final long timeout = SystemClock.elapsedRealtime() + (15 * SECOND_IN_MILLIS); while (SystemClock.elapsedRealtime() < timeout) { - actual = getStatus(); - if (actual == STATUS_SUCCESSFUL || actual == STATUS_FAILED) { - assertEquals(expected, actual); - return; - } else if (actual == expected) { - return; + if (getLongField(DownloadManager.COLUMN_LAST_MODIFIED_TIMESTAMP) >= afterMillis) { + actual = getStatus(); + if (actual == STATUS_SUCCESSFUL || actual == STATUS_FAILED) { + assertEquals(expected, actual); + return; + } else if (actual == expected) { + return; + } } SystemClock.sleep(100); diff --git a/tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java b/tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java index 4159beb6..157cce89 100644 --- a/tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java +++ b/tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java @@ -421,7 +421,7 @@ public class PublicApiFunctionalTest extends AbstractPublicApiTest { mManager.remove(download.mId); // make sure the row is gone from the database - download.waitForStatus(-1); + download.waitForStatus(-1, -1); } public void testDownloadCompleteBroadcast() throws Exception { |