Fix resuming canceled updates.
Resuming an interrupted update was broken in several ways. First,
DeltaPerformer::CanResumeUpdate was checking for the number of
resume-failures to be more than the limit, not less. Then, the
resume logic didn't work for payload v2 when there was a
metadata signature included in the payload. Finally, resuming an
update after reading the payload signature (in payload v2) was not
updating the checkpoint, but storing the signature causing it to
attempt to parse it again and fail.
Bug: 27047026
Bug: chromium:590410
TEST=Manual procedure:
1. Start an update: update_engine_client --update ...
2. Cancel the update: update_engine_client --cancel
3. Re-start the same update: update_engine_client --update ...
-> The update should resume from the previous point.
Change-Id: I60134de155aa073a7ba91174cceea7297e5f8d17
diff --git a/common/constants.cc b/common/constants.cc
index b15c3f4..6b1d416 100644
--- a/common/constants.cc
+++ b/common/constants.cc
@@ -45,6 +45,7 @@
const char kPrefsLastActivePingDay[] = "last-active-ping-day";
const char kPrefsLastRollCallPingDay[] = "last-roll-call-ping-day";
const char kPrefsManifestMetadataSize[] = "manifest-metadata-size";
+const char kPrefsManifestSignatureSize[] = "manifest-signature-size";
const char kPrefsMetricsAttemptLastReportingTime[] =
"metrics-attempt-last-reporting-time";
const char kPrefsMetricsCheckLastReportingTime[] =
diff --git a/common/constants.h b/common/constants.h
index 62f61ce..f9a43c6 100644
--- a/common/constants.h
+++ b/common/constants.h
@@ -50,6 +50,7 @@
extern const char kPrefsLastActivePingDay[];
extern const char kPrefsLastRollCallPingDay[];
extern const char kPrefsManifestMetadataSize[];
+extern const char kPrefsManifestSignatureSize[];
extern const char kPrefsMetricsAttemptLastReportingTime[];
extern const char kPrefsMetricsCheckLastReportingTime[];
extern const char kPrefsNumReboots[];
diff --git a/omaha_request_action.cc b/omaha_request_action.cc
index 1b0c2fe..bc84c9f 100644
--- a/omaha_request_action.cc
+++ b/omaha_request_action.cc
@@ -1074,18 +1074,23 @@
// safe-guards). See http://crbug.com/297170 for an example)
size_t minimum_size = 0;
int64_t manifest_metadata_size = 0;
+ int64_t manifest_signature_size = 0;
int64_t next_data_offset = 0;
int64_t next_data_length = 0;
if (system_state_ &&
system_state_->prefs()->GetInt64(kPrefsManifestMetadataSize,
&manifest_metadata_size) &&
manifest_metadata_size != -1 &&
+ system_state_->prefs()->GetInt64(kPrefsManifestSignatureSize,
+ &manifest_signature_size) &&
+ manifest_signature_size != -1 &&
system_state_->prefs()->GetInt64(kPrefsUpdateStateNextDataOffset,
&next_data_offset) &&
next_data_offset != -1 &&
system_state_->prefs()->GetInt64(kPrefsUpdateStateNextDataLength,
&next_data_length)) {
- minimum_size = manifest_metadata_size + next_data_offset + next_data_length;
+ minimum_size = manifest_metadata_size + manifest_signature_size +
+ next_data_offset + next_data_length;
}
string file_id = utils::CalculateP2PFileId(response.hash, response.size);
diff --git a/payload_consumer/delta_performer.cc b/payload_consumer/delta_performer.cc
index cb3cdb5..f95679c 100644
--- a/payload_consumer/delta_performer.cc
+++ b/payload_consumer/delta_performer.cc
@@ -578,6 +578,9 @@
LOG_IF(WARNING, !prefs_->SetInt64(kPrefsManifestMetadataSize,
metadata_size_))
<< "Unable to save the manifest metadata size.";
+ LOG_IF(WARNING, !prefs_->SetInt64(kPrefsManifestSignatureSize,
+ metadata_signature_size_))
+ << "Unable to save the manifest signature size.";
if (!PrimeUpdateState()) {
*error = ErrorCode::kDownloadStateInitializationError;
@@ -686,8 +689,10 @@
}
// In major version 2, we don't add dummy operation to the payload.
+ // If we already extracted the signature we should skip this step.
if (major_payload_version_ == kBrilloMajorPayloadVersion &&
- manifest_.has_signatures_offset() && manifest_.has_signatures_size()) {
+ manifest_.has_signatures_offset() && manifest_.has_signatures_size() &&
+ signatures_message_data_.empty()) {
if (manifest_.signatures_offset() != buffer_offset_) {
LOG(ERROR) << "Payload signatures offset points to blob offset "
<< manifest_.signatures_offset()
@@ -706,6 +711,10 @@
return false;
}
DiscardBuffer(true, 0);
+ // Since we extracted the SignatureMessage we need to advance the
+ // checkpoint, otherwise we would reload the signature and try to extract
+ // it again.
+ CheckpointUpdateProgress();
}
return true;
@@ -1666,8 +1675,10 @@
return false;
int64_t resumed_update_failures;
- if (!(prefs->GetInt64(kPrefsResumedUpdateFailures, &resumed_update_failures)
- && resumed_update_failures > kMaxResumedUpdateFailures))
+ // Note that storing this value is optional, but if it is there it should not
+ // be more than the limit.
+ if (prefs->GetInt64(kPrefsResumedUpdateFailures, &resumed_update_failures) &&
+ resumed_update_failures > kMaxResumedUpdateFailures)
return false;
// Sanity check the rest.
@@ -1686,6 +1697,12 @@
manifest_metadata_size > 0))
return false;
+ int64_t manifest_signature_size = 0;
+ if (!(prefs->GetInt64(kPrefsManifestSignatureSize,
+ &manifest_signature_size) &&
+ manifest_signature_size >= 0))
+ return false;
+
return true;
}
@@ -1700,6 +1717,7 @@
prefs->SetString(kPrefsUpdateStateSignedSHA256Context, "");
prefs->SetString(kPrefsUpdateStateSignatureBlob, "");
prefs->SetInt64(kPrefsManifestMetadataSize, -1);
+ prefs->SetInt64(kPrefsManifestSignatureSize, -1);
prefs->SetInt64(kPrefsResumedUpdateFailures, 0);
}
return true;
@@ -1786,6 +1804,12 @@
manifest_metadata_size > 0);
metadata_size_ = manifest_metadata_size;
+ int64_t manifest_signature_size = 0;
+ TEST_AND_RETURN_FALSE(
+ prefs_->GetInt64(kPrefsManifestSignatureSize, &manifest_signature_size) &&
+ manifest_signature_size >= 0);
+ metadata_signature_size_ = manifest_signature_size;
+
// Advance the download progress to reflect what doesn't need to be
// re-downloaded.
total_bytes_received_ += buffer_offset_;
diff --git a/payload_consumer/delta_performer_integration_test.cc b/payload_consumer/delta_performer_integration_test.cc
index 34ce0de..3d7f8fa 100644
--- a/payload_consumer/delta_performer_integration_test.cc
+++ b/payload_consumer/delta_performer_integration_test.cc
@@ -693,6 +693,8 @@
MockPrefs prefs;
EXPECT_CALL(prefs, SetInt64(kPrefsManifestMetadataSize,
state->metadata_size)).WillOnce(Return(true));
+ EXPECT_CALL(prefs, SetInt64(kPrefsManifestSignatureSize, 0))
+ .WillOnce(Return(true));
EXPECT_CALL(prefs, SetInt64(kPrefsUpdateStateNextOperation, _))
.WillRepeatedly(Return(true));
EXPECT_CALL(prefs, GetInt64(kPrefsUpdateStateNextOperation, _))
diff --git a/update_attempter.cc b/update_attempter.cc
index b67fcb3..cfd2425 100644
--- a/update_attempter.cc
+++ b/update_attempter.cc
@@ -1380,14 +1380,17 @@
if (response_handler_action_->install_plan().is_resume) {
// Resuming an update so fetch the update manifest metadata first.
int64_t manifest_metadata_size = 0;
+ int64_t manifest_signature_size = 0;
prefs_->GetInt64(kPrefsManifestMetadataSize, &manifest_metadata_size);
- fetcher->AddRange(0, manifest_metadata_size);
+ prefs_->GetInt64(kPrefsManifestSignatureSize, &manifest_signature_size);
+ fetcher->AddRange(0, manifest_metadata_size + manifest_signature_size);
// If there're remaining unprocessed data blobs, fetch them. Be careful not
// to request data beyond the end of the payload to avoid 416 HTTP response
// error codes.
int64_t next_data_offset = 0;
prefs_->GetInt64(kPrefsUpdateStateNextDataOffset, &next_data_offset);
- uint64_t resume_offset = manifest_metadata_size + next_data_offset;
+ uint64_t resume_offset =
+ manifest_metadata_size + manifest_signature_size + next_data_offset;
if (resume_offset < response_handler_action_->install_plan().payload_size) {
fetcher->AddRange(resume_offset);
}
diff --git a/update_attempter_android.cc b/update_attempter_android.cc
index cc478a4..c10b080 100644
--- a/update_attempter_android.cc
+++ b/update_attempter_android.cc
@@ -412,14 +412,18 @@
if (install_plan_.is_resume) {
// Resuming an update so fetch the update manifest metadata first.
int64_t manifest_metadata_size = 0;
+ int64_t manifest_signature_size = 0;
prefs_->GetInt64(kPrefsManifestMetadataSize, &manifest_metadata_size);
- fetcher->AddRange(base_offset_, manifest_metadata_size);
+ prefs_->GetInt64(kPrefsManifestSignatureSize, &manifest_signature_size);
+ fetcher->AddRange(base_offset_,
+ manifest_metadata_size + manifest_signature_size);
// If there're remaining unprocessed data blobs, fetch them. Be careful not
// to request data beyond the end of the payload to avoid 416 HTTP response
// error codes.
int64_t next_data_offset = 0;
prefs_->GetInt64(kPrefsUpdateStateNextDataOffset, &next_data_offset);
- uint64_t resume_offset = manifest_metadata_size + next_data_offset;
+ uint64_t resume_offset =
+ manifest_metadata_size + manifest_signature_size + next_data_offset;
if (!install_plan_.payload_size) {
fetcher->AddRange(base_offset_ + resume_offset);
} else if (resume_offset < install_plan_.payload_size) {