From 1ab598f46c3ff520a67f9d80194847741f3467ab Mon Sep 17 00:00:00 2001 From: Adam Lesinski Date: Fri, 14 Aug 2015 14:26:04 -0700 Subject: AAPT2: Separate out the various steps An early refactor. Some ideas became clearer as development continued. Now the various phases are much clearer and more easily reusable. Also added a ton of tests! Change-Id: Ic8f0a70c8222370352e63533b329c40457c0903e --- tools/aapt2/ResourceUtils.cpp | 481 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 481 insertions(+) create mode 100644 tools/aapt2/ResourceUtils.cpp (limited to 'tools/aapt2/ResourceUtils.cpp') diff --git a/tools/aapt2/ResourceUtils.cpp b/tools/aapt2/ResourceUtils.cpp new file mode 100644 index 000000000000..0db1c372c901 --- /dev/null +++ b/tools/aapt2/ResourceUtils.cpp @@ -0,0 +1,481 @@ +/* + * Copyright (C) 2015 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 "ResourceUtils.h" +#include "util/Util.h" + +#include +#include + +namespace aapt { +namespace ResourceUtils { + +void extractResourceName(const StringPiece16& str, StringPiece16* outPackage, + StringPiece16* outType, StringPiece16* outEntry) { + const char16_t* start = str.data(); + const char16_t* end = start + str.size(); + const char16_t* current = start; + while (current != end) { + if (outType->size() == 0 && *current == u'/') { + outType->assign(start, current - start); + start = current + 1; + } else if (outPackage->size() == 0 && *current == u':') { + outPackage->assign(start, current - start); + start = current + 1; + } + current++; + } + outEntry->assign(start, end - start); +} + +bool tryParseReference(const StringPiece16& str, ResourceNameRef* outRef, bool* outCreate, + bool* outPrivate) { + StringPiece16 trimmedStr(util::trimWhitespace(str)); + if (trimmedStr.empty()) { + return false; + } + + bool create = false; + bool priv = false; + if (trimmedStr.data()[0] == u'@') { + size_t offset = 1; + if (trimmedStr.data()[1] == u'+') { + create = true; + offset += 1; + } else if (trimmedStr.data()[1] == u'*') { + priv = true; + offset += 1; + } + StringPiece16 package; + StringPiece16 type; + StringPiece16 entry; + extractResourceName(trimmedStr.substr(offset, trimmedStr.size() - offset), &package, &type, + &entry); + + const ResourceType* parsedType = parseResourceType(type); + if (!parsedType) { + return false; + } + + if (create && *parsedType != ResourceType::kId) { + return false; + } + + outRef->package = package; + outRef->type = *parsedType; + outRef->entry = entry; + if (outCreate) { + *outCreate = create; + } + if (outPrivate) { + *outPrivate = priv; + } + return true; + } + return false; +} + +bool tryParseAttributeReference(const StringPiece16& str, ResourceNameRef* outRef) { + StringPiece16 trimmedStr(util::trimWhitespace(str)); + if (trimmedStr.empty()) { + return false; + } + + if (*trimmedStr.data() == u'?') { + StringPiece16 package; + StringPiece16 type; + StringPiece16 entry; + extractResourceName(trimmedStr.substr(1, trimmedStr.size() - 1), &package, &type, &entry); + + if (!type.empty() && type != u"attr") { + return false; + } + + outRef->package = package; + outRef->type = ResourceType::kAttr; + outRef->entry = entry; + return true; + } + return false; +} + +/* + * Style parent's are a bit different. We accept the following formats: + * + * @[package:]style/ + * ?[package:]style/ + * :[style/] + * [package:style/] + */ +Maybe parseStyleParentReference(const StringPiece16& str, std::string* outError) { + if (str.empty()) { + return {}; + } + + StringPiece16 name = str; + + bool hasLeadingIdentifiers = false; + bool privateRef = false; + + // Skip over these identifiers. A style's parent is a normal reference. + if (name.data()[0] == u'@' || name.data()[0] == u'?') { + hasLeadingIdentifiers = true; + name = name.substr(1, name.size() - 1); + if (name.data()[0] == u'*') { + privateRef = true; + name = name.substr(1, name.size() - 1); + } + } + + ResourceNameRef ref; + ref.type = ResourceType::kStyle; + + StringPiece16 typeStr; + extractResourceName(name, &ref.package, &typeStr, &ref.entry); + if (!typeStr.empty()) { + // If we have a type, make sure it is a Style. + const ResourceType* parsedType = parseResourceType(typeStr); + if (!parsedType || *parsedType != ResourceType::kStyle) { + std::stringstream err; + err << "invalid resource type '" << typeStr << "' for parent of style"; + *outError = err.str(); + return {}; + } + } else { + // No type was defined, this should not have a leading identifier. + if (hasLeadingIdentifiers) { + std::stringstream err; + err << "invalid parent reference '" << str << "'"; + *outError = err.str(); + return {}; + } + } + + if (!hasLeadingIdentifiers && ref.package.empty() && !typeStr.empty()) { + std::stringstream err; + err << "invalid parent reference '" << str << "'"; + *outError = err.str(); + return {}; + } + + Reference result(ref); + result.privateReference = privateRef; + return result; +} + +std::unique_ptr tryParseReference(const StringPiece16& str, bool* outCreate) { + ResourceNameRef ref; + bool privateRef = false; + if (tryParseReference(str, &ref, outCreate, &privateRef)) { + std::unique_ptr value = util::make_unique(ref); + value->privateReference = privateRef; + return value; + } + + if (tryParseAttributeReference(str, &ref)) { + if (outCreate) { + *outCreate = false; + } + return util::make_unique(ref, Reference::Type::kAttribute); + } + return {}; +} + +std::unique_ptr tryParseNullOrEmpty(const StringPiece16& str) { + StringPiece16 trimmedStr(util::trimWhitespace(str)); + android::Res_value value = { }; + if (trimmedStr == u"@null") { + // TYPE_NULL with data set to 0 is interpreted by the runtime as an error. + // Instead we set the data type to TYPE_REFERENCE with a value of 0. + value.dataType = android::Res_value::TYPE_REFERENCE; + } else if (trimmedStr == u"@empty") { + // TYPE_NULL with value of DATA_NULL_EMPTY is handled fine by the runtime. + value.dataType = android::Res_value::TYPE_NULL; + value.data = android::Res_value::DATA_NULL_EMPTY; + } else { + return {}; + } + return util::make_unique(value); +} + +std::unique_ptr tryParseEnumSymbol(const Attribute* enumAttr, + const StringPiece16& str) { + StringPiece16 trimmedStr(util::trimWhitespace(str)); + for (const Attribute::Symbol& symbol : enumAttr->symbols) { + // Enum symbols are stored as @package:id/symbol resources, + // so we need to match against the 'entry' part of the identifier. + const ResourceName& enumSymbolResourceName = symbol.symbol.name.value(); + if (trimmedStr == enumSymbolResourceName.entry) { + android::Res_value value = { }; + value.dataType = android::Res_value::TYPE_INT_DEC; + value.data = symbol.value; + return util::make_unique(value); + } + } + return {}; +} + +std::unique_ptr tryParseFlagSymbol(const Attribute* flagAttr, + const StringPiece16& str) { + android::Res_value flags = { }; + flags.dataType = android::Res_value::TYPE_INT_DEC; + + for (StringPiece16 part : util::tokenize(str, u'|')) { + StringPiece16 trimmedPart = util::trimWhitespace(part); + + bool flagSet = false; + for (const Attribute::Symbol& symbol : flagAttr->symbols) { + // Flag symbols are stored as @package:id/symbol resources, + // so we need to match against the 'entry' part of the identifier. + const ResourceName& flagSymbolResourceName = symbol.symbol.name.value(); + if (trimmedPart == flagSymbolResourceName.entry) { + flags.data |= symbol.value; + flagSet = true; + break; + } + } + + if (!flagSet) { + return {}; + } + } + return util::make_unique(flags); +} + +static uint32_t parseHex(char16_t c, bool* outError) { + if (c >= u'0' && c <= u'9') { + return c - u'0'; + } else if (c >= u'a' && c <= u'f') { + return c - u'a' + 0xa; + } else if (c >= u'A' && c <= u'F') { + return c - u'A' + 0xa; + } else { + *outError = true; + return 0xffffffffu; + } +} + +std::unique_ptr tryParseColor(const StringPiece16& str) { + StringPiece16 colorStr(util::trimWhitespace(str)); + const char16_t* start = colorStr.data(); + const size_t len = colorStr.size(); + if (len == 0 || start[0] != u'#') { + return {}; + } + + android::Res_value value = { }; + bool error = false; + if (len == 4) { + value.dataType = android::Res_value::TYPE_INT_COLOR_RGB4; + value.data = 0xff000000u; + value.data |= parseHex(start[1], &error) << 20; + value.data |= parseHex(start[1], &error) << 16; + value.data |= parseHex(start[2], &error) << 12; + value.data |= parseHex(start[2], &error) << 8; + value.data |= parseHex(start[3], &error) << 4; + value.data |= parseHex(start[3], &error); + } else if (len == 5) { + value.dataType = android::Res_value::TYPE_INT_COLOR_ARGB4; + value.data |= parseHex(start[1], &error) << 28; + value.data |= parseHex(start[1], &error) << 24; + value.data |= parseHex(start[2], &error) << 20; + value.data |= parseHex(start[2], &error) << 16; + value.data |= parseHex(start[3], &error) << 12; + value.data |= parseHex(start[3], &error) << 8; + value.data |= parseHex(start[4], &error) << 4; + value.data |= parseHex(start[4], &error); + } else if (len == 7) { + value.dataType = android::Res_value::TYPE_INT_COLOR_RGB8; + value.data = 0xff000000u; + value.data |= parseHex(start[1], &error) << 20; + value.data |= parseHex(start[2], &error) << 16; + value.data |= parseHex(start[3], &error) << 12; + value.data |= parseHex(start[4], &error) << 8; + value.data |= parseHex(start[5], &error) << 4; + value.data |= parseHex(start[6], &error); + } else if (len == 9) { + value.dataType = android::Res_value::TYPE_INT_COLOR_ARGB8; + value.data |= parseHex(start[1], &error) << 28; + value.data |= parseHex(start[2], &error) << 24; + value.data |= parseHex(start[3], &error) << 20; + value.data |= parseHex(start[4], &error) << 16; + value.data |= parseHex(start[5], &error) << 12; + value.data |= parseHex(start[6], &error) << 8; + value.data |= parseHex(start[7], &error) << 4; + value.data |= parseHex(start[8], &error); + } else { + return {}; + } + return error ? std::unique_ptr() : util::make_unique(value); +} + +std::unique_ptr tryParseBool(const StringPiece16& str) { + StringPiece16 trimmedStr(util::trimWhitespace(str)); + uint32_t data = 0; + if (trimmedStr == u"true" || trimmedStr == u"TRUE") { + data = 0xffffffffu; + } else if (trimmedStr != u"false" && trimmedStr != u"FALSE") { + return {}; + } + android::Res_value value = { }; + value.dataType = android::Res_value::TYPE_INT_BOOLEAN; + value.data = data; + return util::make_unique(value); +} + +std::unique_ptr tryParseInt(const StringPiece16& str) { + android::Res_value value; + if (!android::ResTable::stringToInt(str.data(), str.size(), &value)) { + return {}; + } + return util::make_unique(value); +} + +std::unique_ptr tryParseFloat(const StringPiece16& str) { + android::Res_value value; + if (!android::ResTable::stringToFloat(str.data(), str.size(), &value)) { + return {}; + } + return util::make_unique(value); +} + +uint32_t androidTypeToAttributeTypeMask(uint16_t type) { + switch (type) { + case android::Res_value::TYPE_NULL: + case android::Res_value::TYPE_REFERENCE: + case android::Res_value::TYPE_ATTRIBUTE: + case android::Res_value::TYPE_DYNAMIC_REFERENCE: + return android::ResTable_map::TYPE_REFERENCE; + + case android::Res_value::TYPE_STRING: + return android::ResTable_map::TYPE_STRING; + + case android::Res_value::TYPE_FLOAT: + return android::ResTable_map::TYPE_FLOAT; + + case android::Res_value::TYPE_DIMENSION: + return android::ResTable_map::TYPE_DIMENSION; + + case android::Res_value::TYPE_FRACTION: + return android::ResTable_map::TYPE_FRACTION; + + case android::Res_value::TYPE_INT_DEC: + case android::Res_value::TYPE_INT_HEX: + return android::ResTable_map::TYPE_INTEGER | android::ResTable_map::TYPE_ENUM + | android::ResTable_map::TYPE_FLAGS; + + case android::Res_value::TYPE_INT_BOOLEAN: + return android::ResTable_map::TYPE_BOOLEAN; + + case android::Res_value::TYPE_INT_COLOR_ARGB8: + case android::Res_value::TYPE_INT_COLOR_RGB8: + case android::Res_value::TYPE_INT_COLOR_ARGB4: + case android::Res_value::TYPE_INT_COLOR_RGB4: + return android::ResTable_map::TYPE_COLOR; + + default: + return 0; + }; +} + +std::unique_ptr parseItemForAttribute( + const StringPiece16& value, uint32_t typeMask, + std::function onCreateReference) { + std::unique_ptr nullOrEmpty = tryParseNullOrEmpty(value); + if (nullOrEmpty) { + return std::move(nullOrEmpty); + } + + bool create = false; + std::unique_ptr reference = tryParseReference(value, &create); + if (reference) { + if (create && onCreateReference) { + onCreateReference(reference->name.value()); + } + return std::move(reference); + } + + if (typeMask & android::ResTable_map::TYPE_COLOR) { + // Try parsing this as a color. + std::unique_ptr color = tryParseColor(value); + if (color) { + return std::move(color); + } + } + + if (typeMask & android::ResTable_map::TYPE_BOOLEAN) { + // Try parsing this as a boolean. + std::unique_ptr boolean = tryParseBool(value); + if (boolean) { + return std::move(boolean); + } + } + + if (typeMask & android::ResTable_map::TYPE_INTEGER) { + // Try parsing this as an integer. + std::unique_ptr integer = tryParseInt(value); + if (integer) { + return std::move(integer); + } + } + + const uint32_t floatMask = android::ResTable_map::TYPE_FLOAT + | android::ResTable_map::TYPE_DIMENSION | android::ResTable_map::TYPE_FRACTION; + if (typeMask & floatMask) { + // Try parsing this as a float. + std::unique_ptr floatingPoint = tryParseFloat(value); + if (floatingPoint) { + if (typeMask & androidTypeToAttributeTypeMask(floatingPoint->value.dataType)) { + return std::move(floatingPoint); + } + } + } + return {}; +} + +/** + * We successively try to parse the string as a resource type that the Attribute + * allows. + */ +std::unique_ptr parseItemForAttribute( + const StringPiece16& str, const Attribute* attr, + std::function onCreateReference) { + const uint32_t typeMask = attr->typeMask; + std::unique_ptr value = parseItemForAttribute(str, typeMask, onCreateReference); + if (value) { + return value; + } + + if (typeMask & android::ResTable_map::TYPE_ENUM) { + // Try parsing this as an enum. + std::unique_ptr enumValue = tryParseEnumSymbol(attr, str); + if (enumValue) { + return std::move(enumValue); + } + } + + if (typeMask & android::ResTable_map::TYPE_FLAGS) { + // Try parsing this as a flag. + std::unique_ptr flagValue = tryParseFlagSymbol(attr, str); + if (flagValue) { + return std::move(flagValue); + } + } + return {}; +} + +} // namespace ResourceUtils +} // namespace aapt -- cgit v1.2.3-59-g8ed1b From 2ae4a877d1623f851040ce69239552c873f1abf0 Mon Sep 17 00:00:00 2001 From: Adam Lesinski Date: Mon, 2 Nov 2015 16:10:55 -0800 Subject: AAPT2: Add Manifest fixing/validation Change-Id: I7f6d8b74d1c590adc356b4da55cb6cb777cdf1da --- tools/aapt2/Android.mk | 4 +- tools/aapt2/Flags.cpp | 10 +- tools/aapt2/ManifestValidator.cpp | 217 ---------------------------- tools/aapt2/ManifestValidator.h | 55 ------- tools/aapt2/ResourceUtils.cpp | 12 +- tools/aapt2/ResourceUtils.h | 5 + tools/aapt2/XmlDom.h | 2 + tools/aapt2/java/ManifestClassGenerator.cpp | 4 +- tools/aapt2/java/ProguardRules.cpp | 16 +- tools/aapt2/link/Link.cpp | 39 +++-- tools/aapt2/link/ManifestFixer.cpp | 100 +++++++++++++ tools/aapt2/link/ManifestFixer.h | 44 ++++++ tools/aapt2/link/ManifestFixer_test.cpp | 165 +++++++++++++++++++++ 13 files changed, 373 insertions(+), 300 deletions(-) delete mode 100644 tools/aapt2/ManifestValidator.cpp delete mode 100644 tools/aapt2/ManifestValidator.h create mode 100644 tools/aapt2/link/ManifestFixer.cpp create mode 100644 tools/aapt2/link/ManifestFixer.h create mode 100644 tools/aapt2/link/ManifestFixer_test.cpp (limited to 'tools/aapt2/ResourceUtils.cpp') diff --git a/tools/aapt2/Android.mk b/tools/aapt2/Android.mk index a41d2d72b754..ceed21edeb1b 100644 --- a/tools/aapt2/Android.mk +++ b/tools/aapt2/Android.mk @@ -32,6 +32,7 @@ sources := \ flatten/TableFlattener.cpp \ flatten/XmlFlattener.cpp \ link/AutoVersioner.cpp \ + link/ManifestFixer.cpp \ link/PrivateAttributeMover.cpp \ link/ReferenceLinker.cpp \ link/TableMerger.cpp \ @@ -67,6 +68,7 @@ testSources := \ flatten/TableFlattener_test.cpp \ flatten/XmlFlattener_test.cpp \ link/AutoVersioner_test.cpp \ + link/ManifestFixer_test.cpp \ link/PrivateAttributeMover_test.cpp \ link/ReferenceLinker_test.cpp \ link/TableMerger_test.cpp \ @@ -113,7 +115,7 @@ else endif cFlags := -Wall -Werror -Wno-unused-parameter -UNDEBUG -cppFlags := -std=c++11 -Wno-missing-field-initializers -fno-exceptions +cppFlags := -std=c++11 -Wno-missing-field-initializers -fno-exceptions -fno-rtti # ========================================================== # Build the host static library: libaapt2 diff --git a/tools/aapt2/Flags.cpp b/tools/aapt2/Flags.cpp index 6ae5af7bb92a..9435396991df 100644 --- a/tools/aapt2/Flags.cpp +++ b/tools/aapt2/Flags.cpp @@ -16,6 +16,7 @@ #include "Flags.h" #include "util/StringPiece.h" +#include "util/Util.h" #include #include @@ -94,7 +95,14 @@ void Flags::usage(const StringPiece& command, std::ostream* out) { if (flag.numArgs > 0) { argLine += " arg"; } - *out << " " << std::setw(30) << std::left << argLine << flag.description << "\n"; + + // Split the description by newlines and write out the argument (which is empty after + // the first line) followed by the description line. This will make sure that multiline + // descriptions are still right justified and aligned. + for (StringPiece line : util::tokenize(flag.description, '\n')) { + *out << " " << std::setw(30) << std::left << argLine << line << "\n"; + argLine = " "; + } } *out << " " << std::setw(30) << std::left << "-h" << "Displays this help menu\n"; out->flush(); diff --git a/tools/aapt2/ManifestValidator.cpp b/tools/aapt2/ManifestValidator.cpp deleted file mode 100644 index 9f971fbcf728..000000000000 --- a/tools/aapt2/ManifestValidator.cpp +++ /dev/null @@ -1,217 +0,0 @@ -/* - * Copyright (C) 2015 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 "Logger.h" -#include "ManifestValidator.h" -#include "util/Maybe.h" -#include "Source.h" -#include "util/Util.h" - -#include - -namespace aapt { - -ManifestValidator::ManifestValidator(const android::ResTable& table) -: mTable(table) { -} - -bool ManifestValidator::validate(const Source& source, android::ResXMLParser* parser) { - SourceLogger logger(source); - - android::ResXMLParser::event_code_t code; - while ((code = parser->next()) != android::ResXMLParser::END_DOCUMENT && - code != android::ResXMLParser::BAD_DOCUMENT) { - if (code != android::ResXMLParser::START_TAG) { - continue; - } - - size_t len = 0; - const StringPiece16 namespaceUri(parser->getElementNamespace(&len), len); - if (!namespaceUri.empty()) { - continue; - } - - const StringPiece16 name(parser->getElementName(&len), len); - if (name.empty()) { - logger.error(parser->getLineNumber()) - << "failed to get the element name." - << std::endl; - return false; - } - - if (name == u"manifest") { - if (!validateManifest(source, parser)) { - return false; - } - } - } - return true; -} - -Maybe ManifestValidator::getAttributeValue(android::ResXMLParser* parser, - size_t idx) { - android::Res_value value; - if (parser->getAttributeValue(idx, &value) < 0) { - return StringPiece16(); - } - - const android::ResStringPool* pool = &parser->getStrings(); - if (value.dataType == android::Res_value::TYPE_REFERENCE) { - ssize_t strIdx = mTable.resolveReference(&value, 0x10000000u); - if (strIdx < 0) { - return {}; - } - pool = mTable.getTableStringBlock(strIdx); - } - - if (value.dataType != android::Res_value::TYPE_STRING || !pool) { - return {}; - } - return util::getString(*pool, value.data); -} - -Maybe ManifestValidator::getAttributeInlineValue(android::ResXMLParser* parser, - size_t idx) { - android::Res_value value; - if (parser->getAttributeValue(idx, &value) < 0) { - return StringPiece16(); - } - - if (value.dataType != android::Res_value::TYPE_STRING) { - return {}; - } - return util::getString(parser->getStrings(), value.data); -} - -bool ManifestValidator::validateInlineAttribute(android::ResXMLParser* parser, size_t idx, - SourceLogger& logger, - const StringPiece16& charSet) { - size_t len = 0; - StringPiece16 element(parser->getElementName(&len), len); - StringPiece16 attributeName(parser->getAttributeName(idx, &len), len); - Maybe result = getAttributeInlineValue(parser, idx); - if (!result) { - logger.error(parser->getLineNumber()) - << "<" - << element - << "> must have a '" - << attributeName - << "' attribute with a string literal value." - << std::endl; - return false; - } - return validateAttributeImpl(element, attributeName, result.value(), charSet, - parser->getLineNumber(), logger); -} - -bool ManifestValidator::validateAttribute(android::ResXMLParser* parser, size_t idx, - SourceLogger& logger, const StringPiece16& charSet) { - size_t len = 0; - StringPiece16 element(parser->getElementName(&len), len); - StringPiece16 attributeName(parser->getAttributeName(idx, &len), len); - Maybe result = getAttributeValue(parser, idx); - if (!result) { - logger.error(parser->getLineNumber()) - << "<" - << element - << "> must have a '" - << attributeName - << "' attribute that points to a string." - << std::endl; - return false; - } - return validateAttributeImpl(element, attributeName, result.value(), charSet, - parser->getLineNumber(), logger); -} - -bool ManifestValidator::validateAttributeImpl(const StringPiece16& element, - const StringPiece16& attributeName, - const StringPiece16& attributeValue, - const StringPiece16& charSet, size_t lineNumber, - SourceLogger& logger) { - StringPiece16::const_iterator badIter = - util::findNonAlphaNumericAndNotInSet(attributeValue, charSet); - if (badIter != attributeValue.end()) { - logger.error(lineNumber) - << "tag <" - << element - << "> attribute '" - << attributeName - << "' has invalid character '" - << StringPiece16(badIter, 1) - << "'." - << std::endl; - return false; - } - - if (!attributeValue.empty()) { - StringPiece16 trimmed = util::trimWhitespace(attributeValue); - if (attributeValue.begin() != trimmed.begin()) { - logger.error(lineNumber) - << "tag <" - << element - << "> attribute '" - << attributeName - << "' can not start with whitespace." - << std::endl; - return false; - } - - if (attributeValue.end() != trimmed.end()) { - logger.error(lineNumber) - << "tag <" - << element - << "> attribute '" - << attributeName - << "' can not end with whitespace." - << std::endl; - return false; - } - } - return true; -} - -constexpr const char16_t* kPackageIdentSet = u"._"; - -bool ManifestValidator::validateManifest(const Source& source, android::ResXMLParser* parser) { - bool error = false; - SourceLogger logger(source); - - const StringPiece16 kAndroid = u"android"; - const StringPiece16 kPackage = u"package"; - const StringPiece16 kSharedUserId = u"sharedUserId"; - - ssize_t idx; - - idx = parser->indexOfAttribute(nullptr, 0, kPackage.data(), kPackage.size()); - if (idx < 0) { - logger.error(parser->getLineNumber()) - << "missing package attribute." - << std::endl; - error = true; - } else { - error |= !validateInlineAttribute(parser, idx, logger, kPackageIdentSet); - } - - idx = parser->indexOfAttribute(kAndroid.data(), kAndroid.size(), - kSharedUserId.data(), kSharedUserId.size()); - if (idx >= 0) { - error |= !validateInlineAttribute(parser, idx, logger, kPackageIdentSet); - } - return !error; -} - -} // namespace aapt diff --git a/tools/aapt2/ManifestValidator.h b/tools/aapt2/ManifestValidator.h deleted file mode 100644 index 1a7f48e9b5ac..000000000000 --- a/tools/aapt2/ManifestValidator.h +++ /dev/null @@ -1,55 +0,0 @@ -/* - * Copyright (C) 2015 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. - */ - -#ifndef AAPT_MANIFEST_VALIDATOR_H -#define AAPT_MANIFEST_VALIDATOR_H - -#include "Logger.h" -#include "util/Maybe.h" -#include "Source.h" -#include "util/StringPiece.h" - -#include - -namespace aapt { - -class ManifestValidator { -public: - ManifestValidator(const android::ResTable& table); - ManifestValidator(const ManifestValidator&) = delete; - - bool validate(const Source& source, android::ResXMLParser* parser); - -private: - bool validateManifest(const Source& source, android::ResXMLParser* parser); - - Maybe getAttributeInlineValue(android::ResXMLParser* parser, size_t idx); - Maybe getAttributeValue(android::ResXMLParser* parser, size_t idx); - - bool validateInlineAttribute(android::ResXMLParser* parser, size_t idx, - SourceLogger& logger, const StringPiece16& charSet); - bool validateAttribute(android::ResXMLParser* parser, size_t idx, SourceLogger& logger, - const StringPiece16& charSet); - bool validateAttributeImpl(const StringPiece16& element, const StringPiece16& attributeName, - const StringPiece16& attributeValue, const StringPiece16& charSet, - size_t lineNumber, SourceLogger& logger); - - const android::ResTable& mTable; -}; - -} // namespace aapt - -#endif // AAPT_MANIFEST_VALIDATOR_H diff --git a/tools/aapt2/ResourceUtils.cpp b/tools/aapt2/ResourceUtils.cpp index 0db1c372c901..ae3b4ff1e363 100644 --- a/tools/aapt2/ResourceUtils.cpp +++ b/tools/aapt2/ResourceUtils.cpp @@ -74,9 +74,11 @@ bool tryParseReference(const StringPiece16& str, ResourceNameRef* outRef, bool* return false; } - outRef->package = package; - outRef->type = *parsedType; - outRef->entry = entry; + if (outRef != nullptr) { + outRef->package = package; + outRef->type = *parsedType; + outRef->entry = entry; + } if (outCreate) { *outCreate = create; } @@ -88,6 +90,10 @@ bool tryParseReference(const StringPiece16& str, ResourceNameRef* outRef, bool* return false; } +bool isReference(const StringPiece16& str) { + return tryParseReference(str, nullptr, nullptr, nullptr); +} + bool tryParseAttributeReference(const StringPiece16& str, ResourceNameRef* outRef) { StringPiece16 trimmedStr(util::trimWhitespace(str)); if (trimmedStr.empty()) { diff --git a/tools/aapt2/ResourceUtils.h b/tools/aapt2/ResourceUtils.h index 118a2ee9d769..3c532de9060a 100644 --- a/tools/aapt2/ResourceUtils.h +++ b/tools/aapt2/ResourceUtils.h @@ -48,6 +48,11 @@ void extractResourceName(const StringPiece16& str, StringPiece16* outPackage, bool tryParseReference(const StringPiece16& str, ResourceNameRef* outReference, bool* outCreate = nullptr, bool* outPrivate = nullptr); +/* + * Returns true if the string is in the form of a resource reference (@[+][package:]type/name). + */ +bool isReference(const StringPiece16& str); + /* * Returns true if the string was parsed as an attribute reference (?[package:]type/name), * with `outReference` set to the parsed reference. diff --git a/tools/aapt2/XmlDom.h b/tools/aapt2/XmlDom.h index b1987f1fd245..9a46bcb9fc5c 100644 --- a/tools/aapt2/XmlDom.h +++ b/tools/aapt2/XmlDom.h @@ -34,6 +34,8 @@ namespace aapt { namespace xml { +constexpr const char16_t* kSchemaAndroid = u"http://schemas.android.com/apk/res/android"; + struct RawVisitor; /** diff --git a/tools/aapt2/java/ManifestClassGenerator.cpp b/tools/aapt2/java/ManifestClassGenerator.cpp index c12da6465a2a..901a344881fc 100644 --- a/tools/aapt2/java/ManifestClassGenerator.cpp +++ b/tools/aapt2/java/ManifestClassGenerator.cpp @@ -25,8 +25,6 @@ namespace aapt { -constexpr const char16_t* kSchemaAndroid = u"http://schemas.android.com/apk/res/android"; - static Maybe extractJavaIdentifier(IDiagnostics* diag, const Source& source, const StringPiece16& value) { const StringPiece16 sep = u"."; @@ -62,7 +60,7 @@ static Maybe extractJavaIdentifier(IDiagnostics* diag, const Sour static bool writeSymbol(IDiagnostics* diag, const Source& source, xml::Element* el, std::ostream* out) { - xml::Attribute* attr = el->findAttribute(kSchemaAndroid, u"name"); + xml::Attribute* attr = el->findAttribute(xml::kSchemaAndroid, u"name"); if (!attr) { diag->error(DiagMessage(source) << "<" << el->name << "> must define 'android:name'"); return false; diff --git a/tools/aapt2/java/ProguardRules.cpp b/tools/aapt2/java/ProguardRules.cpp index 7683f27f40b3..44314772fbd4 100644 --- a/tools/aapt2/java/ProguardRules.cpp +++ b/tools/aapt2/java/ProguardRules.cpp @@ -25,8 +25,6 @@ namespace aapt { namespace proguard { -constexpr const char16_t* kSchemaAndroid = u"http://schemas.android.com/apk/res/android"; - class BaseVisitor : public xml::Visitor { public: BaseVisitor(const Source& source, KeepSet* keepSet) : mSource(source), mKeepSet(keepSet) { @@ -83,7 +81,7 @@ struct LayoutVisitor : public BaseVisitor { bool checkName = false; if (node->namespaceUri.empty()) { checkClass = node->name == u"view" || node->name == u"fragment"; - } else if (node->namespaceUri == kSchemaAndroid) { + } else if (node->namespaceUri == xml::kSchemaAndroid) { checkName = node->name == u"fragment"; } @@ -91,10 +89,10 @@ struct LayoutVisitor : public BaseVisitor { if (checkClass && attr.namespaceUri.empty() && attr.name == u"class" && util::isJavaClassName(attr.value)) { addClass(node->lineNumber, attr.value); - } else if (checkName && attr.namespaceUri == kSchemaAndroid && attr.name == u"name" && - util::isJavaClassName(attr.value)) { + } else if (checkName && attr.namespaceUri == xml::kSchemaAndroid && + attr.name == u"name" && util::isJavaClassName(attr.value)) { addClass(node->lineNumber, attr.value); - } else if (attr.namespaceUri == kSchemaAndroid && attr.name == u"onClick") { + } else if (attr.namespaceUri == xml::kSchemaAndroid && attr.name == u"onClick") { addMethod(node->lineNumber, attr.value); } } @@ -114,7 +112,7 @@ struct XmlResourceVisitor : public BaseVisitor { } if (checkFragment) { - xml::Attribute* attr = node->findAttribute(kSchemaAndroid, u"fragment"); + xml::Attribute* attr = node->findAttribute(xml::kSchemaAndroid, u"fragment"); if (attr && util::isJavaClassName(attr->value)) { addClass(node->lineNumber, attr->value); } @@ -156,7 +154,7 @@ struct ManifestVisitor : public BaseVisitor { } } else if (node->name == u"application") { getName = true; - xml::Attribute* attr = node->findAttribute(kSchemaAndroid, u"backupAgent"); + xml::Attribute* attr = node->findAttribute(xml::kSchemaAndroid, u"backupAgent"); if (attr) { Maybe result = util::getFullyQualifiedClassName(mPackage, attr->value); @@ -171,7 +169,7 @@ struct ManifestVisitor : public BaseVisitor { } if (getName) { - xml::Attribute* attr = node->findAttribute(kSchemaAndroid, u"name"); + xml::Attribute* attr = node->findAttribute(xml::kSchemaAndroid, u"name"); if (attr) { Maybe result = util::getFullyQualifiedClassName(mPackage, attr->value); diff --git a/tools/aapt2/link/Link.cpp b/tools/aapt2/link/Link.cpp index 77918acb2996..0236e988162a 100644 --- a/tools/aapt2/link/Link.cpp +++ b/tools/aapt2/link/Link.cpp @@ -28,6 +28,7 @@ #include "java/ManifestClassGenerator.h" #include "java/ProguardRules.h" #include "link/Linkers.h" +#include "link/ManifestFixer.h" #include "link/TableMerger.h" #include "process/IResourceTableConsumer.h" #include "process/SymbolTable.h" @@ -54,6 +55,8 @@ struct LinkOptions { bool verbose = false; bool outputToDirectory = false; Maybe privateSymbols; + Maybe minSdkVersionDefault; + Maybe targetSdkVersionDefault; }; struct LinkContext : public IAaptContext { @@ -240,15 +243,8 @@ struct LinkCommand { } Maybe extractAppInfoFromManifest(XmlResource* xmlRes) { - xml::Node* node = xmlRes->root.get(); - - // Find the first xml::Element. - while (node && !xml::nodeCast(node)) { - node = !node->children.empty() ? node->children.front().get() : nullptr; - } - // Make sure the first element is with package attribute. - if (xml::Element* manifestEl = xml::nodeCast(node)) { + if (xml::Element* manifestEl = xml::findRootElement(xmlRes->root.get())) { if (manifestEl->namespaceUri.empty() && manifestEl->name == u"manifest") { if (xml::Attribute* packageAttr = manifestEl->findAttribute({}, u"package")) { return AppInfo{ packageAttr->value }; @@ -570,9 +566,16 @@ struct LinkCommand { } { + ManifestFixerOptions manifestFixerOptions; + manifestFixerOptions.minSdkVersionDefault = mOptions.minSdkVersionDefault; + manifestFixerOptions.targetSdkVersionDefault = mOptions.targetSdkVersionDefault; + ManifestFixer manifestFixer(manifestFixerOptions); + if (!manifestFixer.consume(&mContext, manifestXml.get())) { + error = true; + } + XmlReferenceLinker manifestLinker; if (manifestLinker.consume(&mContext, manifestXml.get())) { - if (!proguard::collectProguardRulesForManifest(Source(mOptions.manifestPath), manifestXml.get(), &proguardKeepSet)) { @@ -742,6 +745,7 @@ struct LinkCommand { int link(const std::vector& args) { LinkOptions options; Maybe privateSymbolsPackage; + Maybe minSdkVersion, targetSdkVersion; Flags flags = Flags() .requiredFlag("-o", "Output path", &options.outputPath) .requiredFlag("--manifest", "Path to the Android manifest to build", @@ -757,10 +761,15 @@ int link(const std::vector& args) { .optionalSwitch("--output-to-dir", "Outputs the APK contents to a directory specified " "by -o", &options.outputToDirectory) + .optionalFlag("--min-sdk-version", "Default minimum SDK version to use for " + "AndroidManifest.xml", &minSdkVersion) + .optionalFlag("--target-sdk-version", "Default target SDK version to use for " + "AndroidManifest.xml", &targetSdkVersion) .optionalSwitch("--static-lib", "Generate a static Android library", &options.staticLib) .optionalFlag("--private-symbols", "Package name to use when generating R.java for " - "private symbols. If not specified, public and private symbols will " - "use the application's package name", &privateSymbolsPackage) + "private symbols.\n" + "If not specified, public and private symbols will use the application's " + "package name", &privateSymbolsPackage) .optionalSwitch("-v", "Enables verbose logging", &options.verbose); if (!flags.parse("aapt2 link", args, &std::cerr)) { @@ -771,6 +780,14 @@ int link(const std::vector& args) { options.privateSymbols = util::utf8ToUtf16(privateSymbolsPackage.value()); } + if (minSdkVersion) { + options.minSdkVersionDefault = util::utf8ToUtf16(minSdkVersion.value()); + } + + if (targetSdkVersion) { + options.targetSdkVersionDefault = util::utf8ToUtf16(targetSdkVersion.value()); + } + LinkCommand cmd = { options }; return cmd.run(flags.getArgs()); } diff --git a/tools/aapt2/link/ManifestFixer.cpp b/tools/aapt2/link/ManifestFixer.cpp new file mode 100644 index 000000000000..52d942670243 --- /dev/null +++ b/tools/aapt2/link/ManifestFixer.cpp @@ -0,0 +1,100 @@ +/* + * Copyright (C) 2015 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 "ResourceUtils.h" +#include "XmlDom.h" + +#include "link/ManifestFixer.h" +#include "util/Util.h" + +namespace aapt { + +static bool verifyManifest(IAaptContext* context, const Source& source, xml::Element* manifestEl) { + bool error = false; + + xml::Attribute* attr = manifestEl->findAttribute({}, u"package"); + if (!attr) { + context->getDiagnostics()->error(DiagMessage(source.withLine(manifestEl->lineNumber)) + << "missing 'package' attribute"); + error = true; + } else if (ResourceUtils::isReference(attr->value)) { + context->getDiagnostics()->error(DiagMessage(source.withLine(manifestEl->lineNumber)) + << "value for attribute 'package' must not be a " + "reference"); + error = true; + } else if (!util::isJavaPackageName(attr->value)) { + context->getDiagnostics()->error(DiagMessage(source.withLine(manifestEl->lineNumber)) + << "invalid package name '" << attr->value << "'"); + error = true; + } + + return !error; +} + +static bool fixUsesSdk(IAaptContext* context, const Source& source, xml::Element* el, + const ManifestFixerOptions& options) { + if (options.minSdkVersionDefault && + el->findAttribute(xml::kSchemaAndroid, u"minSdkVersion") == nullptr) { + // There was no minSdkVersion defined and we have a default to assign. + el->attributes.push_back(xml::Attribute{ + xml::kSchemaAndroid, u"minSdkVersion", options.minSdkVersionDefault.value() }); + } + + if (options.targetSdkVersionDefault && + el->findAttribute(xml::kSchemaAndroid, u"targetSdkVersion") == nullptr) { + // There was no targetSdkVersion defined and we have a default to assign. + el->attributes.push_back(xml::Attribute{ + xml::kSchemaAndroid, u"targetSdkVersion", + options.targetSdkVersionDefault.value() }); + } + return true; +} + +bool ManifestFixer::consume(IAaptContext* context, XmlResource* doc) { + xml::Element* root = xml::findRootElement(doc->root.get()); + if (!root || !root->namespaceUri.empty() || root->name != u"manifest") { + context->getDiagnostics()->error(DiagMessage(doc->file.source) + << "root tag must be "); + return false; + } + + if (!verifyManifest(context, doc->file.source, root)) { + return false; + } + + bool foundUsesSdk = false; + for (xml::Element* el : root->getChildElements()) { + if (!el->namespaceUri.empty()) { + continue; + } + + if (el->name == u"uses-sdk") { + foundUsesSdk = true; + fixUsesSdk(context, doc->file.source, el, mOptions); + } + } + + if (!foundUsesSdk && (mOptions.minSdkVersionDefault || mOptions.targetSdkVersionDefault)) { + std::unique_ptr usesSdk = util::make_unique(); + usesSdk->name = u"uses-sdk"; + fixUsesSdk(context, doc->file.source, usesSdk.get(), mOptions); + root->addChild(std::move(usesSdk)); + } + + return true; +} + +} // namespace aapt diff --git a/tools/aapt2/link/ManifestFixer.h b/tools/aapt2/link/ManifestFixer.h new file mode 100644 index 000000000000..16e161dc40d8 --- /dev/null +++ b/tools/aapt2/link/ManifestFixer.h @@ -0,0 +1,44 @@ +/* + * Copyright (C) 2015 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. + */ + +#ifndef AAPT_LINK_MANIFESTFIXER_H +#define AAPT_LINK_MANIFESTFIXER_H + +#include "process/IResourceTableConsumer.h" + +namespace aapt { + +struct ManifestFixerOptions { + Maybe minSdkVersionDefault; + Maybe targetSdkVersionDefault; +}; + +/** + * Verifies that the manifest is correctly formed and inserts defaults + * where specified with ManifestFixerOptions. + */ +struct ManifestFixer : public IXmlResourceConsumer { + ManifestFixerOptions mOptions; + + ManifestFixer(const ManifestFixerOptions& options) : mOptions(options) { + } + + bool consume(IAaptContext* context, XmlResource* doc) override; +}; + +} // namespace aapt + +#endif /* AAPT_LINK_MANIFESTFIXER_H */ diff --git a/tools/aapt2/link/ManifestFixer_test.cpp b/tools/aapt2/link/ManifestFixer_test.cpp new file mode 100644 index 000000000000..5c5d8afa610d --- /dev/null +++ b/tools/aapt2/link/ManifestFixer_test.cpp @@ -0,0 +1,165 @@ +/* + * Copyright (C) 2015 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 "link/ManifestFixer.h" + +#include "test/Builders.h" +#include "test/Context.h" + +#include + +namespace aapt { + +struct ManifestFixerTest : public ::testing::Test { + std::unique_ptr mContext; + + void SetUp() override { + mContext = test::ContextBuilder() + .setCompilationPackage(u"android") + .setPackageId(0x01) + .setNameManglerPolicy(NameManglerPolicy{ u"android" }) + .setSymbolTable(test::StaticSymbolTableBuilder() + .addSymbol(u"@android:attr/package", ResourceId(0x01010000), + test::AttributeBuilder() + .setTypeMask(android::ResTable_map::TYPE_STRING) + .build()) + .addSymbol(u"@android:attr/minSdkVersion", ResourceId(0x01010001), + test::AttributeBuilder() + .setTypeMask(android::ResTable_map::TYPE_STRING | + android::ResTable_map::TYPE_INTEGER) + .build()) + .addSymbol(u"@android:attr/targetSdkVersion", ResourceId(0x01010002), + test::AttributeBuilder() + .setTypeMask(android::ResTable_map::TYPE_STRING | + android::ResTable_map::TYPE_INTEGER) + .build()) + .addSymbol(u"@android:string/str", ResourceId(0x01060000)) + .build()) + .build(); + } + + std::unique_ptr verify(const StringPiece& str) { + return verifyWithOptions(str, {}); + } + + std::unique_ptr verifyWithOptions(const StringPiece& str, + const ManifestFixerOptions& options) { + std::unique_ptr doc = test::buildXmlDom(str); + ManifestFixer fixer(options); + if (fixer.consume(mContext.get(), doc.get())) { + return doc; + } + return {}; + } +}; + +TEST_F(ManifestFixerTest, EnsureManifestIsRootTag) { + EXPECT_EQ(nullptr, verify("")); + EXPECT_EQ(nullptr, verify("")); + EXPECT_NE(nullptr, verify("")); +} + +TEST_F(ManifestFixerTest, EnsureManifestHasPackage) { + EXPECT_NE(nullptr, verify("")); + EXPECT_NE(nullptr, verify("")); + EXPECT_NE(nullptr, verify("")); + EXPECT_EQ(nullptr, verify("")); + EXPECT_EQ(nullptr, + verify("")); + EXPECT_EQ(nullptr, verify("")); +} + + + +TEST_F(ManifestFixerTest, UseDefaultSdkVersionsIfNonePresent) { + ManifestFixerOptions options = { std::u16string(u"8"), std::u16string(u"22") }; + + std::unique_ptr doc = verifyWithOptions(R"EOF( + + + )EOF", options); + ASSERT_NE(nullptr, doc); + + xml::Element* el; + xml::Attribute* attr; + + el = xml::findRootElement(doc->root.get()); + ASSERT_NE(nullptr, el); + el = el->findChild({}, u"uses-sdk"); + ASSERT_NE(nullptr, el); + attr = el->findAttribute(xml::kSchemaAndroid, u"minSdkVersion"); + ASSERT_NE(nullptr, attr); + EXPECT_EQ(u"7", attr->value); + attr = el->findAttribute(xml::kSchemaAndroid, u"targetSdkVersion"); + ASSERT_NE(nullptr, attr); + EXPECT_EQ(u"21", attr->value); + + doc = verifyWithOptions(R"EOF( + + + )EOF", options); + ASSERT_NE(nullptr, doc); + + el = xml::findRootElement(doc->root.get()); + ASSERT_NE(nullptr, el); + el = el->findChild({}, u"uses-sdk"); + ASSERT_NE(nullptr, el); + attr = el->findAttribute(xml::kSchemaAndroid, u"minSdkVersion"); + ASSERT_NE(nullptr, attr); + EXPECT_EQ(u"8", attr->value); + attr = el->findAttribute(xml::kSchemaAndroid, u"targetSdkVersion"); + ASSERT_NE(nullptr, attr); + EXPECT_EQ(u"21", attr->value); + + doc = verifyWithOptions(R"EOF( + + + )EOF", options); + ASSERT_NE(nullptr, doc); + + el = xml::findRootElement(doc->root.get()); + ASSERT_NE(nullptr, el); + el = el->findChild({}, u"uses-sdk"); + ASSERT_NE(nullptr, el); + attr = el->findAttribute(xml::kSchemaAndroid, u"minSdkVersion"); + ASSERT_NE(nullptr, attr); + EXPECT_EQ(u"8", attr->value); + attr = el->findAttribute(xml::kSchemaAndroid, u"targetSdkVersion"); + ASSERT_NE(nullptr, attr); + EXPECT_EQ(u"22", attr->value); + + doc = verifyWithOptions(R"EOF( + )EOF", options); + ASSERT_NE(nullptr, doc); + + el = xml::findRootElement(doc->root.get()); + ASSERT_NE(nullptr, el); + el = el->findChild({}, u"uses-sdk"); + ASSERT_NE(nullptr, el); + attr = el->findAttribute(xml::kSchemaAndroid, u"minSdkVersion"); + ASSERT_NE(nullptr, attr); + EXPECT_EQ(u"8", attr->value); + attr = el->findAttribute(xml::kSchemaAndroid, u"targetSdkVersion"); + ASSERT_NE(nullptr, attr); + EXPECT_EQ(u"22", attr->value); +} + +} // namespace aapt -- cgit v1.2.3-59-g8ed1b From b23f1e077b02a1d62bcf5e34655e8dc979e124fa Mon Sep 17 00:00:00 2001 From: Adam Lesinski Date: Tue, 3 Nov 2015 12:24:17 -0800 Subject: AAPT2: Verify positional Java String format arguments in strings Change-Id: Id415969035a0d5712857c0e11e140155566a960c --- tools/aapt2/ResourceParser.cpp | 37 +++++++++++++--- tools/aapt2/ResourceUtils.cpp | 36 +++++++++++---- tools/aapt2/ResourceUtils.h | 5 +++ tools/aapt2/util/Util.cpp | 99 ++++++++++++++++++++++++++++++++++++++++++ tools/aapt2/util/Util.h | 8 ++++ tools/aapt2/util/Util_test.cpp | 8 ++++ 6 files changed, 179 insertions(+), 14 deletions(-) (limited to 'tools/aapt2/ResourceUtils.cpp') diff --git a/tools/aapt2/ResourceParser.cpp b/tools/aapt2/ResourceParser.cpp index 0c7a4d578be5..2d6c0c2d5b8c 100644 --- a/tools/aapt2/ResourceParser.cpp +++ b/tools/aapt2/ResourceParser.cpp @@ -62,6 +62,7 @@ bool ResourceParser::flattenXmlSubtree(XmlPullParser* parser, std::u16string* ou StyleString* outStyleString) { std::vector spanStack; + bool error = false; outRawString->clear(); outStyleString->spans.clear(); util::StringBuilder builder; @@ -84,7 +85,6 @@ bool ResourceParser::flattenXmlSubtree(XmlPullParser* parser, std::u16string* ou spanStack.pop_back(); } else if (event == XmlPullParser::Event::kText) { - // TODO(adamlesinski): Verify format strings. outRawString->append(parser->getText()); builder.append(parser->getText()); @@ -116,9 +116,10 @@ bool ResourceParser::flattenXmlSubtree(XmlPullParser* parser, std::u16string* ou if (builder.str().size() > std::numeric_limits::max()) { mDiag->error(DiagMessage(mSource.withLine(parser->getLineNumber())) << "style string '" << builder.str() << "' is too long"); - return false; + error = true; + } else { + spanStack.push_back(Span{ spanName, static_cast(builder.str().size()) }); } - spanStack.push_back(Span{ spanName, static_cast(builder.str().size()) }); } else if (event == XmlPullParser::Event::kComment) { // Skip @@ -129,7 +130,7 @@ bool ResourceParser::flattenXmlSubtree(XmlPullParser* parser, std::u16string* ou assert(spanStack.empty() && "spans haven't been fully processed"); outStyleString->str = builder.str(); - return true; + return !error; } bool ResourceParser::parse(XmlPullParser* parser) { @@ -437,13 +438,39 @@ std::unique_ptr ResourceParser::parseXml(XmlPullParser* parser, const uint bool ResourceParser::parseString(XmlPullParser* parser, ParsedResource* outResource) { const Source source = mSource.withLine(parser->getLineNumber()); - // TODO(adamlesinski): Read "untranslateable" attribute. + bool formatted = true; + if (Maybe formattedAttr = findAttribute(parser, u"formatted")) { + if (!ResourceUtils::tryParseBool(formattedAttr.value(), &formatted)) { + mDiag->error(DiagMessage(source) << "invalid value for 'formatted'. Must be a boolean"); + return false; + } + } + + bool untranslateable = false; + if (Maybe untranslateableAttr = findAttribute(parser, u"untranslateable")) { + if (!ResourceUtils::tryParseBool(untranslateableAttr.value(), &untranslateable)) { + mDiag->error(DiagMessage(source) + << "invalid value for 'untranslateable'. Must be a boolean"); + return false; + } + } outResource->value = parseXml(parser, android::ResTable_map::TYPE_STRING, kNoRawString); if (!outResource->value) { mDiag->error(DiagMessage(source) << "not a valid string"); return false; } + + if (formatted || untranslateable) { + if (String* stringValue = valueCast(outResource->value.get())) { + if (!util::verifyJavaStringFormat(*stringValue->value)) { + mDiag->error(DiagMessage(mSource.withLine(parser->getLineNumber())) + << "multiple substitutions specified in non-positional format; " + "did you mean to add the formatted=\"false\" attribute?"); + return false; + } + } + } return true; } diff --git a/tools/aapt2/ResourceUtils.cpp b/tools/aapt2/ResourceUtils.cpp index ae3b4ff1e363..d3c3c1044ae7 100644 --- a/tools/aapt2/ResourceUtils.cpp +++ b/tools/aapt2/ResourceUtils.cpp @@ -328,18 +328,36 @@ std::unique_ptr tryParseColor(const StringPiece16& str) { return error ? std::unique_ptr() : util::make_unique(value); } -std::unique_ptr tryParseBool(const StringPiece16& str) { +bool tryParseBool(const StringPiece16& str, bool* outValue) { StringPiece16 trimmedStr(util::trimWhitespace(str)); - uint32_t data = 0; if (trimmedStr == u"true" || trimmedStr == u"TRUE") { - data = 0xffffffffu; - } else if (trimmedStr != u"false" && trimmedStr != u"FALSE") { - return {}; + if (outValue) { + *outValue = true; + } + return true; + } else if (trimmedStr == u"false" || trimmedStr == u"FALSE") { + if (outValue) { + *outValue = false; + } + return true; } - android::Res_value value = { }; - value.dataType = android::Res_value::TYPE_INT_BOOLEAN; - value.data = data; - return util::make_unique(value); + return false; +} + +std::unique_ptr tryParseBool(const StringPiece16& str) { + bool result = false; + if (tryParseBool(str, &result)) { + android::Res_value value = {}; + value.dataType = android::Res_value::TYPE_INT_BOOLEAN; + + if (result) { + value.data = 0xffffffffu; + } else { + value.data = 0; + } + return util::make_unique(value); + } + return {}; } std::unique_ptr tryParseInt(const StringPiece16& str) { diff --git a/tools/aapt2/ResourceUtils.h b/tools/aapt2/ResourceUtils.h index 3c532de9060a..851edc89fa90 100644 --- a/tools/aapt2/ResourceUtils.h +++ b/tools/aapt2/ResourceUtils.h @@ -59,6 +59,11 @@ bool isReference(const StringPiece16& str); */ bool tryParseAttributeReference(const StringPiece16& str, ResourceNameRef* outReference); +/** + * Returns true if the value is a boolean, putting the result in `outValue`. + */ +bool tryParseBool(const StringPiece16& str, bool* outValue); + /* * Returns a Reference, or None Maybe instance if the string `str` was parsed as a * valid reference to a style. diff --git a/tools/aapt2/util/Util.cpp b/tools/aapt2/util/Util.cpp index 6ef4ce504a63..59b838587a6a 100644 --- a/tools/aapt2/util/Util.cpp +++ b/tools/aapt2/util/Util.cpp @@ -189,6 +189,105 @@ Maybe getFullyQualifiedClassName(const StringPiece16& package, return result; } +static size_t consumeDigits(const char16_t* start, const char16_t* end) { + const char16_t* c = start; + for (; c != end && *c >= u'0' && *c <= u'9'; c++) {} + return static_cast(c - start); +} + +bool verifyJavaStringFormat(const StringPiece16& str) { + const char16_t* c = str.begin(); + const char16_t* const end = str.end(); + + size_t argCount = 0; + bool nonpositional = false; + while (c != end) { + if (*c == u'%' && c + 1 < end) { + c++; + + if (*c == u'%') { + c++; + continue; + } + + argCount++; + + size_t numDigits = consumeDigits(c, end); + if (numDigits > 0) { + c += numDigits; + if (c != end && *c != u'$') { + // The digits were a size, but not a positional argument. + nonpositional = true; + } + } else if (*c == u'<') { + // Reusing last argument, bad idea since positions can be moved around + // during translation. + nonpositional = true; + + c++; + + // Optionally we can have a $ after + if (c != end && *c == u'$') { + c++; + } + } else { + nonpositional = true; + } + + // Ignore size, width, flags, etc. + while (c != end && (*c == u'-' || + *c == u'#' || + *c == u'+' || + *c == u' ' || + *c == u',' || + *c == u'(' || + (*c >= u'0' && *c <= '9'))) { + c++; + } + + /* + * This is a shortcut to detect strings that are going to Time.format() + * instead of String.format() + * + * Comparison of String.format() and Time.format() args: + * + * String: ABC E GH ST X abcdefgh nost x + * Time: DEFGHKMS W Za d hkm s w yz + * + * Therefore we know it's definitely Time if we have: + * DFKMWZkmwyz + */ + if (c != end) { + switch (*c) { + case 'D': + case 'F': + case 'K': + case 'M': + case 'W': + case 'Z': + case 'k': + case 'm': + case 'w': + case 'y': + case 'z': + return true; + } + } + } + + if (c != end) { + c++; + } + } + + if (argCount > 1 && nonpositional) { + // Multiple arguments were specified, but some or all were non positional. Translated + // strings may rearrange the order of the arguments, which will break the string. + return false; + } + return true; +} + static Maybe parseUnicodeCodepoint(const char16_t** start, const char16_t* end) { char16_t code = 0; for (size_t i = 0; i < 4 && *start != end; i++, (*start)++) { diff --git a/tools/aapt2/util/Util.h b/tools/aapt2/util/Util.h index fd3fbb4573a0..80552a540ec3 100644 --- a/tools/aapt2/util/Util.h +++ b/tools/aapt2/util/Util.h @@ -158,6 +158,14 @@ inline StringPiece16 getString(const android::ResStringPool& pool, size_t idx) { return StringPiece16(); } +/** + * Checks that the Java string format contains no non-positional arguments (arguments without + * explicitly specifying an index) when there are more than one argument. This is an error + * because translations may rearrange the order of the arguments in the string, which will + * break the string interpolation. + */ +bool verifyJavaStringFormat(const StringPiece16& str); + class StringBuilder { public: StringBuilder& append(const StringPiece16& str); diff --git a/tools/aapt2/util/Util_test.cpp b/tools/aapt2/util/Util_test.cpp index cdba960b8670..9db9fb7f112a 100644 --- a/tools/aapt2/util/Util_test.cpp +++ b/tools/aapt2/util/Util_test.cpp @@ -185,4 +185,12 @@ TEST(UtilTest, ExtractResourcePathComponents) { EXPECT_EQ(suffix, u"."); } +TEST(UtilTest, VerifyJavaStringFormat) { + ASSERT_TRUE(util::verifyJavaStringFormat(u"%09.34f")); + ASSERT_TRUE(util::verifyJavaStringFormat(u"%9$.34f %8$")); + ASSERT_TRUE(util::verifyJavaStringFormat(u"%% %%")); + ASSERT_FALSE(util::verifyJavaStringFormat(u"%09$f %f")); + ASSERT_FALSE(util::verifyJavaStringFormat(u"%09f %08s")); +} + } // namespace aapt -- cgit v1.2.3-59-g8ed1b From 7298bc9c857541b444b2f1639dbed17599cbe5e9 Mon Sep 17 00:00:00 2001 From: Adam Lesinski Date: Mon, 16 Nov 2015 12:31:52 -0800 Subject: AAPT2: Be more strict parsing references Change-Id: I3d54a519687fff1e66acb8e395ef99ba01a1b845 --- tools/aapt2/ResourceUtils.cpp | 43 +++++++++++++++++++++++++++++++------- tools/aapt2/ResourceUtils.h | 10 +++++++-- tools/aapt2/ResourceUtils_test.cpp | 20 ++++++++++++++++++ 3 files changed, 63 insertions(+), 10 deletions(-) (limited to 'tools/aapt2/ResourceUtils.cpp') diff --git a/tools/aapt2/ResourceUtils.cpp b/tools/aapt2/ResourceUtils.cpp index d3c3c1044ae7..b1a4c7dee73f 100644 --- a/tools/aapt2/ResourceUtils.cpp +++ b/tools/aapt2/ResourceUtils.cpp @@ -23,22 +23,28 @@ namespace aapt { namespace ResourceUtils { -void extractResourceName(const StringPiece16& str, StringPiece16* outPackage, +bool extractResourceName(const StringPiece16& str, StringPiece16* outPackage, StringPiece16* outType, StringPiece16* outEntry) { + bool hasPackageSeparator = false; + bool hasTypeSeparator = false; const char16_t* start = str.data(); const char16_t* end = start + str.size(); const char16_t* current = start; while (current != end) { if (outType->size() == 0 && *current == u'/') { + hasTypeSeparator = true; outType->assign(start, current - start); start = current + 1; } else if (outPackage->size() == 0 && *current == u':') { + hasPackageSeparator = true; outPackage->assign(start, current - start); start = current + 1; } current++; } outEntry->assign(start, end - start); + + return !(hasPackageSeparator && outPackage->empty()) && !(hasTypeSeparator && outType->empty()); } bool tryParseReference(const StringPiece16& str, ResourceNameRef* outRef, bool* outCreate, @@ -62,26 +68,34 @@ bool tryParseReference(const StringPiece16& str, ResourceNameRef* outRef, bool* StringPiece16 package; StringPiece16 type; StringPiece16 entry; - extractResourceName(trimmedStr.substr(offset, trimmedStr.size() - offset), &package, &type, - &entry); + if (!extractResourceName(trimmedStr.substr(offset, trimmedStr.size() - offset), + &package, &type, &entry)) { + return false; + } const ResourceType* parsedType = parseResourceType(type); if (!parsedType) { return false; } + if (entry.empty()) { + return false; + } + if (create && *parsedType != ResourceType::kId) { return false; } - if (outRef != nullptr) { + if (outRef) { outRef->package = package; outRef->type = *parsedType; outRef->entry = entry; } + if (outCreate) { *outCreate = create; } + if (outPrivate) { *outPrivate = priv; } @@ -104,20 +118,33 @@ bool tryParseAttributeReference(const StringPiece16& str, ResourceNameRef* outRe StringPiece16 package; StringPiece16 type; StringPiece16 entry; - extractResourceName(trimmedStr.substr(1, trimmedStr.size() - 1), &package, &type, &entry); + if (!extractResourceName(trimmedStr.substr(1, trimmedStr.size() - 1), + &package, &type, &entry)) { + return false; + } if (!type.empty() && type != u"attr") { return false; } - outRef->package = package; - outRef->type = ResourceType::kAttr; - outRef->entry = entry; + if (entry.empty()) { + return false; + } + + if (outRef) { + outRef->package = package; + outRef->type = ResourceType::kAttr; + outRef->entry = entry; + } return true; } return false; } +bool isAttributeReference(const StringPiece16& str) { + return tryParseAttributeReference(str, nullptr); +} + /* * Style parent's are a bit different. We accept the following formats: * diff --git a/tools/aapt2/ResourceUtils.h b/tools/aapt2/ResourceUtils.h index 851edc89fa90..34daa66b1546 100644 --- a/tools/aapt2/ResourceUtils.h +++ b/tools/aapt2/ResourceUtils.h @@ -34,8 +34,9 @@ namespace ResourceUtils { * * where the package can be empty. Validation must be performed on each * individual extracted piece to verify that the pieces are valid. + * Returns false if there was no package but a ':' was present. */ -void extractResourceName(const StringPiece16& str, StringPiece16* outPackage, +bool extractResourceName(const StringPiece16& str, StringPiece16* outPackage, StringPiece16* outType, StringPiece16* outEntry); /* @@ -54,11 +55,16 @@ bool tryParseReference(const StringPiece16& str, ResourceNameRef* outReference, bool isReference(const StringPiece16& str); /* - * Returns true if the string was parsed as an attribute reference (?[package:]type/name), + * Returns true if the string was parsed as an attribute reference (?[package:][type/]name), * with `outReference` set to the parsed reference. */ bool tryParseAttributeReference(const StringPiece16& str, ResourceNameRef* outReference); +/** + * Returns true if the string is in the form of an attribute reference(?[package:][type/]name). + */ +bool isAttributeReference(const StringPiece16& str); + /** * Returns true if the value is a boolean, putting the result in `outValue`. */ diff --git a/tools/aapt2/ResourceUtils_test.cpp b/tools/aapt2/ResourceUtils_test.cpp index 7de8f4130316..3d2a6e18e2bc 100644 --- a/tools/aapt2/ResourceUtils_test.cpp +++ b/tools/aapt2/ResourceUtils_test.cpp @@ -90,6 +90,26 @@ TEST(ResourceUtilsTest, FailToParseAutoCreateNonIdReference) { &privateRef)); } +TEST(ResourceUtilsTest, ParseAttributeReferences) { + EXPECT_TRUE(ResourceUtils::isAttributeReference(u"?android")); + EXPECT_TRUE(ResourceUtils::isAttributeReference(u"?android:foo")); + EXPECT_TRUE(ResourceUtils::isAttributeReference(u"?attr/foo")); + EXPECT_TRUE(ResourceUtils::isAttributeReference(u"?android:attr/foo")); +} + +TEST(ResourceUtilsTest, FailParseIncompleteReference) { + EXPECT_FALSE(ResourceUtils::isAttributeReference(u"?style/foo")); + EXPECT_FALSE(ResourceUtils::isAttributeReference(u"?android:style/foo")); + EXPECT_FALSE(ResourceUtils::isAttributeReference(u"?android:")); + EXPECT_FALSE(ResourceUtils::isAttributeReference(u"?android:attr/")); + EXPECT_FALSE(ResourceUtils::isAttributeReference(u"?:attr/")); + EXPECT_FALSE(ResourceUtils::isAttributeReference(u"?:attr/foo")); + EXPECT_FALSE(ResourceUtils::isAttributeReference(u"?:/")); + EXPECT_FALSE(ResourceUtils::isAttributeReference(u"?:/foo")); + EXPECT_FALSE(ResourceUtils::isAttributeReference(u"?attr/")); + EXPECT_FALSE(ResourceUtils::isAttributeReference(u"?/foo")); +} + TEST(ResourceUtilsTest, ParseStyleParentReference) { const ResourceName kAndroidStyleFooName = { u"android", ResourceType::kStyle, u"foo" }; const ResourceName kStyleFooName = { {}, ResourceType::kStyle, u"foo" }; -- cgit v1.2.3-59-g8ed1b From 467f171315f9c2037fcd3eb5edcfabc40671bf7b Mon Sep 17 00:00:00 2001 From: Adam Lesinski Date: Mon, 16 Nov 2015 17:35:44 -0800 Subject: AAPT2: Fail compiling when private symbols are referenced Also moved some XML specific stuff into its own directory, and refactored ReferenceLinker a bit. Change-Id: I912247a82023c1bbf72dc191fbdaf62858cbec0c --- tools/aapt2/Android.mk | 10 +- tools/aapt2/ResourceParser.cpp | 239 ++++++------ tools/aapt2/ResourceParser.h | 40 +-- tools/aapt2/ResourceParser_test.cpp | 26 +- tools/aapt2/ResourceUtils.cpp | 60 +++- tools/aapt2/ResourceUtils.h | 7 + tools/aapt2/ResourceUtils_test.cpp | 31 +- tools/aapt2/ResourceValues.cpp | 39 +- tools/aapt2/XmlDom.cpp | 401 --------------------- tools/aapt2/XmlDom.h | 259 ------------- tools/aapt2/XmlDom_test.cpp | 50 --- tools/aapt2/XmlPullParser.cpp | 286 --------------- tools/aapt2/XmlPullParser.h | 282 --------------- tools/aapt2/XmlPullParser_test.cpp | 55 --- tools/aapt2/compile/Compile.cpp | 9 +- tools/aapt2/compile/XmlIdCollector.cpp | 5 +- tools/aapt2/compile/XmlIdCollector.h | 3 +- tools/aapt2/compile/XmlIdCollector_test.cpp | 4 +- tools/aapt2/flatten/ResourceTypeExtensions.h | 2 +- tools/aapt2/flatten/TableFlattener.cpp | 54 ++- tools/aapt2/flatten/TableFlattener_test.cpp | 4 +- tools/aapt2/flatten/XmlFlattener.cpp | 7 +- tools/aapt2/flatten/XmlFlattener.h | 10 +- tools/aapt2/flatten/XmlFlattener_test.cpp | 15 +- tools/aapt2/java/AnnotationProcessor_test.cpp | 6 +- tools/aapt2/java/ClassDefinitionWriter.h | 1 + tools/aapt2/java/ManifestClassGenerator.cpp | 5 +- tools/aapt2/java/ManifestClassGenerator.h | 4 +- tools/aapt2/java/ManifestClassGenerator_test.cpp | 5 +- tools/aapt2/java/ProguardRules.cpp | 12 +- tools/aapt2/java/ProguardRules.h | 7 +- tools/aapt2/link/Link.cpp | 47 ++- tools/aapt2/link/Linkers.h | 23 +- tools/aapt2/link/ManifestFixer.cpp | 5 +- tools/aapt2/link/ManifestFixer.h | 6 +- tools/aapt2/link/ManifestFixer_test.cpp | 11 +- tools/aapt2/link/PrivateAttributeMover.cpp | 1 - tools/aapt2/link/ReferenceLinker.cpp | 249 +++++++++---- tools/aapt2/link/ReferenceLinker.h | 99 +++++ tools/aapt2/link/ReferenceLinkerVisitor.h | 109 ------ tools/aapt2/link/ReferenceLinker_test.cpp | 86 ++++- tools/aapt2/link/XmlReferenceLinker.cpp | 92 +++-- tools/aapt2/link/XmlReferenceLinker_test.cpp | 74 ++-- tools/aapt2/process/IResourceTableConsumer.h | 16 +- tools/aapt2/process/SymbolTable.cpp | 57 ++- tools/aapt2/test/Builders.h | 16 +- tools/aapt2/test/Context.h | 13 +- tools/aapt2/unflatten/BinaryResourceParser.cpp | 92 +++-- tools/aapt2/unflatten/BinaryResourceParser.h | 2 +- tools/aapt2/util/Util.cpp | 15 - tools/aapt2/util/Util.h | 9 - tools/aapt2/xml/XmlDom.cpp | 440 +++++++++++++++++++++++ tools/aapt2/xml/XmlDom.h | 220 ++++++++++++ tools/aapt2/xml/XmlDom_test.cpp | 50 +++ tools/aapt2/xml/XmlPullParser.cpp | 308 ++++++++++++++++ tools/aapt2/xml/XmlPullParser.h | 298 +++++++++++++++ tools/aapt2/xml/XmlPullParser_test.cpp | 55 +++ tools/aapt2/xml/XmlUtil.cpp | 67 ++++ tools/aapt2/xml/XmlUtil.h | 85 +++++ tools/aapt2/xml/XmlUtil_test.cpp | 54 +++ 60 files changed, 2539 insertions(+), 1998 deletions(-) delete mode 100644 tools/aapt2/XmlDom.cpp delete mode 100644 tools/aapt2/XmlDom.h delete mode 100644 tools/aapt2/XmlDom_test.cpp delete mode 100644 tools/aapt2/XmlPullParser.cpp delete mode 100644 tools/aapt2/XmlPullParser.h delete mode 100644 tools/aapt2/XmlPullParser_test.cpp create mode 100644 tools/aapt2/link/ReferenceLinker.h delete mode 100644 tools/aapt2/link/ReferenceLinkerVisitor.h create mode 100644 tools/aapt2/xml/XmlDom.cpp create mode 100644 tools/aapt2/xml/XmlDom.h create mode 100644 tools/aapt2/xml/XmlDom_test.cpp create mode 100644 tools/aapt2/xml/XmlPullParser.cpp create mode 100644 tools/aapt2/xml/XmlPullParser.h create mode 100644 tools/aapt2/xml/XmlPullParser_test.cpp create mode 100644 tools/aapt2/xml/XmlUtil.cpp create mode 100644 tools/aapt2/xml/XmlUtil.h create mode 100644 tools/aapt2/xml/XmlUtil_test.cpp (limited to 'tools/aapt2/ResourceUtils.cpp') diff --git a/tools/aapt2/Android.mk b/tools/aapt2/Android.mk index ec29c3818bdc..d8e0aac678f0 100644 --- a/tools/aapt2/Android.mk +++ b/tools/aapt2/Android.mk @@ -58,8 +58,9 @@ sources := \ ResourceValues.cpp \ SdkConstants.cpp \ StringPool.cpp \ - XmlDom.cpp \ - XmlPullParser.cpp + xml/XmlDom.cpp \ + xml/XmlPullParser.cpp \ + xml/XmlUtil.cpp testSources := \ compile/IdAssigner_test.cpp \ @@ -90,8 +91,9 @@ testSources := \ ResourceUtils_test.cpp \ StringPool_test.cpp \ ValueVisitor_test.cpp \ - XmlDom_test.cpp \ - XmlPullParser_test.cpp + xml/XmlDom_test.cpp \ + xml/XmlPullParser_test.cpp \ + xml/XmlUtil_test.cpp toolSources := \ compile/Compile.cpp \ diff --git a/tools/aapt2/ResourceParser.cpp b/tools/aapt2/ResourceParser.cpp index d292f623a33e..02fe59c0a03b 100644 --- a/tools/aapt2/ResourceParser.cpp +++ b/tools/aapt2/ResourceParser.cpp @@ -19,9 +19,8 @@ #include "ResourceUtils.h" #include "ResourceValues.h" #include "ValueVisitor.h" -#include "XmlPullParser.h" - #include "util/Util.h" +#include "xml/XmlPullParser.h" #include @@ -29,26 +28,6 @@ namespace aapt { constexpr const char16_t* sXliffNamespaceUri = u"urn:oasis:names:tc:xliff:document:1.2"; -static Maybe findAttribute(XmlPullParser* parser, const StringPiece16& name) { - auto iter = parser->findAttribute(u"", name); - if (iter != parser->endAttributes()) { - return StringPiece16(util::trimWhitespace(iter->value)); - } - return {}; -} - -static Maybe findNonEmptyAttribute(XmlPullParser* parser, - const StringPiece16& name) { - auto iter = parser->findAttribute(u"", name); - if (iter != parser->endAttributes()) { - StringPiece16 trimmed = util::trimWhitespace(iter->value); - if (!trimmed.empty()) { - return trimmed; - } - } - return {}; -} - /** * Returns true if the element is or and can be safely ignored. */ @@ -65,7 +44,7 @@ ResourceParser::ResourceParser(IDiagnostics* diag, ResourceTable* table, const S /** * Build a string from XML that converts nested elements into Span objects. */ -bool ResourceParser::flattenXmlSubtree(XmlPullParser* parser, std::u16string* outRawString, +bool ResourceParser::flattenXmlSubtree(xml::XmlPullParser* parser, std::u16string* outRawString, StyleString* outStyleString) { std::vector spanStack; @@ -74,9 +53,9 @@ bool ResourceParser::flattenXmlSubtree(XmlPullParser* parser, std::u16string* ou outStyleString->spans.clear(); util::StringBuilder builder; size_t depth = 1; - while (XmlPullParser::isGoodEvent(parser->next())) { - const XmlPullParser::Event event = parser->getEvent(); - if (event == XmlPullParser::Event::kEndElement) { + while (xml::XmlPullParser::isGoodEvent(parser->next())) { + const xml::XmlPullParser::Event event = parser->getEvent(); + if (event == xml::XmlPullParser::Event::kEndElement) { if (!parser->getElementNamespace().empty()) { // We already warned and skipped the start element, so just skip here too continue; @@ -91,11 +70,11 @@ bool ResourceParser::flattenXmlSubtree(XmlPullParser* parser, std::u16string* ou outStyleString->spans.push_back(spanStack.back()); spanStack.pop_back(); - } else if (event == XmlPullParser::Event::kText) { + } else if (event == xml::XmlPullParser::Event::kText) { outRawString->append(parser->getText()); builder.append(parser->getText()); - } else if (event == XmlPullParser::Event::kStartElement) { + } else if (event == xml::XmlPullParser::Event::kStartElement) { if (!parser->getElementNamespace().empty()) { if (parser->getElementNamespace() != sXliffNamespaceUri) { // Only warn if this isn't an xliff namespace. @@ -128,7 +107,7 @@ bool ResourceParser::flattenXmlSubtree(XmlPullParser* parser, std::u16string* ou spanStack.push_back(Span{ spanName, static_cast(builder.str().size()) }); } - } else if (event == XmlPullParser::Event::kComment) { + } else if (event == xml::XmlPullParser::Event::kComment) { // Skip } else { assert(false); @@ -140,11 +119,11 @@ bool ResourceParser::flattenXmlSubtree(XmlPullParser* parser, std::u16string* ou return !error; } -bool ResourceParser::parse(XmlPullParser* parser) { +bool ResourceParser::parse(xml::XmlPullParser* parser) { bool error = false; const size_t depth = parser->getDepth(); - while (XmlPullParser::nextChildNode(parser, depth)) { - if (parser->getEvent() != XmlPullParser::Event::kStartElement) { + while (xml::XmlPullParser::nextChildNode(parser, depth)) { + if (parser->getEvent() != xml::XmlPullParser::Event::kStartElement) { // Skip comments and text. continue; } @@ -159,7 +138,7 @@ bool ResourceParser::parse(XmlPullParser* parser) { break; }; - if (parser->getEvent() == XmlPullParser::Event::kBadDocument) { + if (parser->getEvent() == xml::XmlPullParser::Event::kBadDocument) { mDiag->error(DiagMessage(mSource.withLine(parser->getLineNumber())) << "xml parser error: " << parser->getLastError()); return false; @@ -167,10 +146,11 @@ bool ResourceParser::parse(XmlPullParser* parser) { return !error; } -static bool shouldStripResource(XmlPullParser* parser, const Maybe productToMatch) { - assert(parser->getEvent() == XmlPullParser::Event::kStartElement); +static bool shouldStripResource(const xml::XmlPullParser* parser, + const Maybe productToMatch) { + assert(parser->getEvent() == xml::XmlPullParser::Event::kStartElement); - if (Maybe maybeProduct = findNonEmptyAttribute(parser, u"product")) { + if (Maybe maybeProduct = xml::findNonEmptyAttribute(parser, u"product")) { if (!productToMatch) { if (maybeProduct.value() != u"default" && maybeProduct.value() != u"phone") { // We didn't specify a product and this is not a default product, so skip. @@ -229,20 +209,20 @@ static bool addResourcesToTable(ResourceTable* table, const ConfigDescription& c return !error; } -bool ResourceParser::parseResources(XmlPullParser* parser) { +bool ResourceParser::parseResources(xml::XmlPullParser* parser) { std::set strippedResources; bool error = false; std::u16string comment; const size_t depth = parser->getDepth(); - while (XmlPullParser::nextChildNode(parser, depth)) { - const XmlPullParser::Event event = parser->getEvent(); - if (event == XmlPullParser::Event::kComment) { + while (xml::XmlPullParser::nextChildNode(parser, depth)) { + const xml::XmlPullParser::Event event = parser->getEvent(); + if (event == xml::XmlPullParser::Event::kComment) { comment = parser->getComment(); continue; } - if (event == XmlPullParser::Event::kText) { + if (event == xml::XmlPullParser::Event::kText) { if (!util::trimWhitespace(parser->getText()).empty()) { mDiag->error(DiagMessage(mSource.withLine(parser->getLineNumber())) << "plain text not allowed here"); @@ -251,7 +231,7 @@ bool ResourceParser::parseResources(XmlPullParser* parser) { continue; } - assert(event == XmlPullParser::Event::kStartElement); + assert(event == xml::XmlPullParser::Event::kStartElement); if (!parser->getElementNamespace().empty()) { // Skip unknown namespace. @@ -266,7 +246,7 @@ bool ResourceParser::parseResources(XmlPullParser* parser) { if (elementName == u"item") { // Items simply have their type encoded in the type attribute. - if (Maybe maybeType = findNonEmptyAttribute(parser, u"type")) { + if (Maybe maybeType = xml::findNonEmptyAttribute(parser, u"type")) { elementName = maybeType.value().toString(); } else { mDiag->error(DiagMessage(mSource.withLine(parser->getLineNumber())) @@ -280,7 +260,7 @@ bool ResourceParser::parseResources(XmlPullParser* parser) { parsedResource.source = mSource.withLine(parser->getLineNumber()); parsedResource.comment = std::move(comment); - if (Maybe maybeName = findNonEmptyAttribute(parser, u"name")) { + if (Maybe maybeName = xml::findNonEmptyAttribute(parser, u"name")) { parsedResource.name.entry = maybeName.value().toString(); } else if (elementName != u"public-group") { @@ -403,7 +383,7 @@ enum { * an Item. If allowRawValue is false, nullptr is returned in this * case. */ -std::unique_ptr ResourceParser::parseXml(XmlPullParser* parser, const uint32_t typeMask, +std::unique_ptr ResourceParser::parseXml(xml::XmlPullParser* parser, const uint32_t typeMask, const bool allowRawValue) { const size_t beginXmlLine = parser->getLineNumber(); @@ -432,10 +412,7 @@ std::unique_ptr ResourceParser::parseXml(XmlPullParser* parser, const uint if (processedItem) { // Fix up the reference. if (Reference* ref = valueCast(processedItem.get())) { - if (Maybe transformedName = - parser->transformPackage(ref->name.value(), u"")) { - ref->name = std::move(transformedName); - } + transformReferenceFromNamespace(parser, u"", ref); } return processedItem; } @@ -456,11 +433,11 @@ std::unique_ptr ResourceParser::parseXml(XmlPullParser* parser, const uint return {}; } -bool ResourceParser::parseString(XmlPullParser* parser, ParsedResource* outResource) { +bool ResourceParser::parseString(xml::XmlPullParser* parser, ParsedResource* outResource) { const Source source = mSource.withLine(parser->getLineNumber()); bool formatted = true; - if (Maybe formattedAttr = findAttribute(parser, u"formatted")) { + if (Maybe formattedAttr = xml::findAttribute(parser, u"formatted")) { if (!ResourceUtils::tryParseBool(formattedAttr.value(), &formatted)) { mDiag->error(DiagMessage(source) << "invalid value for 'formatted'. Must be a boolean"); return false; @@ -468,7 +445,7 @@ bool ResourceParser::parseString(XmlPullParser* parser, ParsedResource* outResou } bool translateable = mOptions.translatable; - if (Maybe translateableAttr = findAttribute(parser, u"translatable")) { + if (Maybe translateableAttr = xml::findAttribute(parser, u"translatable")) { if (!ResourceUtils::tryParseBool(translateableAttr.value(), &translateable)) { mDiag->error(DiagMessage(source) << "invalid value for 'translatable'. Must be a boolean"); @@ -495,7 +472,7 @@ bool ResourceParser::parseString(XmlPullParser* parser, ParsedResource* outResou return true; } -bool ResourceParser::parseColor(XmlPullParser* parser, ParsedResource* outResource) { +bool ResourceParser::parseColor(xml::XmlPullParser* parser, ParsedResource* outResource) { const Source source = mSource.withLine(parser->getLineNumber()); outResource->value = parseXml(parser, android::ResTable_map::TYPE_COLOR, kNoRawString); @@ -506,7 +483,7 @@ bool ResourceParser::parseColor(XmlPullParser* parser, ParsedResource* outResour return true; } -bool ResourceParser::parsePrimitive(XmlPullParser* parser, ParsedResource* outResource) { +bool ResourceParser::parsePrimitive(xml::XmlPullParser* parser, ParsedResource* outResource) { const Source source = mSource.withLine(parser->getLineNumber()); uint32_t typeMask = 0; @@ -540,10 +517,10 @@ bool ResourceParser::parsePrimitive(XmlPullParser* parser, ParsedResource* outRe return true; } -bool ResourceParser::parsePublic(XmlPullParser* parser, ParsedResource* outResource) { +bool ResourceParser::parsePublic(xml::XmlPullParser* parser, ParsedResource* outResource) { const Source source = mSource.withLine(parser->getLineNumber()); - Maybe maybeType = findNonEmptyAttribute(parser, u"type"); + Maybe maybeType = xml::findNonEmptyAttribute(parser, u"type"); if (!maybeType) { mDiag->error(DiagMessage(source) << " must have a 'type' attribute"); return false; @@ -558,7 +535,7 @@ bool ResourceParser::parsePublic(XmlPullParser* parser, ParsedResource* outResou outResource->name.type = *parsedType; - if (Maybe maybeId = findNonEmptyAttribute(parser, u"id")) { + if (Maybe maybeId = xml::findNonEmptyAttribute(parser, u"id")) { android::Res_value val; bool result = android::ResTable::stringToInt(maybeId.value().data(), maybeId.value().size(), &val); @@ -580,10 +557,10 @@ bool ResourceParser::parsePublic(XmlPullParser* parser, ParsedResource* outResou return true; } -bool ResourceParser::parsePublicGroup(XmlPullParser* parser, ParsedResource* outResource) { +bool ResourceParser::parsePublicGroup(xml::XmlPullParser* parser, ParsedResource* outResource) { const Source source = mSource.withLine(parser->getLineNumber()); - Maybe maybeType = findNonEmptyAttribute(parser, u"type"); + Maybe maybeType = xml::findNonEmptyAttribute(parser, u"type"); if (!maybeType) { mDiag->error(DiagMessage(source) << " must have a 'type' attribute"); return false; @@ -596,7 +573,7 @@ bool ResourceParser::parsePublicGroup(XmlPullParser* parser, ParsedResource* out return false; } - Maybe maybeId = findNonEmptyAttribute(parser, u"first-id"); + Maybe maybeId = xml::findNonEmptyAttribute(parser, u"first-id"); if (!maybeId) { mDiag->error(DiagMessage(source) << " must have a 'first-id' attribute"); return false; @@ -615,11 +592,11 @@ bool ResourceParser::parsePublicGroup(XmlPullParser* parser, ParsedResource* out std::u16string comment; bool error = false; const size_t depth = parser->getDepth(); - while (XmlPullParser::nextChildNode(parser, depth)) { - if (parser->getEvent() == XmlPullParser::Event::kComment) { + while (xml::XmlPullParser::nextChildNode(parser, depth)) { + if (parser->getEvent() == xml::XmlPullParser::Event::kComment) { comment = util::trimWhitespace(parser->getComment()).toString(); continue; - } else if (parser->getEvent() != XmlPullParser::Event::kStartElement) { + } else if (parser->getEvent() != xml::XmlPullParser::Event::kStartElement) { // Skip text. continue; } @@ -628,20 +605,20 @@ bool ResourceParser::parsePublicGroup(XmlPullParser* parser, ParsedResource* out const std::u16string& elementNamespace = parser->getElementNamespace(); const std::u16string& elementName = parser->getElementName(); if (elementNamespace.empty() && elementName == u"public") { - Maybe maybeName = findNonEmptyAttribute(parser, u"name"); + Maybe maybeName = xml::findNonEmptyAttribute(parser, u"name"); if (!maybeName) { mDiag->error(DiagMessage(itemSource) << " must have a 'name' attribute"); error = true; continue; } - if (findNonEmptyAttribute(parser, u"id")) { + if (xml::findNonEmptyAttribute(parser, u"id")) { mDiag->error(DiagMessage(itemSource) << "'id' is ignored within "); error = true; continue; } - if (findNonEmptyAttribute(parser, u"type")) { + if (xml::findNonEmptyAttribute(parser, u"type")) { mDiag->error(DiagMessage(itemSource) << "'type' is ignored within "); error = true; continue; @@ -666,10 +643,10 @@ bool ResourceParser::parsePublicGroup(XmlPullParser* parser, ParsedResource* out return !error; } -bool ResourceParser::parseSymbol(XmlPullParser* parser, ParsedResource* outResource) { +bool ResourceParser::parseSymbol(xml::XmlPullParser* parser, ParsedResource* outResource) { const Source source = mSource.withLine(parser->getLineNumber()); - Maybe maybeType = findNonEmptyAttribute(parser, u"type"); + Maybe maybeType = xml::findNonEmptyAttribute(parser, u"type"); if (!maybeType) { mDiag->error(DiagMessage(source) << "<" << parser->getElementName() << "> must have a " "'type' attribute"); @@ -715,17 +692,16 @@ static uint32_t parseFormatAttribute(const StringPiece16& str) { return mask; } - - -bool ResourceParser::parseAttr(XmlPullParser* parser, ParsedResource* outResource) { +bool ResourceParser::parseAttr(xml::XmlPullParser* parser, ParsedResource* outResource) { outResource->source = mSource.withLine(parser->getLineNumber()); return parseAttrImpl(parser, outResource, false); } -bool ResourceParser::parseAttrImpl(XmlPullParser* parser, ParsedResource* outResource, bool weak) { +bool ResourceParser::parseAttrImpl(xml::XmlPullParser* parser, ParsedResource* outResource, + bool weak) { uint32_t typeMask = 0; - Maybe maybeFormat = findAttribute(parser, u"format"); + Maybe maybeFormat = xml::findAttribute(parser, u"format"); if (maybeFormat) { typeMask = parseFormatAttribute(maybeFormat.value()); if (typeMask == 0) { @@ -735,18 +711,6 @@ bool ResourceParser::parseAttrImpl(XmlPullParser* parser, ParsedResource* outRes } } - // If this is a declaration, the package name may be in the name. Separate these out. - // Eg. - // No format attribute is allowed. - if (weak && !maybeFormat) { - StringPiece16 package, type, name; - ResourceUtils::extractResourceName(outResource->name.entry, &package, &type, &name); - if (type.empty() && !package.empty()) { - outResource->name.package = package.toString(); - outResource->name.entry = name.toString(); - } - } - struct SymbolComparator { bool operator()(const Attribute::Symbol& a, const Attribute::Symbol& b) { return a.symbol.name.value() < b.symbol.name.value(); @@ -758,11 +722,11 @@ bool ResourceParser::parseAttrImpl(XmlPullParser* parser, ParsedResource* outRes std::u16string comment; bool error = false; const size_t depth = parser->getDepth(); - while (XmlPullParser::nextChildNode(parser, depth)) { - if (parser->getEvent() == XmlPullParser::Event::kComment) { + while (xml::XmlPullParser::nextChildNode(parser, depth)) { + if (parser->getEvent() == xml::XmlPullParser::Event::kComment) { comment = util::trimWhitespace(parser->getComment()).toString(); continue; - } else if (parser->getEvent() != XmlPullParser::Event::kStartElement) { + } else if (parser->getEvent() != xml::XmlPullParser::Event::kStartElement) { // Skip text. continue; } @@ -834,17 +798,17 @@ bool ResourceParser::parseAttrImpl(XmlPullParser* parser, ParsedResource* outRes return true; } -Maybe ResourceParser::parseEnumOrFlagItem(XmlPullParser* parser, +Maybe ResourceParser::parseEnumOrFlagItem(xml::XmlPullParser* parser, const StringPiece16& tag) { const Source source = mSource.withLine(parser->getLineNumber()); - Maybe maybeName = findNonEmptyAttribute(parser, u"name"); + Maybe maybeName = xml::findNonEmptyAttribute(parser, u"name"); if (!maybeName) { mDiag->error(DiagMessage(source) << "no attribute 'name' found for tag <" << tag << ">"); return {}; } - Maybe maybeValue = findNonEmptyAttribute(parser, u"value"); + Maybe maybeValue = xml::findNonEmptyAttribute(parser, u"value"); if (!maybeValue) { mDiag->error(DiagMessage(source) << "no attribute 'value' found for tag <" << tag << ">"); return {}; @@ -863,12 +827,19 @@ Maybe ResourceParser::parseEnumOrFlagItem(XmlPullParser* pars val.data }; } -static Maybe parseXmlAttributeName(StringPiece16 str) { +static Maybe parseXmlAttributeName(StringPiece16 str) { str = util::trimWhitespace(str); - const char16_t* const start = str.data(); + const char16_t* start = str.data(); const char16_t* const end = start + str.size(); const char16_t* p = start; + Reference ref; + if (p != end && *p == u'*') { + ref.privateReference = true; + start++; + p++; + } + StringPiece16 package; StringPiece16 name; while (p != end) { @@ -880,28 +851,27 @@ static Maybe parseXmlAttributeName(StringPiece16 str) { p++; } - return ResourceName(package.toString(), ResourceType::kAttr, + ref.name = ResourceName(package.toString(), ResourceType::kAttr, name.empty() ? str.toString() : name.toString()); + return Maybe(std::move(ref)); } -bool ResourceParser::parseStyleItem(XmlPullParser* parser, Style* style) { +bool ResourceParser::parseStyleItem(xml::XmlPullParser* parser, Style* style) { const Source source = mSource.withLine(parser->getLineNumber()); - Maybe maybeName = findNonEmptyAttribute(parser, u"name"); + Maybe maybeName = xml::findNonEmptyAttribute(parser, u"name"); if (!maybeName) { mDiag->error(DiagMessage(source) << " must have a 'name' attribute"); return false; } - Maybe maybeKey = parseXmlAttributeName(maybeName.value()); + Maybe maybeKey = parseXmlAttributeName(maybeName.value()); if (!maybeKey) { mDiag->error(DiagMessage(source) << "invalid attribute name '" << maybeName.value() << "'"); return false; } - if (Maybe transformedName = parser->transformPackage(maybeKey.value(), u"")) { - maybeKey = std::move(transformedName); - } + transformReferenceFromNamespace(parser, u"", &maybeKey.value()); std::unique_ptr value = parseXml(parser, 0, kAllowRawString); if (!value) { @@ -909,15 +879,15 @@ bool ResourceParser::parseStyleItem(XmlPullParser* parser, Style* style) { return false; } - style->entries.push_back(Style::Entry{ Reference(maybeKey.value()), std::move(value) }); + style->entries.push_back(Style::Entry{ std::move(maybeKey.value()), std::move(value) }); return true; } -bool ResourceParser::parseStyle(XmlPullParser* parser, ParsedResource* outResource) { +bool ResourceParser::parseStyle(xml::XmlPullParser* parser, ParsedResource* outResource) { const Source source = mSource.withLine(parser->getLineNumber()); std::unique_ptr