updater: Fix the wrong return value for package_extract_file().
'bool success = ExtractEntryToFile()' gives opposite result. Fix the
issue and add testcases.
Change the one-argument version of package_extract_file() to explicitly
abort for non-existent zip entry. Note that this is NOT changing the
behavior. Prior to this CL, it aborts from Evaluate() function, by
giving a general cause code. Now it returns kPackageExtractFileFailure.
BUg: 32903624
Test: recovery_component_test works.
Change-Id: I7a273e9c0d9aaaf8c472b2c778f7b8d90362c24f
diff --git a/error_code.h b/error_code.h
index 92e93bd..92b1574 100644
--- a/error_code.h
+++ b/error_code.h
@@ -18,52 +18,53 @@
#define _ERROR_CODE_H_
enum ErrorCode {
- kNoError = -1,
- kLowBattery = 20,
- kZipVerificationFailure,
- kZipOpenFailure,
- kBootreasonInBlacklist
+ kNoError = -1,
+ kLowBattery = 20,
+ kZipVerificationFailure,
+ kZipOpenFailure,
+ kBootreasonInBlacklist
};
enum CauseCode {
- kNoCause = -1,
- kArgsParsingFailure = 100,
- kStashCreationFailure,
- kFileOpenFailure,
- kLseekFailure,
- kFreadFailure,
- kFwriteFailure,
- kFsyncFailure,
- kLibfecFailure,
- kFileGetPropFailure,
- kFileRenameFailure,
- kSymlinkFailure,
- kSetMetadataFailure,
- kTune2FsFailure,
- kRebootFailure,
- kVendorFailure = 200
+ kNoCause = -1,
+ kArgsParsingFailure = 100,
+ kStashCreationFailure,
+ kFileOpenFailure,
+ kLseekFailure,
+ kFreadFailure,
+ kFwriteFailure,
+ kFsyncFailure,
+ kLibfecFailure,
+ kFileGetPropFailure,
+ kFileRenameFailure,
+ kSymlinkFailure,
+ kSetMetadataFailure,
+ kTune2FsFailure,
+ kRebootFailure,
+ kPackageExtractFileFailure,
+ kVendorFailure = 200
};
enum UncryptErrorCode {
- kUncryptNoError = -1,
- kUncryptErrorHolder = 50,
- kUncryptTimeoutError = 100,
- kUncryptFileRemoveError,
- kUncryptFileOpenError,
- kUncryptSocketOpenError,
- kUncryptSocketWriteError,
- kUncryptSocketListenError,
- kUncryptSocketAcceptError,
- kUncryptFstabReadError,
- kUncryptFileStatError,
- kUncryptBlockOpenError,
- kUncryptIoctlError,
- kUncryptReadError,
- kUncryptWriteError,
- kUncryptFileSyncError,
- kUncryptFileCloseError,
- kUncryptFileRenameError,
- kUncryptPackageMissingError,
+ kUncryptNoError = -1,
+ kUncryptErrorHolder = 50,
+ kUncryptTimeoutError = 100,
+ kUncryptFileRemoveError,
+ kUncryptFileOpenError,
+ kUncryptSocketOpenError,
+ kUncryptSocketWriteError,
+ kUncryptSocketListenError,
+ kUncryptSocketAcceptError,
+ kUncryptFstabReadError,
+ kUncryptFileStatError,
+ kUncryptBlockOpenError,
+ kUncryptIoctlError,
+ kUncryptReadError,
+ kUncryptWriteError,
+ kUncryptFileSyncError,
+ kUncryptFileCloseError,
+ kUncryptFileRenameError,
+ kUncryptPackageMissingError,
};
-#endif
+#endif // _ERROR_CODE_H_
diff --git a/tests/Android.mk b/tests/Android.mk
index e87a229..fdc9470 100644
--- a/tests/Android.mk
+++ b/tests/Android.mk
@@ -92,6 +92,8 @@
libcrypto \
libcutils \
libbz \
+ libziparchive \
+ libutils \
libz \
libbase \
libtune2fs \
diff --git a/tests/common/test_constants.h b/tests/common/test_constants.h
index 97e74a3..93e4ab5 100644
--- a/tests/common/test_constants.h
+++ b/tests/common/test_constants.h
@@ -19,6 +19,15 @@
#include <stdlib.h>
+// Zip entries in ziptest_valid.zip.
+static const std::string kATxtContents("abcdefghabcdefgh\n");
+static const std::string kBTxtContents("abcdefgh\n");
+
+// echo -n -e "abcdefghabcdefgh\n" | sha1sum
+static const std::string kATxtSha1Sum("32c96a03dc8cd20097940f351bca6261ee5a1643");
+// echo -n -e "abcdefgh\n" | sha1sum
+static const std::string kBTxtSha1Sum("e414af7161c9554089f4106d6f1797ef14a73666");
+
static const char* data_root = getenv("ANDROID_DATA");
static std::string from_testdata_base(const std::string& fname) {
diff --git a/tests/component/updater_test.cpp b/tests/component/updater_test.cpp
index 973c19d..a029cf4 100644
--- a/tests/component/updater_test.cpp
+++ b/tests/component/updater_test.cpp
@@ -24,35 +24,40 @@
#include <android-base/properties.h>
#include <android-base/test_utils.h>
#include <gtest/gtest.h>
+#include <ziparchive/zip_archive.h>
+#include "common/test_constants.h"
#include "edify/expr.h"
#include "error_code.h"
#include "updater/install.h"
+#include "updater/updater.h"
struct selabel_handle *sehandle = nullptr;
-static void expect(const char* expected, const char* expr_str, CauseCode cause_code) {
- Expr* e;
- int error_count;
- EXPECT_EQ(parse_string(expr_str, &e, &error_count), 0);
+static void expect(const char* expected, const char* expr_str, CauseCode cause_code,
+ UpdaterInfo* info = nullptr) {
+ Expr* e;
+ int error_count = 0;
+ ASSERT_EQ(0, parse_string(expr_str, &e, &error_count));
+ ASSERT_EQ(0, error_count);
- State state(expr_str, nullptr);
+ State state(expr_str, info);
- std::string result;
- bool status = Evaluate(&state, e, &result);
+ std::string result;
+ bool status = Evaluate(&state, e, &result);
- if (expected == nullptr) {
- EXPECT_FALSE(status);
- } else {
- EXPECT_STREQ(expected, result.c_str());
- }
+ if (expected == nullptr) {
+ ASSERT_FALSE(status);
+ } else {
+ ASSERT_TRUE(status);
+ ASSERT_STREQ(expected, result.c_str());
+ }
- // Error code is set in updater/updater.cpp only, by parsing State.errmsg.
- EXPECT_EQ(kNoError, state.error_code);
+ // Error code is set in updater/updater.cpp only, by parsing State.errmsg.
+ ASSERT_EQ(kNoError, state.error_code);
- // Cause code should always be available.
- EXPECT_EQ(cause_code, state.cause_code);
-
+ // Cause code should always be available.
+ ASSERT_EQ(cause_code, state.cause_code);
}
class UpdaterTest : public ::testing::Test {
@@ -264,3 +269,56 @@
ASSERT_EQ(0, unlink(src1.c_str()));
ASSERT_EQ(0, unlink(src2.c_str()));
}
+
+// TODO: Test extracting to block device.
+TEST_F(UpdaterTest, package_extract_file) {
+ // package_extract_file expects 1 or 2 arguments.
+ expect(nullptr, "package_extract_file()", kArgsParsingFailure);
+ expect(nullptr, "package_extract_file(\"arg1\", \"arg2\", \"arg3\")", kArgsParsingFailure);
+
+ std::string zip_path = from_testdata_base("ziptest_valid.zip");
+ ZipArchiveHandle handle;
+ ASSERT_EQ(0, OpenArchive(zip_path.c_str(), &handle));
+
+ // Need to set up the ziphandle.
+ UpdaterInfo updater_info;
+ updater_info.package_zip = handle;
+
+ // Two-argument version.
+ TemporaryFile temp_file1;
+ std::string script("package_extract_file(\"a.txt\", \"" + std::string(temp_file1.path) + "\")");
+ expect("t", script.c_str(), kNoCause, &updater_info);
+
+ // Verify the extracted entry.
+ std::string data;
+ ASSERT_TRUE(android::base::ReadFileToString(temp_file1.path, &data));
+ ASSERT_EQ(kATxtContents, data);
+
+ // Now extract another entry to the same location, which should overwrite.
+ script = "package_extract_file(\"b.txt\", \"" + std::string(temp_file1.path) + "\")";
+ expect("t", script.c_str(), kNoCause, &updater_info);
+
+ ASSERT_TRUE(android::base::ReadFileToString(temp_file1.path, &data));
+ ASSERT_EQ(kBTxtContents, data);
+
+ // Missing zip entry. The two-argument version doesn't abort.
+ script = "package_extract_file(\"doesntexist\", \"" + std::string(temp_file1.path) + "\")";
+ expect("", script.c_str(), kNoCause, &updater_info);
+
+ // Extract to /dev/full should fail.
+ script = "package_extract_file(\"a.txt\", \"/dev/full\")";
+ expect("", script.c_str(), kNoCause, &updater_info);
+
+ // One-argument version.
+ script = "sha1_check(package_extract_file(\"a.txt\"))";
+ expect(kATxtSha1Sum.c_str(), script.c_str(), kNoCause, &updater_info);
+
+ script = "sha1_check(package_extract_file(\"b.txt\"))";
+ expect(kBTxtSha1Sum.c_str(), script.c_str(), kNoCause, &updater_info);
+
+ // Missing entry. The one-argument version aborts the evaluation.
+ script = "package_extract_file(\"doesntexist\")";
+ expect(nullptr, script.c_str(), kPackageExtractFileFailure, &updater_info);
+
+ CloseArchive(handle);
+}
diff --git a/tests/unit/zip_test.cpp b/tests/unit/zip_test.cpp
index 4972946..ef0ee4c 100644
--- a/tests/unit/zip_test.cpp
+++ b/tests/unit/zip_test.cpp
@@ -30,9 +30,6 @@
#include "common/test_constants.h"
-static const std::string kATxtContents("abcdefghabcdefgh\n");
-static const std::string kBTxtContents("abcdefgh\n");
-
TEST(ZipTest, ExtractPackageRecursive) {
std::string zip_path = from_testdata_base("ziptest_valid.zip");
ZipArchiveHandle handle;
diff --git a/updater/install.cpp b/updater/install.cpp
index 59c54dd..b885f86 100644
--- a/updater/install.cpp
+++ b/updater/install.cpp
@@ -477,93 +477,86 @@
return StringValue(success ? "t" : "");
}
+// package_extract_file(package_file[, dest_file])
+// Extracts a single package_file from the update package and writes it to dest_file,
+// overwriting existing files if necessary. Without the dest_file argument, returns the
+// contents of the package file as a binary blob.
+Value* PackageExtractFileFn(const char* name, State* state, int argc, Expr* argv[]) {
+ if (argc < 1 || argc > 2) {
+ return ErrorAbort(state, kArgsParsingFailure, "%s() expects 1 or 2 args, got %d", name, argc);
+ }
-// package_extract_file(package_path, destination_path)
-// or
-// package_extract_file(package_path)
-// to return the entire contents of the file as the result of this
-// function (the char* returned is actually a FileContents*).
-Value* PackageExtractFileFn(const char* name, State* state,
- int argc, Expr* argv[]) {
- if (argc < 1 || argc > 2) {
- return ErrorAbort(state, kArgsParsingFailure, "%s() expects 1 or 2 args, got %d",
- name, argc);
+ if (argc == 2) {
+ // The two-argument version extracts to a file.
+
+ std::vector<std::string> args;
+ if (!ReadArgs(state, 2, argv, &args)) {
+ return ErrorAbort(state, kArgsParsingFailure, "%s() Failed to parse %d args", name, argc);
}
- bool success = false;
+ const std::string& zip_path = args[0];
+ const std::string& dest_path = args[1];
- if (argc == 2) {
- // The two-argument version extracts to a file.
-
- ZipArchiveHandle za = ((UpdaterInfo*)(state->cookie))->package_zip;
-
- std::vector<std::string> args;
- if (!ReadArgs(state, 2, argv, &args)) {
- return ErrorAbort(state, kArgsParsingFailure, "%s() Failed to parse %d args", name,
- argc);
- }
- const std::string& zip_path = args[0];
- const std::string& dest_path = args[1];
-
- ZipString zip_string_path(zip_path.c_str());
- ZipEntry entry;
- if (FindEntry(za, zip_string_path, &entry) != 0) {
- printf("%s: no %s in package\n", name, zip_path.c_str());
- return StringValue("");
- }
-
- int fd = TEMP_FAILURE_RETRY(ota_open(dest_path.c_str(), O_WRONLY | O_CREAT | O_TRUNC,
- S_IRUSR | S_IWUSR));
- if (fd == -1) {
- printf("%s: can't open %s for write: %s\n", name, dest_path.c_str(), strerror(errno));
- return StringValue("");
- }
- success = ExtractEntryToFile(za, &entry, fd);
- if (ota_fsync(fd) == -1) {
- printf("fsync of \"%s\" failed: %s\n", dest_path.c_str(), strerror(errno));
- success = false;
- }
- if (ota_close(fd) == -1) {
- printf("close of \"%s\" failed: %s\n", dest_path.c_str(), strerror(errno));
- success = false;
- }
-
- return StringValue(success ? "t" : "");
- } else {
- // The one-argument version returns the contents of the file
- // as the result.
-
- std::vector<std::string> args;
- if (!ReadArgs(state, 1, argv, &args)) {
- return ErrorAbort(state, kArgsParsingFailure, "%s() Failed to parse %d args", name,
- argc);
- }
- const std::string& zip_path = args[0];
-
- Value* v = new Value(VAL_INVALID, "");
-
- ZipArchiveHandle za = ((UpdaterInfo*)(state->cookie))->package_zip;
- ZipString zip_string_path(zip_path.c_str());
- ZipEntry entry;
- if (FindEntry(za, zip_string_path, &entry) != 0) {
- printf("%s: no %s in package\n", name, zip_path.c_str());
- return v;
- }
-
- v->data.resize(entry.uncompressed_length);
- if (ExtractToMemory(za, &entry, reinterpret_cast<uint8_t*>(&v->data[0]),
- v->data.size()) != 0) {
- printf("%s: faled to extract %zu bytes to memory\n", name, v->data.size());
- } else {
- success = true;
- }
-
- if (!success) {
- v->data.clear();
- } else {
- v->type = VAL_BLOB;
- }
- return v;
+ ZipArchiveHandle za = static_cast<UpdaterInfo*>(state->cookie)->package_zip;
+ ZipString zip_string_path(zip_path.c_str());
+ ZipEntry entry;
+ if (FindEntry(za, zip_string_path, &entry) != 0) {
+ printf("%s: no %s in package\n", name, zip_path.c_str());
+ return StringValue("");
}
+
+ int fd = TEMP_FAILURE_RETRY(
+ ota_open(dest_path.c_str(), O_WRONLY | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR));
+ if (fd == -1) {
+ printf("%s: can't open %s for write: %s\n", name, dest_path.c_str(), strerror(errno));
+ return StringValue("");
+ }
+
+ bool success = true;
+ int32_t ret = ExtractEntryToFile(za, &entry, fd);
+ if (ret != 0) {
+ printf("%s: Failed to extract entry \"%s\" (%u bytes) to \"%s\": %s\n", name,
+ zip_path.c_str(), entry.uncompressed_length, dest_path.c_str(), ErrorCodeString(ret));
+ success = false;
+ }
+ if (ota_fsync(fd) == -1) {
+ printf("fsync of \"%s\" failed: %s\n", dest_path.c_str(), strerror(errno));
+ success = false;
+ }
+ if (ota_close(fd) == -1) {
+ printf("close of \"%s\" failed: %s\n", dest_path.c_str(), strerror(errno));
+ success = false;
+ }
+
+ return StringValue(success ? "t" : "");
+ } else {
+ // The one-argument version returns the contents of the file as the result.
+
+ std::vector<std::string> args;
+ if (!ReadArgs(state, 1, argv, &args)) {
+ return ErrorAbort(state, kArgsParsingFailure, "%s() Failed to parse %d args", name, argc);
+ }
+ const std::string& zip_path = args[0];
+
+ ZipArchiveHandle za = static_cast<UpdaterInfo*>(state->cookie)->package_zip;
+ ZipString zip_string_path(zip_path.c_str());
+ ZipEntry entry;
+ if (FindEntry(za, zip_string_path, &entry) != 0) {
+ return ErrorAbort(state, kPackageExtractFileFailure, "%s(): no %s in package", name,
+ zip_path.c_str());
+ }
+
+ std::string buffer;
+ buffer.resize(entry.uncompressed_length);
+
+ int32_t ret = ExtractToMemory(za, &entry, reinterpret_cast<uint8_t*>(&buffer[0]), buffer.size());
+ if (ret != 0) {
+ return ErrorAbort(state, kPackageExtractFileFailure,
+ "%s: Failed to extract entry \"%s\" (%zu bytes) to memory: %s", name,
+ zip_path.c_str(), buffer.size(), ErrorCodeString(ret));
+ }
+
+ return new Value(VAL_BLOB, buffer);
+ }
}
// symlink(target, [src1, src2, ...])