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.

(clean cherry-pick)

Bug: 208773835
Change-Id: Ib41ab443e36a694e3fac5f2ab0acabb3009f40a9
Test: m (runs apex tests)
Merged-In: I2c3ad8984ca05ad763bf6162bd478f93ab4ee650
This commit is contained in:
Remi NGUYEN VAN 2021-12-10 00:17:57 +00:00
parent aa7b36e681
commit efb49afc8a
4 changed files with 180 additions and 25 deletions

View file

@ -238,7 +238,7 @@ func neverallowMutator(ctx BottomUpMutatorContext) {
continue
}
if !n.appliesToProperties(properties) {
if !n.appliesToProperties(ctx, properties) {
continue
}
@ -258,8 +258,12 @@ func neverallowMutator(ctx BottomUpMutatorContext) {
}
}
type ValueMatcherContext interface {
Config() Config
}
type ValueMatcher interface {
Test(string) bool
Test(ValueMatcherContext, string) bool
String() string
}
@ -267,7 +271,7 @@ type equalMatcher struct {
expected string
}
func (m *equalMatcher) Test(value string) bool {
func (m *equalMatcher) Test(ctx ValueMatcherContext, value string) bool {
return m.expected == value
}
@ -278,7 +282,7 @@ func (m *equalMatcher) String() string {
type anyMatcher struct {
}
func (m *anyMatcher) Test(value string) bool {
func (m *anyMatcher) Test(ctx ValueMatcherContext, value string) bool {
return true
}
@ -292,7 +296,7 @@ type startsWithMatcher struct {
prefix string
}
func (m *startsWithMatcher) Test(value string) bool {
func (m *startsWithMatcher) Test(ctx ValueMatcherContext, value string) bool {
return strings.HasPrefix(value, m.prefix)
}
@ -304,7 +308,7 @@ type regexMatcher struct {
re *regexp.Regexp
}
func (m *regexMatcher) Test(value string) bool {
func (m *regexMatcher) Test(ctx ValueMatcherContext, value string) bool {
return m.re.MatchString(value)
}
@ -316,7 +320,7 @@ type notInListMatcher struct {
allowed []string
}
func (m *notInListMatcher) Test(value string) bool {
func (m *notInListMatcher) Test(ctx ValueMatcherContext, value string) bool {
return !InList(value, m.allowed)
}
@ -326,7 +330,7 @@ func (m *notInListMatcher) String() string {
type isSetMatcher struct{}
func (m *isSetMatcher) Test(value string) bool {
func (m *isSetMatcher) Test(ctx ValueMatcherContext, value string) bool {
return value != ""
}
@ -336,6 +340,19 @@ func (m *isSetMatcher) String() string {
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 {
fields []string // e.x.: Vndk.Enabled
matcher ValueMatcher
@ -549,9 +566,10 @@ func (r *rule) appliesToModuleType(moduleType string) bool {
return (len(r.moduleTypes) == 0 || InList(moduleType, r.moduleTypes)) && !InList(moduleType, r.unlessModuleTypes)
}
func (r *rule) appliesToProperties(properties []interface{}) bool {
includeProps := hasAllProperties(properties, r.props)
excludeProps := hasAnyProperty(properties, r.unlessProps)
func (r *rule) appliesToProperties(ctx ValueMatcherContext,
properties []interface{}) bool {
includeProps := hasAllProperties(ctx, properties, r.props)
excludeProps := hasAnyProperty(ctx, properties, r.unlessProps)
return includeProps && !excludeProps
}
@ -571,6 +589,16 @@ func NotInList(allowed []string) ValueMatcher {
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
func cleanPaths(paths []string) []string {
@ -589,25 +617,28 @@ func fieldNamesForProperties(propertyNames string) []string {
return names
}
func hasAnyProperty(properties []interface{}, props []ruleProperty) bool {
func hasAnyProperty(ctx ValueMatcherContext, properties []interface{},
props []ruleProperty) bool {
for _, v := range props {
if hasProperty(properties, v) {
if hasProperty(ctx, properties, v) {
return true
}
}
return false
}
func hasAllProperties(properties []interface{}, props []ruleProperty) bool {
func hasAllProperties(ctx ValueMatcherContext, properties []interface{},
props []ruleProperty) bool {
for _, v := range props {
if !hasProperty(properties, v) {
if !hasProperty(ctx, properties, v) {
return false
}
}
return true
}
func hasProperty(properties []interface{}, prop ruleProperty) bool {
func hasProperty(ctx ValueMatcherContext, properties []interface{},
prop ruleProperty) bool {
for _, propertyStruct := range properties {
propertiesValue := reflect.ValueOf(propertyStruct).Elem()
for _, v := range prop.fields {
@ -621,7 +652,7 @@ func hasProperty(properties []interface{}, prop ruleProperty) bool {
}
check := func(value string) bool {
return prop.matcher.Test(value)
return prop.matcher.Test(ctx, value)
}
if matchValue(propertiesValue, check) {

View file

@ -296,6 +296,48 @@ var neverallowTests = []struct {
"Only boot images 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(
@ -379,9 +421,10 @@ func (p *mockCcLibraryModule) GenerateAndroidBuildActions(ModuleContext) {
}
type mockJavaLibraryProperties struct {
Libs []string
Sdk_version *string
Uncompress_dex *bool
Libs []string
Min_sdk_version *string
Sdk_version *string
Uncompress_dex *bool
}
type mockJavaLibraryModule struct {

View file

@ -3049,15 +3049,16 @@ func createApexPermittedPackagesRules(modules_packages map[string][]string) []an
BootclasspathJar().
With("apex_available", module_name).
WithMatcher("permitted_packages", android.NotInList(module_packages)).
WithMatcher("min_sdk_version", android.LessThanSdkVersion("Tiramisu")).
Because("jars that are part of the " + module_name +
" 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)
}
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.
func qModulesPackages() map[string][]string {
return map[string][]string{
@ -3071,7 +3072,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.
func rModulesPackages() map[string][]string {
return map[string][]string{

View file

@ -6897,6 +6897,7 @@ func TestApexPermittedPackagesRules(t *testing.T) {
apex_available: ["myapex"],
sdk_version: "none",
system_modules: "none",
min_sdk_version: "30",
}
java_library {
name: "nonbcp_lib2",
@ -6905,9 +6906,11 @@ func TestApexPermittedPackagesRules(t *testing.T) {
permitted_packages: ["a.b"],
sdk_version: "none",
system_modules: "none",
min_sdk_version: "30",
}
apex {
name: "myapex",
min_sdk_version: "30",
key: "myapex.key",
java_libs: ["bcp_lib1", "nonbcp_lib2"],
updatable: false,
@ -6920,8 +6923,8 @@ func TestApexPermittedPackagesRules(t *testing.T) {
},
},
{
name: "Bootclasspath apex jar not satisfying allowed module packages.",
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.`,
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 with min_sdk < T. Please jarjar or move code around.`,
bp: `
java_library {
name: "bcp_lib1",
@ -6930,6 +6933,7 @@ func TestApexPermittedPackagesRules(t *testing.T) {
permitted_packages: ["foo.bar"],
sdk_version: "none",
system_modules: "none",
min_sdk_version: "29",
}
java_library {
name: "bcp_lib2",
@ -6938,9 +6942,85 @@ func TestApexPermittedPackagesRules(t *testing.T) {
permitted_packages: ["foo.bar", "bar.baz"],
sdk_version: "none",
system_modules: "none",
min_sdk_version: "29",
}
apex {
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",
java_libs: ["bcp_lib1", "bcp_lib2"],
updatable: false,