From fada91ccf2b31d69d72899388b1833186d763d1a Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Thu, 27 Oct 2016 18:16:06 -0700 Subject: applypatch: Switch the parameter of Value** to std::vector. Test: Unit tests and install-recovery.sh pass on angler and dragon. Change-Id: I328e6554edca667cf850f5584ebf1ac211e3d4d1 --- applypatch/applypatch.cpp | 16 +- applypatch/include/applypatch/applypatch.h | 15 +- applypatch/main.cpp | 15 +- tests/component/applypatch_test.cpp | 264 +++++++++++++++-------------- updater/install.cpp | 4 +- 5 files changed, 162 insertions(+), 152 deletions(-) diff --git a/applypatch/applypatch.cpp b/applypatch/applypatch.cpp index 48e4c8386..9d505f9f6 100644 --- a/applypatch/applypatch.cpp +++ b/applypatch/applypatch.cpp @@ -530,8 +530,8 @@ int applypatch(const char* source_filename, const char* target_sha1_str, size_t target_size, const std::vector& patch_sha1_str, - Value** patch_data, - Value* bonus_data) { + const std::vector>& patch_data, + const Value* bonus_data) { printf("patch %s: ", source_filename); if (target_filename[0] == '-' && target_filename[1] == '\0') { @@ -546,8 +546,8 @@ int applypatch(const char* source_filename, FileContents copy_file; FileContents source_file; - const Value* source_patch_value = NULL; - const Value* copy_patch_value = NULL; + const Value* source_patch_value = nullptr; + const Value* copy_patch_value = nullptr; // We try to load the target file into the source_file object. if (LoadFileContents(target_filename, &source_file) == 0) { @@ -571,11 +571,11 @@ int applypatch(const char* source_filename, if (!source_file.data.empty()) { int to_use = FindMatchingPatch(source_file.sha1, patch_sha1_str); if (to_use >= 0) { - source_patch_value = patch_data[to_use]; + source_patch_value = patch_data[to_use].get(); } } - if (source_patch_value == NULL) { + if (source_patch_value == nullptr) { source_file.data.clear(); printf("source file is bad; trying copy\n"); @@ -587,10 +587,10 @@ int applypatch(const char* source_filename, int to_use = FindMatchingPatch(copy_file.sha1, patch_sha1_str); if (to_use >= 0) { - copy_patch_value = patch_data[to_use]; + copy_patch_value = patch_data[to_use].get(); } - if (copy_patch_value == NULL) { + if (copy_patch_value == nullptr) { // fail. printf("copy file doesn't match source SHA-1s either\n"); return 1; diff --git a/applypatch/include/applypatch/applypatch.h b/applypatch/include/applypatch/applypatch.h index 80d681638..ca3dafbc9 100644 --- a/applypatch/include/applypatch/applypatch.h +++ b/applypatch/include/applypatch/applypatch.h @@ -20,10 +20,12 @@ #include #include +#include #include #include -#include "openssl/sha.h" +#include + #include "edify/expr.h" struct FileContents { @@ -41,27 +43,26 @@ struct FileContents { typedef ssize_t (*SinkFn)(const unsigned char*, ssize_t, void*); -// applypatch.c +// applypatch.cpp int ShowLicenses(); size_t FreeSpaceForFile(const char* filename); int CacheSizeCheck(size_t bytes); int ParseSha1(const char* str, uint8_t* digest); -int applypatch_flash(const char* source_filename, const char* target_filename, - const char* target_sha1_str, size_t target_size); int applypatch(const char* source_filename, const char* target_filename, const char* target_sha1_str, size_t target_size, const std::vector& patch_sha1_str, - Value** patch_data, - Value* bonus_data); + const std::vector>& patch_data, + const Value* bonus_data); int applypatch_check(const char* filename, const std::vector& patch_sha1_str); +int applypatch_flash(const char* source_filename, const char* target_filename, + const char* target_sha1_str, size_t target_size); int LoadFileContents(const char* filename, FileContents* file); int SaveFileContents(const char* filename, const FileContents* file); -void FreeFileContents(FileContents* file); int FindMatchingPatch(uint8_t* sha1, const std::vector& patch_sha1_str); // bsdiff.cpp diff --git a/applypatch/main.cpp b/applypatch/main.cpp index 294f7ee21..988b6f9b1 100644 --- a/applypatch/main.cpp +++ b/applypatch/main.cpp @@ -23,9 +23,10 @@ #include #include +#include + #include "applypatch/applypatch.h" #include "edify/expr.h" -#include "openssl/sha.h" static int CheckMode(int argc, char** argv) { if (argc < 3) { @@ -129,15 +130,13 @@ static int PatchMode(int argc, char** argv) { printf("failed to parse patch args\n"); return 1; } - std::vector patches; - std::vector patch_ptrs; + + std::vector> patches; for (size_t i = 0; i < files.size(); ++i) { - patches.push_back(Value(VAL_BLOB, - std::string(files[i].data.cbegin(), files[i].data.cend()))); - patch_ptrs.push_back(&patches[i]); + patches.push_back(std::make_unique( + VAL_BLOB, std::string(files[i].data.cbegin(), files[i].data.cend()))); } - return applypatch(argv[1], argv[2], argv[3], target_size, - sha1s, patch_ptrs.data(), &bonus); + return applypatch(argv[1], argv[2], argv[3], target_size, sha1s, patches, &bonus); } // This program applies binary patches to files in a way that is safe diff --git a/tests/component/applypatch_test.cpp b/tests/component/applypatch_test.cpp index 908a9f5f5..e09c1e7ba 100644 --- a/tests/component/applypatch_test.cpp +++ b/tests/component/applypatch_test.cpp @@ -23,102 +23,98 @@ #include #include +#include #include +#include #include #include #include +#include #include "applypatch/applypatch.h" #include "common/test_constants.h" -#include "openssl/sha.h" #include "print_sha1.h" static const std::string DATA_PATH = getenv("ANDROID_DATA"); static const std::string TESTDATA_PATH = "/recovery/testdata"; -static const std::string WORK_FS = "/data"; -static std::string sha1sum(const std::string& fname) { - uint8_t digest[SHA_DIGEST_LENGTH]; +static void sha1sum(const std::string& fname, std::string* sha1) { + ASSERT_NE(nullptr, sha1); + std::string data; - android::base::ReadFileToString(fname, &data); + ASSERT_TRUE(android::base::ReadFileToString(fname, &data)); - SHA1((const uint8_t*)data.c_str(), data.size(), digest); - return print_sha1(digest); + uint8_t digest[SHA_DIGEST_LENGTH]; + SHA1(reinterpret_cast(data.c_str()), data.size(), digest); + *sha1 = print_sha1(digest); } static void mangle_file(const std::string& fname) { - FILE* fh = fopen(&fname[0], "w"); - int r; - for (int i=0; i < 1024; i++) { - r = rand(); - fwrite(&r, sizeof(short), 1, fh); + std::string content; + content.reserve(1024); + for (size_t i = 0; i < 1024; i++) { + content[i] = rand() % 256; } - fclose(fh); + ASSERT_TRUE(android::base::WriteStringToFile(content, fname)); } -static bool file_cmp(std::string& f1, std::string& f2) { +static bool file_cmp(const std::string& f1, const std::string& f2) { std::string c1; - std::string c2; android::base::ReadFileToString(f1, &c1); + std::string c2; android::base::ReadFileToString(f2, &c2); return c1 == c2; } static std::string from_testdata_base(const std::string& fname) { - return android::base::StringPrintf("%s%s%s/%s", - &DATA_PATH[0], - &NATIVE_TEST_PATH[0], - &TESTDATA_PATH[0], - &fname[0]); + return DATA_PATH + NATIVE_TEST_PATH + TESTDATA_PATH + "/" + fname; } class ApplyPatchTest : public ::testing::Test { - public: - static void SetUpTestCase() { - // set up files - old_file = from_testdata_base("old.file"); - new_file = from_testdata_base("new.file"); - patch_file = from_testdata_base("patch.bsdiff"); - rand_file = "/cache/applypatch_test_rand.file"; - cache_file = "/cache/saved.file"; - - // write stuff to rand_file - android::base::WriteStringToFile("hello", rand_file); - - // set up SHA constants - old_sha1 = sha1sum(old_file); - new_sha1 = sha1sum(new_file); - srand(time(NULL)); - bad_sha1_a = android::base::StringPrintf("%040x", rand()); - bad_sha1_b = android::base::StringPrintf("%040x", rand()); - - struct stat st; - stat(&new_file[0], &st); - new_size = st.st_size; - } - - static std::string old_file; - static std::string new_file; - static std::string rand_file; - static std::string cache_file; - static std::string patch_file; - - static std::string old_sha1; - static std::string new_sha1; - static std::string bad_sha1_a; - static std::string bad_sha1_b; - - static size_t new_size; + public: + static void SetUpTestCase() { + // set up files + old_file = from_testdata_base("old.file"); + new_file = from_testdata_base("new.file"); + patch_file = from_testdata_base("patch.bsdiff"); + rand_file = "/cache/applypatch_test_rand.file"; + cache_file = "/cache/saved.file"; + + // write stuff to rand_file + ASSERT_TRUE(android::base::WriteStringToFile("hello", rand_file)); + + // set up SHA constants + sha1sum(old_file, &old_sha1); + sha1sum(new_file, &new_sha1); + srand(time(NULL)); + bad_sha1_a = android::base::StringPrintf("%040x", rand()); + bad_sha1_b = android::base::StringPrintf("%040x", rand()); + + struct stat st; + stat(&new_file[0], &st); + new_size = st.st_size; + } + + static std::string old_file; + static std::string new_file; + static std::string rand_file; + static std::string cache_file; + static std::string patch_file; + + static std::string old_sha1; + static std::string new_sha1; + static std::string bad_sha1_a; + static std::string bad_sha1_b; + + static size_t new_size; }; std::string ApplyPatchTest::old_file; std::string ApplyPatchTest::new_file; -static void cp(std::string src, std::string tgt) { - std::string cmd = android::base::StringPrintf("cp %s %s", - &src[0], - &tgt[0]); +static void cp(const std::string& src, const std::string& tgt) { + std::string cmd = "cp " + src + " " + tgt; system(&cmd[0]); } @@ -131,59 +127,55 @@ static void restore_old() { } class ApplyPatchCacheTest : public ApplyPatchTest { - public: - virtual void SetUp() { - backup_old(); - } - - virtual void TearDown() { - restore_old(); - } + public: + virtual void SetUp() { + backup_old(); + } + + virtual void TearDown() { + restore_old(); + } }; class ApplyPatchFullTest : public ApplyPatchCacheTest { - public: - static void SetUpTestCase() { - ApplyPatchTest::SetUpTestCase(); - unsigned long free_kb = FreeSpaceForFile(&WORK_FS[0]); - ASSERT_GE(free_kb * 1024, new_size * 3 / 2); - output_f = new TemporaryFile(); - output_loc = std::string(output_f->path); - - struct FileContents fc; - - ASSERT_EQ(0, LoadFileContents(&rand_file[0], &fc)); - Value* patch1 = new Value(VAL_BLOB, std::string(fc.data.begin(), fc.data.end())); - patches.push_back(patch1); - - ASSERT_EQ(0, LoadFileContents(&patch_file[0], &fc)); - Value* patch2 = new Value(VAL_BLOB, std::string(fc.data.begin(), fc.data.end())); - patches.push_back(patch2); - } - static void TearDownTestCase() { - delete output_f; - for (auto it = patches.begin(); it != patches.end(); ++it) { - delete *it; - } - patches.clear(); - } - - static std::vector patches; - static TemporaryFile* output_f; - static std::string output_loc; + public: + static void SetUpTestCase() { + ApplyPatchTest::SetUpTestCase(); + + output_f = new TemporaryFile(); + output_loc = std::string(output_f->path); + + struct FileContents fc; + + ASSERT_EQ(0, LoadFileContents(&rand_file[0], &fc)); + patches.push_back( + std::make_unique(VAL_BLOB, std::string(fc.data.begin(), fc.data.end()))); + + ASSERT_EQ(0, LoadFileContents(&patch_file[0], &fc)); + patches.push_back( + std::make_unique(VAL_BLOB, std::string(fc.data.begin(), fc.data.end()))); + } + + static void TearDownTestCase() { + delete output_f; + } + + static std::vector> patches; + static TemporaryFile* output_f; + static std::string output_loc; }; class ApplyPatchDoubleCacheTest : public ApplyPatchFullTest { - public: - virtual void SetUp() { - ApplyPatchCacheTest::SetUp(); - cp(cache_file, "/cache/reallysaved.file"); - } - - virtual void TearDown() { - cp("/cache/reallysaved.file", cache_file); - ApplyPatchCacheTest::TearDown(); - } + public: + virtual void SetUp() { + ApplyPatchCacheTest::SetUp(); + cp(cache_file, "/cache/reallysaved.file"); + } + + virtual void TearDown() { + cp("/cache/reallysaved.file", cache_file); + ApplyPatchCacheTest::TearDown(); + } }; std::string ApplyPatchTest::rand_file; @@ -196,7 +188,7 @@ std::string ApplyPatchTest::bad_sha1_b; size_t ApplyPatchTest::new_size; -std::vector ApplyPatchFullTest::patches; +std::vector> ApplyPatchFullTest::patches; TemporaryFile* ApplyPatchFullTest::output_f; std::string ApplyPatchFullTest::output_loc; @@ -287,7 +279,7 @@ TEST_F(ApplyPatchFullTest, ApplyInPlace) { &new_sha1[0], new_size, sha1s, - patches.data(), + patches, nullptr); ASSERT_EQ(0, ap_result); ASSERT_TRUE(file_cmp(old_file, new_file)); @@ -297,7 +289,7 @@ TEST_F(ApplyPatchFullTest, ApplyInPlace) { &new_sha1[0], new_size, sha1s, - patches.data(), + patches, nullptr); ASSERT_EQ(0, ap_result); ASSERT_TRUE(file_cmp(old_file, new_file)); @@ -308,75 +300,95 @@ TEST_F(ApplyPatchFullTest, ApplyInNewLocation) { bad_sha1_a, old_sha1 }; - int ap_result = applypatch(&old_file[0], + // Apply bsdiff patch to new location. + ASSERT_EQ(0, applypatch(&old_file[0], &output_loc[0], &new_sha1[0], new_size, sha1s, - patches.data(), - nullptr); - ASSERT_EQ(0, ap_result); + patches, + nullptr)); ASSERT_TRUE(file_cmp(output_loc, new_file)); - ap_result = applypatch(&old_file[0], + + // Reapply to the same location. + ASSERT_EQ(0, applypatch(&old_file[0], &output_loc[0], &new_sha1[0], new_size, sha1s, - patches.data(), - nullptr); - ASSERT_EQ(0, ap_result); + patches, + nullptr)); ASSERT_TRUE(file_cmp(output_loc, new_file)); } TEST_F(ApplyPatchFullTest, ApplyCorruptedInNewLocation) { - mangle_file(old_file); std::vector sha1s = { bad_sha1_a, old_sha1 }; + // Apply bsdiff patch to new location with corrupted source. + mangle_file(old_file); int ap_result = applypatch(&old_file[0], &output_loc[0], &new_sha1[0], new_size, sha1s, - patches.data(), + patches, nullptr); ASSERT_EQ(0, ap_result); ASSERT_TRUE(file_cmp(output_loc, new_file)); + + // Reapply bsdiff patch to new location with corrupted source. ap_result = applypatch(&old_file[0], &output_loc[0], &new_sha1[0], new_size, sha1s, - patches.data(), + patches, nullptr); ASSERT_EQ(0, ap_result); ASSERT_TRUE(file_cmp(output_loc, new_file)); } TEST_F(ApplyPatchDoubleCacheTest, ApplyDoubleCorruptedInNewLocation) { - mangle_file(old_file); - mangle_file(cache_file); - std::vector sha1s = { bad_sha1_a, old_sha1 }; + + // Apply bsdiff patch to new location with corrupted source and copy (no new file). + // Expected to fail. + mangle_file(old_file); + mangle_file(cache_file); int ap_result = applypatch(&old_file[0], &output_loc[0], &new_sha1[0], new_size, sha1s, - patches.data(), + patches, + nullptr); + ASSERT_NE(0, ap_result); + ASSERT_FALSE(file_cmp(output_loc, new_file)); + + // Expected to fail again on retry. + ap_result = applypatch(&old_file[0], + &output_loc[0], + &new_sha1[0], + new_size, + sha1s, + patches, nullptr); ASSERT_NE(0, ap_result); ASSERT_FALSE(file_cmp(output_loc, new_file)); + + // Expected to fail with incorrect new file. + mangle_file(output_loc); ap_result = applypatch(&old_file[0], &output_loc[0], &new_sha1[0], new_size, sha1s, - patches.data(), + patches, nullptr); ASSERT_NE(0, ap_result); ASSERT_FALSE(file_cmp(output_loc, new_file)); diff --git a/updater/install.cpp b/updater/install.cpp index a41c5db3c..5f3f675b1 100644 --- a/updater/install.cpp +++ b/updater/install.cpp @@ -1061,15 +1061,13 @@ Value* ApplyPatchFn(const char* name, State* state, int argc, Expr* argv[]) { } std::vector patch_sha_str; - std::vector patch_ptrs; for (int i = 0; i < patchcount; ++i) { patch_sha_str.push_back(patch_shas[i]->data); - patch_ptrs.push_back(patches[i].get()); } int result = applypatch(source_filename, target_filename, target_sha1, target_size, - patch_sha_str, patch_ptrs.data(), NULL); + patch_sha_str, patches, NULL); return StringValue(result == 0 ? "t" : ""); } -- cgit v1.2.3