Fix loaded apk string pool order

Loading in an APk changed the ordering of strings in the string pool.
When loading an apk, assign the strings to the same index as they
are in the ResStringPool.

Bug: 118831219
Test: "aapt2 dump strings left.apk" prints in the correct order,
"aapt2 convert left.apk --output-format binary -o left_binary.apk" has
entries in the correct order, and aapt2_tests

Change-Id: I00014c02195f39c1152a110e90083d9b14e9216e
This commit is contained in:
Ryan Mitchell
2018-11-13 10:40:07 -08:00
parent 67dd91e6a3
commit 4e9a922ede
9 changed files with 158 additions and 107 deletions

View File

@@ -431,7 +431,7 @@ void Debug::DumpHex(const void* data, size_t len) {
void Debug::DumpResStringPool(const android::ResStringPool* pool, text::Printer* printer) {
using namespace android;
if (pool->getError() == NO_INIT) {
printer->Print("String pool is unitialized.\n");
return;
@@ -460,7 +460,7 @@ void Debug::DumpResStringPool(const android::ResStringPool* pool, text::Printer*
const size_t NS = pool->size();
for (size_t s=0; s<NS; s++) {
String8 str = pool->string8ObjectAt(s);
printer->Print(StringPrintf("String #%zd : %s\n", s, str.string()));
printer->Print(StringPrintf("String #%zd: %s\n", s, str.string()));
}
}

View File

@@ -718,7 +718,7 @@ std::unique_ptr<Item> ParseBinaryResValue(const ResourceType& type, const Config
// This must be a FileReference.
std::unique_ptr<FileReference> file_ref =
util::make_unique<FileReference>(dst_pool->MakeRef(
str, StringPool::Context(StringPool::Context::kHighPriority, config)));
str, StringPool::Context(StringPool::Context::kHighPriority, config), data));
if (type == ResourceType::kRaw) {
file_ref->type = ResourceFile::Type::kUnknown;
} else if (util::EndsWith(*file_ref->path, ".xml")) {
@@ -730,7 +730,7 @@ std::unique_ptr<Item> ParseBinaryResValue(const ResourceType& type, const Config
}
// There are no styles associated with this string, so treat it as a simple string.
return util::make_unique<String>(dst_pool->MakeRef(str, StringPool::Context(config)));
return util::make_unique<String>(dst_pool->MakeRef(str, StringPool::Context(config), data));
}
} break;

View File

@@ -165,12 +165,13 @@ StringPool::Ref StringPool::MakeRef(const StringPiece& str) {
return MakeRefImpl(str, Context{}, true);
}
StringPool::Ref StringPool::MakeRef(const StringPiece& str, const Context& context) {
return MakeRefImpl(str, context, true);
StringPool::Ref StringPool::MakeRef(const StringPiece& str, const Context& context,
Maybe<size_t> index) {
return MakeRefImpl(str, context, true, index);
}
StringPool::Ref StringPool::MakeRefImpl(const StringPiece& str, const Context& context,
bool unique) {
bool unique, Maybe<size_t> index) {
if (unique) {
auto range = indexed_strings_.equal_range(str);
for (auto iter = range.first; iter != range.second; ++iter) {
@@ -180,15 +181,26 @@ StringPool::Ref StringPool::MakeRefImpl(const StringPiece& str, const Context& c
}
}
const size_t size = strings_.size();
// Insert the string at the end of the string vector if no index is specified
const size_t insertion_index = index ? index.value() : size;
std::unique_ptr<Entry> entry(new Entry());
entry->value = str.to_string();
entry->context = context;
entry->index_ = strings_.size();
entry->index_ = insertion_index;
entry->ref_ = 0;
entry->pool_ = this;
Entry* borrow = entry.get();
strings_.emplace_back(std::move(entry));
if (insertion_index == size) {
strings_.emplace_back(std::move(entry));
} else {
// Allocate enough space for the string at the index
strings_.resize(std::max(insertion_index + 1, size));
strings_[insertion_index] = std::move(entry);
}
indexed_strings_.insert(std::make_pair(StringPiece(borrow->value), borrow));
return Ref(borrow);
}

View File

@@ -166,7 +166,8 @@ class StringPool {
// Adds a string to the pool, unless it already exists, with a context object that can be used
// when sorting the string pool. Returns a reference to the string in the pool.
Ref MakeRef(const android::StringPiece& str, const Context& context);
Ref MakeRef(const android::StringPiece& str, const Context& context,
Maybe<size_t> index = {});
// Adds a string from another string pool. Returns a reference to the string in the string pool.
Ref MakeRef(const Ref& ref);
@@ -210,7 +211,8 @@ class StringPool {
static bool Flatten(BigBuffer* out, const StringPool& pool, bool utf8, IDiagnostics* diag);
Ref MakeRefImpl(const android::StringPiece& str, const Context& context, bool unique);
Ref MakeRefImpl(const android::StringPiece& str, const Context& context, bool unique,
Maybe<size_t> index = {});
void ReAssignIndices();
std::vector<std::unique_ptr<Entry>> strings_;

View File

@@ -84,6 +84,24 @@ TEST(StringPoolTest, MaintainInsertionOrderIndex) {
EXPECT_THAT(ref_c.index(), Eq(2u));
}
TEST(StringPoolTest, AssignStringIndex) {
StringPool pool;
StringPool::Ref ref_a = pool.MakeRef("0", StringPool::Context{}, 0u);
StringPool::Ref ref_b = pool.MakeRef("1", StringPool::Context{}, 1u);
StringPool::Ref ref_c = pool.MakeRef("5", StringPool::Context{}, 5u);
StringPool::Ref ref_d = pool.MakeRef("2", StringPool::Context{}, 2u);
StringPool::Ref ref_e = pool.MakeRef("4", StringPool::Context{}, 4u);
StringPool::Ref ref_f = pool.MakeRef("3", StringPool::Context{}, 3u);
EXPECT_THAT(ref_a.index(), Eq(0u));
EXPECT_THAT(ref_b.index(), Eq(1u));
EXPECT_THAT(ref_d.index(), Eq(2u));
EXPECT_THAT(ref_f.index(), Eq(3u));
EXPECT_THAT(ref_e.index(), Eq(4u));
EXPECT_THAT(ref_c.index(), Eq(5u));
}
TEST(StringPoolTest, PruneStringsWithNoReferences) {
StringPool pool;

View File

@@ -57,78 +57,6 @@ class IApkSerializer {
Source source_;
};
bool ConvertApk(IAaptContext* context, unique_ptr<LoadedApk> apk, IApkSerializer* serializer,
IArchiveWriter* writer) {
io::IFile* manifest = apk->GetFileCollection()->FindFile(kAndroidManifestPath);
if (!serializer->SerializeXml(apk->GetManifest(), kAndroidManifestPath, true /*utf16*/, writer,
(manifest != nullptr && manifest->WasCompressed())
? ArchiveEntry::kCompress : 0u)) {
context->GetDiagnostics()->Error(DiagMessage(apk->GetSource())
<< "failed to serialize AndroidManifest.xml");
return false;
}
if (apk->GetResourceTable() != nullptr) {
// The table might be modified by below code.
auto converted_table = apk->GetResourceTable();
// Resources
for (const auto& package : converted_table->packages) {
for (const auto& type : package->types) {
for (const auto& entry : type->entries) {
for (const auto& config_value : entry->values) {
FileReference* file = ValueCast<FileReference>(config_value->value.get());
if (file != nullptr) {
if (file->file == nullptr) {
context->GetDiagnostics()->Error(DiagMessage(apk->GetSource())
<< "no file associated with " << *file);
return false;
}
if (!serializer->SerializeFile(file, writer)) {
context->GetDiagnostics()->Error(DiagMessage(apk->GetSource())
<< "failed to serialize file " << *file->path);
return false;
}
} // file
} // config_value
} // entry
} // type
} // package
// Converted resource table
if (!serializer->SerializeTable(converted_table, writer)) {
context->GetDiagnostics()->Error(DiagMessage(apk->GetSource())
<< "failed to serialize the resource table");
return false;
}
}
// Other files
std::unique_ptr<io::IFileCollectionIterator> iterator = apk->GetFileCollection()->Iterator();
while (iterator->HasNext()) {
io::IFile* file = iterator->Next();
std::string path = file->GetSource().path;
// Manifest, resource table and resources have already been taken care of.
if (path == kAndroidManifestPath ||
path == kApkResourceTablePath ||
path == kProtoResourceTablePath ||
path.find("res/") == 0) {
continue;
}
if (!io::CopyFileToArchivePreserveCompression(context, file, path, writer)) {
context->GetDiagnostics()->Error(DiagMessage(apk->GetSource())
<< "failed to copy file " << path);
return false;
}
}
return true;
}
class BinaryApkSerializer : public IApkSerializer {
public:
BinaryApkSerializer(IAaptContext* context, const Source& source,
@@ -323,12 +251,97 @@ class Context : public IAaptContext {
StdErrDiagnostics diag_;
};
int Convert(IAaptContext* context, LoadedApk* apk, IArchiveWriter* output_writer,
ApkFormat output_format, TableFlattenerOptions& options) {
// Do not change the ordering of strings in the values string pool
options.sort_stringpool_entries = false;
unique_ptr<IApkSerializer> serializer;
if (output_format == ApkFormat::kBinary) {
serializer.reset(new BinaryApkSerializer(context, apk->GetSource(), options));
} else if (output_format == ApkFormat::kProto) {
serializer.reset(new ProtoApkSerializer(context, apk->GetSource()));
} else {
context->GetDiagnostics()->Error(DiagMessage(apk->GetSource())
<< "Cannot convert APK to unknown format");
return 1;
}
io::IFile* manifest = apk->GetFileCollection()->FindFile(kAndroidManifestPath);
if (!serializer->SerializeXml(apk->GetManifest(), kAndroidManifestPath, true /*utf16*/,
output_writer, (manifest != nullptr && manifest->WasCompressed())
? ArchiveEntry::kCompress : 0u)) {
context->GetDiagnostics()->Error(DiagMessage(apk->GetSource())
<< "failed to serialize AndroidManifest.xml");
return 1;
}
if (apk->GetResourceTable() != nullptr) {
// The table might be modified by below code.
auto converted_table = apk->GetResourceTable();
// Resources
for (const auto& package : converted_table->packages) {
for (const auto& type : package->types) {
for (const auto& entry : type->entries) {
for (const auto& config_value : entry->values) {
FileReference* file = ValueCast<FileReference>(config_value->value.get());
if (file != nullptr) {
if (file->file == nullptr) {
context->GetDiagnostics()->Error(DiagMessage(apk->GetSource())
<< "no file associated with " << *file);
return 1;
}
if (!serializer->SerializeFile(file, output_writer)) {
context->GetDiagnostics()->Error(DiagMessage(apk->GetSource())
<< "failed to serialize file " << *file->path);
return 1;
}
} // file
} // config_value
} // entry
} // type
} // package
// Converted resource table
if (!serializer->SerializeTable(converted_table, output_writer)) {
context->GetDiagnostics()->Error(DiagMessage(apk->GetSource())
<< "failed to serialize the resource table");
return 1;
}
}
// Other files
std::unique_ptr<io::IFileCollectionIterator> iterator = apk->GetFileCollection()->Iterator();
while (iterator->HasNext()) {
io::IFile* file = iterator->Next();
std::string path = file->GetSource().path;
// Manifest, resource table and resources have already been taken care of.
if (path == kAndroidManifestPath ||
path == kApkResourceTablePath ||
path == kProtoResourceTablePath ||
path.find("res/") == 0) {
continue;
}
if (!io::CopyFileToArchivePreserveCompression(context, file, path, output_writer)) {
context->GetDiagnostics()->Error(DiagMessage(apk->GetSource())
<< "failed to copy file " << path);
return 1;
}
}
return 0;
}
const char* ConvertCommand::kOutputFormatProto = "proto";
const char* ConvertCommand::kOutputFormatBinary = "binary";
int ConvertCommand::Action(const std::vector<std::string>& args) {
if (args.size() != 1) {
std::cerr << "must supply a single proto APK\n";
std::cerr << "must supply a single APK\n";
Usage(&std::cerr);
return 1;
}
@@ -341,34 +354,31 @@ int ConvertCommand::Action(const std::vector<std::string>& args) {
return 1;
}
Maybe<AppInfo> app_info =
ExtractAppInfoFromBinaryManifest(*apk->GetManifest(), context.GetDiagnostics());
Maybe<AppInfo> app_info = ExtractAppInfoFromBinaryManifest(*apk->GetManifest(),
context.GetDiagnostics());
if (!app_info) {
return 1;
}
context.package_ = app_info.value().package;
unique_ptr<IArchiveWriter> writer =
CreateZipFileArchiveWriter(context.GetDiagnostics(), output_path_);
unique_ptr<IArchiveWriter> writer = CreateZipFileArchiveWriter(context.GetDiagnostics(),
output_path_);
if (writer == nullptr) {
return 1;
}
unique_ptr<IApkSerializer> serializer;
ApkFormat format;
if (!output_format_ || output_format_.value() == ConvertCommand::kOutputFormatBinary) {
serializer.reset(new BinaryApkSerializer(&context, apk->GetSource(), options_));
format = ApkFormat::kBinary;
} else if (output_format_.value() == ConvertCommand::kOutputFormatProto) {
serializer.reset(new ProtoApkSerializer(&context, apk->GetSource()));
format = ApkFormat::kProto;
} else {
context.GetDiagnostics()->Error(DiagMessage(path)
<< "Invalid value for flag --output-format: "
<< output_format_.value());
context.GetDiagnostics()->Error(DiagMessage(path) << "Invalid value for flag --output-format: "
<< output_format_.value());
return 1;
}
return ConvertApk(&context, std::move(apk), serializer.get(), writer.get()) ? 0 : 1;
return Convert(&context, apk.get(), writer.get(), format, options_);
}
} // namespace aapt

View File

@@ -18,6 +18,7 @@
#define AAPT2_CONVERT_H
#include "Command.h"
#include "LoadedApk.h"
#include "format/binary/TableFlattener.h"
namespace aapt {
@@ -49,6 +50,9 @@ class ConvertCommand : public Command {
bool verbose_ = false;
};
}// namespace aapt
int Convert(IAaptContext* context, LoadedApk* input, IArchiveWriter* output_writer,
ApkFormat output_format, TableFlattenerOptions& options);
} // namespace aapt
#endif //AAPT2_CONVERT_H

View File

@@ -573,15 +573,17 @@ class PackageFlattener {
} // namespace
bool TableFlattener::Consume(IAaptContext* context, ResourceTable* table) {
// We must do this before writing the resources, since the string pool IDs may change.
table->string_pool.Prune();
table->string_pool.Sort([](const StringPool::Context& a, const StringPool::Context& b) -> int {
int diff = util::compare(a.priority, b.priority);
if (diff == 0) {
diff = a.config.compare(b.config);
}
return diff;
});
if (options_.sort_stringpool_entries) {
// We must do this before writing the resources, since the string pool IDs may change.
table->string_pool.Prune();
table->string_pool.Sort([](const StringPool::Context &a, const StringPool::Context &b) -> int {
int diff = util::compare(a.priority, b.priority);
if (diff == 0) {
diff = a.config.compare(b.config);
}
return diff;
});
}
// Write the ResTable header.
ChunkWriter table_writer(buffer_);

View File

@@ -43,6 +43,9 @@ struct TableFlattenerOptions {
// Set of whitelisted resource names to avoid altering in key stringpool
std::set<std::string> whitelisted_resources;
// When true, sort the entries in the values string pool by priority and configuration.
bool sort_stringpool_entries = true;
};
class TableFlattener : public IResourceTableConsumer {