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
diff --git a/tools/aapt2/cmd/Link.cpp b/tools/aapt2/cmd/Link.cpp
index 119f56a..13c1047 100644
--- a/tools/aapt2/cmd/Link.cpp
+++ b/tools/aapt2/cmd/Link.cpp
@@ -1842,9 +1842,15 @@
} 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 c6c82b0..5862d31 100644
--- a/tools/aapt2/cmd/Util.cpp
+++ b/tools/aapt2/cmd/Util.cpp
@@ -73,6 +73,7 @@
}
*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 @@
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 b9fb5b2..158ef29 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 @@
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 e991743..b5c33062 100644
--- a/tools/aapt2/split/TableSplitter.cpp
+++ b/tools/aapt2/split/TableSplitter.cpp
@@ -154,6 +154,12 @@
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 6aec257..ed24bc39 100644
--- a/tools/aapt2/split/TableSplitter.h
+++ b/tools/aapt2/split/TableSplitter.h
@@ -30,6 +30,7 @@
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 fd5262a..be6e510 100644
--- a/tools/aapt2/test/Builders.h
+++ b/tools/aapt2/test/Builders.h
@@ -204,6 +204,112 @@
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