AU: Verify that the actual download size matches the size reported by Omaha.
This patch makes sure that the size of the payload received from the server
matches the size reported by Omaha. We could also verify that the HTTP header
size field matches the Omaha size but that seems redundant -- ultimately, it's
important that the number of bytes that AU actually downloads (and calculates
the hash on) matches the Omaha size.
BUG=6579
TEST=unit tests, gmerged on device and tested with a regular as well as a
modified dev server to send incorrect size.
Change-Id: I2f38e2df53e713c0b4f417533028fcb623e2df16
Review URL: http://codereview.chromium.org/3499004
diff --git a/action_processor.h b/action_processor.h
index a395b3d..98fc853 100644
--- a/action_processor.h
+++ b/action_processor.h
@@ -34,6 +34,7 @@
kActionCodeKernelDeviceOpenError = 8,
kActionCodeDownloadTransferError = 9,
kActionCodeDownloadHashMismatchError = 10,
+ kActionCodeDownloadSizeMismatchError = 11,
};
class AbstractAction;
diff --git a/download_action.cc b/download_action.cc
index 9779f2a..6b27722 100644
--- a/download_action.cc
+++ b/download_action.cc
@@ -138,13 +138,18 @@
ActionExitCode code =
successful ? kActionCodeSuccess : kActionCodeDownloadTransferError;
if (code == kActionCodeSuccess) {
- // Make sure hash is correct
+ // Makes sure the hash and size are correct.
omaha_hash_calculator_.Finalize();
if (omaha_hash_calculator_.hash() != install_plan_.download_hash) {
LOG(ERROR) << "Download of " << install_plan_.download_url
- << " failed. Expect hash " << install_plan_.download_hash
+ << " failed. Expected hash " << install_plan_.download_hash
<< " but got hash " << omaha_hash_calculator_.hash();
code = kActionCodeDownloadHashMismatchError;
+ } else if (bytes_received_ != install_plan_.size) {
+ LOG(ERROR) << "Download of " << install_plan_.download_url
+ << " failed. Expected size " << install_plan_.size
+ << " but got size " << bytes_received_;
+ code = kActionCodeDownloadSizeMismatchError;
}
}
diff --git a/download_action_unittest.cc b/download_action_unittest.cc
index d63ae2d..9ae9522 100644
--- a/download_action_unittest.cc
+++ b/download_action_unittest.cc
@@ -85,6 +85,7 @@
void TestWithData(const vector<char>& data,
bool hash_test,
+ bool size_test,
bool use_download_delegate) {
GMainLoop *loop = g_main_loop_new(g_main_context_default(), FALSE);
@@ -96,9 +97,10 @@
string hash = hash_test ?
OmahaHashCalculator::OmahaHashOfString("random string") :
OmahaHashCalculator::OmahaHashOfData(data);
+ uint64_t size = data.size() + (size_test ? 1 : 0);
InstallPlan install_plan(true,
"",
- 0,
+ size,
hash,
output_temp_file.GetPath(),
"");
@@ -116,8 +118,12 @@
EXPECT_CALL(download_delegate, BytesReceived(_, _)).Times(AtLeast(1));
EXPECT_CALL(download_delegate, SetDownloadStatus(false)).Times(1);
}
- DownloadActionTestProcessorDelegate delegate(
- hash_test ? kActionCodeDownloadHashMismatchError : kActionCodeSuccess);
+ ActionExitCode expected_code = kActionCodeSuccess;
+ if (hash_test)
+ expected_code = kActionCodeDownloadHashMismatchError;
+ else if (size_test)
+ expected_code = kActionCodeDownloadSizeMismatchError;
+ DownloadActionTestProcessorDelegate delegate(expected_code);
delegate.loop_ = loop;
delegate.expected_data_ = data;
delegate.path_ = output_temp_file.GetPath();
@@ -136,7 +142,10 @@
vector<char> small;
const char* foo = "foo";
small.insert(small.end(), foo, foo + strlen(foo));
- TestWithData(small, false, true);
+ TestWithData(small,
+ false, // hash_test
+ false, // size_test
+ true); // use_download_delegate
}
TEST(DownloadActionTest, LargeTest) {
@@ -149,21 +158,39 @@
else
c++;
}
- TestWithData(big, false, true);
+ TestWithData(big,
+ false, // hash_test
+ false, // size_test
+ true); // use_download_delegate
}
TEST(DownloadActionTest, BadHashTest) {
vector<char> small;
const char* foo = "foo";
small.insert(small.end(), foo, foo + strlen(foo));
- TestWithData(small, true, true);
+ TestWithData(small,
+ true, // hash_test
+ false, // size_test
+ true); // use_download_delegate
+}
+
+TEST(DownloadActionTest, BadSizeTest) {
+ const char* something = "something";
+ vector<char> small(something, something + strlen(something));
+ TestWithData(small,
+ false, // hash_test
+ true, // size_test
+ true); // use_download_delegate
}
TEST(DownloadActionTest, NoDownloadDelegateTest) {
vector<char> small;
const char* foo = "foofoo";
small.insert(small.end(), foo, foo + strlen(foo));
- TestWithData(small, false, false);
+ TestWithData(small,
+ false, // hash_test
+ false, // size_test
+ false); // use_download_delegate
}
namespace {
@@ -294,7 +321,7 @@
// takes ownership of passed in HttpFetcher
InstallPlan install_plan(true,
"",
- 0,
+ 1,
OmahaHashCalculator::OmahaHashOfString("x"),
"/dev/null",
"/dev/null");