Add additional check on float precision after parsing, only compile the
value to a float when the difference between float and double parsed
from same raw string is smaller than 1.
Bug: b/69347762
Test: Verified affected atests pass and added new atest
Change-Id: I25da0baccba580484db39aa2d0a1bb765706635d
diff --git a/libs/androidfw/ResourceTypes.cpp b/libs/androidfw/ResourceTypes.cpp
index 29d33da..136d6ce 100644
--- a/libs/androidfw/ResourceTypes.cpp
+++ b/libs/androidfw/ResourceTypes.cpp
@@ -5436,37 +5436,66 @@
return U16StringToInt(s, len, outValue);
}
-bool ResTable::stringToFloat(const char16_t* s, size_t len, Res_value* outValue)
-{
- while (len > 0 && isspace16(*s)) {
- s++;
- len--;
+template <typename T>
+bool parseFloatingPoint(const char16_t* inBuf, size_t inLen, char* tempBuf,
+ const char** outEnd, T& out){
+ while (inLen > 0 && isspace16(*inBuf)) {
+ inBuf++;
+ inLen--;
}
- if (len <= 0) {
+ if (inLen <= 0) {
return false;
}
- char buf[128];
int i=0;
- while (len > 0 && *s != 0 && i < 126) {
- if (*s > 255) {
+ while (inLen > 0 && *inBuf != 0 && i < 126) {
+ if (*inBuf > 255) {
return false;
}
- buf[i++] = *s++;
- len--;
+ tempBuf[i++] = *inBuf++;
+ inLen--;
}
- if (len > 0) {
+ if (inLen > 0) {
return false;
}
- if ((buf[0] < '0' || buf[0] > '9') && buf[0] != '.' && buf[0] != '-' && buf[0] != '+') {
+ if ((tempBuf[0] < '0' || tempBuf[0] > '9') && tempBuf[0] != '.' && tempBuf[0] != '-' && tempBuf[0] != '+') {
return false;
}
- buf[i] = 0;
- const char* end;
- float f = strtof(buf, (char**)&end);
+ tempBuf[i] = 0;
+ if constexpr(std::is_same_v<T, float>) {
+ out = strtof(tempBuf, (char**)outEnd);
+ } else {
+ out = strtod(tempBuf, (char**)outEnd);
+ }
+ return true;
+}
+
+bool ResTable::stringToDouble(const char16_t* s, size_t len, double& d){
+ char buf[128];
+ const char* end = nullptr;
+ if (!parseFloatingPoint(s, len, buf, &end, d)) {
+ return false;
+ }
+
+ while (*end != 0 && isspace((unsigned char)*end)) {
+ end++;
+ }
+
+ return *end == 0;
+}
+
+bool ResTable::stringToFloat(const char16_t* s, size_t len, Res_value* outValue)
+{
+ char buf[128];
+ const char* end = nullptr;
+ float f;
+
+ if (!parseFloatingPoint(s, len, buf, &end, f)) {
+ return false;
+ }
if (*end != 0 && !isspace((unsigned char)*end)) {
// Might be a unit...
diff --git a/libs/androidfw/include/androidfw/ResourceTypes.h b/libs/androidfw/include/androidfw/ResourceTypes.h
index 631bda4..4eb1d7a 100644
--- a/libs/androidfw/include/androidfw/ResourceTypes.h
+++ b/libs/androidfw/include/androidfw/ResourceTypes.h
@@ -2162,6 +2162,7 @@
static bool stringToInt(const char16_t* s, size_t len, Res_value* outValue);
static bool stringToFloat(const char16_t* s, size_t len, Res_value* outValue);
+ static bool stringToDouble(const char16_t* s, size_t len, double& outValue);
// Used with stringToValue.
class Accessor
diff --git a/tools/aapt2/ResourceUtils.cpp b/tools/aapt2/ResourceUtils.cpp
index 5a118a9..91f4d60 100644
--- a/tools/aapt2/ResourceUtils.cpp
+++ b/tools/aapt2/ResourceUtils.cpp
@@ -670,8 +670,26 @@
// Try parsing this as a float.
auto floating_point = TryParseFloat(value);
if (floating_point) {
+ // Only check if the parsed result lost precision when the parsed item is
+ // android::Res_value::TYPE_FLOAT and there is other possible types saved in type_mask, like
+ // ResTable_map::TYPE_INTEGER.
if (type_mask & AndroidTypeToAttributeTypeMask(floating_point->value.dataType)) {
- return std::move(floating_point);
+ const bool mayOnlyBeFloat = (type_mask & ~float_mask) == 0;
+ const bool parsedAsFloat = floating_point->value.dataType == android::Res_value::TYPE_FLOAT;
+ if (!mayOnlyBeFloat && parsedAsFloat) {
+ float f = reinterpret_cast<float&>(floating_point->value.data);
+ std::u16string str16 = android::util::Utf8ToUtf16(util::TrimWhitespace(value));
+ double d;
+ if (android::ResTable::stringToDouble(str16.data(), str16.size(), d)) {
+ // Parse as a float only if the difference between float and double parsed from the
+ // same string is smaller than 1, otherwise return as raw string.
+ if (fabs(f - d) < 1) {
+ return std::move(floating_point);
+ }
+ }
+ } else {
+ return std::move(floating_point);
+ }
}
}
}
diff --git a/tools/aapt2/ResourceUtils_test.cpp b/tools/aapt2/ResourceUtils_test.cpp
index 568871a..df47a64 100644
--- a/tools/aapt2/ResourceUtils_test.cpp
+++ b/tools/aapt2/ResourceUtils_test.cpp
@@ -228,6 +228,26 @@
Pointee(ValueEq(BinaryPrimitive(Res_value::TYPE_FLOAT, expected_float_flattened))));
}
+TEST(ResourceUtilsTest, FloatAndBigIntegerParsedCorrectly) {
+ const float expected_float = 0.125f;
+ const uint32_t expected_float_flattened = *(uint32_t*)&expected_float;
+ EXPECT_THAT(ResourceUtils::TryParseItemForAttribute("0.125", ResTable_map::TYPE_FLOAT),
+ Pointee(ValueEq(BinaryPrimitive(Res_value::TYPE_FLOAT, expected_float_flattened))));
+
+ const float special_float = 1.0f;
+ const uint32_t special_float_flattened = *(uint32_t*)&special_float;
+ EXPECT_THAT(ResourceUtils::TryParseItemForAttribute("1.0", ResTable_map::TYPE_FLOAT),
+ Pointee(ValueEq(BinaryPrimitive(Res_value::TYPE_FLOAT, special_float_flattened))));
+
+ EXPECT_EQ(ResourceUtils::TryParseItemForAttribute("1099511627776", ResTable_map::TYPE_INTEGER),
+ std::unique_ptr<Item>(nullptr));
+
+ const float big_float = 1099511627776.0f;
+ const uint32_t big_flattened = *(uint32_t*)&big_float;
+ EXPECT_THAT(ResourceUtils::TryParseItemForAttribute("1099511627776", ResTable_map::TYPE_FLOAT),
+ Pointee(ValueEq(BinaryPrimitive(Res_value::TYPE_FLOAT, big_flattened))));
+}
+
TEST(ResourceUtilsTest, ParseSdkVersionWithCodename) {
EXPECT_THAT(ResourceUtils::ParseSdkVersion("Q"), Eq(std::optional<int>(10000)));
EXPECT_THAT(ResourceUtils::ParseSdkVersion("Q.fingerprint"), Eq(std::optional<int>(10000)));
diff --git a/tools/aapt2/ResourceValues.cpp b/tools/aapt2/ResourceValues.cpp
index a5754e0..166b01b 100644
--- a/tools/aapt2/ResourceValues.cpp
+++ b/tools/aapt2/ResourceValues.cpp
@@ -439,6 +439,21 @@
return str;
}
+// This function is designed to using different specifier to print different floats,
+// which can print more accurate format rather than using %g only.
+const char* BinaryPrimitive::DecideFormat(float f) {
+ // if the float is either too big or too tiny, print it in scientific notation.
+ // eg: "10995116277760000000000" to 1.099512e+22, "0.00000000001" to 1.000000e-11
+ if (fabs(f) > std::numeric_limits<int64_t>::max() || fabs(f) < 1e-10) {
+ return "%e";
+ // Else if the number is an integer exactly, print it without trailing zeros.
+ // eg: "1099511627776" to 1099511627776
+ } else if (int64_t(f) == f) {
+ return "%.0f";
+ }
+ return "%g";
+}
+
void BinaryPrimitive::PrettyPrint(Printer* printer) const {
using ::android::Res_value;
switch (value.dataType) {
@@ -470,7 +485,9 @@
break;
case Res_value::TYPE_FLOAT:
- printer->Print(StringPrintf("%g", *reinterpret_cast<const float*>(&value.data)));
+ float f;
+ f = *reinterpret_cast<const float*>(&value.data);
+ printer->Print(StringPrintf(DecideFormat(f), f));
break;
case Res_value::TYPE_DIMENSION:
diff --git a/tools/aapt2/ResourceValues.h b/tools/aapt2/ResourceValues.h
index 6f9dccb..5192c2b 100644
--- a/tools/aapt2/ResourceValues.h
+++ b/tools/aapt2/ResourceValues.h
@@ -284,6 +284,7 @@
bool Equals(const Value* value) const override;
bool Flatten(android::Res_value* out_value) const override;
void Print(std::ostream* out) const override;
+ static const char* DecideFormat(float f);
void PrettyPrint(text::Printer* printer) const override;
};