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
diff --git a/tools/aapt2/Debug.cpp b/tools/aapt2/Debug.cpp
index 0bc5221..583f14a 100644
--- a/tools/aapt2/Debug.cpp
+++ b/tools/aapt2/Debug.cpp
@@ -431,7 +431,7 @@
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 @@
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()));
}
}
diff --git a/tools/aapt2/ResourceUtils.cpp b/tools/aapt2/ResourceUtils.cpp
index dbe5ac5..da22e88 100644
--- a/tools/aapt2/ResourceUtils.cpp
+++ b/tools/aapt2/ResourceUtils.cpp
@@ -718,7 +718,7 @@
// 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 @@
}
// 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;
diff --git a/tools/aapt2/StringPool.cpp b/tools/aapt2/StringPool.cpp
index 8eabd32..a8c2666 100644
--- a/tools/aapt2/StringPool.cpp
+++ b/tools/aapt2/StringPool.cpp
@@ -165,12 +165,13 @@
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 @@
}
}
+ 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);
}
diff --git a/tools/aapt2/StringPool.h b/tools/aapt2/StringPool.h
index 1006ca9..115d5d3 100644
--- a/tools/aapt2/StringPool.h
+++ b/tools/aapt2/StringPool.h
@@ -166,7 +166,8 @@
// 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 @@
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_;
diff --git a/tools/aapt2/StringPool_test.cpp b/tools/aapt2/StringPool_test.cpp
index 9a7238b..648be7d 100644
--- a/tools/aapt2/StringPool_test.cpp
+++ b/tools/aapt2/StringPool_test.cpp
@@ -84,6 +84,24 @@
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;
diff --git a/tools/aapt2/cmd/Convert.cpp b/tools/aapt2/cmd/Convert.cpp
index 3ea1755..4492f6b 100644
--- a/tools/aapt2/cmd/Convert.cpp
+++ b/tools/aapt2/cmd/Convert.cpp
@@ -57,78 +57,6 @@
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 @@
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 @@
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
diff --git a/tools/aapt2/cmd/Convert.h b/tools/aapt2/cmd/Convert.h
index fcec23d..6a6719c 100644
--- a/tools/aapt2/cmd/Convert.h
+++ b/tools/aapt2/cmd/Convert.h
@@ -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 @@
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
diff --git a/tools/aapt2/format/binary/TableFlattener.cpp b/tools/aapt2/format/binary/TableFlattener.cpp
index 8a86f63a..6c1a9ba 100644
--- a/tools/aapt2/format/binary/TableFlattener.cpp
+++ b/tools/aapt2/format/binary/TableFlattener.cpp
@@ -573,15 +573,17 @@
} // 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_);
diff --git a/tools/aapt2/format/binary/TableFlattener.h b/tools/aapt2/format/binary/TableFlattener.h
index c2e1d4b..635cb21 100644
--- a/tools/aapt2/format/binary/TableFlattener.h
+++ b/tools/aapt2/format/binary/TableFlattener.h
@@ -43,6 +43,9 @@
// 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 {