AAPT2: Fix processing of quotes in XML
When processing attributes in XML, quotes can't be used to mark a
section as whitespace preserving, so the assumption should be that the
entire string is whitespace preserving, which makes quote characters
literals.
Bug: 62840718
Bug: 62840406
Test: make aapt2_tests
Change-Id: I4afff02148b5b8e78833abf1f323c2f5325d6155
diff --git a/tools/aapt2/util/Maybe.h b/tools/aapt2/util/Maybe.h
index b43f8e8..9a82418 100644
--- a/tools/aapt2/util/Maybe.h
+++ b/tools/aapt2/util/Maybe.h
@@ -281,16 +281,12 @@
return Maybe<T>();
}
-/**
- * Define the == operator between Maybe<T> and Maybe<U> only if the operator T
- * == U is defined.
- * That way the compiler will show an error at the callsite when comparing two
- * Maybe<> objects
- * whose inner types can't be compared.
- */
+// Define the == operator between Maybe<T> and Maybe<U> only if the operator T == U is defined.
+// That way the compiler will show an error at the callsite when comparing two Maybe<> objects
+// whose inner types can't be compared.
template <typename T, typename U>
-typename std::enable_if<has_eq_op<T, U>::value, bool>::type operator==(
- const Maybe<T>& a, const Maybe<U>& b) {
+typename std::enable_if<has_eq_op<T, U>::value, bool>::type operator==(const Maybe<T>& a,
+ const Maybe<U>& b) {
if (a && b) {
return a.value() == b.value();
} else if (!a && !b) {
@@ -299,18 +295,22 @@
return false;
}
-/**
- * Same as operator== but negated.
- */
template <typename T, typename U>
-typename std::enable_if<has_eq_op<T, U>::value, bool>::type operator!=(
- const Maybe<T>& a, const Maybe<U>& b) {
+typename std::enable_if<has_eq_op<T, U>::value, bool>::type operator==(const Maybe<T>& a,
+ const U& b) {
+ return a ? a.value() == b : false;
+}
+
+// Same as operator== but negated.
+template <typename T, typename U>
+typename std::enable_if<has_eq_op<T, U>::value, bool>::type operator!=(const Maybe<T>& a,
+ const Maybe<U>& b) {
return !(a == b);
}
template <typename T, typename U>
-typename std::enable_if<has_lt_op<T, U>::value, bool>::type operator<(
- const Maybe<T>& a, const Maybe<U>& b) {
+typename std::enable_if<has_lt_op<T, U>::value, bool>::type operator<(const Maybe<T>& a,
+ const Maybe<U>& b) {
if (a && b) {
return a.value() < b.value();
} else if (!a && !b) {
diff --git a/tools/aapt2/util/Util.cpp b/tools/aapt2/util/Util.cpp
index 9fde1b4..51a75d7 100644
--- a/tools/aapt2/util/Util.cpp
+++ b/tools/aapt2/util/Util.cpp
@@ -327,6 +327,9 @@
return isspace(static_cast<char>(codepoint));
}
+StringBuilder::StringBuilder(bool preserve_spaces) : preserve_spaces_(preserve_spaces) {
+}
+
StringBuilder& StringBuilder::Append(const StringPiece& str) {
if (!error_.empty()) {
return *this;
@@ -372,14 +375,12 @@
}
last_char_was_escape_ = false;
- } else if (codepoint == U'"') {
+ } else if (!preserve_spaces_ && codepoint == U'"') {
if (!quote_ && trailing_space_) {
- // We found an opening quote, and we have
- // trailing space, so we should append that
+ // We found an opening quote, and we have trailing space, so we should append that
// space now.
if (trailing_space_) {
- // We had trailing whitespace, so
- // replace with a single space.
+ // We had trailing whitespace, so replace with a single space.
if (!str_.empty()) {
str_ += ' ';
}
@@ -388,7 +389,7 @@
}
quote_ = !quote_;
- } else if (codepoint == U'\'' && !quote_) {
+ } else if (!preserve_spaces_ && codepoint == U'\'' && !quote_) {
// This should be escaped.
error_ = "unescaped apostrophe";
return *this;
@@ -405,7 +406,7 @@
}
last_char_was_escape_ = true;
} else {
- if (quote_) {
+ if (preserve_spaces_ || quote_) {
// Quotes mean everything is taken, including whitespace.
AppendCodepointToUtf8String(codepoint, &str_);
} else {
diff --git a/tools/aapt2/util/Util.h b/tools/aapt2/util/Util.h
index 8bca9dd..410258c 100644
--- a/tools/aapt2/util/Util.h
+++ b/tools/aapt2/util/Util.h
@@ -171,6 +171,8 @@
class StringBuilder {
public:
+ explicit StringBuilder(bool preserve_spaces = false);
+
StringBuilder& Append(const android::StringPiece& str);
const std::string& ToString() const;
const std::string& Error() const;
@@ -184,6 +186,7 @@
explicit operator bool() const;
private:
+ bool preserve_spaces_;
std::string str_;
size_t utf16_len_ = 0;
bool quote_ = false;
diff --git a/tools/aapt2/util/Util_test.cpp b/tools/aapt2/util/Util_test.cpp
index a09001a..adb5291 100644
--- a/tools/aapt2/util/Util_test.cpp
+++ b/tools/aapt2/util/Util_test.cpp
@@ -20,16 +20,17 @@
#include "test/Test.h"
-using android::StringPiece;
+using ::android::StringPiece;
+using ::testing::Eq;
+using ::testing::Ne;
+using ::testing::SizeIs;
namespace aapt {
TEST(UtilTest, TrimOnlyWhitespace) {
- const std::string full = "\n ";
-
- StringPiece trimmed = util::TrimWhitespace(full);
+ const StringPiece trimmed = util::TrimWhitespace("\n ");
EXPECT_TRUE(trimmed.empty());
- EXPECT_EQ(0u, trimmed.size());
+ EXPECT_THAT(trimmed, SizeIs(0u));
}
TEST(UtilTest, StringEndsWith) {
@@ -41,85 +42,74 @@
}
TEST(UtilTest, StringBuilderSplitEscapeSequence) {
- EXPECT_EQ(StringPiece("this is a new\nline."), util::StringBuilder()
- .Append("this is a new\\")
- .Append("nline.")
- .ToString());
+ EXPECT_THAT(util::StringBuilder().Append("this is a new\\").Append("nline.").ToString(),
+ Eq("this is a new\nline."));
}
TEST(UtilTest, StringBuilderWhitespaceRemoval) {
- EXPECT_EQ(StringPiece("hey guys this is so cool"),
- util::StringBuilder()
- .Append(" hey guys ")
- .Append(" this is so cool ")
- .ToString());
-
- EXPECT_EQ(StringPiece(" wow, so many \t spaces. what?"),
- util::StringBuilder()
- .Append(" \" wow, so many \t ")
- .Append("spaces. \"what? ")
- .ToString());
-
- EXPECT_EQ(StringPiece("where is the pie?"), util::StringBuilder()
- .Append(" where \t ")
- .Append(" \nis the "
- " pie?")
- .ToString());
+ EXPECT_THAT(util::StringBuilder().Append(" hey guys ").Append(" this is so cool ").ToString(),
+ Eq("hey guys this is so cool"));
+ EXPECT_THAT(
+ util::StringBuilder().Append(" \" wow, so many \t ").Append("spaces. \"what? ").ToString(),
+ Eq(" wow, so many \t spaces. what?"));
+ EXPECT_THAT(util::StringBuilder().Append(" where \t ").Append(" \nis the pie?").ToString(),
+ Eq("where is the pie?"));
}
TEST(UtilTest, StringBuilderEscaping) {
- EXPECT_EQ(StringPiece("hey guys\n this \t is so\\ cool"),
- util::StringBuilder()
- .Append(" hey guys\\n ")
- .Append(" this \\t is so\\\\ cool ")
- .ToString());
-
- EXPECT_EQ(StringPiece("@?#\\\'"),
- util::StringBuilder().Append("\\@\\?\\#\\\\\\'").ToString());
+ EXPECT_THAT(util::StringBuilder()
+ .Append(" hey guys\\n ")
+ .Append(" this \\t is so\\\\ cool ")
+ .ToString(),
+ Eq("hey guys\n this \t is so\\ cool"));
+ EXPECT_THAT(util::StringBuilder().Append("\\@\\?\\#\\\\\\'").ToString(), Eq("@?#\\\'"));
}
TEST(UtilTest, StringBuilderMisplacedQuote) {
- util::StringBuilder builder{};
+ util::StringBuilder builder;
EXPECT_FALSE(builder.Append("they're coming!"));
}
TEST(UtilTest, StringBuilderUnicodeCodes) {
- EXPECT_EQ(std::string("\u00AF\u0AF0 woah"),
- util::StringBuilder().Append("\\u00AF\\u0AF0 woah").ToString());
-
+ EXPECT_THAT(util::StringBuilder().Append("\\u00AF\\u0AF0 woah").ToString(),
+ Eq("\u00AF\u0AF0 woah"));
EXPECT_FALSE(util::StringBuilder().Append("\\u00 yo"));
}
+TEST(UtilTest, StringBuilderPreserveSpaces) {
+ EXPECT_THAT(util::StringBuilder(true /*preserve_spaces*/).Append("\"").ToString(), Eq("\""));
+}
+
TEST(UtilTest, TokenizeInput) {
auto tokenizer = util::Tokenize(StringPiece("this| is|the|end"), '|');
auto iter = tokenizer.begin();
- ASSERT_EQ(*iter, StringPiece("this"));
+ ASSERT_THAT(*iter, Eq("this"));
++iter;
- ASSERT_EQ(*iter, StringPiece(" is"));
+ ASSERT_THAT(*iter, Eq(" is"));
++iter;
- ASSERT_EQ(*iter, StringPiece("the"));
+ ASSERT_THAT(*iter, Eq("the"));
++iter;
- ASSERT_EQ(*iter, StringPiece("end"));
+ ASSERT_THAT(*iter, Eq("end"));
++iter;
- ASSERT_EQ(tokenizer.end(), iter);
+ ASSERT_THAT(iter, Eq(tokenizer.end()));
}
TEST(UtilTest, TokenizeEmptyString) {
auto tokenizer = util::Tokenize(StringPiece(""), '|');
auto iter = tokenizer.begin();
- ASSERT_NE(tokenizer.end(), iter);
- ASSERT_EQ(StringPiece(), *iter);
+ ASSERT_THAT(iter, Ne(tokenizer.end()));
+ ASSERT_THAT(*iter, Eq(StringPiece()));
++iter;
- ASSERT_EQ(tokenizer.end(), iter);
+ ASSERT_THAT(iter, Eq(tokenizer.end()));
}
TEST(UtilTest, TokenizeAtEnd) {
auto tokenizer = util::Tokenize(StringPiece("one."), '.');
auto iter = tokenizer.begin();
- ASSERT_EQ(*iter, StringPiece("one"));
+ ASSERT_THAT(*iter, Eq("one"));
++iter;
- ASSERT_NE(iter, tokenizer.end());
- ASSERT_EQ(*iter, StringPiece());
+ ASSERT_THAT(iter, Ne(tokenizer.end()));
+ ASSERT_THAT(*iter, Eq(StringPiece()));
}
TEST(UtilTest, IsJavaClassName) {
@@ -146,52 +136,34 @@
}
TEST(UtilTest, FullyQualifiedClassName) {
- Maybe<std::string> res = util::GetFullyQualifiedClassName("android", ".asdf");
- ASSERT_TRUE(res);
- EXPECT_EQ(res.value(), "android.asdf");
-
- res = util::GetFullyQualifiedClassName("android", ".a.b");
- ASSERT_TRUE(res);
- EXPECT_EQ(res.value(), "android.a.b");
-
- res = util::GetFullyQualifiedClassName("android", "a.b");
- ASSERT_TRUE(res);
- EXPECT_EQ(res.value(), "a.b");
-
- res = util::GetFullyQualifiedClassName("", "a.b");
- ASSERT_TRUE(res);
- EXPECT_EQ(res.value(), "a.b");
-
- res = util::GetFullyQualifiedClassName("android", "Class");
- ASSERT_TRUE(res);
- EXPECT_EQ(res.value(), "android.Class");
-
- res = util::GetFullyQualifiedClassName("", "");
- ASSERT_FALSE(res);
-
- res = util::GetFullyQualifiedClassName("android", "./Apple");
- ASSERT_FALSE(res);
+ EXPECT_THAT(util::GetFullyQualifiedClassName("android", ".asdf"), Eq("android.asdf"));
+ EXPECT_THAT(util::GetFullyQualifiedClassName("android", ".a.b"), Eq("android.a.b"));
+ EXPECT_THAT(util::GetFullyQualifiedClassName("android", "a.b"), Eq("a.b"));
+ EXPECT_THAT(util::GetFullyQualifiedClassName("", "a.b"), Eq("a.b"));
+ EXPECT_THAT(util::GetFullyQualifiedClassName("android", "Class"), Eq("android.Class"));
+ EXPECT_FALSE(util::GetFullyQualifiedClassName("", ""));
+ EXPECT_FALSE(util::GetFullyQualifiedClassName("android", "./Apple"));
}
TEST(UtilTest, ExtractResourcePathComponents) {
StringPiece prefix, entry, suffix;
ASSERT_TRUE(util::ExtractResFilePathParts("res/xml-sw600dp/entry.xml", &prefix, &entry, &suffix));
- EXPECT_EQ(prefix, "res/xml-sw600dp/");
- EXPECT_EQ(entry, "entry");
- EXPECT_EQ(suffix, ".xml");
+ EXPECT_THAT(prefix, Eq("res/xml-sw600dp/"));
+ EXPECT_THAT(entry, Eq("entry"));
+ EXPECT_THAT(suffix, Eq(".xml"));
ASSERT_TRUE(util::ExtractResFilePathParts("res/xml-sw600dp/entry.9.png", &prefix, &entry, &suffix));
- EXPECT_EQ(prefix, "res/xml-sw600dp/");
- EXPECT_EQ(entry, "entry");
- EXPECT_EQ(suffix, ".9.png");
+ EXPECT_THAT(prefix, Eq("res/xml-sw600dp/"));
+ EXPECT_THAT(entry, Eq("entry"));
+ EXPECT_THAT(suffix, Eq(".9.png"));
+
+ ASSERT_TRUE(util::ExtractResFilePathParts("res//.", &prefix, &entry, &suffix));
+ EXPECT_THAT(prefix, Eq("res//"));
+ EXPECT_THAT(entry, Eq(""));
+ EXPECT_THAT(suffix, Eq("."));
EXPECT_FALSE(util::ExtractResFilePathParts("AndroidManifest.xml", &prefix, &entry, &suffix));
EXPECT_FALSE(util::ExtractResFilePathParts("res/.xml", &prefix, &entry, &suffix));
-
- ASSERT_TRUE(util::ExtractResFilePathParts("res//.", &prefix, &entry, &suffix));
- EXPECT_EQ(prefix, "res//");
- EXPECT_EQ(entry, "");
- EXPECT_EQ(suffix, ".");
}
TEST(UtilTest, VerifyJavaStringFormat) {