Remove idmap path 256 length limit
Overlay and target package paths can be longer than 256 characters.
Currently, the idmap will fail to be generated if either path
is longer than 256 characters.
This change removes the 256 character limit and makes parsing variable
length strings easier in libandroidfw.
Bug: 174676094
Test: idmap2_tests && libandroidfw_tests
Change-Id: Ic240cdb8700566b2ac2ade08da58bea852e4ae0c
diff --git a/cmds/idmap2/libidmap2/BinaryStreamVisitor.cpp b/cmds/idmap2/libidmap2/BinaryStreamVisitor.cpp
index 726f6c5..5db09ba 100644
--- a/cmds/idmap2/libidmap2/BinaryStreamVisitor.cpp
+++ b/cmds/idmap2/libidmap2/BinaryStreamVisitor.cpp
@@ -38,13 +38,6 @@
stream_.write(reinterpret_cast<char*>(&x), sizeof(uint32_t));
}
-void BinaryStreamVisitor::WriteString256(const StringPiece& value) {
- char buf[kIdmapStringLength];
- memset(buf, 0, sizeof(buf));
- memcpy(buf, value.data(), std::min(value.size(), sizeof(buf)));
- stream_.write(buf, sizeof(buf));
-}
-
void BinaryStreamVisitor::WriteString(const StringPiece& value) {
// pad with null to nearest word boundary;
size_t padding_size = CalculatePadding(value.size());
@@ -64,8 +57,8 @@
Write32(header.GetOverlayCrc());
Write32(header.GetFulfilledPolicies());
Write32(static_cast<uint8_t>(header.GetEnforceOverlayable()));
- WriteString256(header.GetTargetPath());
- WriteString256(header.GetOverlayPath());
+ WriteString(header.GetTargetPath());
+ WriteString(header.GetOverlayPath());
WriteString(header.GetDebugInfo());
}
diff --git a/cmds/idmap2/libidmap2/Idmap.cpp b/cmds/idmap2/libidmap2/Idmap.cpp
index 1129413..4745cc6 100644
--- a/cmds/idmap2/libidmap2/Idmap.cpp
+++ b/cmds/idmap2/libidmap2/Idmap.cpp
@@ -69,38 +69,26 @@
return false;
}
-// a string is encoded as a kIdmapStringLength char array; the array is always null-terminated
-bool WARN_UNUSED ReadString256(std::istream& stream, char out[kIdmapStringLength]) {
- char buf[kIdmapStringLength];
- memset(buf, 0, sizeof(buf));
- if (!stream.read(buf, sizeof(buf))) {
- return false;
- }
- if (buf[sizeof(buf) - 1] != '\0') {
- return false;
- }
- memcpy(out, buf, sizeof(buf));
- return true;
-}
-
-Result<std::string> ReadString(std::istream& stream) {
+bool WARN_UNUSED ReadString(std::istream& stream, std::string* out) {
uint32_t size;
if (!Read32(stream, &size)) {
- return Error("failed to read string size");
+ return false;
}
if (size == 0) {
- return std::string("");
+ *out = "";
+ return true;
}
std::string buf(size, '\0');
if (!stream.read(buf.data(), size)) {
- return Error("failed to read string of size %u", size);
+ return false;
}
uint32_t padding_size = CalculatePadding(size);
std::string padding(padding_size, '\0');
if (!stream.read(padding.data(), padding_size)) {
- return Error("failed to read string padding of size %u", padding_size);
+ return false;
}
- return buf;
+ *out = buf;
+ return true;
}
} // namespace
@@ -119,28 +107,23 @@
if (!Read32(stream, &idmap_header->magic_) || !Read32(stream, &idmap_header->version_) ||
!Read32(stream, &idmap_header->target_crc_) || !Read32(stream, &idmap_header->overlay_crc_) ||
!Read32(stream, &idmap_header->fulfilled_policies_) ||
- !Read32(stream, &enforce_overlayable) || !ReadString256(stream, idmap_header->target_path_) ||
- !ReadString256(stream, idmap_header->overlay_path_)) {
+ !Read32(stream, &enforce_overlayable) || !ReadString(stream, &idmap_header->target_path_) ||
+ !ReadString(stream, &idmap_header->overlay_path_) ||
+ !ReadString(stream, &idmap_header->debug_info_)) {
return nullptr;
}
idmap_header->enforce_overlayable_ = enforce_overlayable != 0U;
-
- auto debug_str = ReadString(stream);
- if (!debug_str) {
- return nullptr;
- }
- idmap_header->debug_info_ = std::move(*debug_str);
-
return std::move(idmap_header);
}
-Result<Unit> IdmapHeader::IsUpToDate(const char* target_path, const char* overlay_path,
+Result<Unit> IdmapHeader::IsUpToDate(const std::string& target_path,
+ const std::string& overlay_path,
PolicyBitmask fulfilled_policies,
bool enforce_overlayable) const {
const std::unique_ptr<const ZipFile> target_zip = ZipFile::Open(target_path);
if (!target_zip) {
- return Error("failed to open target %s", target_path);
+ return Error("failed to open target %s", target_path.c_str());
}
const Result<uint32_t> target_crc = GetPackageCrc(*target_zip);
@@ -150,7 +133,7 @@
const std::unique_ptr<const ZipFile> overlay_zip = ZipFile::Open(overlay_path);
if (!overlay_zip) {
- return Error("failed to overlay target %s", overlay_path);
+ return Error("failed to overlay target %s", overlay_path.c_str());
}
const Result<uint32_t> overlay_crc = GetPackageCrc(*overlay_zip);
@@ -162,9 +145,9 @@
enforce_overlayable);
}
-Result<Unit> IdmapHeader::IsUpToDate(const char* target_path, const char* overlay_path,
- uint32_t target_crc, uint32_t overlay_crc,
- PolicyBitmask fulfilled_policies,
+Result<Unit> IdmapHeader::IsUpToDate(const std::string& target_path,
+ const std::string& overlay_path, uint32_t target_crc,
+ uint32_t overlay_crc, PolicyBitmask fulfilled_policies,
bool enforce_overlayable) const {
if (magic_ != kIdmapMagic) {
return Error("bad magic: actual 0x%08x, expected 0x%08x", magic_, kIdmapMagic);
@@ -194,14 +177,14 @@
enforce_overlayable ? "true" : "false", enforce_overlayable_ ? "true" : "false");
}
- if (strcmp(target_path, target_path_) != 0) {
- return Error("bad target path: idmap version %s, file system version %s", target_path,
- target_path_);
+ if (target_path != target_path_) {
+ return Error("bad target path: idmap version %s, file system version %s", target_path.c_str(),
+ target_path_.c_str());
}
- if (strcmp(overlay_path, overlay_path_) != 0) {
- return Error("bad overlay path: idmap version %s, file system version %s", overlay_path,
- overlay_path_);
+ if (overlay_path != overlay_path_) {
+ return Error("bad overlay path: idmap version %s, file system version %s", overlay_path.c_str(),
+ overlay_path_.c_str());
}
return Unit{};
@@ -262,12 +245,9 @@
}
// Read raw string pool bytes.
- auto string_pool_data = ReadString(stream);
- if (!string_pool_data) {
+ if (!ReadString(stream, &data->string_pool_data_)) {
return nullptr;
}
- data->string_pool_data_ = std::move(*string_pool_data);
-
return std::move(data);
}
@@ -368,23 +348,10 @@
return Error(crc.GetError(), "failed to get zip CRC for overlay");
}
header->overlay_crc_ = *crc;
-
header->fulfilled_policies_ = fulfilled_policies;
header->enforce_overlayable_ = enforce_overlayable;
-
- if (target_apk_path.size() > sizeof(header->target_path_)) {
- return Error("target apk path \"%s\" longer than maximum size %zu", target_apk_path.c_str(),
- sizeof(header->target_path_));
- }
- memset(header->target_path_, 0, sizeof(header->target_path_));
- memcpy(header->target_path_, target_apk_path.data(), target_apk_path.size());
-
- if (overlay_apk_path.size() > sizeof(header->overlay_path_)) {
- return Error("overlay apk path \"%s\" longer than maximum size %zu", overlay_apk_path.c_str(),
- sizeof(header->target_path_));
- }
- memset(header->overlay_path_, 0, sizeof(header->overlay_path_));
- memcpy(header->overlay_path_, overlay_apk_path.data(), overlay_apk_path.size());
+ header->target_path_ = target_apk_path;
+ header->overlay_path_ = overlay_apk_path;
auto overlay_info = utils::ExtractOverlayManifestInfo(overlay_apk_path);
if (!overlay_info) {
diff --git a/cmds/idmap2/libidmap2/PrettyPrintVisitor.cpp b/cmds/idmap2/libidmap2/PrettyPrintVisitor.cpp
index 3037a79..f56d3d2 100644
--- a/cmds/idmap2/libidmap2/PrettyPrintVisitor.cpp
+++ b/cmds/idmap2/libidmap2/PrettyPrintVisitor.cpp
@@ -49,12 +49,12 @@
}
}
- if (auto target_apk_ = ApkAssets::Load(header.GetTargetPath().to_string())) {
+ if (auto target_apk_ = ApkAssets::Load(header.GetTargetPath())) {
target_am_.SetApkAssets({target_apk_.get()});
apk_assets_.push_back(std::move(target_apk_));
}
- if (auto overlay_apk = ApkAssets::Load(header.GetOverlayPath().to_string())) {
+ if (auto overlay_apk = ApkAssets::Load(header.GetOverlayPath())) {
overlay_am_.SetApkAssets({overlay_apk.get()});
apk_assets_.push_back(std::move(overlay_apk));
}
diff --git a/cmds/idmap2/libidmap2/RawPrintVisitor.cpp b/cmds/idmap2/libidmap2/RawPrintVisitor.cpp
index 82f5d26..d7f0739 100644
--- a/cmds/idmap2/libidmap2/RawPrintVisitor.cpp
+++ b/cmds/idmap2/libidmap2/RawPrintVisitor.cpp
@@ -43,20 +43,17 @@
print(header.GetFulfilledPolicies(), "fulfilled policies: %s",
PoliciesToDebugString(header.GetFulfilledPolicies()).c_str());
print(static_cast<uint32_t>(header.GetEnforceOverlayable()), "enforce overlayable");
- print(header.GetTargetPath().to_string(), kIdmapStringLength, "target path");
- print(header.GetOverlayPath().to_string(), kIdmapStringLength, "overlay path");
+ print(header.GetTargetPath(), true /* print_value */, "target path");
+ print(header.GetOverlayPath(), true /* print_value */, "overlay path");
+ print(header.GetDebugInfo(), false /* print_value */, "debug info");
- uint32_t debug_info_size = header.GetDebugInfo().size();
- print(debug_info_size, "debug info size");
- print("...", debug_info_size + CalculatePadding(debug_info_size), "debug info");
-
- auto target_apk_ = ApkAssets::Load(header.GetTargetPath().to_string());
+ auto target_apk_ = ApkAssets::Load(header.GetTargetPath());
if (target_apk_) {
target_am_.SetApkAssets({target_apk_.get()});
apk_assets_.push_back(std::move(target_apk_));
}
- auto overlay_apk_ = ApkAssets::Load(header.GetOverlayPath().to_string());
+ auto overlay_apk_ = ApkAssets::Load(header.GetOverlayPath());
if (overlay_apk_) {
overlay_am_.SetApkAssets({overlay_apk_.get()});
apk_assets_.push_back(std::move(overlay_apk_));
@@ -100,7 +97,7 @@
print(target_entry.target_id, "target id");
}
- print("...", sizeof(Res_value::size) + sizeof(Res_value::res0), "padding");
+ pad(sizeof(Res_value::size) + sizeof(Res_value::res0));
print(target_entry.value.data_type, "type: %s",
utils::DataTypeToString(target_entry.value.data_type).data());
@@ -143,15 +140,13 @@
}
}
- uint32_t string_pool_size = data.GetStringPoolData().size();
- print(string_pool_size, "string pool size");
- print("...", string_pool_size + CalculatePadding(string_pool_size), "string pool");
+ print(data.GetStringPoolData(), false /* print_value */, "string pool");
}
void RawPrintVisitor::visit(const IdmapData::Header& header) {
print(header.GetTargetPackageId(), "target package id");
print(header.GetOverlayPackageId(), "overlay package id");
- print("...", sizeof(Idmap_data_header::p0), "padding");
+ align();
print(header.GetTargetEntryCount(), "target entry count");
print(header.GetTargetInlineEntryCount(), "target inline entry count");
print(header.GetOverlayEntryCount(), "overlay entry count");
@@ -168,7 +163,6 @@
stream_ << base::StringPrintf("%08zx: %02x", offset_, value) << " " << comment
<< std::endl;
-
offset_ += sizeof(uint8_t);
}
@@ -181,7 +175,6 @@
va_end(ap);
stream_ << base::StringPrintf("%08zx: %04x", offset_, value) << " " << comment << std::endl;
-
offset_ += sizeof(uint16_t);
}
@@ -194,22 +187,35 @@
va_end(ap);
stream_ << base::StringPrintf("%08zx: %08x", offset_, value) << " " << comment << std::endl;
-
offset_ += sizeof(uint32_t);
}
// NOLINTNEXTLINE(cert-dcl50-cpp)
-void RawPrintVisitor::print(const std::string& value, size_t encoded_size, const char* fmt, ...) {
+void RawPrintVisitor::print(const std::string& value, bool print_value, const char* fmt, ...) {
va_list ap;
va_start(ap, fmt);
std::string comment;
base::StringAppendV(&comment, fmt, ap);
va_end(ap);
- stream_ << base::StringPrintf("%08zx: ", offset_) << "........ " << comment << ": " << value
- << std::endl;
+ stream_ << base::StringPrintf("%08zx: %08x", offset_, (uint32_t)value.size()) << " " << comment
+ << " size" << std::endl;
+ offset_ += sizeof(uint32_t);
- offset_ += encoded_size;
+ stream_ << base::StringPrintf("%08zx: ", offset_) << "........ " << comment;
+ offset_ += value.size() + CalculatePadding(value.size());
+
+ if (print_value) {
+ stream_ << ": " << value;
+ }
+ stream_ << std::endl;
}
+void RawPrintVisitor::align() {
+ offset_ += CalculatePadding(offset_);
+}
+
+void RawPrintVisitor::pad(size_t padding) {
+ offset_ += padding;
+}
} // namespace android::idmap2
diff --git a/cmds/idmap2/libidmap2/ResourceMapping.cpp b/cmds/idmap2/libidmap2/ResourceMapping.cpp
index d777cbf..9abb9e4 100644
--- a/cmds/idmap2/libidmap2/ResourceMapping.cpp
+++ b/cmds/idmap2/libidmap2/ResourceMapping.cpp
@@ -230,8 +230,8 @@
base::StringPrintf("%s:%s", target_package->GetPackageName().c_str(), name->c_str());
auto target_resource_result = target_am->GetResourceId(full_name);
if (!target_resource_result.has_value()) {
- log_info.Warning(LogMessage() << "failed to find resource \"" << full_name
- << "\" in target resources");
+ log_info.Warning(LogMessage()
+ << "failed to find resource \"" << full_name << "\" in target resources");
continue;
}