Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 3124e7ca authored by Adam Lesinski's avatar Adam Lesinski
Browse files

AAPT2: Fix issue with enums and integer attributes 

When an attribute had the format "enum|integer", and a max or min
allowed value set, any value set for this attribute would have its
enum symbol's value checked against the valid integer range.

This would lead to the following:

android:numColumns="autofit"

being interpreted as an integer value of -1, which violated the minimum
expected value for numColumns, which was 0.

Bug: 62358540
Test: make aapt2_tests
Change-Id: I3150410448a533d3595a08ac6b2966264db874d8
parent 52feccbf

tools/aapt2/ResourceValues.cpp

+80 −36
Original line number Diff line number Diff line
@@ -533,75 +533,119 @@ void Attribute::Print(std::ostream* out) const { 
  }

}



static void BuildAttributeMismatchMessage(DiagMessage* msg,

                                          const Attribute* attr,

                                          const Item* value) {

  *msg << "expected";

  if (attr->type_mask & android::ResTable_map::TYPE_BOOLEAN) {

    *msg << " boolean";

static void BuildAttributeMismatchMessage(const Attribute& attr, const Item& value,

                                          DiagMessage* out_msg) {

  *out_msg << "expected";

  if (attr.type_mask & android::ResTable_map::TYPE_BOOLEAN) {

    *out_msg << " boolean";

  }



  if (attr->type_mask & android::ResTable_map::TYPE_COLOR) {

    *msg << " color";

  if (attr.type_mask & android::ResTable_map::TYPE_COLOR) {

    *out_msg << " color";

  }



  if (attr->type_mask & android::ResTable_map::TYPE_DIMENSION) {

    *msg << " dimension";

  if (attr.type_mask & android::ResTable_map::TYPE_DIMENSION) {

    *out_msg << " dimension";

  }



  if (attr->type_mask & android::ResTable_map::TYPE_ENUM) {

    *msg << " enum";

  if (attr.type_mask & android::ResTable_map::TYPE_ENUM) {

    *out_msg << " enum";

  }



  if (attr->type_mask & android::ResTable_map::TYPE_FLAGS) {

    *msg << " flags";

  if (attr.type_mask & android::ResTable_map::TYPE_FLAGS) {

    *out_msg << " flags";

  }



  if (attr->type_mask & android::ResTable_map::TYPE_FLOAT) {

    *msg << " float";

  if (attr.type_mask & android::ResTable_map::TYPE_FLOAT) {

    *out_msg << " float";

  }



  if (attr->type_mask & android::ResTable_map::TYPE_FRACTION) {

    *msg << " fraction";

  if (attr.type_mask & android::ResTable_map::TYPE_FRACTION) {

    *out_msg << " fraction";

  }



  if (attr->type_mask & android::ResTable_map::TYPE_INTEGER) {

    *msg << " integer";

  if (attr.type_mask & android::ResTable_map::TYPE_INTEGER) {

    *out_msg << " integer";

  }



  if (attr->type_mask & android::ResTable_map::TYPE_REFERENCE) {

    *msg << " reference";

  if (attr.type_mask & android::ResTable_map::TYPE_REFERENCE) {

    *out_msg << " reference";

  }



  if (attr->type_mask & android::ResTable_map::TYPE_STRING) {

    *msg << " string";

  if (attr.type_mask & android::ResTable_map::TYPE_STRING) {

    *out_msg << " string";

  }



  *msg << " but got " << *value;

  *out_msg << " but got " << value;

}



bool Attribute::Matches(const Item* item, DiagMessage* out_msg) const {

bool Attribute::Matches(const Item& item, DiagMessage* out_msg) const {

  constexpr const uint32_t TYPE_ENUM = android::ResTable_map::TYPE_ENUM;

  constexpr const uint32_t TYPE_FLAGS = android::ResTable_map::TYPE_FLAGS;

  constexpr const uint32_t TYPE_INTEGER = android::ResTable_map::TYPE_INTEGER;

  constexpr const uint32_t TYPE_REFERENCE = android::ResTable_map::TYPE_REFERENCE;



  android::Res_value val = {};

  item->Flatten(&val);

  item.Flatten(&val);



  const uint32_t flattened_data = util::DeviceToHost32(val.data);



  // Always allow references.

  const uint32_t mask = type_mask | android::ResTable_map::TYPE_REFERENCE;

  if (!(mask & ResourceUtils::AndroidTypeToAttributeTypeMask(val.dataType))) {

  const uint32_t actual_type = ResourceUtils::AndroidTypeToAttributeTypeMask(val.dataType);



  // Only one type must match between the actual and expected.

  if ((actual_type & (type_mask | TYPE_REFERENCE)) == 0) {

    if (out_msg) {

      BuildAttributeMismatchMessage(*this, item, out_msg);

    }

    return false;

  }



  // Enums and flags are encoded as integers, so check them first before doing any range checks.

  if ((type_mask & TYPE_ENUM) != 0 && (actual_type & TYPE_ENUM) != 0) {

    for (const Symbol& s : symbols) {

      if (flattened_data == s.value) {

        return true;

      }

    }



    // If the attribute accepts integers, we can't fail here.

    if ((type_mask & TYPE_INTEGER) == 0) {

      if (out_msg) {

      BuildAttributeMismatchMessage(out_msg, this, item);

        *out_msg << item << " is not a valid enum";

      }

      return false;

    }

  }



  if ((type_mask & TYPE_FLAGS) != 0 && (actual_type & TYPE_FLAGS) != 0) {

    uint32_t mask = 0u;

    for (const Symbol& s : symbols) {

      mask |= s.value;

    }



    // Check if the flattened data is covered by the flag bit mask.

    // If the attribute accepts integers, we can't fail here.

    if ((mask & flattened_data) == flattened_data) {

      return true;

    } else if ((type_mask & TYPE_INTEGER) == 0) {

      if (out_msg) {

        *out_msg << item << " is not a valid flag";

      }

      return false;

    }

  }



  } else if (ResourceUtils::AndroidTypeToAttributeTypeMask(val.dataType) &

             android::ResTable_map::TYPE_INTEGER) {

    if (static_cast<int32_t>(util::DeviceToHost32(val.data)) < min_int) {

  // Finally check the integer range of the value.

  if ((type_mask & TYPE_INTEGER) != 0 && (actual_type & TYPE_INTEGER) != 0) {

    if (static_cast<int32_t>(flattened_data) < min_int) {

      if (out_msg) {

        *out_msg << *item << " is less than minimum integer " << min_int;

        *out_msg << item << " is less than minimum integer " << min_int;

      }

      return false;

    } else if (static_cast<int32_t>(util::DeviceToHost32(val.data)) > max_int) {

    } else if (static_cast<int32_t>(flattened_data) > max_int) {

      if (out_msg) {

        *out_msg << *item << " is greater than maximum integer " << max_int;

        *out_msg << item << " is greater than maximum integer " << max_int;

      }

      return false;

    }

tools/aapt2/ResourceValues.h

+1 −1
Original line number Diff line number Diff line
@@ -264,7 +264,7 @@ struct Attribute : public BaseValue<Attribute> { 
  Attribute* Clone(StringPool* new_pool) const override;

  void PrintMask(std::ostream* out) const;

  void Print(std::ostream* out) const override;

  bool Matches(const Item* item, DiagMessage* out_msg) const;

  bool Matches(const Item& item, DiagMessage* out_msg = nullptr) const;

};



struct Style : public BaseValue<Style> {

tools/aapt2/ResourceValues_test.cpp

+48 −0
Original line number Diff line number Diff line
@@ -190,4 +190,52 @@ TEST(ResourcesValuesTest, EmptyReferenceFlattens) { 
  EXPECT_EQ(0x0u, value.data);

}



TEST(ResourcesValuesTest, AttributeMatches) {

  constexpr const uint32_t TYPE_DIMENSION = android::ResTable_map::TYPE_DIMENSION;

  constexpr const uint32_t TYPE_ENUM = android::ResTable_map::TYPE_ENUM;

  constexpr const uint32_t TYPE_FLAGS = android::ResTable_map::TYPE_FLAGS;

  constexpr const uint32_t TYPE_INTEGER = android::ResTable_map::TYPE_INTEGER;

  constexpr const uint8_t TYPE_INT_DEC = android::Res_value::TYPE_INT_DEC;



  Attribute attr1(false /*weak*/, TYPE_DIMENSION);

  EXPECT_FALSE(attr1.Matches(*ResourceUtils::TryParseColor("#7fff00")));

  EXPECT_TRUE(attr1.Matches(*ResourceUtils::TryParseFloat("23dp")));

  EXPECT_TRUE(attr1.Matches(*ResourceUtils::TryParseReference("@android:string/foo")));



  Attribute attr2(false /*weak*/, TYPE_INTEGER | TYPE_ENUM);

  attr2.min_int = 0;

  attr2.symbols.push_back(Attribute::Symbol{Reference(test::ParseNameOrDie("android:id/foo")),

                                            static_cast<uint32_t>(-1)});

  EXPECT_FALSE(attr2.Matches(*ResourceUtils::TryParseColor("#7fff00")));

  EXPECT_TRUE(attr2.Matches(BinaryPrimitive(TYPE_INT_DEC, static_cast<uint32_t>(-1))));

  EXPECT_TRUE(attr2.Matches(BinaryPrimitive(TYPE_INT_DEC, 1u)));

  EXPECT_FALSE(attr2.Matches(BinaryPrimitive(TYPE_INT_DEC, static_cast<uint32_t>(-2))));



  Attribute attr3(false /*weak*/, TYPE_INTEGER | TYPE_FLAGS);

  attr3.max_int = 100;

  attr3.symbols.push_back(

      Attribute::Symbol{Reference(test::ParseNameOrDie("android:id/foo")), 0x01u});

  attr3.symbols.push_back(

      Attribute::Symbol{Reference(test::ParseNameOrDie("android:id/bar")), 0x02u});

  attr3.symbols.push_back(

      Attribute::Symbol{Reference(test::ParseNameOrDie("android:id/baz")), 0x04u});

  attr3.symbols.push_back(

      Attribute::Symbol{Reference(test::ParseNameOrDie("android:id/bat")), 0x80u});

  EXPECT_FALSE(attr3.Matches(*ResourceUtils::TryParseColor("#7fff00")));

  EXPECT_TRUE(attr3.Matches(BinaryPrimitive(TYPE_INT_DEC, 0x01u | 0x02u)));

  EXPECT_TRUE(attr3.Matches(BinaryPrimitive(TYPE_INT_DEC, 0x01u | 0x02u | 0x80u)));



  // Not a flag, but a value less than max_int.

  EXPECT_TRUE(attr3.Matches(BinaryPrimitive(TYPE_INT_DEC, 0x08u)));



  // Not a flag and greater than max_int.

  EXPECT_FALSE(attr3.Matches(BinaryPrimitive(TYPE_INT_DEC, 127u)));



  Attribute attr4(false /*weak*/, TYPE_ENUM);

  attr4.symbols.push_back(

      Attribute::Symbol{Reference(test::ParseNameOrDie("android:id/foo")), 0x01u});

  EXPECT_TRUE(attr4.Matches(BinaryPrimitive(TYPE_INT_DEC, 0x01u)));

  EXPECT_FALSE(attr4.Matches(BinaryPrimitive(TYPE_INT_DEC, 0x02u)));

}



} // namespace aapt

tools/aapt2/integration-tests/AppOne/res/values/styles.xml

+1 −0
Original line number Diff line number Diff line
@@ -25,6 +25,7 @@ 
    <style name="Pop">

        <item name="custom">@android:drawable/btn_default</item>

        <item name="android:focusable">true</item>

        <item name="android:numColumns">auto_fit</item>

    </style>

    <string name="yo">@string/wow</string>

tools/aapt2/link/ReferenceLinker.cpp

+4 −7
Original line number Diff line number Diff line
@@ -109,18 +109,15 @@ class ReferenceLinkerVisitor : public ValueVisitor { 
        entry.value->Accept(this);



        // Now verify that the type of this item is compatible with the

        // attribute it

        // is defined for. We pass `nullptr` as the DiagMessage so that this

        // check is

        // fast and we avoid creating a DiagMessage when the match is

        // successful.

        if (!symbol->attribute->Matches(entry.value.get(), nullptr)) {

        // attribute it is defined for. We pass `nullptr` as the DiagMessage so that this

        // check is fast and we avoid creating a DiagMessage when the match is successful.

        if (!symbol->attribute->Matches(*entry.value, nullptr)) {

          // The actual type of this item is incompatible with the attribute.

          DiagMessage msg(entry.key.GetSource());



          // Call the matches method again, this time with a DiagMessage so we

          // fill in the actual error message.

          symbol->attribute->Matches(entry.value.get(), &msg);

          symbol->attribute->Matches(*entry.value, &msg);

          context_->GetDiagnostics()->Error(msg);

          error_ = true;

        }