From e39fa0fd2fddf0f1a9688d136a3b074e8d77af89 Mon Sep 17 00:00:00 2001 From: Khaled Abdelmohsen Date: Mon, 25 Nov 2019 17:24:22 +0000 Subject: [PATCH] Support optional rule attributes in XML parser Bug: 143697198 Test: atest FrameworksServicesTests:RuleXmlParserTest Change-Id: Ib7118228f325f8fcb88fa27cf88d48af0319e320 --- .../integrity/parser/RuleXmlParser.java | 23 ++++--- .../integrity/parser/RuleXmlParserTest.java | 60 +++++++++++++------ 2 files changed, 53 insertions(+), 30 deletions(-) diff --git a/services/core/java/com/android/server/integrity/parser/RuleXmlParser.java b/services/core/java/com/android/server/integrity/parser/RuleXmlParser.java index 11a397948b145..5459a77705f2e 100644 --- a/services/core/java/com/android/server/integrity/parser/RuleXmlParser.java +++ b/services/core/java/com/android/server/integrity/parser/RuleXmlParser.java @@ -32,6 +32,7 @@ import java.io.StringReader; import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.List; +import java.util.Optional; /** * A helper class to parse rules into the {@link Rule} model from Xml representation. @@ -106,7 +107,7 @@ public final class RuleXmlParser implements RuleParser { private static Rule parseRule(XmlPullParser parser) throws IOException, XmlPullParserException { Formula formula = null; - @Rule.Effect int effect = Integer.parseInt(extractAttributeValue(parser, EFFECT_ATTRIBUTE)); + int effect = Integer.parseInt(extractAttributeValue(parser, EFFECT_ATTRIBUTE).orElse("-1")); int eventType; while ((eventType = parser.next()) != XmlPullParser.END_DOCUMENT) { @@ -139,8 +140,8 @@ public final class RuleXmlParser implements RuleParser { private static Formula parseOpenFormula(XmlPullParser parser) throws IOException, XmlPullParserException { - @OpenFormula.Connector int connector = Integer.parseInt( - extractAttributeValue(parser, CONNECTOR_ATTRIBUTE)); + int connector = Integer.parseInt( + extractAttributeValue(parser, CONNECTOR_ATTRIBUTE).orElse("-1")); List formulas = new ArrayList<>(); int eventType; @@ -174,10 +175,10 @@ public final class RuleXmlParser implements RuleParser { private static Formula parseAtomicFormula(XmlPullParser parser) throws IOException, XmlPullParserException { - @AtomicFormula.Key int key = Integer.parseInt(extractAttributeValue(parser, KEY_ATTRIBUTE)); - @AtomicFormula.Operator int operator = Integer.parseInt( - extractAttributeValue(parser, OPERATOR_ATTRIBUTE)); - String value = extractAttributeValue(parser, VALUE_ATTRIBUTE); + int key = Integer.parseInt(extractAttributeValue(parser, KEY_ATTRIBUTE).orElse("-1")); + int operator = Integer.parseInt( + extractAttributeValue(parser, OPERATOR_ATTRIBUTE).orElse("-1")); + String value = extractAttributeValue(parser, VALUE_ATTRIBUTE).orElse(null); int eventType; while ((eventType = parser.next()) != XmlPullParser.END_DOCUMENT) { @@ -205,11 +206,7 @@ public final class RuleXmlParser implements RuleParser { } } - private static String extractAttributeValue(XmlPullParser parser, String attribute) { - String attributeValue = parser.getAttributeValue(NAMESPACE, attribute); - if (attributeValue == null) { - throw new RuntimeException(String.format("Attribute not found: %s", attribute)); - } - return attributeValue; + private static Optional extractAttributeValue(XmlPullParser parser, String attribute) { + return Optional.ofNullable(parser.getAttributeValue(NAMESPACE, attribute)); } } diff --git a/services/tests/servicestests/src/com/android/server/integrity/parser/RuleXmlParserTest.java b/services/tests/servicestests/src/com/android/server/integrity/parser/RuleXmlParserTest.java index be6a31fa949eb..eca1b8556496a 100644 --- a/services/tests/servicestests/src/com/android/server/integrity/parser/RuleXmlParserTest.java +++ b/services/tests/servicestests/src/com/android/server/integrity/parser/RuleXmlParserTest.java @@ -43,7 +43,6 @@ public class RuleXmlParserTest { public void testXmlStream_validOpenFormula() throws Exception { Map atomicFormulaAttrs = new HashMap<>(); atomicFormulaAttrs.put("K", String.valueOf(AtomicFormula.PACKAGE_NAME)); - atomicFormulaAttrs.put("O", String.valueOf(AtomicFormula.EQ)); atomicFormulaAttrs.put("V", "com.app.test"); String ruleXmlOpenFormula = "" + generateTagWithAttribute(/* tag= */ "R", @@ -71,7 +70,6 @@ public class RuleXmlParserTest { public void testXmlString_validOpenFormula_notConnector() throws Exception { Map packageNameAttrs = new HashMap<>(); packageNameAttrs.put("K", String.valueOf(AtomicFormula.PACKAGE_NAME)); - packageNameAttrs.put("O", String.valueOf(AtomicFormula.EQ)); packageNameAttrs.put("V", "com.app.test"); String ruleXmlOpenFormula = "" + generateTagWithAttribute(/* tag= */ "R", @@ -98,11 +96,9 @@ public class RuleXmlParserTest { public void testXmlString_validOpenFormula_andConnector() throws Exception { Map packageNameAttrs = new HashMap<>(); packageNameAttrs.put("K", String.valueOf(AtomicFormula.PACKAGE_NAME)); - packageNameAttrs.put("O", String.valueOf(AtomicFormula.EQ)); packageNameAttrs.put("V", "com.app.test"); Map appCertificateAttrs = new HashMap<>(); appCertificateAttrs.put("K", String.valueOf(AtomicFormula.APP_CERTIFICATE)); - appCertificateAttrs.put("O", String.valueOf(AtomicFormula.EQ)); appCertificateAttrs.put("V", "test_cert"); String ruleXmlOpenFormula = "" + generateTagWithAttribute(/* tag= */ "R", @@ -131,11 +127,9 @@ public class RuleXmlParserTest { public void testXmlString_validOpenFormula_orConnector() throws Exception { Map packageNameAttrs = new HashMap<>(); packageNameAttrs.put("K", String.valueOf(AtomicFormula.PACKAGE_NAME)); - packageNameAttrs.put("O", String.valueOf(AtomicFormula.EQ)); packageNameAttrs.put("V", "com.app.test"); Map appCertificateAttrs = new HashMap<>(); appCertificateAttrs.put("K", String.valueOf(AtomicFormula.APP_CERTIFICATE)); - appCertificateAttrs.put("O", String.valueOf(AtomicFormula.EQ)); appCertificateAttrs.put("V", "test_cert"); String ruleXmlOpenFormula = "" + generateTagWithAttribute(/* tag= */ "R", @@ -165,7 +159,6 @@ public class RuleXmlParserTest { Map packageNameAttrs = new HashMap<>(); packageNameAttrs.put("K", String.valueOf(AtomicFormula.PACKAGE_NAME)); packageNameAttrs.put("V", "com.app.test"); - packageNameAttrs.put("O", String.valueOf(AtomicFormula.EQ)); String ruleXmlOpenFormula = "" + generateTagWithAttribute(/* tag= */ "R", Collections.singletonMap("E", String.valueOf(Rule.DENY)), @@ -191,7 +184,6 @@ public class RuleXmlParserTest { public void testXmlString_invalidOpenFormula_invalidNumberOfFormulas() throws Exception { Map packageNameAttrs = new HashMap<>(); packageNameAttrs.put("K", String.valueOf(AtomicFormula.PACKAGE_NAME)); - packageNameAttrs.put("O", String.valueOf(AtomicFormula.EQ)); packageNameAttrs.put("V", "com.app.test"); Map versionCodeAttrs = new HashMap<>(); versionCodeAttrs.put("K", String.valueOf(AtomicFormula.VERSION_CODE)); @@ -246,7 +238,6 @@ public class RuleXmlParserTest { public void testXmlString_invalidOpenFormula_invalidEffect() throws Exception { Map packageNameAttrs = new HashMap<>(); packageNameAttrs.put("K", String.valueOf(AtomicFormula.PACKAGE_NAME)); - packageNameAttrs.put("O", String.valueOf(AtomicFormula.EQ)); packageNameAttrs.put("V", "com.app.test"); String ruleXmlOpenFormula = "" + generateTagWithAttribute(/* tag= */ "R", @@ -271,7 +262,6 @@ public class RuleXmlParserTest { public void testXmlString_invalidOpenFormula_invalidTags() throws Exception { Map packageNameAttrs = new HashMap<>(); packageNameAttrs.put("K", String.valueOf(AtomicFormula.PACKAGE_NAME)); - packageNameAttrs.put("O", String.valueOf(AtomicFormula.EQ)); packageNameAttrs.put("V", "com.app.test"); String ruleXmlOpenFormula = "" + generateTagWithAttribute(/* tag= */ "R", @@ -297,7 +287,6 @@ public class RuleXmlParserTest { public void testXmlString_validAtomicFormula_stringValue() throws Exception { Map packageNameAttrs = new HashMap<>(); packageNameAttrs.put("K", String.valueOf(AtomicFormula.PACKAGE_NAME)); - packageNameAttrs.put("O", String.valueOf(AtomicFormula.EQ)); packageNameAttrs.put("V", "com.app.test"); String ruleXmlAtomicFormula = "" + generateTagWithAttribute(/* tag= */ "R", @@ -343,7 +332,6 @@ public class RuleXmlParserTest { public void testXmlString_validAtomicFormula_booleanValue() throws Exception { Map preInstalledAttrs = new HashMap<>(); preInstalledAttrs.put("K", String.valueOf(AtomicFormula.PRE_INSTALLED)); - preInstalledAttrs.put("O", String.valueOf(AtomicFormula.EQ)); preInstalledAttrs.put("V", "true"); String ruleXmlAtomicFormula = "" + generateTagWithAttribute(/* tag= */ "R", @@ -367,7 +355,6 @@ public class RuleXmlParserTest { Map packageNameAttrs = new HashMap<>(); packageNameAttrs.put("K", String.valueOf(AtomicFormula.PACKAGE_NAME)); packageNameAttrs.put("V", "com.app.test"); - packageNameAttrs.put("O", String.valueOf(AtomicFormula.EQ)); String ruleXmlAtomicFormula = "" + generateTagWithAttribute(/* tag= */ "R", Collections.singletonMap("E", String.valueOf(Rule.DENY)), @@ -389,7 +376,6 @@ public class RuleXmlParserTest { public void testXmlString_invalidAtomicFormula_invalidAttribute() throws Exception { Map packageNameAttrs = new HashMap<>(); packageNameAttrs.put("BadKey", String.valueOf(AtomicFormula.PACKAGE_NAME)); - packageNameAttrs.put("O", String.valueOf(AtomicFormula.EQ)); packageNameAttrs.put("V", "com.app.test"); String ruleXmlAtomicFormula = "" + generateTagWithAttribute(/* tag= */ "R", @@ -402,10 +388,52 @@ public class RuleXmlParserTest { assertExpectException( RuleParseException.class, - /* expectedExceptionMessageRegex */ "Attribute not found: K", + /* expectedExceptionMessageRegex */ "Found unexpected key: -1", () -> xmlParser.parse(ruleXmlAtomicFormula)); } + @Test + public void testXmlString_invalidRule_invalidAttribute() throws Exception { + Map packageNameAttrs = new HashMap<>(); + packageNameAttrs.put("K", String.valueOf(AtomicFormula.PACKAGE_NAME)); + packageNameAttrs.put("V", "com.app.test"); + String ruleXmlAtomicFormula = "" + + generateTagWithAttribute(/* tag= */ "R", + Collections.singletonMap("BadEffect", String.valueOf(Rule.DENY)), + /* closed= */ false) + + generateTagWithAttribute(/* tag= */ "AF", packageNameAttrs, /* closed= */ true) + + "" + + ""; + RuleParser xmlParser = new RuleXmlParser(); + assertExpectException( + RuleParseException.class, + /* expectedExceptionMessageRegex */ "Unknown effect: -1", + () -> xmlParser.parse(ruleXmlAtomicFormula)); + } + + @Test + public void testXmlString_invalidOpenFormula_invalidAttribute() throws Exception { + Map packageNameAttrs = new HashMap<>(); + packageNameAttrs.put("K", String.valueOf(AtomicFormula.PACKAGE_NAME)); + packageNameAttrs.put("V", "com.app.test"); + String ruleXmlOpenFormula = "" + + generateTagWithAttribute(/* tag= */ "R", + Collections.singletonMap("E", String.valueOf(Rule.DENY)), + /* closed= */ false) + + generateTagWithAttribute(/* tag= */ "OF", + Collections.singletonMap("BadConnector", String.valueOf(OpenFormula.NOT)), + /* closed= */ false) + + generateTagWithAttribute(/* tag= */ "AF", packageNameAttrs, /* closed= */ true) + + "" + + "" + + ""; + RuleParser xmlParser = new RuleXmlParser(); + assertExpectException( + RuleParseException.class, + /* expectedExceptionMessageRegex */ "Unknown connector: -1", + () -> xmlParser.parse(ruleXmlOpenFormula)); + } + @Test public void testXmlString_invalidAtomicFormula() throws Exception { Map packageNameAttrs = new HashMap<>(); @@ -431,7 +459,6 @@ public class RuleXmlParserTest { public void testXmlString_withNoRuleList() { Map atomicFormulaAttrs = new HashMap<>(); atomicFormulaAttrs.put("K", String.valueOf(AtomicFormula.PACKAGE_NAME)); - atomicFormulaAttrs.put("O", String.valueOf(AtomicFormula.EQ)); atomicFormulaAttrs.put("V", "com.app.test"); String ruleXmlWithNoRuleList = generateTagWithAttribute(/* tag= */ "R", Collections.singletonMap("E", String.valueOf(Rule.DENY)), @@ -454,7 +481,6 @@ public class RuleXmlParserTest { public void testXmlStream_withNoRuleList() { Map atomicFormulaAttrs = new HashMap<>(); atomicFormulaAttrs.put("K", String.valueOf(AtomicFormula.PACKAGE_NAME)); - atomicFormulaAttrs.put("O", String.valueOf(AtomicFormula.EQ)); atomicFormulaAttrs.put("V", "com.app.test"); String ruleXmlWithNoRuleList = generateTagWithAttribute(/* tag= */ "R", Collections.singletonMap("E", String.valueOf(Rule.DENY)),