update_engine: Client limits consecutive updates
For details refer to bug.
BUG=b:201737820
TEST=FEATURES=test emerge-$B update_engine
Change-Id: I8e1816c99539170753f5efaa36b084e24475a06c
Reviewed-on: https://chromium-review.googlesource.com/c/aosp/platform/system/update_engine/+/3335760
Tested-by: Jae Hoon Kim <[email protected]>
Auto-Submit: Jae Hoon Kim <[email protected]>
Commit-Queue: Jae Hoon Kim <[email protected]>
Reviewed-by: Henry Barnor <[email protected]>
Commit-Queue: Henry Barnor <[email protected]>
(cherry picked from commit b43659d5c5dcde58fa1b8b1b696dee8dc6b01f10)
Reviewed-on: https://chromium-review.googlesource.com/c/aosp/platform/system/update_engine/+/3337981
diff --git a/common/constants.h b/common/constants.h
index e655385..fb1a1f1 100644
--- a/common/constants.h
+++ b/common/constants.h
@@ -17,6 +17,8 @@
#ifndef UPDATE_ENGINE_COMMON_CONSTANTS_H_
#define UPDATE_ENGINE_COMMON_CONSTANTS_H_
+#include <stdint.h>
+
namespace chromeos_update_engine {
// The root path of all exclusion prefs.
@@ -226,6 +228,9 @@
// Size in bytes of SHA256 hash.
const int kSHA256Size = 32;
+// Limit on the number of consecutive updates.
+const int64_t kConsecutiveUpdateLimit = 3;
+
} // namespace chromeos_update_engine
#endif // UPDATE_ENGINE_COMMON_CONSTANTS_H_
diff --git a/cros/mock_update_attempter.h b/cros/mock_update_attempter.h
index 8eb0c1f..2146194 100644
--- a/cros/mock_update_attempter.h
+++ b/cros/mock_update_attempter.h
@@ -63,6 +63,8 @@
MOCK_CONST_METHOD0(consecutive_failed_update_checks, unsigned int(void));
MOCK_CONST_METHOD0(server_dictated_poll_interval, unsigned int(void));
+
+ MOCK_METHOD0(IsRepeatedUpdatesEnabled, bool(void));
};
} // namespace chromeos_update_engine
diff --git a/cros/omaha_request_action_unittest.cc b/cros/omaha_request_action_unittest.cc
index 3f7105d..8512cdd 100644
--- a/cros/omaha_request_action_unittest.cc
+++ b/cros/omaha_request_action_unittest.cc
@@ -446,6 +446,9 @@
.expected_download_error_code = metrics::DownloadErrorCode::kUnset,
};
+ ON_CALL(*FakeSystemState::Get()->mock_update_attempter(),
+ IsRepeatedUpdatesEnabled())
+ .WillByDefault(Return(true));
ON_CALL(*FakeSystemState::Get()->mock_update_attempter(), GetExcluder())
.WillByDefault(Return(&mock_excluder_));
}
diff --git a/cros/omaha_request_builder_xml_unittest.cc b/cros/omaha_request_builder_xml_unittest.cc
index 7e6b273..ae79abd 100644
--- a/cros/omaha_request_builder_xml_unittest.cc
+++ b/cros/omaha_request_builder_xml_unittest.cc
@@ -66,6 +66,10 @@
FakeSystemState::CreateInstance();
params_.set_hw_details(false);
FakeSystemState::Get()->set_request_params(¶ms_);
+
+ ON_CALL(*FakeSystemState::Get()->mock_update_attempter(),
+ IsRepeatedUpdatesEnabled())
+ .WillByDefault(Return(true));
}
void TearDown() override {}
diff --git a/cros/update_attempter.cc b/cros/update_attempter.cc
index 3f227aa..35c1315 100644
--- a/cros/update_attempter.cc
+++ b/cros/update_attempter.cc
@@ -2013,12 +2013,25 @@
}
bool UpdateAttempter::IsRepeatedUpdatesEnabled() {
+ auto* prefs = SystemState::Get()->prefs();
+
+ // Limit the number of repeated updates allowed as a safeguard on client.
+ // Whether consecutive update feature is allowed or not.
+ // Refer to b/201737820.
+ int64_t consecutive_updates = 0;
+ prefs->GetInt64(kPrefsConsecutiveUpdateCount, &consecutive_updates);
+ if (consecutive_updates >= kConsecutiveUpdateLimit) {
+ LOG(WARNING) << "Not allowing repeated updates as limit reached.";
+ return false;
+ }
+
bool allow_repeated_updates;
if (!SystemState::Get()->prefs()->GetBoolean(kPrefsAllowRepeatedUpdates,
&allow_repeated_updates)) {
// Defaulting to true.
- allow_repeated_updates = true;
+ return true;
}
+
return allow_repeated_updates;
}
diff --git a/cros/update_attempter.h b/cros/update_attempter.h
index eedfda7..ab9b9db 100644
--- a/cros/update_attempter.h
+++ b/cros/update_attempter.h
@@ -245,7 +245,7 @@
// Returns whether repeated updates are enabled. Defaults to true if the pref
// is unset or unable to be read.
- bool IsRepeatedUpdatesEnabled();
+ virtual bool IsRepeatedUpdatesEnabled();
// |DaemonStateInterface| overrides.
bool StartUpdater() override;
@@ -327,6 +327,7 @@
FRIEND_TEST(UpdateAttempterTest, FirstUpdateBeforeReboot);
FRIEND_TEST(UpdateAttempterTest, InvalidateLastUpdate);
FRIEND_TEST(UpdateAttempterTest, ConsecutiveUpdateBeforeRebootSuccess);
+ FRIEND_TEST(UpdateAttempterTest, ConsecutiveUpdateBeforeRebootLimited);
FRIEND_TEST(UpdateAttempterTest, ConsecutiveUpdateFailureMetric);
FRIEND_TEST(UpdateAttempterTest, ResetUpdatePrefs);
FRIEND_TEST(UpdateAttempterTest, ProcessingDoneSkipApplying);
diff --git a/cros/update_attempter_unittest.cc b/cros/update_attempter_unittest.cc
index bc0ac88..cf7499c 100644
--- a/cros/update_attempter_unittest.cc
+++ b/cros/update_attempter_unittest.cc
@@ -266,6 +266,11 @@
.WillRepeatedly(ReturnPointee(&actual_using_p2p_for_sharing_));
}
+ void TearDown() override {
+ prefs_->Delete(kPrefsAllowRepeatedUpdates);
+ prefs_->Delete(kPrefsConsecutiveUpdateCount);
+ }
+
public:
void ScheduleQuitMainLoop();
@@ -2464,15 +2469,39 @@
// Consecutive update metric should be updated.
int64_t num_updates;
- FakeSystemState::Get()->prefs()->GetInt64(kPrefsConsecutiveUpdateCount,
- &num_updates);
+ prefs_->GetInt64(kPrefsConsecutiveUpdateCount, &num_updates);
EXPECT_EQ(num_updates, 1);
+
+ // Validate that consecutive count gets incremented on updates.
+ TestProcessingDone();
+ prefs_->GetInt64(kPrefsConsecutiveUpdateCount, &num_updates);
+ EXPECT_EQ(num_updates, 2);
+}
+
+TEST_F(UpdateAttempterTest, ConsecutiveUpdateBeforeRebootLimited) {
+ // Default should be true.
+ EXPECT_TRUE(attempter_.IsRepeatedUpdatesEnabled());
+
+ // Disabled feature.
+ EXPECT_TRUE(prefs_->SetBoolean(kPrefsAllowRepeatedUpdates, false));
+ EXPECT_FALSE(attempter_.IsRepeatedUpdatesEnabled());
+
+ EXPECT_TRUE(prefs_->SetBoolean(kPrefsAllowRepeatedUpdates, true));
+ EXPECT_TRUE(attempter_.IsRepeatedUpdatesEnabled());
+
+ EXPECT_TRUE(prefs_->SetInt64(kPrefsConsecutiveUpdateCount,
+ kConsecutiveUpdateLimit - 1));
+ EXPECT_TRUE(attempter_.IsRepeatedUpdatesEnabled());
+
+ EXPECT_TRUE(
+ prefs_->SetInt64(kPrefsConsecutiveUpdateCount, kConsecutiveUpdateLimit));
+ EXPECT_FALSE(attempter_.IsRepeatedUpdatesEnabled());
}
TEST_F(UpdateAttempterTest, ConsecutiveUpdateFailureMetric) {
MockAction action;
// Two previous consecutive updates.
- FakeSystemState::Get()->prefs()->SetInt64(kPrefsConsecutiveUpdateCount, 2);
+ prefs_->SetInt64(kPrefsConsecutiveUpdateCount, 2);
// Fail an update.
EXPECT_CALL(action, Type()).WillRepeatedly(Return("MockAction"));
