Log CT log list download failures
This CL adds a DownloadStatus class inside the DownloadHelper. The class
acts as a thin wrapper around the status and reason Ids returned by the
DownloadManager, with some utility methods to help with the
interpretation of the download status.
Bug: 319829948
Test: atest NetworkSecurityUnitTests
Change-Id: I111388ee37c09e30c02bb9bc9b3d65273961b91d
diff --git a/networksecurity/service/Android.bp b/networksecurity/service/Android.bp
index 52667ae..a41e6a0 100644
--- a/networksecurity/service/Android.bp
+++ b/networksecurity/service/Android.bp
@@ -32,6 +32,15 @@
"service-connectivity-pre-jarjar",
],
+ static_libs: [
+ "auto_value_annotations",
+ ],
+
+ plugins: [
+ "auto_value_plugin",
+ "auto_annotation_plugin",
+ ],
+
// This is included in service-connectivity which is 30+
// TODO (b/293613362): allow APEXes to have service jars with higher min_sdk than the APEX
// (service-connectivity is only used on 31+) and use 31 here
diff --git a/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyDownloader.java b/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyDownloader.java
index 16f32c4..f86d127 100644
--- a/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyDownloader.java
+++ b/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyDownloader.java
@@ -29,6 +29,8 @@
import androidx.annotation.VisibleForTesting;
+import com.android.server.net.ct.DownloadHelper.DownloadStatus;
+
import java.io.IOException;
import java.io.InputStream;
import java.security.GeneralSecurityException;
@@ -147,19 +149,18 @@
}
private void handleMetadataDownloadCompleted(long downloadId) {
- if (!mDownloadHelper.isSuccessful(downloadId)) {
- Log.w(TAG, "Metadata download failed.");
- // TODO: re-attempt download
+ DownloadStatus status = mDownloadHelper.getDownloadStatus(downloadId);
+ if (!status.isSuccessful()) {
+ handleDownloadFailed(status);
return;
}
-
startContentDownload(mDataStore.getProperty(Config.CONTENT_URL_PENDING));
}
private void handleContentDownloadCompleted(long downloadId) {
- if (!mDownloadHelper.isSuccessful(downloadId)) {
- Log.w(TAG, "Content download failed.");
- // TODO: re-attempt download
+ DownloadStatus status = mDownloadHelper.getDownloadStatus(downloadId);
+ if (!status.isSuccessful()) {
+ handleDownloadFailed(status);
return;
}
@@ -202,6 +203,11 @@
}
}
+ private void handleDownloadFailed(DownloadStatus status) {
+ Log.e(TAG, "Content download failed with " + status);
+ // TODO(378626065): Report failure via statsd.
+ }
+
private boolean verify(Uri file, Uri signature) throws IOException, GeneralSecurityException {
if (!mPublicKey.isPresent()) {
throw new InvalidKeyException("Missing public key for signature verification");
diff --git a/networksecurity/service/src/com/android/server/net/ct/DownloadHelper.java b/networksecurity/service/src/com/android/server/net/ct/DownloadHelper.java
index cc8c4c0..5748416 100644
--- a/networksecurity/service/src/com/android/server/net/ct/DownloadHelper.java
+++ b/networksecurity/service/src/com/android/server/net/ct/DownloadHelper.java
@@ -24,6 +24,8 @@
import androidx.annotation.VisibleForTesting;
+import com.google.auto.value.AutoValue;
+
/** Class to handle downloads for Certificate Transparency. */
public class DownloadHelper {
@@ -53,25 +55,22 @@
}
/**
- * Returns true if the specified download completed successfully.
+ * Returns the status of the provided download id.
*
* @param downloadId the download.
- * @return true if the download completed successfully.
+ * @return {@link DownloadStatus} of the download.
*/
- public boolean isSuccessful(long downloadId) {
+ public DownloadStatus getDownloadStatus(long downloadId) {
+ DownloadStatus.Builder builder = DownloadStatus.builder().setDownloadId(downloadId);
try (Cursor cursor = mDownloadManager.query(new Query().setFilterById(downloadId))) {
- if (cursor == null) {
- return false;
- }
- if (cursor.moveToFirst()) {
- int status =
- cursor.getInt(cursor.getColumnIndexOrThrow(DownloadManager.COLUMN_STATUS));
- if (DownloadManager.STATUS_SUCCESSFUL == status) {
- return true;
- }
+ if (cursor != null && cursor.moveToFirst()) {
+ builder.setStatus(
+ cursor.getInt(cursor.getColumnIndexOrThrow(DownloadManager.COLUMN_STATUS)));
+ builder.setReason(
+ cursor.getInt(cursor.getColumnIndex(DownloadManager.COLUMN_REASON)));
}
}
- return false;
+ return builder.build();
}
/**
@@ -87,4 +86,58 @@
}
return mDownloadManager.getUriForDownloadedFile(downloadId);
}
+
+ /** A wrapper around the status and reason Ids returned by the {@link DownloadManager}. */
+ @AutoValue
+ public abstract static class DownloadStatus {
+
+ abstract long downloadId();
+
+ abstract int status();
+
+ abstract int reason();
+
+ boolean isSuccessful() {
+ return status() == DownloadManager.STATUS_SUCCESSFUL;
+ }
+
+ boolean isStorageError() {
+ int status = status();
+ int reason = reason();
+ return status == DownloadManager.STATUS_FAILED
+ && (reason == DownloadManager.ERROR_DEVICE_NOT_FOUND
+ || reason == DownloadManager.ERROR_FILE_ERROR
+ || reason == DownloadManager.ERROR_FILE_ALREADY_EXISTS
+ || reason == DownloadManager.ERROR_INSUFFICIENT_SPACE);
+ }
+
+ boolean isHttpError() {
+ int status = status();
+ int reason = reason();
+ return status == DownloadManager.STATUS_FAILED
+ && (reason == DownloadManager.ERROR_HTTP_DATA_ERROR
+ || reason == DownloadManager.ERROR_TOO_MANY_REDIRECTS
+ || reason == DownloadManager.ERROR_UNHANDLED_HTTP_CODE
+ // If an HTTP error occurred, reason will hold the HTTP status code.
+ || (400 <= reason && reason < 600));
+ }
+
+ @AutoValue.Builder
+ abstract static class Builder {
+ abstract Builder setDownloadId(long downloadId);
+
+ abstract Builder setStatus(int status);
+
+ abstract Builder setReason(int reason);
+
+ abstract DownloadStatus build();
+ }
+
+ static Builder builder() {
+ return new AutoValue_DownloadHelper_DownloadStatus.Builder()
+ .setDownloadId(-1)
+ .setStatus(-1)
+ .setReason(-1);
+ }
+ }
}