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

6 files changed, 414 insertions, 4 deletions

diff --git a/tools/aapt2/cmd/Link.cpp b/tools/aapt2/cmd/Link.cpp
index 119f56a5013c..13c10478ba3e 100644
--- a/tools/aapt2/cmd/Link.cpp
+++ b/tools/aapt2/cmd/Link.cpp
@@ -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;
diff --git a/tools/aapt2/cmd/Util.cpp b/tools/aapt2/cmd/Util.cpp
index c6c82b04ff2e..5862d31e3f94 100644
--- a/tools/aapt2/cmd/Util.cpp
+++ b/tools/aapt2/cmd/Util.cpp
@@ -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;
diff --git a/tools/aapt2/cmd/Util_test.cpp b/tools/aapt2/cmd/Util_test.cpp
index b9fb5b2868a7..158ef299f6e2 100644
--- a/tools/aapt2/cmd/Util_test.cpp
+++ b/tools/aapt2/cmd/Util_test.cpp
@@ -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
diff --git a/tools/aapt2/split/TableSplitter.cpp b/tools/aapt2/split/TableSplitter.cpp
index e99174302d26..b5c330622c72 100644
--- a/tools/aapt2/split/TableSplitter.cpp
+++ b/tools/aapt2/split/TableSplitter.cpp
@@ -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()) {
diff --git a/tools/aapt2/split/TableSplitter.h b/tools/aapt2/split/TableSplitter.h
index 6aec2576261e..ed24bc3991b4 100644
--- a/tools/aapt2/split/TableSplitter.h
+++ b/tools/aapt2/split/TableSplitter.h
@@ -30,6 +30,7 @@ namespace aapt {
 
 struct SplitConstraints {
   std::set<ConfigDescription> configs;
+  std::string name;
 };
 
 struct TableSplitterOptions {
diff --git a/tools/aapt2/test/Builders.h b/tools/aapt2/test/Builders.h
index fd5262af6e48..be6e51050c86 100644
--- a/tools/aapt2/test/Builders.h
+++ b/tools/aapt2/test/Builders.h
@@ -204,6 +204,112 @@ class PostProcessingConfigurationBuilder {
   configuration::PostProcessingConfiguration config_;
 };
 
+class ConfigDescriptionBuilder {
+ public:
+  ConfigDescriptionBuilder() = default;
+
+  ConfigDescriptionBuilder& setMcc(uint16_t mcc) {
+    config_.mcc = mcc;
+    return *this;
+  }
+  ConfigDescriptionBuilder& setMnc(uint16_t mnc) {
+    config_.mnc = mnc;
+    return *this;
+  }
+  ConfigDescriptionBuilder& setLanguage(uint16_t language) {
+    config_.language[0] = language >> 8;
+    config_.language[1] = language & 0xff;
+    return *this;
+  }
+  ConfigDescriptionBuilder& setCountry(uint16_t country) {
+    config_.country[0] = country >> 8;
+    config_.country[1] = country & 0xff;
+    return *this;
+  }
+  ConfigDescriptionBuilder& setOrientation(uint8_t orientation) {
+    config_.orientation = orientation;
+    return *this;
+  }
+  ConfigDescriptionBuilder& setTouchscreen(uint8_t touchscreen) {
+    config_.touchscreen = touchscreen;
+    return *this;
+  }
+  ConfigDescriptionBuilder& setDensity(uint16_t density) {
+    config_.density = density;
+    return *this;
+  }
+  ConfigDescriptionBuilder& setKeyboard(uint8_t keyboard) {
+    config_.keyboard = keyboard;
+    return *this;
+  }
+  ConfigDescriptionBuilder& setNavigation(uint8_t navigation) {
+    config_.navigation = navigation;
+    return *this;
+  }
+  ConfigDescriptionBuilder& setInputFlags(uint8_t inputFlags) {
+    config_.inputFlags = inputFlags;
+    return *this;
+  }
+  ConfigDescriptionBuilder& setInputPad0(uint8_t inputPad0) {
+    config_.inputPad0 = inputPad0;
+    return *this;
+  }
+  ConfigDescriptionBuilder& setScreenWidth(uint16_t screenWidth) {
+    config_.screenWidth = screenWidth;
+    return *this;
+  }
+  ConfigDescriptionBuilder& setScreenHeight(uint16_t screenHeight) {
+    config_.screenHeight = screenHeight;
+    return *this;
+  }
+  ConfigDescriptionBuilder& setSdkVersion(uint16_t sdkVersion) {
+    config_.sdkVersion = sdkVersion;
+    return *this;
+  }
+  ConfigDescriptionBuilder& setMinorVersion(uint16_t minorVersion) {
+    config_.minorVersion = minorVersion;
+    return *this;
+  }
+  ConfigDescriptionBuilder& setScreenLayout(uint8_t screenLayout) {
+    config_.screenLayout = screenLayout;
+    return *this;
+  }
+  ConfigDescriptionBuilder& setUiMode(uint8_t uiMode) {
+    config_.uiMode = uiMode;
+    return *this;
+  }
+  ConfigDescriptionBuilder& setSmallestScreenWidthDp(uint16_t smallestScreenWidthDp) {
+    config_.smallestScreenWidthDp = smallestScreenWidthDp;
+    return *this;
+  }
+  ConfigDescriptionBuilder& setScreenWidthDp(uint16_t screenWidthDp) {
+    config_.screenWidthDp = screenWidthDp;
+    return *this;
+  }
+  ConfigDescriptionBuilder& setScreenHeightDp(uint16_t screenHeightDp) {
+    config_.screenHeightDp = screenHeightDp;
+    return *this;
+  }
+  ConfigDescriptionBuilder& setScreenLayout2(uint8_t screenLayout2) {
+    config_.screenLayout2 = screenLayout2;
+    return *this;
+  }
+  ConfigDescriptionBuilder& setColorMode(uint8_t colorMode) {
+    config_.colorMode = colorMode;
+    return *this;
+  }
+  ConfigDescriptionBuilder& setScreenConfigPad2(uint16_t screenConfigPad2) {
+    config_.screenConfigPad2 = screenConfigPad2;
+    return *this;
+  }
+  ConfigDescription Build() {
+    return config_;
+  }
+
+ private:
+  ConfigDescription config_;
+};
+
 }  // namespace test
 }  // namespace aapt