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

Commit ad9e1324 authored by Izabela Orlowska's avatar Izabela Orlowska
Browse files

AAPT2: treat manifest validation errors as warnings when asked 

Bug: 65670329
Test: updated
Change-Id: Ic554cc20134fce66aa9ddf8d16ddffe0131c50e9
parent 8226fd9e

tools/aapt2/cmd/Link.cpp

+3 −0
Original line number Original line Diff line number Diff line
@@ -2121,6 +2121,9 @@ int Link(const std::vector<StringPiece>& args, IDiagnostics* diagnostics) { 
                        &options.manifest_fixer_options.rename_instrumentation_target_package)

                        &options.manifest_fixer_options.rename_instrumentation_target_package)

          .OptionalFlagList("-0", "File extensions not to compress.",

          .OptionalFlagList("-0", "File extensions not to compress.",

                            &options.extensions_to_not_compress)

                            &options.extensions_to_not_compress)

          .OptionalSwitch("--warn-manifest-validation",

                          "Treat manifest validation errors as warnings.",

                          &options.manifest_fixer_options.warn_validation)

          .OptionalFlagList("--split",

          .OptionalFlagList("--split",

                            "Split resources matching a set of configs out to a Split APK.\n"

                            "Split resources matching a set of configs out to a Split APK.\n"

                            "Syntax: path/to/output.apk:<config>[,<config>[...]].\n"

                            "Syntax: path/to/output.apk:<config>[,<config>[...]].\n"

tools/aapt2/link/ManifestFixer.cpp

+4 −1
Original line number Original line Diff line number Diff line
@@ -430,7 +430,10 @@ bool ManifestFixer::Consume(IAaptContext* context, xml::XmlResource* doc) { 
    return false;

    return false;

  }

  }





  if (!executor.Execute(xml::XmlActionExecutorPolicy::kWhitelist, context->GetDiagnostics(), doc)) {

  xml::XmlActionExecutorPolicy policy = options_.warn_validation

                                            ? xml::XmlActionExecutorPolicy::kWhitelistWarning

                                            : xml::XmlActionExecutorPolicy::kWhitelist;

  if (!executor.Execute(policy, context->GetDiagnostics(), doc)) {

    return false;

    return false;

  }

  }

tools/aapt2/link/ManifestFixer.h

+5 −0
Original line number Original line Diff line number Diff line
@@ -57,6 +57,11 @@ struct ManifestFixerOptions { 
  // The version codename of the framework being compiled against to set for

  // The version codename of the framework being compiled against to set for

  // 'android:compileSdkVersionCodename' in the <manifest> tag.

  // 'android:compileSdkVersionCodename' in the <manifest> tag.

  Maybe<std::string> compile_sdk_version_codename;

  Maybe<std::string> compile_sdk_version_codename;



  // Wether validation errors should be treated only as warnings. If this is 'true', then an

  // incorrect node will not result in an error, but only as a warning, and the parsing will

  // continue.

  bool warn_validation = false;

};

};





// Verifies that the manifest is correctly formed and inserts defaults where specified with

// Verifies that the manifest is correctly formed and inserts defaults where specified with

tools/aapt2/link/ManifestFixer_test.cpp

+29 −2
Original line number Original line Diff line number Diff line
@@ -112,7 +112,9 @@ TEST_F(ManifestFixerTest, AllowMetaData) { 
}

}





TEST_F(ManifestFixerTest, UseDefaultSdkVersionsIfNonePresent) {

TEST_F(ManifestFixerTest, UseDefaultSdkVersionsIfNonePresent) {

  ManifestFixerOptions options = {std::string("8"), std::string("22")};

  ManifestFixerOptions options;

  options.min_sdk_version_default = std::string("8");

  options.target_sdk_version_default = std::string("22");





  std::unique_ptr<xml::XmlResource> doc = VerifyWithOptions(R"EOF(

  std::unique_ptr<xml::XmlResource> doc = VerifyWithOptions(R"EOF(

      <manifest xmlns:android="http://schemas.android.com/apk/res/android"

      <manifest xmlns:android="http://schemas.android.com/apk/res/android"
@@ -193,7 +195,9 @@ TEST_F(ManifestFixerTest, UseDefaultSdkVersionsIfNonePresent) { 
}

}





TEST_F(ManifestFixerTest, UsesSdkMustComeBeforeApplication) {

TEST_F(ManifestFixerTest, UsesSdkMustComeBeforeApplication) {

  ManifestFixerOptions options = {std::string("8"), std::string("22")};

  ManifestFixerOptions options;

  options.min_sdk_version_default = std::string("8");

  options.target_sdk_version_default = std::string("22");

  std::unique_ptr<xml::XmlResource> doc = VerifyWithOptions(R"EOF(

  std::unique_ptr<xml::XmlResource> doc = VerifyWithOptions(R"EOF(

          <manifest xmlns:android="http://schemas.android.com/apk/res/android"

          <manifest xmlns:android="http://schemas.android.com/apk/res/android"

                    package="android">

                    package="android">
@@ -467,4 +471,27 @@ TEST_F(ManifestFixerTest, InsertCompileSdkVersions) { 
  EXPECT_THAT(attr->value, StrEq("P"));

  EXPECT_THAT(attr->value, StrEq("P"));

}

}





TEST_F(ManifestFixerTest, UnexpectedElementsInManifest) {

  std::string input = R"(

      <manifest xmlns:android="http://schemas.android.com/apk/res/android"

          package="android">

        <beep/>

      </manifest>)";

  ManifestFixerOptions options;

  options.warn_validation = true;



  // Unexpected element should result in a warning if the flag is set to 'true'.

  std::unique_ptr<xml::XmlResource> manifest = VerifyWithOptions(input, options);

  ASSERT_THAT(manifest, NotNull());



  // Unexpected element should result in an error if the flag is set to 'false'.

  options.warn_validation = false;

  manifest = VerifyWithOptions(input, options);

  ASSERT_THAT(manifest, IsNull());



  // By default the flag should be set to 'false'.

  manifest = Verify(input);

  ASSERT_THAT(manifest, IsNull());

}



}  // namespace aapt
 
}  // namespace aapt

tools/aapt2/xml/XmlActionExecutor.cpp

+9 −3
Original line number Original line Diff line number Diff line
@@ -66,7 +66,7 @@ bool XmlNodeAction::Execute(XmlActionExecutorPolicy policy, std::vector<StringPi 
        continue;

        continue;

      }

      }





      if (policy == XmlActionExecutorPolicy::kWhitelist) {

      if (policy != XmlActionExecutorPolicy::kNone) {

        DiagMessage error_msg(child_el->line_number);

        DiagMessage error_msg(child_el->line_number);

        error_msg << "unexpected element ";

        error_msg << "unexpected element ";

        PrintElementToDiagMessage(child_el, &error_msg);

        PrintElementToDiagMessage(child_el, &error_msg);
@@ -74,11 +74,17 @@ bool XmlNodeAction::Execute(XmlActionExecutorPolicy policy, std::vector<StringPi 
        for (const StringPiece& element : *bread_crumb) {

        for (const StringPiece& element : *bread_crumb) {

          error_msg << "<" << element << ">";

          error_msg << "<" << element << ">";

        }

        }

        if (policy == XmlActionExecutorPolicy::kWhitelistWarning) {

          // Treat the error only as a warning.

          diag->Warn(error_msg);

        } else {

          // Policy is XmlActionExecutorPolicy::kWhitelist, we should fail.

          diag->Error(error_msg);

          diag->Error(error_msg);

          error = true;

          error = true;

        }

        }

      }

      }

    }

    }

  }

  return !error;

  return !error;

}

}