Relax apex package restriction for T+ jars

The ART AOT exemption only applies to Q/R/S, so module jars that have
min_sdk T+ do not need to follow the module package restriction, even if
they are part of a Q/R/S module (but not loaded on Q/R/S).

Relax the restriction to only apply to jars that have min_sdk before T.

Bug: 208773835
Test: m (runs apex tests)
Change-Id: I2c3ad8984ca05ad763bf6162bd478f93ab4ee650
This commit is contained in:
Remi NGUYEN VAN
2021-12-02 19:39:35 +09:00
parent ea8b7771f1
commit 1fdd6ca88a
4 changed files with 180 additions and 25 deletions

View File

@@ -252,7 +252,7 @@ func neverallowMutator(ctx BottomUpMutatorContext) {
continue continue
} }
if !n.appliesToProperties(properties) { if !n.appliesToProperties(ctx, properties) {
continue continue
} }
@@ -272,8 +272,12 @@ func neverallowMutator(ctx BottomUpMutatorContext) {
} }
} }
type ValueMatcherContext interface {
Config() Config
}
type ValueMatcher interface { type ValueMatcher interface {
Test(string) bool Test(ValueMatcherContext, string) bool
String() string String() string
} }
@@ -281,7 +285,7 @@ type equalMatcher struct {
expected string expected string
} }
func (m *equalMatcher) Test(value string) bool { func (m *equalMatcher) Test(ctx ValueMatcherContext, value string) bool {
return m.expected == value return m.expected == value
} }
@@ -292,7 +296,7 @@ func (m *equalMatcher) String() string {
type anyMatcher struct { type anyMatcher struct {
} }
func (m *anyMatcher) Test(value string) bool { func (m *anyMatcher) Test(ctx ValueMatcherContext, value string) bool {
return true return true
} }
@@ -306,7 +310,7 @@ type startsWithMatcher struct {
prefix string prefix string
} }
func (m *startsWithMatcher) Test(value string) bool { func (m *startsWithMatcher) Test(ctx ValueMatcherContext, value string) bool {
return strings.HasPrefix(value, m.prefix) return strings.HasPrefix(value, m.prefix)
} }
@@ -318,7 +322,7 @@ type regexMatcher struct {
re *regexp.Regexp re *regexp.Regexp
} }
func (m *regexMatcher) Test(value string) bool { func (m *regexMatcher) Test(ctx ValueMatcherContext, value string) bool {
return m.re.MatchString(value) return m.re.MatchString(value)
} }
@@ -330,7 +334,7 @@ type notInListMatcher struct {
allowed []string allowed []string
} }
func (m *notInListMatcher) Test(value string) bool { func (m *notInListMatcher) Test(ctx ValueMatcherContext, value string) bool {
return !InList(value, m.allowed) return !InList(value, m.allowed)
} }
@@ -340,7 +344,7 @@ func (m *notInListMatcher) String() string {
type isSetMatcher struct{} type isSetMatcher struct{}
func (m *isSetMatcher) Test(value string) bool { func (m *isSetMatcher) Test(ctx ValueMatcherContext, value string) bool {
return value != "" return value != ""
} }
@@ -350,6 +354,19 @@ func (m *isSetMatcher) String() string {
var isSetMatcherInstance = &isSetMatcher{} var isSetMatcherInstance = &isSetMatcher{}
type sdkVersionMatcher struct {
condition func(ctx ValueMatcherContext, spec SdkSpec) bool
description string
}
func (m *sdkVersionMatcher) Test(ctx ValueMatcherContext, value string) bool {
return m.condition(ctx, SdkSpecFromWithConfig(ctx.Config(), value))
}
func (m *sdkVersionMatcher) String() string {
return ".sdk-version(" + m.description + ")"
}
type ruleProperty struct { type ruleProperty struct {
fields []string // e.x.: Vndk.Enabled fields []string // e.x.: Vndk.Enabled
matcher ValueMatcher matcher ValueMatcher
@@ -563,9 +580,10 @@ func (r *rule) appliesToModuleType(moduleType string) bool {
return (len(r.moduleTypes) == 0 || InList(moduleType, r.moduleTypes)) && !InList(moduleType, r.unlessModuleTypes) return (len(r.moduleTypes) == 0 || InList(moduleType, r.moduleTypes)) && !InList(moduleType, r.unlessModuleTypes)
} }
func (r *rule) appliesToProperties(properties []interface{}) bool { func (r *rule) appliesToProperties(ctx ValueMatcherContext,
includeProps := hasAllProperties(properties, r.props) properties []interface{}) bool {
excludeProps := hasAnyProperty(properties, r.unlessProps) includeProps := hasAllProperties(ctx, properties, r.props)
excludeProps := hasAnyProperty(ctx, properties, r.unlessProps)
return includeProps && !excludeProps return includeProps && !excludeProps
} }
@@ -585,6 +603,16 @@ func NotInList(allowed []string) ValueMatcher {
return &notInListMatcher{allowed} return &notInListMatcher{allowed}
} }
func LessThanSdkVersion(sdk string) ValueMatcher {
return &sdkVersionMatcher{
condition: func(ctx ValueMatcherContext, spec SdkSpec) bool {
return spec.ApiLevel.LessThan(
SdkSpecFromWithConfig(ctx.Config(), sdk).ApiLevel)
},
description: "lessThan=" + sdk,
}
}
// assorted utils // assorted utils
func cleanPaths(paths []string) []string { func cleanPaths(paths []string) []string {
@@ -603,25 +631,28 @@ func fieldNamesForProperties(propertyNames string) []string {
return names return names
} }
func hasAnyProperty(properties []interface{}, props []ruleProperty) bool { func hasAnyProperty(ctx ValueMatcherContext, properties []interface{},
props []ruleProperty) bool {
for _, v := range props { for _, v := range props {
if hasProperty(properties, v) { if hasProperty(ctx, properties, v) {
return true return true
} }
} }
return false return false
} }
func hasAllProperties(properties []interface{}, props []ruleProperty) bool { func hasAllProperties(ctx ValueMatcherContext, properties []interface{},
props []ruleProperty) bool {
for _, v := range props { for _, v := range props {
if !hasProperty(properties, v) { if !hasProperty(ctx, properties, v) {
return false return false
} }
} }
return true return true
} }
func hasProperty(properties []interface{}, prop ruleProperty) bool { func hasProperty(ctx ValueMatcherContext, properties []interface{},
prop ruleProperty) bool {
for _, propertyStruct := range properties { for _, propertyStruct := range properties {
propertiesValue := reflect.ValueOf(propertyStruct).Elem() propertiesValue := reflect.ValueOf(propertyStruct).Elem()
for _, v := range prop.fields { for _, v := range prop.fields {
@@ -635,7 +666,7 @@ func hasProperty(properties []interface{}, prop ruleProperty) bool {
} }
check := func(value string) bool { check := func(value string) bool {
return prop.matcher.Test(value) return prop.matcher.Test(ctx, value)
} }
if matchValue(propertiesValue, check) { if matchValue(propertiesValue, check) {

View File

@@ -296,6 +296,48 @@ var neverallowTests = []struct {
"Only boot images and seapp contexts may be imported as a makefile goal.", "Only boot images and seapp contexts may be imported as a makefile goal.",
}, },
}, },
{
name: "min_sdk too low",
fs: map[string][]byte{
"Android.bp": []byte(`
java_library {
name: "min_sdk_too_low",
min_sdk_version: "30",
}`),
},
rules: []Rule{
NeverAllow().WithMatcher("min_sdk_version", LessThanSdkVersion("31")),
},
expectedErrors: []string{
"module \"min_sdk_too_low\": violates neverallow",
},
},
{
name: "min_sdk high enough",
fs: map[string][]byte{
"Android.bp": []byte(`
java_library {
name: "min_sdk_high_enough",
min_sdk_version: "31",
}`),
},
rules: []Rule{
NeverAllow().WithMatcher("min_sdk_version", LessThanSdkVersion("31")),
},
},
{
name: "current min_sdk high enough",
fs: map[string][]byte{
"Android.bp": []byte(`
java_library {
name: "current_min_sdk_high_enough",
min_sdk_version: "current",
}`),
},
rules: []Rule{
NeverAllow().WithMatcher("min_sdk_version", LessThanSdkVersion("31")),
},
},
} }
var prepareForNeverAllowTest = GroupFixturePreparers( var prepareForNeverAllowTest = GroupFixturePreparers(
@@ -380,6 +422,7 @@ func (p *mockCcLibraryModule) GenerateAndroidBuildActions(ModuleContext) {
type mockJavaLibraryProperties struct { type mockJavaLibraryProperties struct {
Libs []string Libs []string
Min_sdk_version *string
Sdk_version *string Sdk_version *string
Uncompress_dex *bool Uncompress_dex *bool
} }

View File

@@ -3200,15 +3200,16 @@ func createApexPermittedPackagesRules(modules_packages map[string][]string) []an
BootclasspathJar(). BootclasspathJar().
With("apex_available", module_name). With("apex_available", module_name).
WithMatcher("permitted_packages", android.NotInList(module_packages)). WithMatcher("permitted_packages", android.NotInList(module_packages)).
WithMatcher("min_sdk_version", android.LessThanSdkVersion("Tiramisu")).
Because("jars that are part of the " + module_name + Because("jars that are part of the " + module_name +
" module may only allow these packages: " + strings.Join(module_packages, ",") + " module may only allow these packages: " + strings.Join(module_packages, ",") +
". Please jarjar or move code around.") " with min_sdk < T. Please jarjar or move code around.")
rules = append(rules, permittedPackagesRule) rules = append(rules, permittedPackagesRule)
} }
return rules return rules
} }
// DO NOT EDIT! These are the package prefixes that are exempted from being AOT'ed by ART. // DO NOT EDIT! These are the package prefixes that are exempted from being AOT'ed by ART on Q/R/S.
// Adding code to the bootclasspath in new packages will cause issues on module update. // Adding code to the bootclasspath in new packages will cause issues on module update.
func qModulesPackages() map[string][]string { func qModulesPackages() map[string][]string {
return map[string][]string{ return map[string][]string{
@@ -3222,7 +3223,7 @@ func qModulesPackages() map[string][]string {
} }
} }
// DO NOT EDIT! These are the package prefixes that are exempted from being AOT'ed by ART. // DO NOT EDIT! These are the package prefixes that are exempted from being AOT'ed by ART on R/S.
// Adding code to the bootclasspath in new packages will cause issues on module update. // Adding code to the bootclasspath in new packages will cause issues on module update.
func rModulesPackages() map[string][]string { func rModulesPackages() map[string][]string {
return map[string][]string{ return map[string][]string{

View File

@@ -7359,6 +7359,7 @@ func TestApexPermittedPackagesRules(t *testing.T) {
apex_available: ["myapex"], apex_available: ["myapex"],
sdk_version: "none", sdk_version: "none",
system_modules: "none", system_modules: "none",
min_sdk_version: "30",
} }
java_library { java_library {
name: "nonbcp_lib2", name: "nonbcp_lib2",
@@ -7367,9 +7368,11 @@ func TestApexPermittedPackagesRules(t *testing.T) {
permitted_packages: ["a.b"], permitted_packages: ["a.b"],
sdk_version: "none", sdk_version: "none",
system_modules: "none", system_modules: "none",
min_sdk_version: "30",
} }
apex { apex {
name: "myapex", name: "myapex",
min_sdk_version: "30",
key: "myapex.key", key: "myapex.key",
java_libs: ["bcp_lib1", "nonbcp_lib2"], java_libs: ["bcp_lib1", "nonbcp_lib2"],
updatable: false, updatable: false,
@@ -7382,8 +7385,8 @@ func TestApexPermittedPackagesRules(t *testing.T) {
}, },
}, },
{ {
name: "Bootclasspath apex jar not satisfying allowed module packages.", name: "Bootclasspath apex jar not satisfying allowed module packages on Q.",
expectedError: `module "bcp_lib2" .* which is restricted because jars that are part of the myapex module may only allow these packages: foo.bar. Please jarjar or move code around.`, expectedError: `module "bcp_lib2" .* which is restricted because jars that are part of the myapex module may only allow these packages: foo.bar with min_sdk < T. Please jarjar or move code around.`,
bp: ` bp: `
java_library { java_library {
name: "bcp_lib1", name: "bcp_lib1",
@@ -7392,6 +7395,7 @@ func TestApexPermittedPackagesRules(t *testing.T) {
permitted_packages: ["foo.bar"], permitted_packages: ["foo.bar"],
sdk_version: "none", sdk_version: "none",
system_modules: "none", system_modules: "none",
min_sdk_version: "29",
} }
java_library { java_library {
name: "bcp_lib2", name: "bcp_lib2",
@@ -7400,9 +7404,85 @@ func TestApexPermittedPackagesRules(t *testing.T) {
permitted_packages: ["foo.bar", "bar.baz"], permitted_packages: ["foo.bar", "bar.baz"],
sdk_version: "none", sdk_version: "none",
system_modules: "none", system_modules: "none",
min_sdk_version: "29",
} }
apex { apex {
name: "myapex", name: "myapex",
min_sdk_version: "29",
key: "myapex.key",
java_libs: ["bcp_lib1", "bcp_lib2"],
updatable: false,
}
`,
bootJars: []string{"bcp_lib1", "bcp_lib2"},
modulesPackages: map[string][]string{
"myapex": []string{
"foo.bar",
},
},
},
{
name: "Bootclasspath apex jar not satisfying allowed module packages on R.",
expectedError: `module "bcp_lib2" .* which is restricted because jars that are part of the myapex module may only allow these packages: foo.bar with min_sdk < T. Please jarjar or move code around.`,
bp: `
java_library {
name: "bcp_lib1",
srcs: ["lib1/src/*.java"],
apex_available: ["myapex"],
permitted_packages: ["foo.bar"],
sdk_version: "none",
system_modules: "none",
min_sdk_version: "30",
}
java_library {
name: "bcp_lib2",
srcs: ["lib2/src/*.java"],
apex_available: ["myapex"],
permitted_packages: ["foo.bar", "bar.baz"],
sdk_version: "none",
system_modules: "none",
min_sdk_version: "30",
}
apex {
name: "myapex",
min_sdk_version: "30",
key: "myapex.key",
java_libs: ["bcp_lib1", "bcp_lib2"],
updatable: false,
}
`,
bootJars: []string{"bcp_lib1", "bcp_lib2"},
modulesPackages: map[string][]string{
"myapex": []string{
"foo.bar",
},
},
},
{
name: "Bootclasspath apex jar >= T not satisfying Q/R/S allowed module packages.",
expectedError: "",
bp: `
java_library {
name: "bcp_lib1",
srcs: ["lib1/src/*.java"],
apex_available: ["myapex"],
permitted_packages: ["foo.bar"],
sdk_version: "none",
system_modules: "none",
min_sdk_version: "current",
}
java_library {
name: "bcp_lib2",
srcs: ["lib2/src/*.java"],
apex_available: ["myapex"],
permitted_packages: ["foo.bar", "bar.baz"],
sdk_version: "none",
system_modules: "none",
min_sdk_version: "current",
}
apex {
name: "myapex",
min_sdk_version: "current",
key: "myapex.key", key: "myapex.key",
java_libs: ["bcp_lib1", "bcp_lib2"], java_libs: ["bcp_lib1", "bcp_lib2"],
updatable: false, updatable: false,