Improve how we exempt resources from getting their names collapsed.
Removed the --whitelist-path flag, which is poorly named and we can already specify these resources with --resources-config-path. Renamed TableFlattenerOptions.whitelisted_resources to keep_resources. It holds ResourceName instead of std::string. This lets us include type when specifying what gets exempted, for correctness. Bug: 111115201 Test: make aapt2_tests Change-Id: Ifa5df924b5e2265c32cdcf8ca7dfa4a3992a0468
This commit is contained in:
@@ -53,9 +53,9 @@ using ::android::ConfigDescription;
|
||||
using ::android::ResTable_config;
|
||||
using ::android::StringPiece;
|
||||
using ::android::base::ReadFileToString;
|
||||
using ::android::base::WriteStringToFile;
|
||||
using ::android::base::StringAppendF;
|
||||
using ::android::base::StringPrintf;
|
||||
using ::android::base::WriteStringToFile;
|
||||
|
||||
namespace aapt {
|
||||
|
||||
@@ -300,29 +300,7 @@ class Optimizer {
|
||||
OptimizeContext* context_;
|
||||
};
|
||||
|
||||
bool ExtractObfuscationWhitelistFromConfig(const std::string& path, OptimizeContext* context,
|
||||
OptimizeOptions* options) {
|
||||
std::string contents;
|
||||
if (!ReadFileToString(path, &contents, true)) {
|
||||
context->GetDiagnostics()->Error(DiagMessage()
|
||||
<< "failed to parse whitelist from config file: " << path);
|
||||
return false;
|
||||
}
|
||||
for (StringPiece resource_name : util::Tokenize(contents, ',')) {
|
||||
options->table_flattener_options.whitelisted_resources.insert(
|
||||
resource_name.to_string());
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
bool ExtractConfig(const std::string& path, OptimizeContext* context,
|
||||
OptimizeOptions* options) {
|
||||
std::string content;
|
||||
if (!android::base::ReadFileToString(path, &content, true /*follow_symlinks*/)) {
|
||||
context->GetDiagnostics()->Error(DiagMessage(path) << "failed reading whitelist");
|
||||
return false;
|
||||
}
|
||||
|
||||
bool ParseConfig(const std::string& content, IAaptContext* context, OptimizeOptions* options) {
|
||||
size_t line_no = 0;
|
||||
for (StringPiece line : util::Tokenize(content, '\n')) {
|
||||
line_no++;
|
||||
@@ -351,15 +329,24 @@ bool ExtractConfig(const std::string& path, OptimizeContext* context,
|
||||
for (StringPiece directive : util::Tokenize(directives, ',')) {
|
||||
if (directive == "remove") {
|
||||
options->resources_blacklist.insert(resource_name.ToResourceName());
|
||||
} else if (directive == "no_obfuscate") {
|
||||
options->table_flattener_options.whitelisted_resources.insert(
|
||||
resource_name.entry.to_string());
|
||||
} else if (directive == "no_collapse" || directive == "no_obfuscate") {
|
||||
options->table_flattener_options.name_collapse_exemptions.insert(
|
||||
resource_name.ToResourceName());
|
||||
}
|
||||
}
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
bool ExtractConfig(const std::string& path, IAaptContext* context, OptimizeOptions* options) {
|
||||
std::string content;
|
||||
if (!android::base::ReadFileToString(path, &content, true /*follow_symlinks*/)) {
|
||||
context->GetDiagnostics()->Error(DiagMessage(path) << "failed reading config file");
|
||||
return false;
|
||||
}
|
||||
return ParseConfig(content, context, options);
|
||||
}
|
||||
|
||||
bool ExtractAppDataFromManifest(OptimizeContext* context, const LoadedApk* apk,
|
||||
OptimizeOptions* out_options) {
|
||||
const xml::XmlResource* manifest = apk->GetManifest();
|
||||
@@ -467,15 +454,6 @@ int OptimizeCommand::Action(const std::vector<std::string>& args) {
|
||||
}
|
||||
}
|
||||
|
||||
if (options_.table_flattener_options.collapse_key_stringpool) {
|
||||
if (whitelist_path_) {
|
||||
std::string& path = whitelist_path_.value();
|
||||
if (!ExtractObfuscationWhitelistFromConfig(path, &context, &options_)) {
|
||||
return 1;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (resources_config_path_) {
|
||||
std::string& path = resources_config_path_.value();
|
||||
if (!ExtractConfig(path, &context, &options_)) {
|
||||
|
||||
@@ -78,10 +78,6 @@ class OptimizeCommand : public Command {
|
||||
"All the resources that would be unused on devices of the given densities will be \n"
|
||||
"removed from the APK.",
|
||||
&target_densities_);
|
||||
AddOptionalFlag("--whitelist-path",
|
||||
"Path to the whitelist.cfg file containing whitelisted resources \n"
|
||||
"whose names should not be altered in final resource tables.",
|
||||
&whitelist_path_);
|
||||
AddOptionalFlag("--resources-config-path",
|
||||
"Path to the resources.cfg file containing the list of resources and \n"
|
||||
"directives to each resource. \n"
|
||||
@@ -125,7 +121,6 @@ class OptimizeCommand : public Command {
|
||||
const std::string &file_path);
|
||||
|
||||
Maybe<std::string> config_path_;
|
||||
Maybe<std::string> whitelist_path_;
|
||||
Maybe<std::string> resources_config_path_;
|
||||
Maybe<std::string> target_densities_;
|
||||
std::vector<std::string> configs_;
|
||||
|
||||
68
tools/aapt2/cmd/Optimize_test.cpp
Normal file
68
tools/aapt2/cmd/Optimize_test.cpp
Normal file
@@ -0,0 +1,68 @@
|
||||
/*
|
||||
* Copyright (C) 2019 The Android Open Source Project
|
||||
*
|
||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||
* you may not use this file except in compliance with the License.
|
||||
* You may obtain a copy of the License at
|
||||
*
|
||||
* http://www.apache.org/licenses/LICENSE-2.0
|
||||
*
|
||||
* Unless required by applicable law or agreed to in writing, software
|
||||
* distributed under the License is distributed on an "AS IS" BASIS,
|
||||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
* See the License for the specific language governing permissions and
|
||||
* limitations under the License.
|
||||
*/
|
||||
|
||||
#include "Optimize.h"
|
||||
|
||||
#include "AppInfo.h"
|
||||
#include "Diagnostics.h"
|
||||
#include "LoadedApk.h"
|
||||
#include "Resource.h"
|
||||
#include "test/Test.h"
|
||||
|
||||
using testing::Contains;
|
||||
using testing::Eq;
|
||||
|
||||
namespace aapt {
|
||||
|
||||
bool ParseConfig(const std::string&, IAaptContext*, OptimizeOptions*);
|
||||
|
||||
using OptimizeTest = CommandTestFixture;
|
||||
|
||||
TEST_F(OptimizeTest, ParseConfigWithNoCollapseExemptions) {
|
||||
const std::string& content = R"(
|
||||
string/foo#no_collapse
|
||||
dimen/bar#no_collapse
|
||||
)";
|
||||
aapt::test::Context context;
|
||||
OptimizeOptions options;
|
||||
ParseConfig(content, &context, &options);
|
||||
|
||||
const std::set<ResourceName>& name_collapse_exemptions =
|
||||
options.table_flattener_options.name_collapse_exemptions;
|
||||
|
||||
ASSERT_THAT(name_collapse_exemptions.size(), Eq(2));
|
||||
EXPECT_THAT(name_collapse_exemptions, Contains(ResourceName({}, ResourceType::kString, "foo")));
|
||||
EXPECT_THAT(name_collapse_exemptions, Contains(ResourceName({}, ResourceType::kDimen, "bar")));
|
||||
}
|
||||
|
||||
TEST_F(OptimizeTest, ParseConfigWithNoObfuscateExemptions) {
|
||||
const std::string& content = R"(
|
||||
string/foo#no_obfuscate
|
||||
dimen/bar#no_obfuscate
|
||||
)";
|
||||
aapt::test::Context context;
|
||||
OptimizeOptions options;
|
||||
ParseConfig(content, &context, &options);
|
||||
|
||||
const std::set<ResourceName>& name_collapse_exemptions =
|
||||
options.table_flattener_options.name_collapse_exemptions;
|
||||
|
||||
ASSERT_THAT(name_collapse_exemptions.size(), Eq(2));
|
||||
EXPECT_THAT(name_collapse_exemptions, Contains(ResourceName({}, ResourceType::kString, "foo")));
|
||||
EXPECT_THAT(name_collapse_exemptions, Contains(ResourceName({}, ResourceType::kDimen, "bar")));
|
||||
}
|
||||
|
||||
} // namespace aapt
|
||||
@@ -228,14 +228,15 @@ class PackageFlattener {
|
||||
public:
|
||||
PackageFlattener(IAaptContext* context, ResourceTablePackage* package,
|
||||
const std::map<size_t, std::string>* shared_libs, bool use_sparse_entries,
|
||||
bool collapse_key_stringpool, const std::set<std::string>& whitelisted_resources)
|
||||
bool collapse_key_stringpool,
|
||||
const std::set<ResourceName>& name_collapse_exemptions)
|
||||
: context_(context),
|
||||
diag_(context->GetDiagnostics()),
|
||||
package_(package),
|
||||
shared_libs_(shared_libs),
|
||||
use_sparse_entries_(use_sparse_entries),
|
||||
collapse_key_stringpool_(collapse_key_stringpool),
|
||||
whitelisted_resources_(whitelisted_resources) {
|
||||
name_collapse_exemptions_(name_collapse_exemptions) {
|
||||
}
|
||||
|
||||
bool FlattenPackage(BigBuffer* buffer) {
|
||||
@@ -652,11 +653,12 @@ class PackageFlattener {
|
||||
|
||||
for (ResourceEntry* entry : sorted_entries) {
|
||||
uint32_t local_key_index;
|
||||
ResourceName resource_name({}, type->type, entry->name);
|
||||
if (!collapse_key_stringpool_ ||
|
||||
whitelisted_resources_.find(entry->name) != whitelisted_resources_.end()) {
|
||||
name_collapse_exemptions_.find(resource_name) != name_collapse_exemptions_.end()) {
|
||||
local_key_index = (uint32_t)key_pool_.MakeRef(entry->name).index();
|
||||
} else {
|
||||
// resource isn't whitelisted, add it as obfuscated value
|
||||
// resource isn't exempt from collapse, add it as obfuscated value
|
||||
local_key_index = (uint32_t)key_pool_.MakeRef(obfuscated_resource_name).index();
|
||||
}
|
||||
// Group values by configuration.
|
||||
@@ -712,7 +714,7 @@ class PackageFlattener {
|
||||
StringPool type_pool_;
|
||||
StringPool key_pool_;
|
||||
bool collapse_key_stringpool_;
|
||||
const std::set<std::string>& whitelisted_resources_;
|
||||
const std::set<ResourceName>& name_collapse_exemptions_;
|
||||
};
|
||||
|
||||
} // namespace
|
||||
@@ -760,7 +762,7 @@ bool TableFlattener::Consume(IAaptContext* context, ResourceTable* table) {
|
||||
|
||||
PackageFlattener flattener(context, package.get(), &table->included_packages_,
|
||||
options_.use_sparse_entries, options_.collapse_key_stringpool,
|
||||
options_.whitelisted_resources);
|
||||
options_.name_collapse_exemptions);
|
||||
if (!flattener.FlattenPackage(&package_buffer)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
@@ -19,6 +19,7 @@
|
||||
|
||||
#include "android-base/macros.h"
|
||||
|
||||
#include "Resource.h"
|
||||
#include "ResourceTable.h"
|
||||
#include "process/IResourceTableConsumer.h"
|
||||
#include "util/BigBuffer.h"
|
||||
@@ -41,8 +42,8 @@ struct TableFlattenerOptions {
|
||||
// have name indices that point to this single value
|
||||
bool collapse_key_stringpool = false;
|
||||
|
||||
// Set of whitelisted resource names to avoid altering in key stringpool
|
||||
std::set<std::string> whitelisted_resources;
|
||||
// Set of resources to avoid collapsing to a single entry in key stringpool.
|
||||
std::set<ResourceName> name_collapse_exemptions;
|
||||
|
||||
// Map from original resource paths to shortened resource paths.
|
||||
std::map<std::string, std::string> shortened_path_map;
|
||||
|
||||
@@ -518,7 +518,7 @@ TEST_F(TableFlattenerTest, LongSharedLibraryPackageNameIsIllegal) {
|
||||
ASSERT_FALSE(Flatten(context.get(), {}, table.get(), &result));
|
||||
}
|
||||
|
||||
TEST_F(TableFlattenerTest, ObfuscatingResourceNamesNoWhitelistSucceeds) {
|
||||
TEST_F(TableFlattenerTest, ObfuscatingResourceNamesNoNameCollapseExemptionsSucceeds) {
|
||||
std::unique_ptr<ResourceTable> table =
|
||||
test::ResourceTableBuilder()
|
||||
.SetPackageId("com.app.test", 0x7f)
|
||||
@@ -572,7 +572,7 @@ TEST_F(TableFlattenerTest, ObfuscatingResourceNamesNoWhitelistSucceeds) {
|
||||
ResourceId(0x7f050000), {}, Res_value::TYPE_STRING, (uint32_t)idx, 0u));
|
||||
}
|
||||
|
||||
TEST_F(TableFlattenerTest, ObfuscatingResourceNamesWithWhitelistSucceeds) {
|
||||
TEST_F(TableFlattenerTest, ObfuscatingResourceNamesWithNameCollapseExemptionsSucceeds) {
|
||||
std::unique_ptr<ResourceTable> table =
|
||||
test::ResourceTableBuilder()
|
||||
.SetPackageId("com.app.test", 0x7f)
|
||||
@@ -591,21 +591,22 @@ TEST_F(TableFlattenerTest, ObfuscatingResourceNamesWithWhitelistSucceeds) {
|
||||
|
||||
TableFlattenerOptions options;
|
||||
options.collapse_key_stringpool = true;
|
||||
options.whitelisted_resources.insert("test");
|
||||
options.whitelisted_resources.insert("three");
|
||||
options.name_collapse_exemptions.insert(ResourceName({}, ResourceType::kId, "one"));
|
||||
options.name_collapse_exemptions.insert(ResourceName({}, ResourceType::kString, "test"));
|
||||
ResTable res_table;
|
||||
|
||||
ASSERT_TRUE(Flatten(context_.get(), options, table.get(), &res_table));
|
||||
|
||||
EXPECT_TRUE(Exists(&res_table, "com.app.test:id/0_resource_name_obfuscated",
|
||||
EXPECT_TRUE(Exists(&res_table, "com.app.test:id/one",
|
||||
ResourceId(0x7f020000), {}, Res_value::TYPE_INT_BOOLEAN, 0u, 0u));
|
||||
|
||||
EXPECT_TRUE(Exists(&res_table, "com.app.test:id/0_resource_name_obfuscated",
|
||||
ResourceId(0x7f020001), {}, Res_value::TYPE_INT_BOOLEAN, 0u, 0u));
|
||||
|
||||
EXPECT_TRUE(Exists(&res_table, "com.app.test:id/three", ResourceId(0x7f020002), {},
|
||||
Res_value::TYPE_REFERENCE, 0x7f020000u, 0u));
|
||||
EXPECT_TRUE(Exists(&res_table, "com.app.test:id/0_resource_name_obfuscated",
|
||||
ResourceId(0x7f020002), {}, Res_value::TYPE_REFERENCE, 0x7f020000u, 0u));
|
||||
|
||||
// Note that this resource is also named "one", but it's a different type, so gets obfuscated.
|
||||
EXPECT_TRUE(Exists(&res_table, "com.app.test:integer/0_resource_name_obfuscated",
|
||||
ResourceId(0x7f030000), {}, Res_value::TYPE_INT_DEC, 1u,
|
||||
ResTable_config::CONFIG_VERSION));
|
||||
|
||||
Reference in New Issue
Block a user