From 8c3f31f022f7e094fd227ef0c2987e0846cb3e43 Mon Sep 17 00:00:00 2001 From: Adam Lesinski Date: Wed, 7 Sep 2016 13:45:13 -0700 Subject: [PATCH] AAPT2: Fix issue with styled string indices Styled strings use spans to denote which part is styled (, , etc). Spans are simply a range of indices into the original string. In Java, we use String and its internal representation, meaning we must encode the indices using UTF16 lengths. When the internal AAPT2 representation of strings switched to UTF8, the indices also began to index into the UTF8 string. This change reverts the indices to use UTF16 lengths. Bug:31170115 Change-Id: I07b8b5b67d2542c7e0a855b601cdbd3ac4ebffb0 --- tools/aapt2/ResourceParser.cpp | 6 ++--- tools/aapt2/ResourceParser_test.cpp | 34 +++++++++++++++++++++++++++++ tools/aapt2/util/Util.cpp | 20 ++++++++++++----- tools/aapt2/util/Util.h | 10 +++++++++ 4 files changed, 62 insertions(+), 8 deletions(-) diff --git a/tools/aapt2/ResourceParser.cpp b/tools/aapt2/ResourceParser.cpp index 32e5cfd573bc3..c430c46378991 100644 --- a/tools/aapt2/ResourceParser.cpp +++ b/tools/aapt2/ResourceParser.cpp @@ -152,7 +152,7 @@ bool ResourceParser::flattenXmlSubtree(xml::XmlPullParser* parser, std::string* break; } - spanStack.back().lastChar = builder.str().size() - 1; + spanStack.back().lastChar = builder.utf16Len() - 1; outStyleString->spans.push_back(spanStack.back()); spanStack.pop_back(); @@ -185,12 +185,12 @@ bool ResourceParser::flattenXmlSubtree(xml::XmlPullParser* parser, std::string* spanName += attrIter->value; } - if (builder.str().size() > std::numeric_limits::max()) { + if (builder.utf16Len() > std::numeric_limits::max()) { mDiag->error(DiagMessage(mSource.withLine(parser->getLineNumber())) << "style string '" << builder.str() << "' is too long"); error = true; } else { - spanStack.push_back(Span{ spanName, static_cast(builder.str().size()) }); + spanStack.push_back(Span{ spanName, static_cast(builder.utf16Len()) }); } } else if (event == xml::XmlPullParser::Event::kComment) { diff --git a/tools/aapt2/ResourceParser_test.cpp b/tools/aapt2/ResourceParser_test.cpp index 3d03a882cd020..e097740966f6d 100644 --- a/tools/aapt2/ResourceParser_test.cpp +++ b/tools/aapt2/ResourceParser_test.cpp @@ -90,6 +90,40 @@ TEST_F(ResourceParserTest, ParseFormattedString) { ASSERT_TRUE(testParse(input)); } +TEST_F(ResourceParserTest, ParseStyledString) { + // Use a surrogate pair unicode point so that we can verify that the span indices + // use UTF-16 length and not UTF-18 length. + std::string input = "This is my aunt\u2019s string"; + ASSERT_TRUE(testParse(input)); + + StyledString* str = test::getValue(&mTable, "string/foo"); + ASSERT_NE(nullptr, str); + + const std::string expectedStr = "This is my aunt\u2019s string"; + EXPECT_EQ(expectedStr, *str->value->str); + EXPECT_EQ(1u, str->value->spans.size()); + + EXPECT_EQ(std::string("b"), *str->value->spans[0].name); + EXPECT_EQ(17u, str->value->spans[0].firstChar); + EXPECT_EQ(23u, str->value->spans[0].lastChar); +} + +TEST_F(ResourceParserTest, ParseStringWithWhitespace) { + std::string input = " This is what I think "; + ASSERT_TRUE(testParse(input)); + + String* str = test::getValue(&mTable, "string/foo"); + ASSERT_NE(nullptr, str); + EXPECT_EQ(std::string("This is what I think"), *str->value); + + input = "\" This is what I think \""; + ASSERT_TRUE(testParse(input)); + + str = test::getValue(&mTable, "string/foo2"); + ASSERT_NE(nullptr, str); + EXPECT_EQ(std::string(" This is what I think "), *str->value); +} + TEST_F(ResourceParserTest, IgnoreXliffTags) { std::string input = "\n" diff --git a/tools/aapt2/util/Util.cpp b/tools/aapt2/util/Util.cpp index e743247be8a9a..b0bec624cc9cd 100644 --- a/tools/aapt2/util/Util.cpp +++ b/tools/aapt2/util/Util.cpp @@ -314,6 +314,9 @@ StringBuilder& StringBuilder::append(const StringPiece& str) { return *this; } + // Where the new data will be appended to. + size_t newDataIndex = mStr.size(); + const char* const end = str.end(); const char* start = str.begin(); const char* current = start; @@ -422,6 +425,16 @@ StringBuilder& StringBuilder::append(const StringPiece& str) { current++; } mStr.append(start, end - start); + + // Accumulate the added string's UTF-16 length. + ssize_t len = utf8_to_utf16_length( + reinterpret_cast(mStr.data()) + newDataIndex, + mStr.size() - newDataIndex); + if (len < 0) { + mError = "invalid unicode code point"; + return *this; + } + mUtf16Len += len; return *this; } @@ -434,11 +447,8 @@ std::u16string utf8ToUtf16(const StringPiece& utf8) { std::u16string utf16; utf16.resize(utf16Length); - utf8_to_utf16( - reinterpret_cast(utf8.data()), - utf8.length(), - &*utf16.begin(), - (size_t) utf16Length + 1); + utf8_to_utf16(reinterpret_cast(utf8.data()), utf8.length(), + &*utf16.begin(), utf16Length + 1); return utf16; } diff --git a/tools/aapt2/util/Util.h b/tools/aapt2/util/Util.h index 998ecf7702bda..9c88354dfaf11 100644 --- a/tools/aapt2/util/Util.h +++ b/tools/aapt2/util/Util.h @@ -163,10 +163,16 @@ public: StringBuilder& append(const StringPiece& str); const std::string& str() const; const std::string& error() const; + + // When building StyledStrings, we need UTF-16 indices into the string, + // which is what the Java layer expects when dealing with java String.charAt(). + size_t utf16Len() const; + operator bool() const; private: std::string mStr; + size_t mUtf16Len = 0; bool mQuote = false; bool mTrailingSpace = false; bool mLastCharWasEscape = false; @@ -181,6 +187,10 @@ inline const std::string& StringBuilder::error() const { return mError; } +inline size_t StringBuilder::utf16Len() const { + return mUtf16Len; +} + inline StringBuilder::operator bool() const { return mError.empty(); }