Merge "updater: Fix an off-by-1 bug in file_getprop()."

This commit is contained in:
Tao Bao 2016-11-04 04:30:24 +00:00 committed by Gerrit Code Review
commit da2b34b5d0
2 changed files with 59 additions and 6 deletions

View file

@ -16,7 +16,9 @@
#include <string>
#include <android-base/file.h>
#include <android-base/properties.h>
#include <android-base/test_utils.h>
#include <gtest/gtest.h>
#include "edify/expr.h"
@ -97,3 +99,51 @@ TEST_F(UpdaterTest, sha1_check) {
// sha1_check() expects at least one argument.
expect(nullptr, "sha1_check()", kArgsParsingFailure);
}
TEST_F(UpdaterTest, file_getprop) {
// file_getprop() expects two arguments.
expect(nullptr, "file_getprop()", kArgsParsingFailure);
expect(nullptr, "file_getprop(\"arg1\")", kArgsParsingFailure);
expect(nullptr, "file_getprop(\"arg1\", \"arg2\", \"arg3\")", kArgsParsingFailure);
// File doesn't exist.
expect(nullptr, "file_getprop(\"/doesntexist\", \"key1\")", kFileGetPropFailure);
// Reject too large files (current limit = 65536).
TemporaryFile temp_file1;
std::string buffer(65540, '\0');
ASSERT_TRUE(android::base::WriteStringToFile(buffer, temp_file1.path));
// Read some keys.
TemporaryFile temp_file2;
std::string content("ro.product.name=tardis\n"
"# comment\n\n\n"
"ro.product.model\n"
"ro.product.board = magic \n");
ASSERT_TRUE(android::base::WriteStringToFile(content, temp_file2.path));
std::string script1("file_getprop(\"" + std::string(temp_file2.path) +
"\", \"ro.product.name\")");
expect("tardis", script1.c_str(), kNoCause);
std::string script2("file_getprop(\"" + std::string(temp_file2.path) +
"\", \"ro.product.board\")");
expect("magic", script2.c_str(), kNoCause);
// No match.
std::string script3("file_getprop(\"" + std::string(temp_file2.path) +
"\", \"ro.product.wrong\")");
expect("", script3.c_str(), kNoCause);
std::string script4("file_getprop(\"" + std::string(temp_file2.path) +
"\", \"ro.product.name=\")");
expect("", script4.c_str(), kNoCause);
std::string script5("file_getprop(\"" + std::string(temp_file2.path) +
"\", \"ro.product.nam\")");
expect("", script5.c_str(), kNoCause);
std::string script6("file_getprop(\"" + std::string(temp_file2.path) +
"\", \"ro.product.model\")");
expect("", script6.c_str(), kNoCause);
}

View file

@ -860,9 +860,13 @@ Value* GetPropFn(const char* name, State* state, int argc, Expr* argv[]) {
// file_getprop(file, key)
//
// interprets 'file' as a getprop-style file (key=value pairs, one
// per line. # comment lines,blank lines, lines without '=' ignored),
// per line. # comment lines, blank lines, lines without '=' ignored),
// and returns the value for 'key' (or "" if it isn't defined).
Value* FileGetPropFn(const char* name, State* state, int argc, Expr* argv[]) {
if (argc != 2) {
return ErrorAbort(state, kArgsParsingFailure, "%s() expects 2 args, got %d", name, argc);
}
std::vector<std::string> args;
if (!ReadArgs(state, 2, argv, &args)) {
return ErrorAbort(state, kArgsParsingFailure, "%s() Failed to parse the argument(s)", name);
@ -876,11 +880,10 @@ Value* FileGetPropFn(const char* name, State* state, int argc, Expr* argv[]) {
filename.c_str(), strerror(errno));
}
#define MAX_FILE_GETPROP_SIZE 65536
constexpr off_t MAX_FILE_GETPROP_SIZE = 65536;
if (st.st_size > MAX_FILE_GETPROP_SIZE) {
return ErrorAbort(state, kFileGetPropFailure, "%s too large for %s (max %d)",
filename.c_str(), name, MAX_FILE_GETPROP_SIZE);
return ErrorAbort(state, kFileGetPropFailure, "%s too large for %s (max %lld)",
filename.c_str(), name, static_cast<long long>(MAX_FILE_GETPROP_SIZE));
}
std::string buffer(st.st_size, '\0');
@ -913,7 +916,7 @@ Value* FileGetPropFn(const char* name, State* state, int argc, Expr* argv[]) {
}
// trim whitespace between key and '='
std::string str = android::base::Trim(line.substr(0, equal_pos - 1));
std::string str = android::base::Trim(line.substr(0, equal_pos));
// not the key we're looking for
if (key != str) continue;