diff options
author | 2022-04-14 09:04:56 +0000 | |
---|---|---|
committer | 2022-04-14 14:47:46 +0000 | |
commit | 2b19a8e706d05d963191f19333669cef0b6aaee4 (patch) | |
tree | a9f6331ea72532e11a6f52271d579a41d15e4280 | |
parent | 0b9a9f7bb27cae07a1702acaaaf430e09e51c8f8 (diff) |
Add the infrastructure to allow OEMs to make small modifications
It can be expensive for OEMs to overlay the whole XML config.
Make every field in the XML config accept string references.
Remove some prohibited checks so that we can add placeholder values that
can be overlayed later on if the source state is also overlayed.
Fix flaky overlay test as best as we can.
Fix linter to be able to read string references.
Add missing presubmit test mappings.
Test: m out/soong/.intermediates/packages/modules/Permission/SafetyCenter/Resources/SafetyCenterResources/android_common/lint/lint-report.html
Test: atest CtsSafetyCenterTestCases
Test: atest SafetyCenterConfigTests
Test: atest ConfigLintCheckerTest --host
Bug: 229192140
Change-Id: If8fa41db9d2424ca5a311338e88cb9bb92a632b8
19 files changed, 590 insertions, 386 deletions
diff --git a/SafetyCenter/Config/TEST_MAPPING b/SafetyCenter/Config/TEST_MAPPING new file mode 100644 index 000000000..a39176e27 --- /dev/null +++ b/SafetyCenter/Config/TEST_MAPPING @@ -0,0 +1,7 @@ +{ + "presubmit": [ + { + "name": "SafetyCenterConfigTests" + } + ] +} diff --git a/SafetyCenter/Config/java/com/android/safetycenter/config/SafetyCenterConfigParser.java b/SafetyCenter/Config/java/com/android/safetycenter/config/SafetyCenterConfigParser.java index fc933bdcd..8081ae472 100644 --- a/SafetyCenter/Config/java/com/android/safetycenter/config/SafetyCenterConfigParser.java +++ b/SafetyCenter/Config/java/com/android/safetycenter/config/SafetyCenterConfigParser.java @@ -79,8 +79,7 @@ public final class SafetyCenterConfigParser { private static final String ENUM_INITIAL_DISPLAY_STATE_DISABLED = "disabled"; private static final String ENUM_INITIAL_DISPLAY_STATE_HIDDEN = "hidden"; - private SafetyCenterConfigParser() { - } + private SafetyCenterConfigParser() {} /** * Parses and validates the given XML resource into a {@link SafetyCenterConfig} object. @@ -88,9 +87,9 @@ public final class SafetyCenterConfigParser { * <p>It throws a {@link ParseException} if the given XML resource does not comply with the * safety_center_config.xsd schema. * - * @param in the raw XML resource representing the Safety Center configuration + * @param in the raw XML resource representing the Safety Center configuration * @param resources the {@link Resources} retrieved from the package that contains the Safety - * Center configuration + * Center configuration */ @NonNull public static SafetyCenterConfig parseXmlResource( @@ -106,8 +105,7 @@ public final class SafetyCenterConfigParser { } parser.nextTag(); validateElementStart(parser, TAG_SAFETY_CENTER_CONFIG); - SafetyCenterConfig safetyCenterConfig = - parseSafetyCenterConfig(parser, resources); + SafetyCenterConfig safetyCenterConfig = parseSafetyCenterConfig(parser, resources); if (parser.getEventType() == TEXT && parser.isWhitespace()) { parser.next(); } @@ -132,8 +130,7 @@ public final class SafetyCenterConfigParser { parser.nextTag(); while (parser.getEventType() == START_TAG && parser.getName().equals(TAG_SAFETY_SOURCES_GROUP)) { - builder.addSafetySourcesGroup( - parseSafetySourcesGroup(parser, resources)); + builder.addSafetySourcesGroup(parseSafetySourcesGroup(parser, resources)); } validateElementEnd(parser, TAG_SAFETY_SOURCES_CONFIG); parser.nextTag(); @@ -155,22 +152,36 @@ public final class SafetyCenterConfigParser { for (int i = 0; i < parser.getAttributeCount(); i++) { switch (parser.getAttributeName(i)) { case ATTR_SAFETY_SOURCES_GROUP_ID: - builder.setId(parser.getAttributeValue(i)); + builder.setId( + parseStringResourceValue( + parser.getAttributeValue(i), + name, + parser.getAttributeName(i), + resources)); break; case ATTR_SAFETY_SOURCES_GROUP_TITLE: builder.setTitleResId( - parseStringResourceName(parser.getAttributeValue(i), name, - parser.getAttributeName(i), resources)); + parseStringResourceName( + parser.getAttributeValue(i), + name, + parser.getAttributeName(i), + resources)); break; case ATTR_SAFETY_SOURCES_GROUP_SUMMARY: builder.setSummaryResId( - parseStringResourceName(parser.getAttributeValue(i), name, - parser.getAttributeName(i), resources)); + parseStringResourceName( + parser.getAttributeValue(i), + name, + parser.getAttributeName(i), + resources)); break; case ATTR_SAFETY_SOURCES_GROUP_STATELESS_ICON_TYPE: builder.setStatelessIconType( parseStatelessIconType( - parser.getAttributeValue(i), name, parser.getAttributeName(i))); + parser.getAttributeValue(i), + name, + parser.getAttributeName(i), + resources)); break; default: throw attributeUnexpected(name, parser.getAttributeName(i)); @@ -193,8 +204,7 @@ public final class SafetyCenterConfigParser { default: break loop; } - builder.addSafetySource( - parseSafetySource(parser, resources, type, parser.getName())); + builder.addSafetySource(parseSafetySource(parser, resources, type, parser.getName())); } validateElementEnd(parser, name); parser.nextTag(); @@ -210,64 +220,106 @@ public final class SafetyCenterConfigParser { @NonNull XmlPullParser parser, @NonNull Resources resources, int safetySourceType, - @NonNull String name - ) throws XmlPullParserException, IOException, ParseException { + @NonNull String name) + throws XmlPullParserException, IOException, ParseException { SafetySource.Builder builder = new SafetySource.Builder(safetySourceType); for (int i = 0; i < parser.getAttributeCount(); i++) { switch (parser.getAttributeName(i)) { case ATTR_SAFETY_SOURCE_ID: - builder.setId(parser.getAttributeValue(i)); + builder.setId( + parseStringResourceValue( + parser.getAttributeValue(i), + name, + parser.getAttributeName(i), + resources)); break; case ATTR_SAFETY_SOURCE_PACKAGE_NAME: - builder.setPackageName(parser.getAttributeValue(i)); + builder.setPackageName( + parseStringResourceValue( + parser.getAttributeValue(i), + name, + parser.getAttributeName(i), + resources)); break; case ATTR_SAFETY_SOURCE_TITLE: builder.setTitleResId( - parseStringResourceName(parser.getAttributeValue(i), name, - parser.getAttributeName(i), resources)); + parseStringResourceName( + parser.getAttributeValue(i), + name, + parser.getAttributeName(i), + resources)); break; case ATTR_SAFETY_SOURCE_TITLE_FOR_WORK: builder.setTitleForWorkResId( - parseStringResourceName(parser.getAttributeValue(i), name, - parser.getAttributeName(i), resources)); + parseStringResourceName( + parser.getAttributeValue(i), + name, + parser.getAttributeName(i), + resources)); break; case ATTR_SAFETY_SOURCE_SUMMARY: builder.setSummaryResId( - parseStringResourceName(parser.getAttributeValue(i), name, - parser.getAttributeName(i), resources)); + parseStringResourceName( + parser.getAttributeValue(i), + name, + parser.getAttributeName(i), + resources)); break; case ATTR_SAFETY_SOURCE_INTENT_ACTION: - builder.setIntentAction(parser.getAttributeValue(i)); + builder.setIntentAction( + parseStringResourceValue( + parser.getAttributeValue(i), + name, + parser.getAttributeName(i), + resources)); break; case ATTR_SAFETY_SOURCE_PROFILE: builder.setProfile( - parseProfile(parser.getAttributeValue(i), name, - parser.getAttributeName(i))); + parseProfile( + parser.getAttributeValue(i), + name, + parser.getAttributeName(i), + resources)); break; case ATTR_SAFETY_SOURCE_INITIAL_DISPLAY_STATE: builder.setInitialDisplayState( parseInitialDisplayState( - parser.getAttributeValue(i), name, parser.getAttributeName(i))); + parser.getAttributeValue(i), + name, + parser.getAttributeName(i), + resources)); break; case ATTR_SAFETY_SOURCE_MAX_SEVERITY_LEVEL: builder.setMaxSeverityLevel( - parseInteger(parser.getAttributeValue(i), name, - parser.getAttributeName(i))); + parseInteger( + parser.getAttributeValue(i), + name, + parser.getAttributeName(i), + resources)); break; case ATTR_SAFETY_SOURCE_SEARCH_TERMS: builder.setSearchTermsResId( - parseStringResourceName(parser.getAttributeValue(i), name, - parser.getAttributeName(i), resources)); + parseStringResourceName( + parser.getAttributeValue(i), + name, + parser.getAttributeName(i), + resources)); break; case ATTR_SAFETY_SOURCE_LOGGING_ALLOWED: builder.setLoggingAllowed( - parseBoolean(parser.getAttributeValue(i), name, - parser.getAttributeName(i))); + parseBoolean( + parser.getAttributeValue(i), + name, + parser.getAttributeName(i), + resources)); break; case ATTR_SAFETY_SOURCE_REFRESH_ON_PAGE_OPEN_ALLOWED: builder.setRefreshOnPageOpenAllowed( - parseBoolean(parser.getAttributeValue(i), name, - parser.getAttributeName(i))); + parseBoolean( + parser.getAttributeValue(i), + name, + parser.getAttributeName(i), + resources)); break; default: throw attributeUnexpected(name, parser.getAttributeName(i)); @@ -320,109 +372,160 @@ public final class SafetyCenterConfigParser { return new ParseException(String.format("Element %s invalid", name), e); } - private static ParseException attributeUnexpected(@NonNull String parent, - @NonNull String name) { + private static ParseException attributeUnexpected( + @NonNull String parent, @NonNull String name) { return new ParseException(String.format("Unexpected attribute %s.%s", parent, name)); } - private static ParseException attributeInvalid(@NonNull String parent, @NonNull String name) { - return new ParseException(String.format("Attribute %s.%s invalid", parent, name)); + private static String attributeInvalidString( + @NonNull String valueString, @NonNull String parent, @NonNull String name) { + return String.format("Attribute value \"%s\" in %s.%s invalid", valueString, parent, name); + } + + private static ParseException attributeInvalid( + @NonNull String valueString, @NonNull String parent, @NonNull String name) { + return new ParseException(attributeInvalidString(valueString, parent, name)); + } + + private static ParseException attributeInvalid( + @NonNull String valueString, + @NonNull String parent, + @NonNull String name, + @NonNull Throwable ex) { + return new ParseException(attributeInvalidString(valueString, parent, name), ex); } private static int parseInteger( - @NonNull String valueString, @NonNull String parent, @NonNull String name) + @NonNull String valueString, + @NonNull String parent, + @NonNull String name, + @NonNull Resources resources) throws ParseException { + String valueToParse = getValueToParse(valueString, parent, name, resources); try { - return Integer.parseInt(valueString); + return Integer.parseInt(valueToParse); } catch (NumberFormatException e) { - throw new ParseException(String.format("Attribute %s.%s invalid", parent, name), e); + throw attributeInvalid(valueToParse, parent, name, e); } } private static boolean parseBoolean( - @NonNull String valueString, @NonNull String parent, @NonNull String name) + @NonNull String valueString, + @NonNull String parent, + @NonNull String name, + @NonNull Resources resources) throws ParseException { - String valueLowerString = valueString.toLowerCase(ROOT); - if (valueLowerString.equals("true")) { + String valueToParse = + getValueToParse(valueString, parent, name, resources).toLowerCase(ROOT); + if (valueToParse.equals("true")) { return true; - } else if (!valueLowerString.equals("false")) { - throw new ParseException(String.format("Attribute %s.%s invalid", parent, name)); + } else if (!valueToParse.equals("false")) { + throw attributeInvalid(valueToParse, parent, name); } return false; } @StringRes private static int parseStringResourceName( - @NonNull String valueString, @NonNull String parent, @NonNull String name, - @NonNull Resources resources) throws ParseException { + @NonNull String valueString, + @NonNull String parent, + @NonNull String name, + @NonNull Resources resources) + throws ParseException { if (valueString.isEmpty()) { throw new ParseException( String.format("Resource name in %s.%s cannot be empty", parent, name)); } if (valueString.charAt(0) != '@') { throw new ParseException( - String.format("Resource name %s in %s.%s does not start with @", valueString, - parent, name)); + String.format( + "Resource name \"%s\" in %s.%s does not start with @", + valueString, parent, name)); } String[] colonSplit = valueString.substring(1).split(":", 2); if (colonSplit.length != 2 || colonSplit[0].isEmpty()) { throw new ParseException( - String.format("Resource name %s in %s.%s does not specify a package", + String.format( + "Resource name \"%s\" in %s.%s does not specify a package", valueString, parent, name)); } String packageName = colonSplit[0]; String[] slashSplit = colonSplit[1].split("/", 2); if (slashSplit.length != 2 || slashSplit[0].isEmpty()) { throw new ParseException( - String.format("Resource name %s in %s.%s does not specify a type", + String.format( + "Resource name \"%s\" in %s.%s does not specify a type", valueString, parent, name)); } String type = slashSplit[0]; if (!type.equals("string")) { throw new ParseException( - String.format("Resource name %s in %s.%s is not a string", valueString, parent, - name)); + String.format( + "Resource name \"%s\" in %s.%s is not a string", + valueString, parent, name)); } String entry = slashSplit[1]; int id = resources.getIdentifier(entry, type, packageName); if (id == Resources.ID_NULL) { throw new ParseException( - String.format("Resource name %s in %s.%s missing or invalid", valueString, - parent, name)); + String.format( + "Resource name \"%s\" in %s.%s missing or invalid", + valueString, parent, name)); } return id; } + @NonNull + private static String parseStringResourceValue( + @NonNull String valueString, + @NonNull String parent, + @NonNull String name, + @NonNull Resources resources) { + return getValueToParse(valueString, parent, name, resources); + } + private static int parseStatelessIconType( - @NonNull String valueString, @NonNull String parent, @NonNull String name) + @NonNull String valueString, + @NonNull String parent, + @NonNull String name, + @NonNull Resources resources) throws ParseException { - switch (valueString) { + String valueToParse = getValueToParse(valueString, parent, name, resources); + switch (valueToParse) { case ENUM_STATELESS_ICON_TYPE_NONE: return SafetySourcesGroup.STATELESS_ICON_TYPE_NONE; case ENUM_STATELESS_ICON_TYPE_PRIVACY: return SafetySourcesGroup.STATELESS_ICON_TYPE_PRIVACY; default: - throw attributeInvalid(parent, name); + throw attributeInvalid(valueToParse, parent, name); } } private static int parseProfile( - @NonNull String valueString, @NonNull String parent, @NonNull String name) + @NonNull String valueString, + @NonNull String parent, + @NonNull String name, + @NonNull Resources resources) throws ParseException { - switch (valueString) { + String valueToParse = getValueToParse(valueString, parent, name, resources); + switch (valueToParse) { case ENUM_PROFILE_PRIMARY: return SafetySource.PROFILE_PRIMARY; case ENUM_PROFILE_ALL: return SafetySource.PROFILE_ALL; default: - throw attributeInvalid(parent, name); + throw attributeInvalid(valueToParse, parent, name); } } private static int parseInitialDisplayState( - @NonNull String valueString, @NonNull String parent, @NonNull String name) + @NonNull String valueString, + @NonNull String parent, + @NonNull String name, + @NonNull Resources resources) throws ParseException { - switch (valueString) { + String valueToParse = getValueToParse(valueString, parent, name, resources); + switch (valueToParse) { case ENUM_INITIAL_DISPLAY_STATE_ENABLED: return SafetySource.INITIAL_DISPLAY_STATE_ENABLED; case ENUM_INITIAL_DISPLAY_STATE_DISABLED: @@ -430,7 +533,21 @@ public final class SafetyCenterConfigParser { case ENUM_INITIAL_DISPLAY_STATE_HIDDEN: return SafetySource.INITIAL_DISPLAY_STATE_HIDDEN; default: - throw attributeInvalid(parent, name); + throw attributeInvalid(valueToParse, parent, name); + } + } + + @NonNull + private static String getValueToParse( + @NonNull String valueString, + @NonNull String parent, + @NonNull String name, + @NonNull Resources resources) { + try { + int id = parseStringResourceName(valueString, parent, name, resources); + return resources.getString(id); + } catch (ParseException e) { + return valueString; } } } diff --git a/SafetyCenter/Config/tests/java/com/android/safetycenter/config/Coroutines.kt b/SafetyCenter/Config/tests/java/com/android/safetycenter/config/Coroutines.kt index 6237348dd..9b1d4c5f9 100644 --- a/SafetyCenter/Config/tests/java/com/android/safetycenter/config/Coroutines.kt +++ b/SafetyCenter/Config/tests/java/com/android/safetycenter/config/Coroutines.kt @@ -16,6 +16,7 @@ package com.android.safetycenter.config +import android.util.Log import kotlinx.coroutines.delay import kotlinx.coroutines.runBlocking import kotlinx.coroutines.withTimeout @@ -52,6 +53,22 @@ object Coroutines { return waitFor(checkPeriod, condition) } + /** Retries a test until no assertions or exceptions are thrown or a timeout occurs. */ + fun waitForTestToPass(test: () -> Unit) { + waitForWithTimeout { + try { + test() + true + } catch (ex: Throwable) { + Log.w(TAG, "Encountered test failure, retrying until timeout: $ex") + false + } + } + } + + /** A medium period, to be used for conditions that are expected to change. */ + private val TAG = "Coroutines" + /** A medium period, to be used for conditions that are expected to change. */ private val CHECK_PERIOD = Duration.ofMillis(250) diff --git a/SafetyCenter/Config/tests/java/com/android/safetycenter/config/ParserConfigInvalidTest.kt b/SafetyCenter/Config/tests/java/com/android/safetycenter/config/ParserConfigInvalidTest.kt index 56a68125d..a06eaa9fb 100644 --- a/SafetyCenter/Config/tests/java/com/android/safetycenter/config/ParserConfigInvalidTest.kt +++ b/SafetyCenter/Config/tests/java/com/android/safetycenter/config/ParserConfigInvalidTest.kt @@ -41,16 +41,16 @@ class ParserConfigInvalidTest { override fun toString() = testName } - @Parameterized.Parameter - lateinit var params: Params + @Parameterized.Parameter lateinit var params: Params @Test fun invalidConfig_throws() { val inputStream = context.resources.openRawResource(params.configResourceId) - val thrown = assertThrows(ParseException::class.java) { - SafetyCenterConfigParser.parseXmlResource(inputStream, context.resources) - } + val thrown = + assertThrows(ParseException::class.java) { + SafetyCenterConfigParser.parseXmlResource(inputStream, context.resources) + } assertThat(thrown).hasMessageThat().isEqualTo(params.errorMessage) if (params.causeErrorMessage != null) { @@ -67,362 +67,288 @@ class ParserConfigInvalidTest { "ConfigDynamicSafetySourceAllDisabledNoWork", R.raw.config_dynamic_safety_source_all_disabled_no_work, "Element dynamic-safety-source invalid", - "Required attribute titleForWork missing" - ), + "Required attribute titleForWork missing"), Params( "ConfigDynamicSafetySourceAllHiddenWithSearchNoWork", R.raw.config_dynamic_safety_source_all_hidden_with_search_no_work, "Element dynamic-safety-source invalid", - "Required attribute titleForWork missing" - ), + "Required attribute titleForWork missing"), Params( "ConfigDynamicSafetySourceAllNoWork", R.raw.config_dynamic_safety_source_all_no_work, "Element dynamic-safety-source invalid", - "Required attribute titleForWork missing" - ), + "Required attribute titleForWork missing"), Params( "ConfigDynamicSafetySourceDisabledNoSummary", R.raw.config_dynamic_safety_source_disabled_no_summary, "Element dynamic-safety-source invalid", - "Required attribute summary missing" - ), + "Required attribute summary missing"), Params( "ConfigDynamicSafetySourceDisabledNoTitle", R.raw.config_dynamic_safety_source_disabled_no_title, "Element dynamic-safety-source invalid", - "Required attribute title missing" - ), + "Required attribute title missing"), Params( "ConfigDynamicSafetySourceDuplicateKey", R.raw.config_dynamic_safety_source_duplicate_key, "Element safety-sources-config invalid", - "Duplicate id id among safety sources" - ), - Params( - "ConfigDynamicSafetySourceHiddenWithIntent", - R.raw.config_dynamic_safety_source_hidden_with_intent, - "Element dynamic-safety-source invalid", - "Prohibited attribute intentAction present" - ), + "Duplicate id id among safety sources"), Params( "ConfigDynamicSafetySourceHiddenWithSearchNoTitle", R.raw.config_dynamic_safety_source_hidden_with_search_no_title, "Element dynamic-safety-source invalid", - "Required attribute title missing" - ), - Params( - "ConfigDynamicSafetySourceHiddenWithSummary", - R.raw.config_dynamic_safety_source_hidden_with_summary, - "Element dynamic-safety-source invalid", - "Prohibited attribute summary present" - ), - Params( - "ConfigDynamicSafetySourceHiddenWithTitle", - R.raw.config_dynamic_safety_source_hidden_with_title, - "Element dynamic-safety-source invalid", - "Prohibited attribute title present" - ), + "Required attribute title missing"), Params( "ConfigDynamicSafetySourceInvalidDisplay", R.raw.config_dynamic_safety_source_invalid_display, - "Attribute dynamic-safety-source.initialDisplayState invalid", - null - ), + "Attribute value \"invalid\" in dynamic-safety-source.initialDisplayState " + + "invalid", + null), Params( "ConfigDynamicSafetySourceInvalidProfile", R.raw.config_dynamic_safety_source_invalid_profile, - "Attribute dynamic-safety-source.profile invalid", - null - ), + "Attribute value \"invalid\" in dynamic-safety-source.profile invalid", + null), Params( "ConfigDynamicSafetySourceNoId", R.raw.config_dynamic_safety_source_no_id, "Element dynamic-safety-source invalid", - "Required attribute id missing" - ), + "Required attribute id missing"), Params( "ConfigDynamicSafetySourceNoIntent", R.raw.config_dynamic_safety_source_no_intent, "Element dynamic-safety-source invalid", - "Required attribute intentAction missing" - ), + "Required attribute intentAction missing"), Params( "ConfigDynamicSafetySourceNoPackage", R.raw.config_dynamic_safety_source_no_package, "Element dynamic-safety-source invalid", - "Required attribute packageName missing" - ), + "Required attribute packageName missing"), Params( "ConfigDynamicSafetySourceNoProfile", R.raw.config_dynamic_safety_source_no_profile, "Element dynamic-safety-source invalid", - "Required attribute profile missing" - ), + "Required attribute profile missing"), Params( "ConfigDynamicSafetySourceNoSummary", R.raw.config_dynamic_safety_source_no_summary, "Element dynamic-safety-source invalid", - "Required attribute summary missing" - ), + "Required attribute summary missing"), Params( "ConfigDynamicSafetySourceNoTitle", R.raw.config_dynamic_safety_source_no_title, "Element dynamic-safety-source invalid", - "Required attribute title missing" - ), + "Required attribute title missing"), Params( "ConfigDynamicSafetySourcePrimaryHiddenWithWork", R.raw.config_dynamic_safety_source_primary_hidden_with_work, "Element dynamic-safety-source invalid", - "Prohibited attribute titleForWork present" - ), + "Prohibited attribute titleForWork present"), Params( "ConfigDynamicSafetySourcePrimaryWithWork", R.raw.config_dynamic_safety_source_primary_with_work, "Element dynamic-safety-source invalid", - "Prohibited attribute titleForWork present" - ), + "Prohibited attribute titleForWork present"), Params( "ConfigFileCorrupted", R.raw.config_file_corrupted, "Exception while parsing the XML resource", - null - ), + null), Params( "ConfigIssueOnlySafetySourceDuplicateKey", R.raw.config_issue_only_safety_source_duplicate_key, "Element safety-sources-config invalid", - "Duplicate id id among safety sources" - ), + "Duplicate id id among safety sources"), Params( "ConfigIssueOnlySafetySourceInvalidProfile", R.raw.config_issue_only_safety_source_invalid_profile, - "Attribute issue-only-safety-source.profile invalid", - null - ), + "Attribute value \"invalid\" in issue-only-safety-source.profile invalid", + null), Params( "ConfigIssueOnlySafetySourceNoId", R.raw.config_issue_only_safety_source_no_id, "Element issue-only-safety-source invalid", - "Required attribute id missing" - ), + "Required attribute id missing"), Params( "ConfigIssueOnlySafetySourceNoPackage", R.raw.config_issue_only_safety_source_no_package, "Element issue-only-safety-source invalid", - "Required attribute packageName missing" - ), + "Required attribute packageName missing"), Params( "ConfigIssueOnlySafetySourceNoProfile", R.raw.config_issue_only_safety_source_no_profile, "Element issue-only-safety-source invalid", - "Required attribute profile missing" - ), + "Required attribute profile missing"), Params( "ConfigIssueOnlySafetySourceWithDisplay", R.raw.config_issue_only_safety_source_with_display, "Element issue-only-safety-source invalid", - "Prohibited attribute initialDisplayState present" - ), + "Prohibited attribute initialDisplayState present"), Params( "ConfigIssueOnlySafetySourceWithIntent", R.raw.config_issue_only_safety_source_with_intent, "Element issue-only-safety-source invalid", - "Prohibited attribute intentAction present" - ), + "Prohibited attribute intentAction present"), Params( "ConfigIssueOnlySafetySourceWithSearch", R.raw.config_issue_only_safety_source_with_search, "Element issue-only-safety-source invalid", - "Prohibited attribute searchTerms present" - ), + "Prohibited attribute searchTerms present"), Params( "ConfigIssueOnlySafetySourceWithSummary", R.raw.config_issue_only_safety_source_with_summary, "Element issue-only-safety-source invalid", - "Prohibited attribute summary present" - ), + "Prohibited attribute summary present"), Params( "ConfigIssueOnlySafetySourceWithTitle", R.raw.config_issue_only_safety_source_with_title, "Element issue-only-safety-source invalid", - "Prohibited attribute title present" - ), + "Prohibited attribute title present"), Params( "ConfigIssueOnlySafetySourceWithWork", R.raw.config_issue_only_safety_source_with_work, "Element issue-only-safety-source invalid", - "Prohibited attribute titleForWork present" - ), + "Prohibited attribute titleForWork present"), Params( "ConfigMixedSafetySourceDuplicateKey", R.raw.config_mixed_safety_source_duplicate_key, "Element safety-sources-config invalid", - "Duplicate id id among safety sources" - ), + "Duplicate id id among safety sources"), Params( "ConfigSafetyCenterConfigMissing", R.raw.config_safety_center_config_missing, "Element safety-center-config missing", - null - ), + null), Params( "ConfigSafetySourcesConfigEmpty", R.raw.config_safety_sources_config_empty, "Element safety-sources-config invalid", - "No safety sources groups present" - ), + "No safety sources groups present"), Params( "ConfigSafetySourcesConfigMissing", R.raw.config_safety_sources_config_missing, "Element safety-sources-config missing", - null - ), + null), Params( "ConfigSafetySourcesGroupDuplicateId", R.raw.config_safety_sources_group_duplicate_id, "Element safety-sources-config invalid", - "Duplicate id id among safety sources groups" - ), + "Duplicate id id among safety sources groups"), Params( "ConfigSafetySourcesGroupEmpty", R.raw.config_safety_sources_group_empty, "Element safety-sources-group invalid", - "Safety sources group empty" - ), + "Safety sources group empty"), Params( "ConfigSafetySourcesGroupInvalidIcon", R.raw.config_safety_sources_group_invalid_icon, - "Attribute safety-sources-group.statelessIconType invalid", - null - ), + "Attribute value \"invalid\" in safety-sources-group.statelessIconType invalid", + null), Params( "ConfigSafetySourcesGroupNoId", R.raw.config_safety_sources_group_no_id, "Element safety-sources-group invalid", - "Required attribute id missing" - ), + "Required attribute id missing"), Params( "ConfigSafetySourcesGroupNoTitle", R.raw.config_safety_sources_group_no_title, "Element safety-sources-group invalid", - "Required attribute title missing" - ), + "Required attribute title missing"), Params( "ConfigStaticSafetySourceDuplicateKey", R.raw.config_static_safety_source_duplicate_key, "Element safety-sources-config invalid", - "Duplicate id id among safety sources" - ), + "Duplicate id id among safety sources"), Params( "ConfigStaticSafetySourceInvalidProfile", R.raw.config_static_safety_source_invalid_profile, - "Attribute static-safety-source.profile invalid", - null - ), + "Attribute value \"invalid\" in static-safety-source.profile invalid", + null), Params( "ConfigStaticSafetySourceNoId", R.raw.config_static_safety_source_no_id, "Element static-safety-source invalid", - "Required attribute id missing" - ), + "Required attribute id missing"), Params( "ConfigStaticSafetySourceNoIntent", R.raw.config_static_safety_source_no_intent, "Element static-safety-source invalid", - "Required attribute intentAction missing" - ), + "Required attribute intentAction missing"), Params( "ConfigStaticSafetySourceNoProfile", R.raw.config_static_safety_source_no_profile, "Element static-safety-source invalid", - "Required attribute profile missing" - ), + "Required attribute profile missing"), Params( "ConfigStaticSafetySourceNoTitle", R.raw.config_static_safety_source_no_title, "Element static-safety-source invalid", - "Required attribute title missing" - ), + "Required attribute title missing"), Params( "ConfigStaticSafetySourceWithDisplay", R.raw.config_static_safety_source_with_display, "Element static-safety-source invalid", - "Prohibited attribute initialDisplayState present" - ), + "Prohibited attribute initialDisplayState present"), Params( "ConfigStaticSafetySourceWithLogging", R.raw.config_static_safety_source_with_logging, "Element static-safety-source invalid", - "Prohibited attribute loggingAllowed present" - ), + "Prohibited attribute loggingAllowed present"), Params( "ConfigStaticSafetySourceWithPackage", R.raw.config_static_safety_source_with_package, "Element static-safety-source invalid", - "Prohibited attribute packageName present" - ), + "Prohibited attribute packageName present"), Params( "ConfigStaticSafetySourceWithPrimaryAndWork", R.raw.config_static_safety_source_with_primary_and_work, "Element static-safety-source invalid", - "Prohibited attribute titleForWork present" - ), + "Prohibited attribute titleForWork present"), Params( "ConfigStaticSafetySourceWithRefresh", R.raw.config_static_safety_source_with_refresh, "Element static-safety-source invalid", - "Prohibited attribute refreshOnPageOpenAllowed present" - ), + "Prohibited attribute refreshOnPageOpenAllowed present"), Params( "ConfigStaticSafetySourceWithSeverity", R.raw.config_static_safety_source_with_severity, "Element static-safety-source invalid", - "Prohibited attribute maxSeverityLevel present" - ), + "Prohibited attribute maxSeverityLevel present"), Params( "ConfigStringResourceNameInvalidEmpty", R.raw.config_string_resource_name_empty, "Resource name in safety-sources-group.title cannot be empty", - null - ), + null), Params( "ConfigStringResourceNameInvalidNoAt", R.raw.config_string_resource_name_invalid_no_at, - "Resource name com.android.safetycenter.config.tests:string/reference in " + + "Resource name \"com.android.safetycenter.config.tests:string/reference\" in " + "safety-sources-group.title does not start with @", - null - ), + null), Params( "ConfigStringResourceNameInvalidNoPackage", R.raw.config_string_resource_name_invalid_no_package, - "Resource name @string/reference in safety-sources-group.title does not " + + "Resource name \"@string/reference\" in safety-sources-group.title does not " + "specify a package", - null - ), + null), Params( "ConfigStringResourceNameInvalidNoType", R.raw.config_string_resource_name_invalid_no_type, - "Resource name @com.android.safetycenter.config.tests:reference in " + + "Resource name \"@com.android.safetycenter.config.tests:reference\" in " + "safety-sources-group.title does not specify a type", - null - ), + null), Params( "ConfigStringResourceNameInvalidWrongType", R.raw.config_string_resource_name_invalid_wrong_type, - "Resource name @com.android.safetycenter.config.tests:raw/" + - "config_string_resource_name_invalid_wrong_type in " + + "Resource name \"@com.android.safetycenter.config.tests:raw/" + + "config_string_resource_name_invalid_wrong_type\" in " + "safety-sources-group.title is not a string", - null - ), + null), Params( "ConfigStringResourceNameMissing", R.raw.config_string_resource_name_missing, - "Resource name @com.android.safetycenter.config.tests:string/missing in " + + "Resource name \"@com.android.safetycenter.config.tests:string/missing\" in " + "safety-sources-group.title missing or invalid", - null - ) - ) + null)) } } diff --git a/SafetyCenter/Config/tests/java/com/android/safetycenter/config/ParserConfigOverlayTest.kt b/SafetyCenter/Config/tests/java/com/android/safetycenter/config/ParserConfigOverlayTest.kt index 26dd3dbcf..a78d8e502 100644 --- a/SafetyCenter/Config/tests/java/com/android/safetycenter/config/ParserConfigOverlayTest.kt +++ b/SafetyCenter/Config/tests/java/com/android/safetycenter/config/ParserConfigOverlayTest.kt @@ -22,13 +22,13 @@ import androidx.test.core.app.ApplicationProvider.getApplicationContext import androidx.test.ext.junit.runners.AndroidJUnit4 import androidx.test.filters.SdkSuppress import com.android.compatibility.common.util.SystemUtil.runShellCommand +import com.android.safetycenter.config.Coroutines.waitForTestToPass import com.android.safetycenter.config.Coroutines.waitForWithTimeout import com.android.safetycenter.config.tests.R -import com.google.common.collect.Range import com.google.common.truth.Truth.assertThat -import org.junit.After +import org.junit.AfterClass import org.junit.Assert.assertThrows -import org.junit.Before +import org.junit.BeforeClass import org.junit.Test import org.junit.runner.RunWith @@ -37,21 +37,8 @@ import org.junit.runner.RunWith class ParserConfigOverlayTest { private val context: Context = getApplicationContext() - @Before - fun install() { - runShellCommand("pm install -r --force-sdk --force-queryable $OVERLAY_PATH") - waitForWithTimeout { getStateForOverlay(OVERLAY_PACKAGE) == STATE_DISABLED } - runShellCommand("cmd overlay enable --user 0 $OVERLAY_PACKAGE") - waitForWithTimeout { getStateForOverlay(OVERLAY_PACKAGE) == STATE_ENABLED } - } - - @After - fun uninstall() { - runShellCommand("pm uninstall $OVERLAY_PACKAGE") - } - @Test - fun validNotOverlayableConfig_matchesExpected() { + fun validNotOverlayableConfig_matchesExpected() = waitForTestToPass { val inputStream = context.resources.openRawResource(R.raw.config_valid_not_overlayable) val safetyCenterConfig = @@ -85,7 +72,7 @@ class ParserConfigOverlayTest { } @Test - fun validOverlayableConfig_matchesExpected() { + fun validOverlayableConfig_matchesExpected() = waitForTestToPass { val inputStream = context.resources.openRawResource(R.raw.config_valid_overlayable) val safetyCenterConfig = @@ -116,19 +103,20 @@ class ParserConfigOverlayTest { } @Test - fun invalidOverlayableConfig_StringResourceNameInvalid_throws() { - val inputStream = context.resources.openRawResource( - R.raw.config_string_resource_name_invalid_overlayable - ) + fun invalidOverlayableConfig_StringResourceNameInvalid_throws() = waitForTestToPass { + val inputStream = + context.resources.openRawResource(R.raw.config_string_resource_name_invalid_overlayable) - val thrown = assertThrows(ParseException::class.java) { - SafetyCenterConfigParser.parseXmlResource(inputStream, context.resources) - } + val thrown = + assertThrows(ParseException::class.java) { + SafetyCenterConfigParser.parseXmlResource(inputStream, context.resources) + } - assertThat(thrown).hasMessageThat().isEqualTo( - "Resource name @com.android.safetycenter.config.tests:string/reference_overlay in " + - "static-safety-source.summary missing or invalid" - ) + assertThat(thrown) + .hasMessageThat() + .isEqualTo( + "Resource name \"@com.android.safetycenter.config.tests:string/reference_overlay" + + "\" in static-safety-source.summary missing or invalid") } companion object { @@ -136,25 +124,29 @@ class ParserConfigOverlayTest { private const val OVERLAY_PACKAGE = "com.android.safetycenter.config.tests.overlay" private const val OVERLAY_PATH = "/data/local/tmp/com/safetycenter/config/tests/SafetyCenterConfigTestsOverlay.apk" - private const val OVERLAY_WAIT_TIMEOUT_MILLIS = 10000 private const val STATE_ENABLED = "STATE_ENABLED" - private const val STATE_DISABLED = "STATE_DISABLED" private fun getStateForOverlay(overlayPackage: String): String? { - val result: String = runShellCommand("cmd overlay dump") - val startIndex = result.indexOf("$overlayPackage:0") - if (startIndex < 0) { + val result: String = runShellCommand("cmd overlay dump --user 0 state $overlayPackage") + if (!result.startsWith("STATE_")) { return null } - val endIndex = result.indexOf('}', startIndex) - assertThat(endIndex).isGreaterThan(startIndex) - val stateIndex = result.indexOf("mState", startIndex) - assertThat(stateIndex).isIn(Range.open(startIndex, endIndex)) - val colonIndex = result.indexOf(':', stateIndex) - assertThat(colonIndex).isIn(Range.open(stateIndex, endIndex)) - val endLineIndex = result.indexOf('\n', colonIndex) - assertThat(endLineIndex).isIn(Range.open(colonIndex, endIndex)) - return result.substring(colonIndex + 2, endLineIndex) + return result.trim() + } + + @JvmStatic + @BeforeClass + fun install() { + runShellCommand("pm install -r --force-sdk --force-queryable $OVERLAY_PATH") + waitForWithTimeout { getStateForOverlay(OVERLAY_PACKAGE) != null } + runShellCommand("cmd overlay enable --user 0 $OVERLAY_PACKAGE") + waitForWithTimeout { getStateForOverlay(OVERLAY_PACKAGE) == STATE_ENABLED } + } + + @JvmStatic + @AfterClass + fun uninstall() { + runShellCommand("pm uninstall $OVERLAY_PACKAGE") } } } diff --git a/SafetyCenter/Config/tests/java/com/android/safetycenter/config/ParserConfigValidTest.kt b/SafetyCenter/Config/tests/java/com/android/safetycenter/config/ParserConfigValidTest.kt index 0effff7ba..dcb6d9cd2 100644 --- a/SafetyCenter/Config/tests/java/com/android/safetycenter/config/ParserConfigValidTest.kt +++ b/SafetyCenter/Config/tests/java/com/android/safetycenter/config/ParserConfigValidTest.kt @@ -76,6 +76,22 @@ class ParserConfigValidTest { ) .addSafetySource( SafetySource.Builder(SafetySource.SAFETY_SOURCE_TYPE_DYNAMIC) + .setId("dynamic_all_references") + .setPackageName("package") + .setTitleResId(R.string.reference) + .setTitleForWorkResId(R.string.reference) + .setSummaryResId(R.string.reference) + .setIntentAction("intent") + .setProfile(SafetySource.PROFILE_ALL) + .setInitialDisplayState(SafetySource.INITIAL_DISPLAY_STATE_DISABLED) + .setMaxSeverityLevel(300) + .setSearchTermsResId(R.string.reference) + .setLoggingAllowed(false) + .setRefreshOnPageOpenAllowed(true) + .build() + ) + .addSafetySource( + SafetySource.Builder(SafetySource.SAFETY_SOURCE_TYPE_DYNAMIC) .setId("dynamic_disabled") .setPackageName("package") .setTitleResId(R.string.reference) @@ -98,6 +114,8 @@ class ParserConfigValidTest { .setPackageName("package") .setTitleResId(R.string.reference) .setTitleForWorkResId(R.string.reference) + .setSummaryResId(R.string.reference) + .setIntentAction("intent") .setProfile(SafetySource.PROFILE_ALL) .setInitialDisplayState(SafetySource.INITIAL_DISPLAY_STATE_HIDDEN) .setSearchTermsResId(R.string.reference) diff --git a/SafetyCenter/Config/tests/res/raw/config_dynamic_safety_source_hidden_with_intent.xml b/SafetyCenter/Config/tests/res/raw/config_dynamic_safety_source_hidden_with_intent.xml deleted file mode 100644 index faee1c9e1..000000000 --- a/SafetyCenter/Config/tests/res/raw/config_dynamic_safety_source_hidden_with_intent.xml +++ /dev/null @@ -1,15 +0,0 @@ -<safety-center-config> - <safety-sources-config> - <safety-sources-group - id="id" - title="@com.android.safetycenter.config.tests:string/reference" - summary="@com.android.safetycenter.config.tests:string/reference"> - <dynamic-safety-source - id="id" - packageName="package" - intentAction="intent" - profile="primary_profile_only" - initialDisplayState="hidden"/> - </safety-sources-group> - </safety-sources-config> -</safety-center-config> diff --git a/SafetyCenter/Config/tests/res/raw/config_dynamic_safety_source_hidden_with_summary.xml b/SafetyCenter/Config/tests/res/raw/config_dynamic_safety_source_hidden_with_summary.xml deleted file mode 100644 index 9e9384350..000000000 --- a/SafetyCenter/Config/tests/res/raw/config_dynamic_safety_source_hidden_with_summary.xml +++ /dev/null @@ -1,15 +0,0 @@ -<safety-center-config> - <safety-sources-config> - <safety-sources-group - id="id" - title="@com.android.safetycenter.config.tests:string/reference" - summary="@com.android.safetycenter.config.tests:string/reference"> - <dynamic-safety-source - id="id" - packageName="package" - summary="@com.android.safetycenter.config.tests:string/reference" - profile="primary_profile_only" - initialDisplayState="hidden"/> - </safety-sources-group> - </safety-sources-config> -</safety-center-config> diff --git a/SafetyCenter/Config/tests/res/raw/config_dynamic_safety_source_hidden_with_title.xml b/SafetyCenter/Config/tests/res/raw/config_dynamic_safety_source_hidden_with_title.xml deleted file mode 100644 index a79f438f7..000000000 --- a/SafetyCenter/Config/tests/res/raw/config_dynamic_safety_source_hidden_with_title.xml +++ /dev/null @@ -1,15 +0,0 @@ -<safety-center-config> - <safety-sources-config> - <safety-sources-group - id="id" - title="@com.android.safetycenter.config.tests:string/reference" - summary="@com.android.safetycenter.config.tests:string/reference"> - <dynamic-safety-source - id="id" - packageName="package" - title="@com.android.safetycenter.config.tests:string/reference" - profile="primary_profile_only" - initialDisplayState="hidden"/> - </safety-sources-group> - </safety-sources-config> -</safety-center-config> diff --git a/SafetyCenter/Config/tests/res/raw/config_valid.xml b/SafetyCenter/Config/tests/res/raw/config_valid.xml index 03708b818..8a3ee0917 100644 --- a/SafetyCenter/Config/tests/res/raw/config_valid.xml +++ b/SafetyCenter/Config/tests/res/raw/config_valid.xml @@ -26,6 +26,19 @@ loggingAllowed="false" refreshOnPageOpenAllowed="true"/> <dynamic-safety-source + id="@com.android.safetycenter.config.tests:string/dynamic_all_references_id" + packageName="@com.android.safetycenter.config.tests:string/dynamic_all_references_package_name" + title="@com.android.safetycenter.config.tests:string/reference" + titleForWork="@com.android.safetycenter.config.tests:string/reference" + summary="@com.android.safetycenter.config.tests:string/reference" + intentAction="@com.android.safetycenter.config.tests:string/dynamic_all_references_intent_action" + profile="@com.android.safetycenter.config.tests:string/dynamic_all_references_profile" + initialDisplayState="@com.android.safetycenter.config.tests:string/dynamic_all_references_initial_display_state" + maxSeverityLevel="@com.android.safetycenter.config.tests:string/dynamic_all_references_max_severity_level" + searchTerms="@com.android.safetycenter.config.tests:string/reference" + loggingAllowed="@com.android.safetycenter.config.tests:string/dynamic_all_references_logging_allowed" + refreshOnPageOpenAllowed="@com.android.safetycenter.config.tests:string/dynamic_all_references_refresh_on_page_open_allowed"/> + <dynamic-safety-source id="dynamic_disabled" packageName="package" title="@com.android.safetycenter.config.tests:string/reference" @@ -42,6 +55,8 @@ packageName="package" title="@com.android.safetycenter.config.tests:string/reference" titleForWork="@com.android.safetycenter.config.tests:string/reference" + summary="@com.android.safetycenter.config.tests:string/reference" + intentAction="intent" profile="all_profiles" initialDisplayState="hidden" searchTerms="@com.android.safetycenter.config.tests:string/reference"/> diff --git a/SafetyCenter/Config/tests/res/values/strings.xml b/SafetyCenter/Config/tests/res/values/strings.xml index 0f2303648..195f56c2a 100644 --- a/SafetyCenter/Config/tests/res/values/strings.xml +++ b/SafetyCenter/Config/tests/res/values/strings.xml @@ -15,8 +15,15 @@ ~ limitations under the License. --> <resources xmlns:xliff="urn:oasis:names:tc:xliff:document:1.2"> - <!-- Test reference --> <string name="reference" translatable="false">Reference</string> <string name="reference_overlayable" translatable="false">Base</string> <string name="reference_not_overlayable" translatable="false">Base</string> + <string name="dynamic_all_references_id" translatable="false">dynamic_all_references</string> + <string name="dynamic_all_references_package_name" translatable="false">package</string> + <string name="dynamic_all_references_intent_action" translatable="false">intent</string> + <string name="dynamic_all_references_profile" translatable="false">all_profiles</string> + <string name="dynamic_all_references_initial_display_state" translatable="false">disabled</string> + <string name="dynamic_all_references_max_severity_level" translatable="false">300</string> + <string name="dynamic_all_references_logging_allowed" translatable="false">false</string> + <string name="dynamic_all_references_refresh_on_page_open_allowed" translatable="false">true</string> </resources> diff --git a/SafetyCenter/ConfigLintChecker/jarjar-rules.txt b/SafetyCenter/ConfigLintChecker/jarjar-rules.txt index c4a25fe33..46d2cfe4d 100644 --- a/SafetyCenter/ConfigLintChecker/jarjar-rules.txt +++ b/SafetyCenter/ConfigLintChecker/jarjar-rules.txt @@ -7,4 +7,5 @@ # reference by `LintJarVerifier`. To work around this, preserve the dynamically linked Android Lint # API references and rename any other `com.android` reference. rule com.android.tools.lint.** @0 +rule com.android.resources.ResourceFolderType @0 rule com.android.** android.safetycenter.lint.jarjar.@0 diff --git a/SafetyCenter/ConfigLintChecker/java/android/content/res/Resources.java b/SafetyCenter/ConfigLintChecker/java/android/content/res/Resources.java index 8dff0a4ae..dcf60b204 100644 --- a/SafetyCenter/ConfigLintChecker/java/android/content/res/Resources.java +++ b/SafetyCenter/ConfigLintChecker/java/android/content/res/Resources.java @@ -16,16 +16,49 @@ package android.content.res; +import java.util.Map; + /** Stub class to compile the linter for host execution. */ public final class Resources { /** Constant used in the Safety Center config parser. */ public static final int ID_NULL = 0; + private static final String STRING_TYPE = "string"; + + private final String mPackageName; + private final Map<String, Integer> mNameToIndex; + private final Map<Integer, String> mIndexToValue; + /** Class used in the Safety Center config parser. */ - public Resources() {} + public Resources( + String packageName, + Map<String, Integer> nameToIndex, + Map<Integer, String> indexToValue) { + mPackageName = packageName; + mNameToIndex = nameToIndex; + mIndexToValue = indexToValue; + } + + /** This exception is thrown by the resource APIs when a requested resource can not be found. */ + public static final class NotFoundException extends RuntimeException { + public NotFoundException() {} + } /** Method used in the Safety Center config parser. */ public int getIdentifier(String name, String defType, String defPackage) { - return ID_NULL + 1; + if (!mPackageName.equals(defPackage) + || !STRING_TYPE.equals(defType) + || !mNameToIndex.containsKey(name)) { + return ID_NULL; + } + return mNameToIndex.get(name); + } + + /** Method used in the Safety Center config parser. */ + public String getString(int id) { + if (mIndexToValue.containsKey(id)) { + return mIndexToValue.get(id); + } + throw new NotFoundException(); } } diff --git a/SafetyCenter/ConfigLintChecker/java/android/safetycenter/lint/ParserExceptionDetector.kt b/SafetyCenter/ConfigLintChecker/java/android/safetycenter/lint/ParserExceptionDetector.kt index f9cb62a8f..e512b7e54 100644 --- a/SafetyCenter/ConfigLintChecker/java/android/safetycenter/lint/ParserExceptionDetector.kt +++ b/SafetyCenter/ConfigLintChecker/java/android/safetycenter/lint/ParserExceptionDetector.kt @@ -17,6 +17,8 @@ package android.safetycenter.lint import android.content.res.Resources +import com.android.SdkConstants.ATTR_NAME +import com.android.SdkConstants.TAG_STRING import com.android.resources.ResourceFolderType import com.android.safetycenter.config.ParseException import com.android.safetycenter.config.SafetyCenterConfigParser @@ -29,43 +31,93 @@ import com.android.tools.lint.detector.api.Location import com.android.tools.lint.detector.api.OtherFileScanner import com.android.tools.lint.detector.api.Scope import com.android.tools.lint.detector.api.Severity +import com.android.tools.lint.detector.api.XmlContext +import com.android.tools.lint.detector.api.XmlScanner +import java.util.EnumSet +import org.w3c.dom.Element +import org.w3c.dom.Node /** Lint check for detecting invalid Safety Center configs */ -class ParserExceptionDetector : Detector(), OtherFileScanner { +class ParserExceptionDetector : Detector(), OtherFileScanner, XmlScanner { companion object { - val ISSUE = Issue.create( - id = "InvalidSafetyCenterConfig", - briefDescription = "The Safety Center config parser detected an error", - explanation = """The Safety Center config must follow all constraints defined in \ + val ISSUE = + Issue.create( + id = "InvalidSafetyCenterConfig", + briefDescription = "The Safety Center config parser detected an error", + explanation = + """The Safety Center config must follow all constraints defined in \ safety_center_config.xsd. Check the error message to find out the specific \ constraint not met by the current config.""", - category = Category.CORRECTNESS, - severity = Severity.ERROR, - implementation = Implementation( - ParserExceptionDetector::class.java, - Scope.OTHER_SCOPE - ), - androidSpecific = true - ) + category = Category.CORRECTNESS, + severity = Severity.ERROR, + implementation = + Implementation( + ParserExceptionDetector::class.java, + EnumSet.of(Scope.RESOURCE_FILE, Scope.OTHER)), + androidSpecific = true) + + val STRING_MAP_BUILD_PHASE = 1 + val CONFIG_PARSE_PHASE = 2 } override fun appliesTo(folderType: ResourceFolderType): Boolean { - return folderType == ResourceFolderType.RAW + return folderType == ResourceFolderType.RAW || folderType == ResourceFolderType.VALUES + } + + override fun afterCheckEachProject(context: Context) { + context.driver.requestRepeat(this, Scope.OTHER_SCOPE) + } + + /** Implements XmlScanner and builds a map of string resources in the first phase */ + val mNameToIndex: MutableMap<String, Int> = mutableMapOf() + val mIndexToValue: MutableMap<Int, String> = mutableMapOf() + var mIndex = 1000 + + override fun getApplicableElements(): Collection<String>? { + return listOf(TAG_STRING) + } + + override fun visitElement(context: XmlContext, element: Element) { + if (context.driver.phase != STRING_MAP_BUILD_PHASE || + context.resourceFolderType != ResourceFolderType.VALUES) { + return + } + val name = element.getAttribute(ATTR_NAME) + var value = "" + for (index in 0 until element.childNodes.length) { + val child = element.childNodes.item(index) + if (child.nodeType == Node.TEXT_NODE) { + value = child.nodeValue + break + } + } + mNameToIndex[name] = mIndex + mIndexToValue[mIndex] = value + mIndex++ } + /** Implements OtherFileScanner and parses the XML config in the second phase */ override fun run(context: Context) { - if (context.file.name != "safety_center_config.xml") { + if (context.driver.phase != CONFIG_PARSE_PHASE || + context.file.name != "safety_center_config.xml") { return } try { - SafetyCenterConfigParser.parseXmlResource(context.file.inputStream(), Resources()) + SafetyCenterConfigParser.parseXmlResource( + context.file.inputStream(), + // Note: using a map of the string resources present in the APK under analysis is + // necessary in order to get the value of string resources that are resolved and + // validated at parse time. The drawback of this is that the linter cannot be used + // on overlay packages that refer to resources in the target package or on packages + // that refer to Android global resources. However, we cannot use custom a linter + // with the default soong overlay build rule regardless. + Resources(context.project.`package`, mNameToIndex, mIndexToValue)) } catch (e: ParseException) { context.report( ISSUE, Location.create(context.file), - "Parser exception: \"${e.message}\", cause: \"${e.cause?.message}\"" - ) + "Parser exception: \"${e.message}\", cause: \"${e.cause?.message}\"") } } -}
\ No newline at end of file +} diff --git a/SafetyCenter/ConfigLintChecker/tests/java/android/safetycenter/lint/test/ParserExceptionDetectorTest.kt b/SafetyCenter/ConfigLintChecker/tests/java/android/safetycenter/lint/test/ParserExceptionDetectorTest.kt index 8c859ae1d..ad7d36685 100644 --- a/SafetyCenter/ConfigLintChecker/tests/java/android/safetycenter/lint/test/ParserExceptionDetectorTest.kt +++ b/SafetyCenter/ConfigLintChecker/tests/java/android/safetycenter/lint/test/ParserExceptionDetectorTest.kt @@ -36,44 +36,59 @@ class ParserExceptionDetectorTest : LintDetectorTest() { @Test fun validConfig_doesNotThrow() { - lint().files(( - xml("res/raw/safety_center_config.xml", - """ + lint() + .files( + (xml( + "res/raw/safety_center_config.xml", + """ <safety-center-config> <safety-sources-config> <safety-sources-group id="group" - title="@package:string/reference" - summary="@package:string/reference"> + title="@lint.test.pkg:string/reference" + summary="@lint.test.pkg:string/reference"> <static-safety-source id="source" - title="@package:string/reference" - summary="@package:string/reference" + title="@lint.test.pkg:string/reference" + summary="@lint.test.pkg:string/reference" intentAction="intent" profile="primary_profile_only"/> </safety-sources-group> </safety-sources-config> </safety-center-config> - """))).run().expectClean() + """)), + (xml( + "res/values/strings.xml", + """ +<resources xmlns:xliff="urn:oasis:names:tc:xliff:document:1.2"> + <string name="reference" translatable="false">Reference</string> +</resources> + """))) + .run() + .expectClean() } @Test fun invalidConfig_throws() { - lint().files((xml("res/raw/safety_center_config.xml", "<invalid-root/>"))) - .run().expect("res/raw/safety_center_config.xml: Error: Parser exception: " + - "\"Element safety-center-config missing\", cause: \"null\" " + - "[InvalidSafetyCenterConfig]\n1 errors, 0 warnings") + lint() + .files((xml("res/raw/safety_center_config.xml", "<invalid-root/>"))) + .run() + .expect( + "res/raw/safety_center_config.xml: Error: Parser exception: " + + "\"Element safety-center-config missing\", cause: \"null\" " + + "[InvalidSafetyCenterConfig]\n1 errors, 0 warnings") } @Test fun unrelatedFile_doesNotThrow() { - lint().files((xml("res/raw/some_other_config.xml", "<some-other-root/>"))) - .run().expectClean() + lint() + .files((xml("res/raw/some_other_config.xml", "<some-other-root/>"))) + .run() + .expectClean() } @Test fun unrelatedFolder_doesNotThrow() { - lint().files((xml("res/values/strings.xml", "<some-other-root/>"))) - .run().expectClean() + lint().files((xml("res/values/strings.xml", "<some-other-root/>"))).run().expectClean() } } diff --git a/framework-s/java/android/safetycenter/config/SafetySource.java b/framework-s/java/android/safetycenter/config/SafetySource.java index 7caa5439b..ab4a8929b 100644 --- a/framework-s/java/android/safetycenter/config/SafetySource.java +++ b/framework-s/java/android/safetycenter/config/SafetySource.java @@ -588,36 +588,33 @@ public final class SafetySource implements Parcelable { isIssueOnly); boolean isDynamicHiddenWithSearch = isDynamic && isHidden && searchTermsResId != Resources.ID_NULL; - boolean isDynamicHiddenWithoutSearch = - isDynamic && isHidden && searchTermsResId == Resources.ID_NULL; boolean titleRequired = isDynamicNotHidden || isDynamicHiddenWithSearch || isStatic; - boolean titleProhibited = isIssueOnly || isDynamicHiddenWithoutSearch; int titleResId = BuilderUtils.validateResId( mTitleResId, "title", titleRequired, - titleProhibited); + isIssueOnly); int titleForWorkResId = BuilderUtils.validateResId( mTitleForWorkResId, "titleForWork", hasWork && titleRequired, - !hasWork || titleProhibited); + !hasWork || isIssueOnly); int summaryResId = BuilderUtils.validateResId( mSummaryResId, "summary", isDynamicNotHidden, - isIssueOnly || isHidden); + isIssueOnly); BuilderUtils.validateAttribute( mIntentAction, "intentAction", (isDynamic && isEnabled) || isStatic, - isIssueOnly || isHidden); + isIssueOnly); int maxSeverityLevel = BuilderUtils.validateInteger( diff --git a/framework-s/java/android/safetycenter/config/TEST_MAPPING b/framework-s/java/android/safetycenter/config/TEST_MAPPING new file mode 100644 index 000000000..a39176e27 --- /dev/null +++ b/framework-s/java/android/safetycenter/config/TEST_MAPPING @@ -0,0 +1,7 @@ +{ + "presubmit": [ + { + "name": "SafetyCenterConfigTests" + } + ] +} diff --git a/framework-s/java/android/safetycenter/config/safety_center_config.xsd b/framework-s/java/android/safetycenter/config/safety_center_config.xsd index 1147b16e6..8549df80f 100644 --- a/framework-s/java/android/safetycenter/config/safety_center_config.xsd +++ b/framework-s/java/android/safetycenter/config/safety_center_config.xsd @@ -41,61 +41,85 @@ <xsd:element name="issue-only-safety-source" type="issue-only-safety-source"/> </xsd:choice> <!-- id must be unique among safety sources groups --> - <xsd:attribute name="id" type="xsd:string" use="required"/> + <xsd:attribute name="id" type="stringOrStringResourceName" use="required"/> <!-- title is required unless the group contains issue only and/or internal sources --> - <xsd:attribute name="title" type="stringResourceName"/> - <xsd:attribute name="summary" type="stringResourceName"/> - <xsd:attribute name="statelessIconType" type="statelessIconType" default="none"/> + <xsd:attribute name="title" type="runtimeStringResourceName"/> + <xsd:attribute name="summary" type="runtimeStringResourceName"/> + <xsd:attribute name="statelessIconType" type="statelessIconTypeOrStringResourceName" default="none"/> </xsd:complexType> <xsd:complexType name="dynamic-safety-source"> <!-- id must be unique among safety sources --> - <xsd:attribute name="id" type="xsd:string" use="required"/> - <xsd:attribute name="packageName" type="xsd:string" use="required"/> + <xsd:attribute name="id" type="stringOrStringResourceName" use="required"/> + <xsd:attribute name="packageName" type="stringOrStringResourceName" use="required"/> <!-- title is required if initialDisplayState is not set to hidden or if searchTerms are provided --> - <!-- title is prohibited if initialDisplayState is set to hidden and if searchTerms are not provided --> - <xsd:attribute name="title" type="stringResourceName"/> + <xsd:attribute name="title" type="runtimeStringResourceName"/> <!-- titleForWork is required if profile is set to all_profiles, and initialDisplayState is not set to hidden or if searchTerms are provided --> - <!-- titleForWork is prohibited if profile is set to primary_profile_only, or initialDisplayState is set to hidden and if searchTerms are not provided --> - <xsd:attribute name="titleForWork" type="stringResourceName"/> + <!-- titleForWork is prohibited if profile is set to primary_profile_only --> + <xsd:attribute name="titleForWork" type="runtimeStringResourceName"/> <!-- summary is required if initialDisplayState is not set to hidden --> - <!-- summary is prohibited if initialDisplayState is set to hidden --> - <xsd:attribute name="summary" type="stringResourceName"/> + <xsd:attribute name="summary" type="runtimeStringResourceName"/> <!-- intentAction is required if initialDisplayState is set to enabled --> - <!-- intentAction is optional if initialDisplayState is set to disabled --> - <!-- intentAction is prohibited if initialDisplayState is set to hidden --> - <xsd:attribute name="intentAction" type="xsd:string"/> + <xsd:attribute name="intentAction" type="stringOrStringResourceName"/> <xsd:attribute name="profile" type="profile" use="required"/> - <xsd:attribute name="initialDisplayState" type="initialDisplayState" default="enabled"/> - <xsd:attribute name="maxSeverityLevel" type="xsd:int" default="2147483647"/> - <xsd:attribute name="searchTerms" type="stringResourceName"/> - <xsd:attribute name="loggingAllowed" type="xsd:boolean" default="true"/> - <xsd:attribute name="refreshOnPageOpenAllowed" type="xsd:boolean" default="false"/> + <xsd:attribute name="initialDisplayState" type="initialDisplayStateOrStringResourceName" default="enabled"/> + <xsd:attribute name="maxSeverityLevel" type="intOrStringResourceName" default="2147483647"/> + <xsd:attribute name="searchTerms" type="runtimeStringResourceName"/> + <xsd:attribute name="loggingAllowed" type="booleanOrStringResourceName" default="true"/> + <xsd:attribute name="refreshOnPageOpenAllowed" type="booleanOrStringResourceName" default="false"/> </xsd:complexType> <xsd:complexType name="issue-only-safety-source"> <!-- id must be unique among safety sources --> - <xsd:attribute name="id" type="xsd:string" use="required"/> - <xsd:attribute name="packageName" type="xsd:string" use="required"/> - <xsd:attribute name="profile" type="profile" use="required"/> - <xsd:attribute name="maxSeverityLevel" type="xsd:int" default="2147483647"/> - <xsd:attribute name="loggingAllowed" type="xsd:boolean" default="true"/> - <xsd:attribute name="refreshOnPageOpenAllowed" type="xsd:boolean" default="false"/> + <xsd:attribute name="id" type="stringOrStringResourceName" use="required"/> + <xsd:attribute name="packageName" type="stringOrStringResourceName" use="required"/> + <xsd:attribute name="profile" type="profileOrStringResourceName" use="required"/> + <xsd:attribute name="maxSeverityLevel" type="intOrStringResourceName" default="2147483647"/> + <xsd:attribute name="loggingAllowed" type="booleanOrStringResourceName" default="true"/> + <xsd:attribute name="refreshOnPageOpenAllowed" type="booleanOrStringResourceName" default="false"/> </xsd:complexType> <xsd:complexType name="static-safety-source"> <!-- id must be unique among safety sources --> - <xsd:attribute name="id" type="xsd:string" use="required"/> - <xsd:attribute name="title" type="stringResourceName" use="required"/> + <xsd:attribute name="id" type="stringOrStringResourceName" use="required"/> + <xsd:attribute name="title" type="runtimeStringResourceName" use="required"/> <!-- titleForWork is required if profile is set to all_profiles --> <!-- titleForWork is prohibited if profile is set to primary_profile_only --> - <xsd:attribute name="titleForWork" type="stringResourceName"/> - <xsd:attribute name="summary" type="stringResourceName"/> - <xsd:attribute name="intentAction" type="xsd:string" use="required"/> - <xsd:attribute name="profile" type="profile" use="required"/> - <xsd:attribute name="searchTerms" type="stringResourceName"/> + <xsd:attribute name="titleForWork" type="runtimeStringResourceName"/> + <xsd:attribute name="summary" type="runtimeStringResourceName"/> + <xsd:attribute name="intentAction" type="stringOrStringResourceName" use="required"/> + <xsd:attribute name="profile" type="profileOrStringResourceName" use="required"/> + <xsd:attribute name="searchTerms" type="runtimeStringResourceName"/> </xsd:complexType> + <xsd:simpleType name="intOrStringResourceName"> + <!-- String resource names will be resolved only once at parse time. --> + <!-- Locale changes and device config changes will be ignored. --> + <!-- The value of the string resource must be of type xsd:int. --> + <xsd:union memberTypes="stringResourceName xsd:int" /> + </xsd:simpleType> + + <xsd:simpleType name="booleanOrStringResourceName"> + <!-- String resource names will be resolved only once at parse time. --> + <!-- Locale changes and device config changes will be ignored. --> + <!-- The value of the string resource must be of type xsd:boolean. --> + <xsd:union memberTypes="stringResourceName xsd:boolean" /> + </xsd:simpleType> + + <xsd:simpleType name="stringOrStringResourceName"> + <!-- String resource names will be resolved only once at parse time. --> + <!-- Locale changes and device config changes will be ignored. --> + <!-- The value of the string resource must be of type xsd:string. --> + <xsd:union memberTypes="stringResourceName xsd:string" /> + </xsd:simpleType> + + <xsd:simpleType name="statelessIconTypeOrStringResourceName"> + <!-- String resource names will be resolved only once at parse time. --> + <!-- Locale changes and device config changes will be ignored. --> + <!-- The value of the string resource must be of type statelessIconType. --> + <xsd:union memberTypes="stringResourceName statelessIconType" /> + </xsd:simpleType> + <xsd:simpleType name="statelessIconType"> <xsd:restriction base="xsd:string"> <xsd:enumeration value="none"/> @@ -103,6 +127,13 @@ </xsd:restriction> </xsd:simpleType> + <xsd:simpleType name="profileOrStringResourceName"> + <!-- String resource names will be resolved only once at parse time. --> + <!-- Locale changes and device config changes will be ignored. --> + <!-- The value of the string resource must be of type profile. --> + <xsd:union memberTypes="stringResourceName profile" /> + </xsd:simpleType> + <xsd:simpleType name="profile"> <xsd:restriction base="xsd:string"> <xsd:enumeration value="primary_profile_only"/> @@ -110,6 +141,13 @@ </xsd:restriction> </xsd:simpleType> + <xsd:simpleType name="initialDisplayStateOrStringResourceName"> + <!-- String resource names will be resolved only once at parse time. --> + <!-- Locale changes and device config changes will be ignored. --> + <!-- The value of the string resource must be of type initialDisplayState. --> + <xsd:union memberTypes="stringResourceName initialDisplayState" /> + </xsd:simpleType> + <xsd:simpleType name="initialDisplayState"> <xsd:restriction base="xsd:string"> <xsd:enumeration value="enabled"/> @@ -118,8 +156,13 @@ </xsd:restriction> </xsd:simpleType> - <!-- NOTE: stringResourceNames will be ignored for any attribute not explicitly marked as stringResourceName in this schema. --> - <!-- A stringResourceNames is a fully qualified resource name of the form "@package:string/entry". Package is required. --> + <xsd:simpleType name="runtimeStringResourceName"> + <!-- String resource names will be resolved at runtime whenever the string value is used. --> + <xsd:union memberTypes="stringResourceName" /> + </xsd:simpleType> + + <!-- String resource names will be ignored for any attribute not directly or indirectly marked as stringResourceName. --> + <!-- A stringResourceName is a fully qualified resource name of the form "@package:string/entry". Package is required. --> <xsd:simpleType name="stringResourceName"> <xsd:restriction base="xsd:string"> <xsd:pattern value="@([a-z]+\.)*[a-z]+:string/.+"/> diff --git a/tests/cts/safetycenter/src/android/safetycenter/cts/config/SafetySourceTest.kt b/tests/cts/safetycenter/src/android/safetycenter/cts/config/SafetySourceTest.kt index 9f33c78ae..3c9ebe9e0 100644 --- a/tests/cts/safetycenter/src/android/safetycenter/cts/config/SafetySourceTest.kt +++ b/tests/cts/safetycenter/src/android/safetycenter/cts/config/SafetySourceTest.kt @@ -114,7 +114,7 @@ class SafetySourceTest { assertThat(DYNAMIC_ALL_OPTIONAL.summaryResId).isEqualTo(REFERENCE_RES_ID) assertThat(DYNAMIC_DISABLED.summaryResId).isEqualTo(REFERENCE_RES_ID) assertThat(DYNAMIC_HIDDEN.summaryResId).isEqualTo(Resources.ID_NULL) - assertThat(DYNAMIC_HIDDEN_WITH_SEARCH.summaryResId).isEqualTo(Resources.ID_NULL) + assertThat(DYNAMIC_HIDDEN_WITH_SEARCH.summaryResId).isEqualTo(REFERENCE_RES_ID) assertThat(STATIC_BAREBONE.summaryResId).isEqualTo(Resources.ID_NULL) assertThat(STATIC_ALL_OPTIONAL.summaryResId).isEqualTo(REFERENCE_RES_ID) assertThrows(UnsupportedOperationException::class.java) { ISSUE_ONLY_BAREBONE.summaryResId } @@ -128,7 +128,7 @@ class SafetySourceTest { assertThat(DYNAMIC_ALL_OPTIONAL.intentAction).isEqualTo(INTENT_ACTION) assertThat(DYNAMIC_DISABLED.intentAction).isNull() assertThat(DYNAMIC_HIDDEN.intentAction).isNull() - assertThat(DYNAMIC_HIDDEN_WITH_SEARCH.intentAction).isNull() + assertThat(DYNAMIC_HIDDEN_WITH_SEARCH.intentAction).isEqualTo(INTENT_ACTION) assertThat(STATIC_BAREBONE.intentAction).isEqualTo(INTENT_ACTION) assertThat(STATIC_ALL_OPTIONAL.intentAction).isEqualTo(INTENT_ACTION) assertThrows(UnsupportedOperationException::class.java) { ISSUE_ONLY_BAREBONE.intentAction } @@ -542,6 +542,8 @@ class SafetySourceTest { .setPackageName(PACKAGE_NAME) .setTitleResId(REFERENCE_RES_ID) .setTitleForWorkResId(REFERENCE_RES_ID) + .setSummaryResId(REFERENCE_RES_ID) + .setIntentAction(INTENT_ACTION) .setProfile(SafetySource.PROFILE_ALL) .setInitialDisplayState(SafetySource.INITIAL_DISPLAY_STATE_HIDDEN) .setSearchTermsResId(REFERENCE_RES_ID) |