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

Commit 9fbdf89d authored by Todd Kennedy's avatar Todd Kennedy
Browse files

Don't allow splitting on an empty configuration 

When aapt breaks out splits, it may remove the SDK constraint [if
it's lower than the min sdk]. If that is the only constraint, we
would create a resource split with no constraints. Don't allow
that situation. There must always be _some_ constraint.

Bug: 113115970
Test: atest CtsAppSecurityHostTestCases:SplitTests
Test: aapt2_tests
Change-Id: I424c875677c3be2a3ff5ddd39100b998bd650a4b
parent 5ed02df4

tools/aapt2/cmd/Link.cpp

+6 −0
Original line number Diff line number Diff line
@@ -1842,9 +1842,15 @@ class Linker { 
    } else {

      // Adjust the SplitConstraints so that their SDK version is stripped if it is less than or

      // equal to the minSdk.

      const size_t origConstraintSize = options_.split_constraints.size();

      options_.split_constraints =

          AdjustSplitConstraintsForMinSdk(context_->GetMinSdkVersion(), options_.split_constraints);



      if (origConstraintSize != options_.split_constraints.size()) {

        context_->GetDiagnostics()->Warn(DiagMessage()

                                         << "requested to split resources prior to min sdk of "

                                         << context_->GetMinSdkVersion());

      }

      TableSplitter table_splitter(options_.split_constraints, options_.table_splitter_options);

      if (!table_splitter.VerifySplitConstraints(context_)) {

        return 1;

tools/aapt2/cmd/Util.cpp

+8 −4
Original line number Diff line number Diff line
@@ -73,6 +73,7 @@ bool ParseSplitParameter(const StringPiece& arg, IDiagnostics* diag, std::string 
  }



  *out_path = parts[0];

  out_split->name = parts[1];

  for (const StringPiece& config_str : util::Tokenize(parts[1], ',')) {

    ConfigDescription config;

    if (!ConfigDescription::Parse(config_str, &config)) {
@@ -119,12 +120,15 @@ std::vector<SplitConstraints> AdjustSplitConstraintsForMinSdk( 
  for (const SplitConstraints& constraints : split_constraints) {

    SplitConstraints constraint;

    for (const ConfigDescription& config : constraints.configs) {

      if (config.sdkVersion <= min_sdk) {

        constraint.configs.insert(config.CopyWithoutSdkVersion());

      } else {

        constraint.configs.insert(config);

      const ConfigDescription &configToInsert = (config.sdkVersion <= min_sdk)

          ? config.CopyWithoutSdkVersion()

          : config;

      // only add the config if it actually selects something

      if (configToInsert != ConfigDescription::DefaultConfig()) {

        constraint.configs.insert(configToInsert);

      }

    }

    constraint.name = constraints.name;

    adjusted_constraints.push_back(std::move(constraint));

  }

  return adjusted_constraints;

tools/aapt2/cmd/Util_test.cpp

+287 −0
Original line number Diff line number Diff line
@@ -22,6 +22,10 @@ 
#include "test/Test.h"



namespace aapt {

#define EXPECT_CONFIG_EQ(constraints, config) \

    EXPECT_EQ(constraints.configs.size(), 1); \

    EXPECT_EQ(*constraints.configs.begin(), config); \

    constraints.configs.clear();



TEST(UtilTest, SplitNamesAreSanitized) {

    AppInfo app_info{"com.pkg"};
@@ -84,4 +88,287 @@ TEST (UtilTest, LongVersionCodeUndefined) { 
  EXPECT_EQ(compiled_version_code_major->value.data, 0x61);

}





TEST (UtilTest, ParseSplitParameter) {

  IDiagnostics* diagnostics = test::ContextBuilder().Build().get()->GetDiagnostics();

  std::string path;

  SplitConstraints constraints;

  ConfigDescription expected_configuration;



  // ========== Test IMSI ==========

  // mcc: 'mcc[0-9]{3}'

  // mnc: 'mnc[0-9]{1,3}'

  ASSERT_TRUE(ParseSplitParameter(":mcc310",

                                  diagnostics, &path, &constraints));

  expected_configuration = test::ConfigDescriptionBuilder()

      .setMcc(0x0136)

      .Build();

  EXPECT_CONFIG_EQ(constraints, expected_configuration);



  ASSERT_TRUE(ParseSplitParameter(":mcc310-mnc004",

                                  diagnostics, &path, &constraints));

  expected_configuration = test::ConfigDescriptionBuilder()

      .setMcc(0x0136)

      .setMnc(0x0004)

      .Build();

  EXPECT_CONFIG_EQ(constraints, expected_configuration);



  ASSERT_TRUE(ParseSplitParameter(":mcc310-mnc000",

                                  diagnostics, &path, &constraints));

  expected_configuration = test::ConfigDescriptionBuilder()

      .setMcc(0x0136)

      .setMnc(0xFFFF)

      .Build();

  EXPECT_CONFIG_EQ(constraints, expected_configuration);



  // ========== Test LOCALE ==========

  // locale: '[a-z]{2,3}(-r[a-z]{2})?'

  // locale: 'b+[a-z]{2,3}(+[a-z[0-9]]{2})?'

  ASSERT_TRUE(ParseSplitParameter(":es",

                                  diagnostics, &path, &constraints));

  expected_configuration = test::ConfigDescriptionBuilder()

      .setLanguage(0x6573)

      .Build();

  EXPECT_CONFIG_EQ(constraints, expected_configuration);



  ASSERT_TRUE(ParseSplitParameter(":fr-rCA",

                                  diagnostics, &path, &constraints));

  expected_configuration = test::ConfigDescriptionBuilder()

      .setLanguage(0x6672)

      .setCountry(0x4341)

      .Build();

  EXPECT_CONFIG_EQ(constraints, expected_configuration);



  ASSERT_TRUE(ParseSplitParameter(":b+es+419",

                                  diagnostics, &path, &constraints));

  expected_configuration = test::ConfigDescriptionBuilder()

      .setLanguage(0x6573)

      .setCountry(0xA424)

      .Build();

  EXPECT_CONFIG_EQ(constraints, expected_configuration);



  // ========== Test SCREEN_TYPE ==========

  // orientation: '(port|land|square)'

  // touchscreen: '(notouch|stylus|finger)'

  // density" '(anydpi|nodpi|ldpi|mdpi|tvdpi|hdpi|xhdpi|xxhdpi|xxxhdpi|[0-9]*dpi)'

  ASSERT_TRUE(ParseSplitParameter(":square",

                                  diagnostics, &path, &constraints));

  expected_configuration = test::ConfigDescriptionBuilder()

      .setOrientation(0x03)

      .Build();

  EXPECT_CONFIG_EQ(constraints, expected_configuration);



  ASSERT_TRUE(ParseSplitParameter(":stylus",

                                  diagnostics, &path, &constraints));

  expected_configuration = test::ConfigDescriptionBuilder()

      .setTouchscreen(0x02)

      .Build();

  EXPECT_CONFIG_EQ(constraints, expected_configuration);



  ASSERT_TRUE(ParseSplitParameter(":xxxhdpi",

                                  diagnostics, &path, &constraints));

  expected_configuration = test::ConfigDescriptionBuilder()

      .setDensity(0x0280)

      .setSdkVersion(0x0004) // version [any density requires donut]

      .Build();

  EXPECT_CONFIG_EQ(constraints, expected_configuration);



  ASSERT_TRUE(ParseSplitParameter(":land-xhdpi-finger",

                                  diagnostics, &path, &constraints));

  expected_configuration = test::ConfigDescriptionBuilder()

      .setOrientation(0x02)

      .setTouchscreen(0x03)

      .setDensity(0x0140)

      .setSdkVersion(0x0004) // version [any density requires donut]

      .Build();

  EXPECT_CONFIG_EQ(constraints, expected_configuration);



  // ========== Test INPUT ==========

  // keyboard: '(nokeys|qwerty|12key)'

  // navigation: '(nonav|dpad|trackball|wheel)'

  // inputFlags: '(keysexposed|keyshidden|keyssoft)'

  // inputFlags: '(navexposed|navhidden)'

  ASSERT_TRUE(ParseSplitParameter(":qwerty",

                                  diagnostics, &path, &constraints));

  expected_configuration = test::ConfigDescriptionBuilder()

      .setKeyboard(0x02)

      .Build();

  EXPECT_CONFIG_EQ(constraints, expected_configuration);



  ASSERT_TRUE(ParseSplitParameter(":dpad",

                                  diagnostics, &path, &constraints));

  expected_configuration = test::ConfigDescriptionBuilder()

      .setNavigation(0x02)

      .Build();

  EXPECT_CONFIG_EQ(constraints, expected_configuration);



  ASSERT_TRUE(ParseSplitParameter(":keyssoft-navhidden",

                                  diagnostics, &path, &constraints));

  expected_configuration = test::ConfigDescriptionBuilder()

      .setInputFlags(0x0B)

      .Build();

  EXPECT_CONFIG_EQ(constraints, expected_configuration);



  ASSERT_TRUE(ParseSplitParameter(":keyshidden-nokeys-navexposed-trackball",

                                  diagnostics, &path, &constraints));

  expected_configuration = test::ConfigDescriptionBuilder()

      .setKeyboard(0x01)

      .setNavigation(0x03)

      .setInputFlags(0x06)

      .Build();

  EXPECT_CONFIG_EQ(constraints, expected_configuration);



  // ========== Test SCREEN_SIZE ==========

  // screenWidth/screenHeight: '[0-9]+x[0-9]+'

  ASSERT_TRUE(ParseSplitParameter(":1920x1080",

                                  diagnostics, &path, &constraints));

  expected_configuration = test::ConfigDescriptionBuilder()

      .setScreenWidth(0x0780)

      .setScreenHeight(0x0438)

      .Build();

  EXPECT_CONFIG_EQ(constraints, expected_configuration);



  // ========== Test VERSION ==========

  // version 'v[0-9]+'



  // ========== Test SCREEN_CONFIG ==========

  // screenLayout [direction]: '(ldltr|ldrtl)'

  // screenLayout [size]: '(small|normal|large|xlarge)'

  // screenLayout [long]: '(long|notlong)'

  // uiMode [type]: '(desk|car|television|appliance|watch|vrheadset)'

  // uiMode [night]: '(night|notnight)'

  // smallestScreenWidthDp: 'sw[0-9]dp'

  ASSERT_TRUE(ParseSplitParameter(":ldrtl",

                                  diagnostics, &path, &constraints));

  expected_configuration = test::ConfigDescriptionBuilder()

      .setScreenLayout(0x80)

      .Build();

  EXPECT_CONFIG_EQ(constraints, expected_configuration);



  ASSERT_TRUE(ParseSplitParameter(":small",

                                  diagnostics, &path, &constraints));

  expected_configuration = test::ConfigDescriptionBuilder()

      .setScreenLayout(0x01)

      .setSdkVersion(0x0004) // screenLayout (size) requires donut

      .Build();

  EXPECT_CONFIG_EQ(constraints, expected_configuration);



  ASSERT_TRUE(ParseSplitParameter(":notlong",

                                  diagnostics, &path, &constraints));

  expected_configuration = test::ConfigDescriptionBuilder()

      .setScreenLayout(0x10)

      .setSdkVersion(0x0004) // screenLayout (long) requires donut

      .Build();

  EXPECT_CONFIG_EQ(constraints, expected_configuration);



  ASSERT_TRUE(ParseSplitParameter(":ldltr-normal-long",

                                  diagnostics, &path, &constraints));

  expected_configuration = test::ConfigDescriptionBuilder()

      .setScreenLayout(0x62)

      .setSdkVersion(0x0004) // screenLayout (size|long) requires donut

      .Build();

  EXPECT_CONFIG_EQ(constraints, expected_configuration);



  ASSERT_TRUE(ParseSplitParameter(":car",

                                  diagnostics, &path, &constraints));

  expected_configuration = test::ConfigDescriptionBuilder()

      .setUiMode(0x03)

      .setSdkVersion(0x0008) // uiMode requires froyo

      .Build();

  EXPECT_CONFIG_EQ(constraints, expected_configuration);



  ASSERT_TRUE(ParseSplitParameter(":vrheadset",

                                  diagnostics, &path, &constraints));

  expected_configuration = test::ConfigDescriptionBuilder()

      .setUiMode(0x07)

      .setSdkVersion(0x001A) // uiMode 'vrheadset' requires oreo

      .Build();

  EXPECT_CONFIG_EQ(constraints, expected_configuration);



  ASSERT_TRUE(ParseSplitParameter(":television-night",

                                  diagnostics, &path, &constraints));

  expected_configuration = test::ConfigDescriptionBuilder()

      .setUiMode(0x24)

      .setSdkVersion(0x0008) // uiMode requires froyo

      .Build();

  EXPECT_CONFIG_EQ(constraints, expected_configuration);



  ASSERT_TRUE(ParseSplitParameter(":sw1920dp",

                                  diagnostics, &path, &constraints));

  expected_configuration = test::ConfigDescriptionBuilder()

      .setSmallestScreenWidthDp(0x0780)

      .setSdkVersion(0x000D) // smallestScreenWidthDp requires honeycomb mr2

      .Build();

  EXPECT_CONFIG_EQ(constraints, expected_configuration);



  // ========== Test SCREEN_SIZE_DP ==========

  // screenWidthDp: 'w[0-9]dp'

  // screenHeightDp: 'h[0-9]dp'

  ASSERT_TRUE(ParseSplitParameter(":w1920dp",

                                  diagnostics, &path, &constraints));

  expected_configuration = test::ConfigDescriptionBuilder()

      .setScreenWidthDp(0x0780)

      .setSdkVersion(0x000D) // screenWidthDp requires honeycomb mr2

      .Build();

  EXPECT_CONFIG_EQ(constraints, expected_configuration);



  ASSERT_TRUE(ParseSplitParameter(":h1080dp",

                                  diagnostics, &path, &constraints));

  expected_configuration = test::ConfigDescriptionBuilder()

      .setScreenHeightDp(0x0438)

      .setSdkVersion(0x000D) // screenHeightDp requires honeycomb mr2

      .Build();

  EXPECT_CONFIG_EQ(constraints, expected_configuration);



  // ========== Test SCREEN_CONFIG_2 ==========

  // screenLayout2: '(round|notround)'

  // colorMode: '(widecg|nowidecg)'

  // colorMode: '(highhdr|lowdr)'

  ASSERT_TRUE(ParseSplitParameter(":round",

                                  diagnostics, &path, &constraints));

  expected_configuration = test::ConfigDescriptionBuilder()

      .setScreenLayout2(0x02)

      .setSdkVersion(0x0017) // screenLayout2 (round) requires marshmallow

      .Build();

  EXPECT_CONFIG_EQ(constraints, expected_configuration);



  ASSERT_TRUE(ParseSplitParameter(":widecg-highdr",

                                  diagnostics, &path, &constraints));

  expected_configuration = test::ConfigDescriptionBuilder()

      .setColorMode(0x0A)

      .setSdkVersion(0x001A) // colorMode (hdr|colour gamut) requires oreo

      .Build();

  EXPECT_CONFIG_EQ(constraints, expected_configuration);

}



TEST (UtilTest, AdjustSplitConstraintsForMinSdk) {

  std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build();



  IDiagnostics* diagnostics = context.get()->GetDiagnostics();

  std::vector<SplitConstraints> test_constraints;

  std::string path;



  test_constraints.push_back({});

  ASSERT_TRUE(ParseSplitParameter(":v7",

                                  diagnostics, &path, &test_constraints.back()));

  test_constraints.push_back({});

  ASSERT_TRUE(ParseSplitParameter(":xhdpi",

                                  diagnostics, &path, &test_constraints.back()));

  EXPECT_EQ(test_constraints.size(), 2);

  EXPECT_EQ(test_constraints[0].name, "v7");

  EXPECT_EQ(test_constraints[0].configs.size(), 1);

  EXPECT_NE(*test_constraints[0].configs.begin(), ConfigDescription::DefaultConfig());

  EXPECT_EQ(test_constraints[1].name, "xhdpi");

  EXPECT_EQ(test_constraints[1].configs.size(), 1);

  EXPECT_NE(*test_constraints[0].configs.begin(), ConfigDescription::DefaultConfig());



  auto adjusted_contraints = AdjustSplitConstraintsForMinSdk(26, test_constraints);

  EXPECT_EQ(adjusted_contraints.size(), 2);

  EXPECT_EQ(adjusted_contraints[0].name, "v7");

  EXPECT_EQ(adjusted_contraints[0].configs.size(), 0);

  EXPECT_EQ(adjusted_contraints[1].name, "xhdpi");

  EXPECT_EQ(adjusted_contraints[1].configs.size(), 1);

  EXPECT_NE(*adjusted_contraints[1].configs.begin(), ConfigDescription::DefaultConfig());

}



}  // namespace aapt

tools/aapt2/split/TableSplitter.cpp

+6 −0
Original line number Diff line number Diff line
@@ -154,6 +154,12 @@ static void MarkNonPreferredDensitiesAsClaimed( 
bool TableSplitter::VerifySplitConstraints(IAaptContext* context) {

  bool error = false;

  for (size_t i = 0; i < split_constraints_.size(); i++) {

    if (split_constraints_[i].configs.size() == 0) {

      // For now, treat this as a warning. We may consider aborting processing.

      context->GetDiagnostics()->Warn(DiagMessage()

                                       << "no configurations for constraint '"

                                       << split_constraints_[i].name << "'");

    }

    for (size_t j = i + 1; j < split_constraints_.size(); j++) {

      for (const ConfigDescription& config : split_constraints_[i].configs) {

        if (split_constraints_[j].configs.find(config) != split_constraints_[j].configs.end()) {

tools/aapt2/split/TableSplitter.h

+1 −0
Original line number Diff line number Diff line
@@ -30,6 +30,7 @@ namespace aapt { 


struct SplitConstraints {

  std::set<ConfigDescription> configs;

  std::string name;

};



struct TableSplitterOptions {