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

Commit 61f92e7e authored by Colin Cross's avatar Colin Cross Committed by Gerrit Code Review
Browse files

Merge changes from topic "aapt2-warn-manifest-validation" 

* changes:
  AAPT2: treat manifest validation errors as warnings when asked
  AAPT2: Better error messages for ManifestFixer
  AAPT2: Differentiate between Android and Java package names
parents 1e0a5d39 533c09b0

tools/aapt2/cmd/Link.cpp

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

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

                            &options.extensions_to_not_compress)

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

                          "Treat manifest validation errors as warnings.",

                          &options.manifest_fixer_options.warn_validation)

          .OptionalFlagList("--split",

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

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

tools/aapt2/java/ManifestClassGenerator.cpp

+5 −6
Original line number Diff line number Diff line
@@ -22,9 +22,11 @@ 
#include "java/AnnotationProcessor.h"

#include "java/ClassDefinition.h"

#include "util/Maybe.h"

#include "text/Unicode.h"

#include "xml/XmlDom.h"



using android::StringPiece;

using ::android::StringPiece;

using ::aapt::text::IsJavaIdentifier;



namespace aapt {
@@ -46,11 +48,8 @@ static Maybe<StringPiece> ExtractJavaIdentifier(IDiagnostics* diag, 
    return {};

  }



  iter = util::FindNonAlphaNumericAndNotInSet(result, "_");

  if (iter != result.end()) {

    diag->Error(DiagMessage(source) << "invalid character '"

                                    << StringPiece(iter, 1) << "' in '"

                                    << result << "'");

  if (!IsJavaIdentifier(result)) {

    diag->Error(DiagMessage(source) << "invalid Java identifier '" << result << "'");

    return {};

  }

tools/aapt2/link/ManifestFixer.cpp

+6 −3
Original line number Diff line number Diff line
@@ -127,9 +127,9 @@ static bool VerifyManifest(xml::Element* el, SourcePathDiagnostics* diag) { 
    diag->Error(DiagMessage(el->line_number)

                << "attribute 'package' in <manifest> tag must not be a reference");

    return false;

  } else if (!util::IsJavaPackageName(attr->value)) {

  } else if (!util::IsAndroidPackageName(attr->value)) {

    diag->Error(DiagMessage(el->line_number)

                << "attribute 'package' in <manifest> tag is not a valid Java package name: '"

                << "attribute 'package' in <manifest> tag is not a valid Android package name: '"

                << attr->value << "'");

    return false;

  }
@@ -409,7 +409,10 @@ bool ManifestFixer::Consume(IAaptContext* context, xml::XmlResource* doc) { 
    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;

  }

tools/aapt2/link/ManifestFixer.h

+5 −0
Original line number Diff line number Diff line
@@ -35,6 +35,11 @@ struct ManifestFixerOptions { 
  Maybe<std::string> rename_instrumentation_target_package;

  Maybe<std::string> version_name_default;

  Maybe<std::string> version_code_default;



  // 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;

};



/**

tools/aapt2/link/ManifestFixer_test.cpp

+30 −2
Original line number Diff line number Diff line
@@ -19,6 +19,7 @@ 
#include "test/Test.h"



using ::android::StringPiece;

using ::testing::IsNull;

using ::testing::NotNull;



namespace aapt {
@@ -109,7 +110,9 @@ TEST_F(ManifestFixerTest, AllowMetaData) { 
}



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(

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



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(

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

                    package="android">
@@ -439,4 +444,27 @@ TEST_F(ManifestFixerTest, SupportKeySets) { 
  EXPECT_THAT(Verify(input), NotNull());

}



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