From 8d0c2c0b5f8c93c1c08d7f010f06fca67a86a080 Mon Sep 17 00:00:00 2001 From: Bessie Jiang Date: Wed, 18 Dec 2024 18:20:21 +0000 Subject: [PATCH] Add new APIs to DataStore to allow for int storage The failure count for CTLogListUpdateFailed is an integer, not a long. We need APIs in DataStore to handle this different data type, as otherwise it will result in an unnecessary cast from long <-> int, which is a lossy conversion. Bug: 384473786 Test: atest NetworkSecurityUnitTests Change-Id: I3b31fc9faef4ac0dec77bc5adb8235a155a4a3d1 --- .../ct/CertificateTransparencyDownloader.java | 10 ++-- .../src/com/android/server/net/ct/Config.java | 2 +- .../com/android/server/net/ct/DataStore.java | 8 ++++ ...CertificateTransparencyDownloaderTest.java | 48 +++++++++---------- 4 files changed, 38 insertions(+), 30 deletions(-) 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 ef0d0eae27..d16a7608ef 100644 --- a/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyDownloader.java +++ b/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyDownloader.java @@ -221,7 +221,7 @@ class CertificateTransparencyDownloader extends BroadcastReceiver { mDataStore.setProperty(Config.VERSION, version); // Reset the number of consecutive log list failure updates back to zero. - mDataStore.setPropertyLong(Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* value= */ 0L); + mDataStore.setPropertyInt(Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* value= */ 0); mDataStore.store(); } else { if (updateFailureCount()) { @@ -244,11 +244,11 @@ class CertificateTransparencyDownloader extends BroadcastReceiver { * @return whether the failure count exceeds the threshold and should be logged. */ private boolean updateFailureCount() { - long failure_count = mDataStore.getPropertyLong( - Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0L); - long new_failure_count = failure_count + 1L; + int failure_count = mDataStore.getPropertyInt( + Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0); + int new_failure_count = failure_count + 1; - mDataStore.setPropertyLong(Config.LOG_LIST_UPDATE_FAILURE_COUNT, new_failure_count); + mDataStore.setPropertyInt(Config.LOG_LIST_UPDATE_FAILURE_COUNT, new_failure_count); mDataStore.store(); boolean shouldReport = new_failure_count >= Config.LOG_LIST_UPDATE_FAILURE_THRESHOLD; diff --git a/networksecurity/service/src/com/android/server/net/ct/Config.java b/networksecurity/service/src/com/android/server/net/ct/Config.java index edd6a805d0..aafee60f91 100644 --- a/networksecurity/service/src/com/android/server/net/ct/Config.java +++ b/networksecurity/service/src/com/android/server/net/ct/Config.java @@ -63,5 +63,5 @@ final class Config { static final String URL_PUBLIC_KEY = URL_PREFIX + "log_list.pub"; // Threshold amounts - static final long LOG_LIST_UPDATE_FAILURE_THRESHOLD = 10L; + static final int LOG_LIST_UPDATE_FAILURE_THRESHOLD = 10; } diff --git a/networksecurity/service/src/com/android/server/net/ct/DataStore.java b/networksecurity/service/src/com/android/server/net/ct/DataStore.java index cd6aebf6a6..377926922d 100644 --- a/networksecurity/service/src/com/android/server/net/ct/DataStore.java +++ b/networksecurity/service/src/com/android/server/net/ct/DataStore.java @@ -64,4 +64,12 @@ class DataStore extends Properties { Object setPropertyLong(String key, long value) { return setProperty(key, Long.toString(value)); } + + int getPropertyInt(String key, int defaultValue) { + return Optional.ofNullable(getProperty(key)).map(Integer::parseInt).orElse(defaultValue); + } + + Object setPropertyInt(String key, int value) { + return setProperty(key, Integer.toString(value)); + } } diff --git a/networksecurity/tests/unit/src/com/android/server/net/ct/CertificateTransparencyDownloaderTest.java b/networksecurity/tests/unit/src/com/android/server/net/ct/CertificateTransparencyDownloaderTest.java index 5c4a4e5416..4748043598 100644 --- a/networksecurity/tests/unit/src/com/android/server/net/ct/CertificateTransparencyDownloaderTest.java +++ b/networksecurity/tests/unit/src/com/android/server/net/ct/CertificateTransparencyDownloaderTest.java @@ -190,7 +190,7 @@ public class CertificateTransparencyDownloaderTest { throws Exception { long publicKeyId = mCertificateTransparencyDownloader.startPublicKeyDownload(); // Set the failure count to just below the threshold - mDataStore.setPropertyLong(Config.LOG_LIST_UPDATE_FAILURE_COUNT, + mDataStore.setPropertyInt(Config.LOG_LIST_UPDATE_FAILURE_COUNT, Config.LOG_LIST_UPDATE_FAILURE_THRESHOLD - 1); setFailedDownload( publicKeyId, // Failure cases where we give up on the download. @@ -200,8 +200,8 @@ public class CertificateTransparencyDownloaderTest { mCertificateTransparencyDownloader.onReceive(mContext, downloadCompleteIntent); - assertThat(mDataStore.getPropertyLong( - Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0L)) + assertThat(mDataStore.getPropertyInt( + Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0)) .isEqualTo(Config.LOG_LIST_UPDATE_FAILURE_THRESHOLD); // TODO(378626065): Verify logged failure via statsd. } @@ -211,7 +211,7 @@ public class CertificateTransparencyDownloaderTest { throws Exception { long publicKeyId = mCertificateTransparencyDownloader.startPublicKeyDownload(); // Set the failure count to just below the threshold - mDataStore.setPropertyLong(Config.LOG_LIST_UPDATE_FAILURE_COUNT, 0); + mDataStore.setPropertyInt(Config.LOG_LIST_UPDATE_FAILURE_COUNT, 0); setFailedDownload( publicKeyId, // Failure cases where we give up on the download. DownloadManager.ERROR_INSUFFICIENT_SPACE, @@ -220,8 +220,8 @@ public class CertificateTransparencyDownloaderTest { mCertificateTransparencyDownloader.onReceive(mContext, downloadCompleteIntent); - assertThat(mDataStore.getPropertyLong( - Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0L)) + assertThat(mDataStore.getPropertyInt( + Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0)) .isEqualTo(1); // TODO(378626065): Verify no failure logged via statsd. } @@ -260,7 +260,7 @@ public class CertificateTransparencyDownloaderTest { throws Exception { long metadataId = mCertificateTransparencyDownloader.startMetadataDownload(); // Set the failure count to just below the threshold - mDataStore.setPropertyLong(Config.LOG_LIST_UPDATE_FAILURE_COUNT, + mDataStore.setPropertyInt(Config.LOG_LIST_UPDATE_FAILURE_COUNT, Config.LOG_LIST_UPDATE_FAILURE_THRESHOLD - 1); setFailedDownload( metadataId, @@ -271,8 +271,8 @@ public class CertificateTransparencyDownloaderTest { mCertificateTransparencyDownloader.onReceive(mContext, downloadCompleteIntent); - assertThat(mDataStore.getPropertyLong( - Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0L)) + assertThat(mDataStore.getPropertyInt( + Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0)) .isEqualTo(Config.LOG_LIST_UPDATE_FAILURE_THRESHOLD); // TODO(378626065): Verify logged failure via statsd. } @@ -282,7 +282,7 @@ public class CertificateTransparencyDownloaderTest { throws Exception { long metadataId = mCertificateTransparencyDownloader.startMetadataDownload(); // Set the failure count to just below the threshold - mDataStore.setPropertyLong(Config.LOG_LIST_UPDATE_FAILURE_COUNT, 0); + mDataStore.setPropertyInt(Config.LOG_LIST_UPDATE_FAILURE_COUNT, 0); setFailedDownload( metadataId, // Failure cases where we give up on the download. @@ -292,8 +292,8 @@ public class CertificateTransparencyDownloaderTest { mCertificateTransparencyDownloader.onReceive(mContext, downloadCompleteIntent); - assertThat(mDataStore.getPropertyLong( - Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0L)) + assertThat(mDataStore.getPropertyInt( + Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0)) .isEqualTo(1); // TODO(378626065): Verify no failure logged via statsd. } @@ -342,7 +342,7 @@ public class CertificateTransparencyDownloaderTest { throws Exception { long contentId = mCertificateTransparencyDownloader.startContentDownload(); // Set the failure count to just below the threshold - mDataStore.setPropertyLong(Config.LOG_LIST_UPDATE_FAILURE_COUNT, + mDataStore.setPropertyInt(Config.LOG_LIST_UPDATE_FAILURE_COUNT, Config.LOG_LIST_UPDATE_FAILURE_THRESHOLD - 1); setFailedDownload( contentId, @@ -353,8 +353,8 @@ public class CertificateTransparencyDownloaderTest { mCertificateTransparencyDownloader.onReceive(mContext, downloadCompleteIntent); - assertThat(mDataStore.getPropertyLong( - Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0L)) + assertThat(mDataStore.getPropertyInt( + Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0)) .isEqualTo(Config.LOG_LIST_UPDATE_FAILURE_THRESHOLD); // TODO(378626065): Verify logged failure via statsd. } @@ -364,7 +364,7 @@ public class CertificateTransparencyDownloaderTest { throws Exception { long contentId = mCertificateTransparencyDownloader.startContentDownload(); // Set the failure count to just below the threshold - mDataStore.setPropertyLong(Config.LOG_LIST_UPDATE_FAILURE_COUNT, 0); + mDataStore.setPropertyInt(Config.LOG_LIST_UPDATE_FAILURE_COUNT, 0); setFailedDownload( contentId, // Failure cases where we give up on the download. @@ -374,8 +374,8 @@ public class CertificateTransparencyDownloaderTest { mCertificateTransparencyDownloader.onReceive(mContext, downloadCompleteIntent); - assertThat(mDataStore.getPropertyLong( - Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0L)) + assertThat(mDataStore.getPropertyInt( + Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0)) .isEqualTo(1); // TODO(378626065): Verify no failure logged via statsd. } @@ -413,7 +413,7 @@ public class CertificateTransparencyDownloaderTest { long contentId = mCertificateTransparencyDownloader.startContentDownload(); setSuccessfulDownload(contentId, logListFile); // Set the failure count to just below the threshold - mDataStore.setPropertyLong(Config.LOG_LIST_UPDATE_FAILURE_COUNT, + mDataStore.setPropertyInt(Config.LOG_LIST_UPDATE_FAILURE_COUNT, Config.LOG_LIST_UPDATE_FAILURE_THRESHOLD - 1); when(mCertificateTransparencyInstaller.install( eq(Config.COMPATIBILITY_VERSION), any(), anyString())) @@ -422,8 +422,8 @@ public class CertificateTransparencyDownloaderTest { mCertificateTransparencyDownloader.onReceive( mContext, makeDownloadCompleteIntent(contentId)); - assertThat(mDataStore.getPropertyLong( - Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0L)) + assertThat(mDataStore.getPropertyInt( + Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0)) .isEqualTo(Config.LOG_LIST_UPDATE_FAILURE_THRESHOLD); // TODO(378626065): Verify logged failure via statsd. } @@ -440,7 +440,7 @@ public class CertificateTransparencyDownloaderTest { long contentId = mCertificateTransparencyDownloader.startContentDownload(); setSuccessfulDownload(contentId, logListFile); // Set the failure count to just below the threshold - mDataStore.setPropertyLong(Config.LOG_LIST_UPDATE_FAILURE_COUNT, 0); + mDataStore.setPropertyInt(Config.LOG_LIST_UPDATE_FAILURE_COUNT, 0); when(mCertificateTransparencyInstaller.install( eq(Config.COMPATIBILITY_VERSION), any(), anyString())) .thenReturn(false); @@ -448,8 +448,8 @@ public class CertificateTransparencyDownloaderTest { mCertificateTransparencyDownloader.onReceive( mContext, makeDownloadCompleteIntent(contentId)); - assertThat(mDataStore.getPropertyLong( - Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0L)) + assertThat(mDataStore.getPropertyInt( + Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0)) .isEqualTo(1); // TODO(378626065): Verify no failure logged via statsd. }