Merge "AAPT2: Allow arbitrary entry names with aapt2 optimize" into oc-dev

This commit is contained in:
Adam Lesinski
2017-03-30 00:30:28 +00:00
committed by Android (Google) Code Review
3 changed files with 95 additions and 97 deletions

View File

@@ -32,22 +32,19 @@ using android::StringPiece;
namespace aapt { namespace aapt {
static bool less_than_type(const std::unique_ptr<ResourceTableType>& lhs, static bool less_than_type(const std::unique_ptr<ResourceTableType>& lhs, ResourceType rhs) {
ResourceType rhs) {
return lhs->type < rhs; return lhs->type < rhs;
} }
template <typename T> template <typename T>
static bool less_than_struct_with_name(const std::unique_ptr<T>& lhs, static bool less_than_struct_with_name(const std::unique_ptr<T>& lhs, const StringPiece& rhs) {
const StringPiece& rhs) {
return lhs->name.compare(0, lhs->name.size(), rhs.data(), rhs.size()) < 0; return lhs->name.compare(0, lhs->name.size(), rhs.data(), rhs.size()) < 0;
} }
ResourceTablePackage* ResourceTable::FindPackage(const StringPiece& name) { ResourceTablePackage* ResourceTable::FindPackage(const StringPiece& name) {
const auto last = packages.end(); const auto last = packages.end();
auto iter = auto iter = std::lower_bound(packages.begin(), last, name,
std::lower_bound(packages.begin(), last, name, less_than_struct_with_name<ResourceTablePackage>);
less_than_struct_with_name<ResourceTablePackage>);
if (iter != last && name == (*iter)->name) { if (iter != last && name == (*iter)->name) {
return iter->get(); return iter->get();
} }
@@ -63,8 +60,7 @@ ResourceTablePackage* ResourceTable::FindPackageById(uint8_t id) {
return nullptr; return nullptr;
} }
ResourceTablePackage* ResourceTable::CreatePackage(const StringPiece& name, ResourceTablePackage* ResourceTable::CreatePackage(const StringPiece& name, Maybe<uint8_t> id) {
Maybe<uint8_t> id) {
ResourceTablePackage* package = FindOrCreatePackage(name); ResourceTablePackage* package = FindOrCreatePackage(name);
if (id && !package->id) { if (id && !package->id) {
package->id = id; package->id = id;
@@ -77,18 +73,15 @@ ResourceTablePackage* ResourceTable::CreatePackage(const StringPiece& name,
return package; return package;
} }
ResourceTablePackage* ResourceTable::FindOrCreatePackage( ResourceTablePackage* ResourceTable::FindOrCreatePackage(const StringPiece& name) {
const StringPiece& name) {
const auto last = packages.end(); const auto last = packages.end();
auto iter = auto iter = std::lower_bound(packages.begin(), last, name,
std::lower_bound(packages.begin(), last, name, less_than_struct_with_name<ResourceTablePackage>);
less_than_struct_with_name<ResourceTablePackage>);
if (iter != last && name == (*iter)->name) { if (iter != last && name == (*iter)->name) {
return iter->get(); return iter->get();
} }
std::unique_ptr<ResourceTablePackage> new_package = std::unique_ptr<ResourceTablePackage> new_package = util::make_unique<ResourceTablePackage>();
util::make_unique<ResourceTablePackage>();
new_package->name = name.to_string(); new_package->name = name.to_string();
return packages.emplace(iter, std::move(new_package))->get(); return packages.emplace(iter, std::move(new_package))->get();
} }
@@ -113,8 +106,8 @@ ResourceTableType* ResourceTablePackage::FindOrCreateType(ResourceType type) {
ResourceEntry* ResourceTableType::FindEntry(const StringPiece& name) { ResourceEntry* ResourceTableType::FindEntry(const StringPiece& name) {
const auto last = entries.end(); const auto last = entries.end();
auto iter = std::lower_bound(entries.begin(), last, name, auto iter =
less_than_struct_with_name<ResourceEntry>); std::lower_bound(entries.begin(), last, name, less_than_struct_with_name<ResourceEntry>);
if (iter != last && name == (*iter)->name) { if (iter != last && name == (*iter)->name) {
return iter->get(); return iter->get();
} }
@@ -123,8 +116,8 @@ ResourceEntry* ResourceTableType::FindEntry(const StringPiece& name) {
ResourceEntry* ResourceTableType::FindOrCreateEntry(const StringPiece& name) { ResourceEntry* ResourceTableType::FindOrCreateEntry(const StringPiece& name) {
auto last = entries.end(); auto last = entries.end();
auto iter = std::lower_bound(entries.begin(), last, name, auto iter =
less_than_struct_with_name<ResourceEntry>); std::lower_bound(entries.begin(), last, name, less_than_struct_with_name<ResourceEntry>);
if (iter != last && name == (*iter)->name) { if (iter != last && name == (*iter)->name) {
return iter->get(); return iter->get();
} }
@@ -140,8 +133,7 @@ struct ConfigKey {
const StringPiece& product; const StringPiece& product;
}; };
bool ltConfigKeyRef(const std::unique_ptr<ResourceConfigValue>& lhs, bool ltConfigKeyRef(const std::unique_ptr<ResourceConfigValue>& lhs, const ConfigKey& rhs) {
const ConfigKey& rhs) {
int cmp = lhs->config.compare(*rhs.config); int cmp = lhs->config.compare(*rhs.config);
if (cmp == 0) { if (cmp == 0) {
cmp = StringPiece(lhs->product).compare(rhs.product); cmp = StringPiece(lhs->product).compare(rhs.product);
@@ -151,8 +143,8 @@ bool ltConfigKeyRef(const std::unique_ptr<ResourceConfigValue>& lhs,
ResourceConfigValue* ResourceEntry::FindValue(const ConfigDescription& config, ResourceConfigValue* ResourceEntry::FindValue(const ConfigDescription& config,
const StringPiece& product) { const StringPiece& product) {
auto iter = std::lower_bound(values.begin(), values.end(), auto iter =
ConfigKey{&config, product}, ltConfigKeyRef); std::lower_bound(values.begin(), values.end(), ConfigKey{&config, product}, ltConfigKeyRef);
if (iter != values.end()) { if (iter != values.end()) {
ResourceConfigValue* value = iter->get(); ResourceConfigValue* value = iter->get();
if (value->config == config && StringPiece(value->product) == product) { if (value->config == config && StringPiece(value->product) == product) {
@@ -162,10 +154,10 @@ ResourceConfigValue* ResourceEntry::FindValue(const ConfigDescription& config,
return nullptr; return nullptr;
} }
ResourceConfigValue* ResourceEntry::FindOrCreateValue( ResourceConfigValue* ResourceEntry::FindOrCreateValue(const ConfigDescription& config,
const ConfigDescription& config, const StringPiece& product) { const StringPiece& product) {
auto iter = std::lower_bound(values.begin(), values.end(), auto iter =
ConfigKey{&config, product}, ltConfigKeyRef); std::lower_bound(values.begin(), values.end(), ConfigKey{&config, product}, ltConfigKeyRef);
if (iter != values.end()) { if (iter != values.end()) {
ResourceConfigValue* value = iter->get(); ResourceConfigValue* value = iter->get();
if (value->config == config && StringPiece(value->product) == product) { if (value->config == config && StringPiece(value->product) == product) {
@@ -173,14 +165,11 @@ ResourceConfigValue* ResourceEntry::FindOrCreateValue(
} }
} }
ResourceConfigValue* newValue = ResourceConfigValue* newValue =
values values.insert(iter, util::make_unique<ResourceConfigValue>(config, product))->get();
.insert(iter, util::make_unique<ResourceConfigValue>(config, product))
->get();
return newValue; return newValue;
} }
std::vector<ResourceConfigValue*> ResourceEntry::findAllValues( std::vector<ResourceConfigValue*> ResourceEntry::FindAllValues(const ConfigDescription& config) {
const ConfigDescription& config) {
std::vector<ResourceConfigValue*> results; std::vector<ResourceConfigValue*> results;
auto iter = values.begin(); auto iter = values.begin();
@@ -237,8 +226,8 @@ std::vector<ResourceConfigValue*> ResourceEntry::FindValuesIf(
* format for there to be * format for there to be
* no error. * no error.
*/ */
ResourceTable::CollisionResult ResourceTable::ResolveValueCollision( ResourceTable::CollisionResult ResourceTable::ResolveValueCollision(Value* existing,
Value* existing, Value* incoming) { Value* incoming) {
Attribute* existing_attr = ValueCast<Attribute>(existing); Attribute* existing_attr = ValueCast<Attribute>(existing);
Attribute* incoming_attr = ValueCast<Attribute>(incoming); Attribute* incoming_attr = ValueCast<Attribute>(incoming);
if (!incoming_attr) { if (!incoming_attr) {
@@ -278,18 +267,15 @@ ResourceTable::CollisionResult ResourceTable::ResolveValueCollision(
// The two attributes are both DECLs, but they are plain attributes // The two attributes are both DECLs, but they are plain attributes
// with the same formats. // with the same formats.
// Keep the strongest one. // Keep the strongest one.
return existing_attr->IsWeak() ? CollisionResult::kTakeNew return existing_attr->IsWeak() ? CollisionResult::kTakeNew : CollisionResult::kKeepOriginal;
: CollisionResult::kKeepOriginal;
} }
if (existing_attr->IsWeak() && if (existing_attr->IsWeak() && existing_attr->type_mask == android::ResTable_map::TYPE_ANY) {
existing_attr->type_mask == android::ResTable_map::TYPE_ANY) {
// Any incoming attribute is better than this. // Any incoming attribute is better than this.
return CollisionResult::kTakeNew; return CollisionResult::kTakeNew;
} }
if (incoming_attr->IsWeak() && if (incoming_attr->IsWeak() && incoming_attr->type_mask == android::ResTable_map::TYPE_ANY) {
incoming_attr->type_mask == android::ResTable_map::TYPE_ANY) {
// The incoming attribute may be a USE instead of a DECL. // The incoming attribute may be a USE instead of a DECL.
// Keep the existing attribute. // Keep the existing attribute.
return CollisionResult::kKeepOriginal; return CollisionResult::kKeepOriginal;
@@ -298,15 +284,26 @@ ResourceTable::CollisionResult ResourceTable::ResolveValueCollision(
} }
static constexpr const char* kValidNameChars = "._-"; static constexpr const char* kValidNameChars = "._-";
static constexpr const char* kValidNameMangledChars = "._-$";
static StringPiece ValidateName(const StringPiece& name) {
auto iter = util::FindNonAlphaNumericAndNotInSet(name, kValidNameChars);
if (iter != name.end()) {
return StringPiece(iter, 1);
}
return {};
}
static StringPiece SkipValidateName(const StringPiece& /*name*/) {
return {};
}
bool ResourceTable::AddResource(const ResourceNameRef& name, bool ResourceTable::AddResource(const ResourceNameRef& name,
const ConfigDescription& config, const ConfigDescription& config,
const StringPiece& product, const StringPiece& product,
std::unique_ptr<Value> value, std::unique_ptr<Value> value,
IDiagnostics* diag) { IDiagnostics* diag) {
return AddResourceImpl(name, {}, config, product, std::move(value), return AddResourceImpl(name, {}, config, product, std::move(value), ValidateName,
kValidNameChars, ResolveValueCollision, diag); ResolveValueCollision, diag);
} }
bool ResourceTable::AddResource(const ResourceNameRef& name, bool ResourceTable::AddResource(const ResourceNameRef& name,
@@ -315,8 +312,8 @@ bool ResourceTable::AddResource(const ResourceNameRef& name,
const StringPiece& product, const StringPiece& product,
std::unique_ptr<Value> value, std::unique_ptr<Value> value,
IDiagnostics* diag) { IDiagnostics* diag) {
return AddResourceImpl(name, res_id, config, product, std::move(value), return AddResourceImpl(name, res_id, config, product, std::move(value), ValidateName,
kValidNameChars, ResolveValueCollision, diag); ResolveValueCollision, diag);
} }
bool ResourceTable::AddFileReference(const ResourceNameRef& name, bool ResourceTable::AddFileReference(const ResourceNameRef& name,
@@ -324,29 +321,26 @@ bool ResourceTable::AddFileReference(const ResourceNameRef& name,
const Source& source, const Source& source,
const StringPiece& path, const StringPiece& path,
IDiagnostics* diag) { IDiagnostics* diag) {
return AddFileReferenceImpl(name, config, source, path, nullptr, return AddFileReferenceImpl(name, config, source, path, nullptr, ValidateName, diag);
kValidNameChars, diag);
} }
bool ResourceTable::AddFileReferenceAllowMangled( bool ResourceTable::AddFileReferenceAllowMangled(
const ResourceNameRef& name, const ConfigDescription& config, const ResourceNameRef& name, const ConfigDescription& config,
const Source& source, const StringPiece& path, io::IFile* file, const Source& source, const StringPiece& path, io::IFile* file,
IDiagnostics* diag) { IDiagnostics* diag) {
return AddFileReferenceImpl(name, config, source, path, file, return AddFileReferenceImpl(name, config, source, path, file, SkipValidateName, diag);
kValidNameMangledChars, diag);
} }
bool ResourceTable::AddFileReferenceImpl( bool ResourceTable::AddFileReferenceImpl(const ResourceNameRef& name,
const ResourceNameRef& name, const ConfigDescription& config, const ConfigDescription& config, const Source& source,
const Source& source, const StringPiece& path, io::IFile* file, const StringPiece& path, io::IFile* file,
const char* valid_chars, IDiagnostics* diag) { NameValidator name_validator, IDiagnostics* diag) {
std::unique_ptr<FileReference> fileRef = std::unique_ptr<FileReference> fileRef =
util::make_unique<FileReference>(string_pool.MakeRef(path)); util::make_unique<FileReference>(string_pool.MakeRef(path));
fileRef->SetSource(source); fileRef->SetSource(source);
fileRef->file = file; fileRef->file = file;
return AddResourceImpl(name, ResourceId{}, config, StringPiece{}, return AddResourceImpl(name, ResourceId{}, config, StringPiece{}, std::move(fileRef),
std::move(fileRef), valid_chars, ResolveValueCollision, name_validator, ResolveValueCollision, diag);
diag);
} }
bool ResourceTable::AddResourceAllowMangled(const ResourceNameRef& name, bool ResourceTable::AddResourceAllowMangled(const ResourceNameRef& name,
@@ -354,8 +348,8 @@ bool ResourceTable::AddResourceAllowMangled(const ResourceNameRef& name,
const StringPiece& product, const StringPiece& product,
std::unique_ptr<Value> value, std::unique_ptr<Value> value,
IDiagnostics* diag) { IDiagnostics* diag) {
return AddResourceImpl(name, ResourceId{}, config, product, std::move(value), return AddResourceImpl(name, ResourceId{}, config, product, std::move(value), SkipValidateName,
kValidNameMangledChars, ResolveValueCollision, diag); ResolveValueCollision, diag);
} }
bool ResourceTable::AddResourceAllowMangled(const ResourceNameRef& name, bool ResourceTable::AddResourceAllowMangled(const ResourceNameRef& name,
@@ -364,25 +358,24 @@ bool ResourceTable::AddResourceAllowMangled(const ResourceNameRef& name,
const StringPiece& product, const StringPiece& product,
std::unique_ptr<Value> value, std::unique_ptr<Value> value,
IDiagnostics* diag) { IDiagnostics* diag) {
return AddResourceImpl(name, id, config, product, std::move(value), return AddResourceImpl(name, id, config, product, std::move(value), SkipValidateName,
kValidNameMangledChars, ResolveValueCollision, diag); ResolveValueCollision, diag);
} }
bool ResourceTable::AddResourceImpl( bool ResourceTable::AddResourceImpl(const ResourceNameRef& name, const ResourceId& res_id,
const ResourceNameRef& name, const ResourceId& res_id, const ConfigDescription& config, const StringPiece& product,
const ConfigDescription& config, const StringPiece& product, std::unique_ptr<Value> value, NameValidator name_validator,
std::unique_ptr<Value> value, const char* valid_chars, const CollisionResolverFunc& conflictResolver,
const CollisionResolverFunc& conflictResolver, IDiagnostics* diag) { IDiagnostics* diag) {
CHECK(value != nullptr); CHECK(value != nullptr);
CHECK(diag != nullptr); CHECK(diag != nullptr);
auto bad_char_iter = const StringPiece bad_char = name_validator(name.entry);
util::FindNonAlphaNumericAndNotInSet(name.entry, valid_chars); if (!bad_char.empty()) {
if (bad_char_iter != name.entry.end()) { diag->Error(DiagMessage(value->GetSource()) << "resource '" << name
diag->Error(DiagMessage(value->GetSource()) << "' has invalid entry name '" << name.entry
<< "resource '" << name << "' has invalid entry name '" << "'. Invalid character '" << bad_char << "'");
<< name.entry << "'. Invalid character '"
<< StringPiece(bad_char_iter, 1) << "'");
return false; return false;
} }
@@ -450,30 +443,26 @@ bool ResourceTable::AddResourceImpl(
bool ResourceTable::SetSymbolState(const ResourceNameRef& name, bool ResourceTable::SetSymbolState(const ResourceNameRef& name,
const ResourceId& res_id, const ResourceId& res_id,
const Symbol& symbol, IDiagnostics* diag) { const Symbol& symbol, IDiagnostics* diag) {
return SetSymbolStateImpl(name, res_id, symbol, kValidNameChars, diag); return SetSymbolStateImpl(name, res_id, symbol, ValidateName, diag);
} }
bool ResourceTable::SetSymbolStateAllowMangled(const ResourceNameRef& name, bool ResourceTable::SetSymbolStateAllowMangled(const ResourceNameRef& name,
const ResourceId& res_id, const ResourceId& res_id,
const Symbol& symbol, const Symbol& symbol,
IDiagnostics* diag) { IDiagnostics* diag) {
return SetSymbolStateImpl(name, res_id, symbol, kValidNameMangledChars, diag); return SetSymbolStateImpl(name, res_id, symbol, SkipValidateName, diag);
} }
bool ResourceTable::SetSymbolStateImpl(const ResourceNameRef& name, bool ResourceTable::SetSymbolStateImpl(const ResourceNameRef& name, const ResourceId& res_id,
const ResourceId& res_id, const Symbol& symbol, NameValidator name_validator,
const Symbol& symbol,
const char* valid_chars,
IDiagnostics* diag) { IDiagnostics* diag) {
CHECK(diag != nullptr); CHECK(diag != nullptr);
auto bad_char_iter = const StringPiece bad_char = name_validator(name.entry);
util::FindNonAlphaNumericAndNotInSet(name.entry, valid_chars); if (!bad_char.empty()) {
if (bad_char_iter != name.entry.end()) { diag->Error(DiagMessage(symbol.source) << "resource '" << name << "' has invalid entry name '"
diag->Error(DiagMessage(symbol.source) << name.entry << "'. Invalid character '" << bad_char
<< "resource '" << name << "' has invalid entry name '" << "'");
<< name.entry << "'. Invalid character '"
<< StringPiece(bad_char_iter, 1) << "'");
return false; return false;
} }
@@ -532,8 +521,7 @@ bool ResourceTable::SetSymbolStateImpl(const ResourceNameRef& name,
return true; return true;
} }
Maybe<ResourceTable::SearchResult> ResourceTable::FindResource( Maybe<ResourceTable::SearchResult> ResourceTable::FindResource(const ResourceNameRef& name) {
const ResourceNameRef& name) {
ResourceTablePackage* package = FindPackage(name.package); ResourceTablePackage* package = FindPackage(name.package);
if (!package) { if (!package) {
return {}; return {};

View File

@@ -113,8 +113,7 @@ class ResourceEntry {
const android::StringPiece& product); const android::StringPiece& product);
ResourceConfigValue* FindOrCreateValue(const ConfigDescription& config, ResourceConfigValue* FindOrCreateValue(const ConfigDescription& config,
const android::StringPiece& product); const android::StringPiece& product);
std::vector<ResourceConfigValue*> findAllValues( std::vector<ResourceConfigValue*> FindAllValues(const ConfigDescription& config);
const ConfigDescription& config);
std::vector<ResourceConfigValue*> FindValuesIf( std::vector<ResourceConfigValue*> FindValuesIf(
const std::function<bool(ResourceConfigValue*)>& f); const std::function<bool(ResourceConfigValue*)>& f);
@@ -189,8 +188,7 @@ class ResourceTable {
* When a collision of resources occurs, this method decides which value to * When a collision of resources occurs, this method decides which value to
* keep. * keep.
*/ */
static CollisionResult ResolveValueCollision(Value* existing, static CollisionResult ResolveValueCollision(Value* existing, Value* incoming);
Value* incoming);
bool AddResource(const ResourceNameRef& name, const ConfigDescription& config, bool AddResource(const ResourceNameRef& name, const ConfigDescription& config,
const android::StringPiece& product, std::unique_ptr<Value> value, const android::StringPiece& product, std::unique_ptr<Value> value,
@@ -274,20 +272,24 @@ class ResourceTable {
std::map<size_t, std::string> included_packages_; std::map<size_t, std::string> included_packages_;
private: private:
// The function type that validates a symbol name. Returns a non-empty StringPiece representing
// the offending character (which may be more than one byte in UTF-8). Returns an empty string
// if the name was valid.
using NameValidator = android::StringPiece(const android::StringPiece&);
ResourceTablePackage* FindOrCreatePackage(const android::StringPiece& name); ResourceTablePackage* FindOrCreatePackage(const android::StringPiece& name);
bool AddResourceImpl(const ResourceNameRef& name, const ResourceId& res_id, bool AddResourceImpl(const ResourceNameRef& name, const ResourceId& res_id,
const ConfigDescription& config, const android::StringPiece& product, const ConfigDescription& config, const android::StringPiece& product,
std::unique_ptr<Value> value, const char* valid_chars, std::unique_ptr<Value> value, NameValidator name_validator,
const CollisionResolverFunc& conflict_resolver, IDiagnostics* diag); const CollisionResolverFunc& conflict_resolver, IDiagnostics* diag);
bool AddFileReferenceImpl(const ResourceNameRef& name, const ConfigDescription& config, bool AddFileReferenceImpl(const ResourceNameRef& name, const ConfigDescription& config,
const Source& source, const android::StringPiece& path, io::IFile* file, const Source& source, const android::StringPiece& path, io::IFile* file,
const char* valid_chars, IDiagnostics* diag); NameValidator name_validator, IDiagnostics* diag);
bool SetSymbolStateImpl(const ResourceNameRef& name, const ResourceId& res_id, bool SetSymbolStateImpl(const ResourceNameRef& name, const ResourceId& res_id,
const Symbol& symbol, const char* valid_chars, const Symbol& symbol, NameValidator name_validator, IDiagnostics* diag);
IDiagnostics* diag);
DISALLOW_COPY_AND_ASSIGN(ResourceTable); DISALLOW_COPY_AND_ASSIGN(ResourceTable);
}; };

View File

@@ -40,6 +40,14 @@ TEST(ResourceTableTest, FailToAddResourceWithBadName) {
test::GetDiagnostics())); test::GetDiagnostics()));
} }
TEST(ResourceTableTest, AddResourceWithWeirdNameWhenAddingMangledResources) {
ResourceTable table;
EXPECT_TRUE(table.AddResourceAllowMangled(
test::ParseNameOrDie("android:id/heythere "), ConfigDescription{}, "",
test::ValueBuilder<Id>().SetSource("test.xml", 21u).Build(), test::GetDiagnostics()));
}
TEST(ResourceTableTest, AddOneResource) { TEST(ResourceTableTest, AddOneResource) {
ResourceTable table; ResourceTable table;
@@ -130,7 +138,7 @@ TEST(ResourceTableTest, ProductVaryingValues) {
table.FindResource(test::ParseNameOrDie("android:string/foo")); table.FindResource(test::ParseNameOrDie("android:string/foo"));
AAPT_ASSERT_TRUE(sr); AAPT_ASSERT_TRUE(sr);
std::vector<ResourceConfigValue*> values = std::vector<ResourceConfigValue*> values =
sr.value().entry->findAllValues(test::ParseConfigOrDie("land")); sr.value().entry->FindAllValues(test::ParseConfigOrDie("land"));
ASSERT_EQ(2u, values.size()); ASSERT_EQ(2u, values.size());
EXPECT_EQ(std::string("phone"), values[0]->product); EXPECT_EQ(std::string("phone"), values[0]->product);
EXPECT_EQ(std::string("tablet"), values[1]->product); EXPECT_EQ(std::string("tablet"), values[1]->product);