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

Commit 1bb1fe06 authored by Ryan Mitchell's avatar Ryan Mitchell
Browse files

Refactor policy parsing 

This change removes the ability for an overlayable resource to be
defined in multiple policy blocks within the same overlayable. This
change also changes aapt2 to use a bit mask to keep track of the parsed
policies.

Bug: 110869880
Bug: 120298168
Test: aapt2_tests
Change-Id: Ie26cd913f94a16c0b312f222bccfa48f62feceaa
parent c622083d

libs/androidfw/include/androidfw/ResourceTypes.h

+1 −1
Original line number Diff line number Diff line
@@ -1622,7 +1622,7 @@ struct ResTable_overlayable_policy_header 
{

  struct ResChunk_header header;



  enum PolicyFlags {

  enum PolicyFlags : uint32_t {

    // Any overlay can overlay these resources.

    POLICY_PUBLIC = 0x00000001,

tools/aapt2/Debug.cpp

+0 −25
Original line number Diff line number Diff line
@@ -306,31 +306,6 @@ void Debug::PrintTable(const ResourceTable& table, const DebugPrintTableOptions& 
            break;

        }



        for (size_t i = 0; i < entry->overlayable_declarations.size(); i++) {

          printer->Print((i == 0) ? " " : "|");

          printer->Print("OVERLAYABLE");



          if (entry->overlayable_declarations[i].policy) {

            switch (entry->overlayable_declarations[i].policy.value()) {

              case Overlayable::Policy::kProduct:

                printer->Print("_PRODUCT");

                break;

              case Overlayable::Policy::kProductServices:

                printer->Print("_PRODUCT_SERVICES");

                break;

              case Overlayable::Policy::kSystem:

                printer->Print("_SYSTEM");

                break;

              case Overlayable::Policy::kVendor:

                printer->Print("_VENDOR");

                break;

              case Overlayable::Policy::kPublic:

                printer->Print("_PUBLIC");

                break;

            }

          }

        }



        printer->Println();



        if (options.show_values) {

tools/aapt2/ResourceParser.cpp

+48 −76
Original line number Diff line number Diff line
@@ -99,7 +99,7 @@ struct ParsedResource { 
  ResourceId id;

  Visibility::Level visibility_level = Visibility::Level::kUndefined;

  bool allow_new = false;

  std::vector<Overlayable> overlayable_declarations;

  Maybe<Overlayable> overlayable;



  std::string comment;

  std::unique_ptr<Value> value;
@@ -133,8 +133,8 @@ static bool AddResourcesToTable(ResourceTable* table, IDiagnostics* diag, Parsed 
    }

  }



  for (auto& overlayable : res->overlayable_declarations) {

    if (!table->AddOverlayable(res->name, overlayable, diag)) {

  if (res->overlayable) {

    if (!table->SetOverlayable(res->name, res->overlayable.value(), diag)) {

      return false;

    }

  }
@@ -1063,20 +1063,19 @@ bool ResourceParser::ParseOverlayable(xml::XmlPullParser* parser, ParsedResource 
                    << "' for <overlayable> tag");

  }



  std::string comment;

  std::vector<Overlayable::Policy> policies;



  bool error = false;

  std::string comment;

  Overlayable::PolicyFlags current_policies = Overlayable::Policy::kNone;

  const size_t start_depth = parser->depth();

  while (xml::XmlPullParser::IsGoodEvent(parser->Next())) {

    xml::XmlPullParser::Event event = parser->event();

    if (event == xml::XmlPullParser::Event::kEndElement && parser->depth() == start_depth) {

      // Break the loop when exiting the overyabale element

      // Break the loop when exiting the overlayable element

      break;

    } else if (event == xml::XmlPullParser::Event::kEndElement

               && parser->depth() == start_depth + 1) {

      // Clear the current policies when exiting the policy element

      policies.clear();

      current_policies = Overlayable::Policy::kNone;

      continue;

    } else if (event == xml::XmlPullParser::Event::kComment) {

      // Get the comment of individual item elements
@@ -1090,34 +1089,62 @@ bool ResourceParser::ParseOverlayable(xml::XmlPullParser* parser, ParsedResource 
    const Source item_source = source_.WithLine(parser->line_number());

    const std::string& element_name = parser->element_name();

    const std::string& element_namespace = parser->element_namespace();



    if (element_namespace.empty() && element_name == "item") {

      if (!ParseOverlayableItem(parser, policies, comment, out_resource)) {

      // Items specify the name and type of resource that should be overlayable

      Maybe<StringPiece> maybe_name = xml::FindNonEmptyAttribute(parser, "name");

      if (!maybe_name) {

        diag_->Error(DiagMessage(item_source)

                     << "<item> within an <overlayable> tag must have a 'name' attribute");

        error = true;

        continue;

      }



      Maybe<StringPiece> maybe_type = xml::FindNonEmptyAttribute(parser, "type");

      if (!maybe_type) {

        diag_->Error(DiagMessage(item_source)

                     << "<item> within an <overlayable> tag must have a 'type' attribute");

        error = true;

        continue;

      }



      const ResourceType* type = ParseResourceType(maybe_type.value());

      if (type == nullptr) {

        diag_->Error(DiagMessage(item_source)

                     << "invalid resource type '" << maybe_type.value()

                     << "' in <item> within an <overlayable>");

        error = true;

        continue;

      }



      ParsedResource child_resource;

      child_resource.name.type = *type;

      child_resource.name.entry = maybe_name.value().to_string();

      child_resource.overlayable = Overlayable{current_policies, item_source, comment};

      out_resource->child_resources.push_back(std::move(child_resource));



    } else if (element_namespace.empty() && element_name == "policy") {

      if (!policies.empty()) {

      if (current_policies != Overlayable::Policy::kNone) {

        // If the policy list is not empty, then we are currently inside a policy element

        diag_->Error(DiagMessage(item_source) << "<policy> blocks cannot be recursively nested");

        error = true;

        break;

      } else if (Maybe<StringPiece> maybe_type = xml::FindNonEmptyAttribute(parser, "type")) {

        // Parse the polices separated by vertical bar characters to allow for specifying multiple

        // policies at once

        // policies

        for (StringPiece part : util::Tokenize(maybe_type.value(), '|')) {

          StringPiece trimmed_part = util::TrimWhitespace(part);

          if (trimmed_part == "public") {

            policies.push_back(Overlayable::Policy::kPublic);

            current_policies |= Overlayable::Policy::kPublic;

          } else if (trimmed_part == "product") {

            policies.push_back(Overlayable::Policy::kProduct);

            current_policies |= Overlayable::Policy::kProduct;

          } else if (trimmed_part == "product_services") {

            policies.push_back(Overlayable::Policy::kProductServices);

            current_policies |= Overlayable::Policy::kProductServices;

          } else if (trimmed_part == "system") {

            policies.push_back(Overlayable::Policy::kSystem);

            current_policies |= Overlayable::Policy::kSystem;

          } else if (trimmed_part == "vendor") {

            policies.push_back(Overlayable::Policy::kVendor);

            current_policies |= Overlayable::Policy::kVendor;

          } else {

            diag_->Error(DiagMessage(out_resource->source)

            diag_->Error(DiagMessage(item_source)

                         << "<policy> has unsupported type '" << trimmed_part << "'");

            error = true;

            continue;
@@ -1125,8 +1152,8 @@ bool ResourceParser::ParseOverlayable(xml::XmlPullParser* parser, ParsedResource 
        }

      }

    } else if (!ShouldIgnoreElement(element_namespace, element_name)) {

      diag_->Error(DiagMessage(item_source) << "invalid element <" << element_name << "> in "

                                            << " <overlayable>");

      diag_->Error(DiagMessage(item_source) << "invalid element <" << element_name << "> "

                                            << " in <overlayable>");

      error = true;

      break;

    }
@@ -1135,61 +1162,6 @@ bool ResourceParser::ParseOverlayable(xml::XmlPullParser* parser, ParsedResource 
  return !error;

}



bool ResourceParser::ParseOverlayableItem(xml::XmlPullParser* parser,

                                          const std::vector<Overlayable::Policy>& policies,

                                          const std::string& comment,

                                          ParsedResource* out_resource) {

  const Source item_source = source_.WithLine(parser->line_number());



  Maybe<StringPiece> maybe_name = xml::FindNonEmptyAttribute(parser, "name");

  if (!maybe_name) {

    diag_->Error(DiagMessage(item_source)

                     << "<item> within an <overlayable> tag must have a 'name' attribute");

    return false;

  }



  Maybe<StringPiece> maybe_type = xml::FindNonEmptyAttribute(parser, "type");

  if (!maybe_type) {

    diag_->Error(DiagMessage(item_source)

                     << "<item> within an <overlayable> tag must have a 'type' attribute");

    return false;

  }



  const ResourceType* type = ParseResourceType(maybe_type.value());

  if (type == nullptr) {

    diag_->Error(DiagMessage(out_resource->source)

                     << "invalid resource type '" << maybe_type.value()

                     << "' in <item> within an <overlayable>");

    return false;

  }



  ParsedResource child_resource;

  child_resource.name.type = *type;

  child_resource.name.entry = maybe_name.value().to_string();

  child_resource.source = item_source;



  if (policies.empty()) {

    Overlayable overlayable;

    overlayable.source = item_source;

    overlayable.comment = comment;

    child_resource.overlayable_declarations.push_back(overlayable);

  } else {

    for (Overlayable::Policy policy : policies) {

      Overlayable overlayable;

      overlayable.policy = policy;

      overlayable.source = item_source;

      overlayable.comment = comment;

      child_resource.overlayable_declarations.push_back(overlayable);

    }

  }



  if (options_.visibility) {

    child_resource.visibility_level = options_.visibility.value();

  }

  out_resource->child_resources.push_back(std::move(child_resource));

  return true;

}



bool ResourceParser::ParseAddResource(xml::XmlPullParser* parser, ParsedResource* out_resource) {

  if (ParseSymbolImpl(parser, out_resource)) {

    out_resource->visibility_level = Visibility::Level::kUndefined;

tools/aapt2/ResourceParser.h

+0 −4
Original line number Diff line number Diff line
@@ -96,10 +96,6 @@ class ResourceParser { 
  bool ParseSymbolImpl(xml::XmlPullParser* parser, ParsedResource* out_resource);

  bool ParseSymbol(xml::XmlPullParser* parser, ParsedResource* out_resource);

  bool ParseOverlayable(xml::XmlPullParser* parser, ParsedResource* out_resource);

  bool ParseOverlayableItem(xml::XmlPullParser* parser,

                            const std::vector<Overlayable::Policy>& policies,

                            const std::string& comment,

                            ParsedResource* out_resource);

  bool ParseAddResource(xml::XmlPullParser* parser, ParsedResource* out_resource);

  bool ParseAttr(xml::XmlPullParser* parser, ParsedResource* out_resource);

  bool ParseAttrImpl(xml::XmlPullParser* parser, ParsedResource* out_resource, bool weak);

tools/aapt2/ResourceParser_test.cpp

+43 −67
Original line number Diff line number Diff line
@@ -905,16 +905,16 @@ TEST_F(ResourceParserTest, ParseOverlayable) { 
  auto search_result = table_.FindResource(test::ParseNameOrDie("string/foo"));

  ASSERT_TRUE(search_result);

  ASSERT_THAT(search_result.value().entry, NotNull());

  EXPECT_THAT(search_result.value().entry->visibility.level, Eq(Visibility::Level::kUndefined));

  EXPECT_THAT(search_result.value().entry->overlayable_declarations.size(), Eq(1));

  EXPECT_FALSE(search_result.value().entry->overlayable_declarations[0].policy);

  ASSERT_TRUE(search_result.value().entry->overlayable);

  EXPECT_THAT(search_result.value().entry->overlayable.value().policies,

              Eq(Overlayable::Policy::kNone));



  search_result = table_.FindResource(test::ParseNameOrDie("drawable/bar"));

  ASSERT_TRUE(search_result);

  ASSERT_THAT(search_result.value().entry, NotNull());

  EXPECT_THAT(search_result.value().entry->visibility.level, Eq(Visibility::Level::kUndefined));

  EXPECT_THAT(search_result.value().entry->overlayable_declarations.size(), Eq(1));

  EXPECT_FALSE(search_result.value().entry->overlayable_declarations[0].policy);

  ASSERT_TRUE(search_result.value().entry->overlayable);

  EXPECT_THAT(search_result.value().entry->overlayable.value().policies,

              Eq(Overlayable::Policy::kNone));

}



TEST_F(ResourceParserTest, ParseOverlayablePolicy) {
@@ -945,49 +945,44 @@ TEST_F(ResourceParserTest, ParseOverlayablePolicy) { 
  auto search_result = table_.FindResource(test::ParseNameOrDie("string/foo"));

  ASSERT_TRUE(search_result);

  ASSERT_THAT(search_result.value().entry, NotNull());

  EXPECT_THAT(search_result.value().entry->visibility.level, Eq(Visibility::Level::kUndefined));

  EXPECT_THAT(search_result.value().entry->overlayable_declarations.size(), Eq(1));

  EXPECT_FALSE(search_result.value().entry->overlayable_declarations[0].policy);

  ASSERT_TRUE(search_result.value().entry->overlayable);

  Overlayable& overlayable = search_result.value().entry->overlayable.value();

  EXPECT_THAT(overlayable.policies, Eq(Overlayable::Policy::kNone));



  search_result = table_.FindResource(test::ParseNameOrDie("string/bar"));

  ASSERT_TRUE(search_result);

  ASSERT_THAT(search_result.value().entry, NotNull());

  EXPECT_THAT(search_result.value().entry->visibility.level, Eq(Visibility::Level::kUndefined));

  EXPECT_THAT(search_result.value().entry->overlayable_declarations.size(), Eq(1));

  EXPECT_THAT(search_result.value().entry->overlayable_declarations[0].policy,

              Eq(Overlayable::Policy::kProduct));

  ASSERT_TRUE(search_result.value().entry->overlayable);

  overlayable = search_result.value().entry->overlayable.value();

  EXPECT_THAT(overlayable.policies, Eq(Overlayable::Policy::kProduct));



  search_result = table_.FindResource(test::ParseNameOrDie("string/baz"));

  ASSERT_TRUE(search_result);

  ASSERT_THAT(search_result.value().entry, NotNull());

  EXPECT_THAT(search_result.value().entry->visibility.level, Eq(Visibility::Level::kUndefined));

  EXPECT_THAT(search_result.value().entry->overlayable_declarations.size(), Eq(1));

  EXPECT_THAT(search_result.value().entry->overlayable_declarations[0].policy,

              Eq(Overlayable::Policy::kProductServices));

  ASSERT_TRUE(search_result.value().entry->overlayable);

  overlayable = search_result.value().entry->overlayable.value();

  EXPECT_THAT(overlayable.policies, Eq(Overlayable::Policy::kProductServices));



  search_result = table_.FindResource(test::ParseNameOrDie("string/fiz"));

  ASSERT_TRUE(search_result);

  ASSERT_THAT(search_result.value().entry, NotNull());

  EXPECT_THAT(search_result.value().entry->visibility.level, Eq(Visibility::Level::kUndefined));

  EXPECT_THAT(search_result.value().entry->overlayable_declarations.size(), Eq(1));

  EXPECT_THAT(search_result.value().entry->overlayable_declarations[0].policy,

              Eq(Overlayable::Policy::kSystem));

  ASSERT_TRUE(search_result.value().entry->overlayable);

  overlayable = search_result.value().entry->overlayable.value();

  EXPECT_THAT(overlayable.policies, Eq(Overlayable::Policy::kSystem));



  search_result = table_.FindResource(test::ParseNameOrDie("string/fuz"));

  ASSERT_TRUE(search_result);

  ASSERT_THAT(search_result.value().entry, NotNull());

  EXPECT_THAT(search_result.value().entry->visibility.level, Eq(Visibility::Level::kUndefined));

  EXPECT_THAT(search_result.value().entry->overlayable_declarations.size(), Eq(1));

  EXPECT_THAT(search_result.value().entry->overlayable_declarations[0].policy,

              Eq(Overlayable::Policy::kVendor));

  ASSERT_TRUE(search_result.value().entry->overlayable);

  overlayable = search_result.value().entry->overlayable.value();

  EXPECT_THAT(overlayable.policies, Eq(Overlayable::Policy::kVendor));



  search_result = table_.FindResource(test::ParseNameOrDie("string/faz"));

  ASSERT_TRUE(search_result);

  ASSERT_THAT(search_result.value().entry, NotNull());

  EXPECT_THAT(search_result.value().entry->visibility.level, Eq(Visibility::Level::kUndefined));

  EXPECT_THAT(search_result.value().entry->overlayable_declarations.size(), Eq(1));

  EXPECT_THAT(search_result.value().entry->overlayable_declarations[0].policy,

              Eq(Overlayable::Policy::kPublic));

  ASSERT_TRUE(search_result.value().entry->overlayable);

  overlayable = search_result.value().entry->overlayable.value();

  EXPECT_THAT(overlayable.policies, Eq(Overlayable::Policy::kPublic));

}



TEST_F(ResourceParserTest, ParseOverlayableBadPolicyError) {
@@ -1031,22 +1026,18 @@ TEST_F(ResourceParserTest, ParseOverlayableMultiplePolicy) { 
  auto search_result = table_.FindResource(test::ParseNameOrDie("string/foo"));

  ASSERT_TRUE(search_result);

  ASSERT_THAT(search_result.value().entry, NotNull());

  EXPECT_THAT(search_result.value().entry->visibility.level, Eq(Visibility::Level::kUndefined));

  EXPECT_THAT(search_result.value().entry->overlayable_declarations.size(), Eq(2));

  EXPECT_THAT(search_result.value().entry->overlayable_declarations[0].policy,

              Eq(Overlayable::Policy::kVendor));

  EXPECT_THAT(search_result.value().entry->overlayable_declarations[1].policy,

              Eq(Overlayable::Policy::kProductServices));

  ASSERT_TRUE(search_result.value().entry->overlayable);

  Overlayable& overlayable = search_result.value().entry->overlayable.value();

  EXPECT_THAT(overlayable.policies, Eq(Overlayable::Policy::kVendor

                                       | Overlayable::Policy::kProductServices));



  search_result = table_.FindResource(test::ParseNameOrDie("string/bar"));

  ASSERT_TRUE(search_result);

  ASSERT_THAT(search_result.value().entry, NotNull());

  EXPECT_THAT(search_result.value().entry->visibility.level, Eq(Visibility::Level::kUndefined));

  EXPECT_THAT(search_result.value().entry->overlayable_declarations.size(), Eq(2));

  EXPECT_THAT(search_result.value().entry->overlayable_declarations[0].policy,

              Eq(Overlayable::Policy::kProduct));

  EXPECT_THAT(search_result.value().entry->overlayable_declarations[1].policy,

              Eq(Overlayable::Policy::kSystem));

  ASSERT_TRUE(search_result.value().entry->overlayable);

  overlayable = search_result.value().entry->overlayable.value();

  EXPECT_THAT(overlayable.policies, Eq(Overlayable::Policy::kProduct

                                       | Overlayable::Policy::kSystem));

}



TEST_F(ResourceParserTest, DuplicateOverlayableIsError) {
@@ -1067,7 +1058,7 @@ TEST_F(ResourceParserTest, DuplicateOverlayableIsError) { 
  EXPECT_FALSE(TestParse(input));



  input = R"(

      <overlayable">

      <overlayable>

        <policy type="product">

          <item type="string" name="foo" />

          <item type="string" name="foo" />
@@ -1080,45 +1071,30 @@ TEST_F(ResourceParserTest, DuplicateOverlayableIsError) { 
        <policy type="product">

          <item type="string" name="foo" />

        </policy>

      </overlayable>



      <overlayable>

        <policy type="product">

          <item type="string" name="foo" />

        </policy>

      </overlayable>)";

  EXPECT_FALSE(TestParse(input));

}



TEST_F(ResourceParserTest, PolicyAndNonPolicyOverlayableError) {

  std::string input = R"(

        <overlayable policy="product">

          <item type="string" name="foo" />

        </overlayable>

        <overlayable policy="">

        <item type="string" name="foo" />

      </overlayable>)";

  EXPECT_FALSE(TestParse(input));



  input = R"(

        <overlayable policy="">

      <overlayable>

        <policy type="product">

          <item type="string" name="foo" />

        </overlayable>

        <overlayable policy="product">

        </policy>

        <policy type="vendor">

          <item type="string" name="foo" />

        </policy>

      </overlayable>)";

  EXPECT_FALSE(TestParse(input));

}



TEST_F(ResourceParserTest, DuplicateOverlayableMultiplePolicyError) {

  std::string input = R"(

  input = R"(

      <overlayable>

        <policy type="vendor|product">

        <policy type="product">

          <item type="string" name="foo" />

        </policy>

      </overlayable>



      <overlayable>

        <policy type="product_services|vendor">

        <policy type="product">

          <item type="string" name="foo" />

        </policy>

      </overlayable>)";