commit b23f1e077b02a1d62bcf5e34655e8dc979e124fa
author Adam Lesinski <adamlesinski@google.com> Tue Nov 03 12:24:17 2015 -0800
committer Adam Lesinski <adamlesinski@google.com> Tue Nov 03 12:58:12 2015 -0800
tree 854c4a7afe2e62e9ead4a10ce17422e3923a1502
parent 2ae4a877d1623f851040ce69239552c873f1abf0

AAPT2: Verify positional Java String format arguments in strings

Change-Id: Id415969035a0d5712857c0e11e140155566a960c

tools/aapt2/ResourceParser.cpp

diff --git a/tools/aapt2/ResourceParser.cpp b/tools/aapt2/ResourceParser.cpp
index 0c7a4d5..2d6c0c2 100644
--- a/tools/aapt2/ResourceParser.cpp
+++ b/tools/aapt2/ResourceParser.cpp
@@ -62,6 +62,7 @@
                                        StyleString* outStyleString) {
     std::vector<Span> spanStack;
 
+    bool error = false;
     outRawString->clear();
     outStyleString->spans.clear();
     util::StringBuilder builder;
@@ -84,7 +85,6 @@
             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 @@
             if (builder.str().size() > std::numeric_limits<uint32_t>::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<uint32_t>(builder.str().size()) });
             }
-            spanStack.push_back(Span{ spanName, static_cast<uint32_t>(builder.str().size()) });
 
         } else if (event == XmlPullParser::Event::kComment) {
             // Skip
@@ -129,7 +130,7 @@
     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 @@
 bool ResourceParser::parseString(XmlPullParser* parser, ParsedResource* outResource) {
     const Source source = mSource.withLine(parser->getLineNumber());
 
-    // TODO(adamlesinski): Read "untranslateable" attribute.
+    bool formatted = true;
+    if (Maybe<StringPiece16> 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<StringPiece16> 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<String>(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;
 }