ctl.<action>$<service_name> is a valid prop name am: 87d439d671 am: 3e932f216d

am: 834f379c44

Change-Id: I5d30c804e481fdbfeb198ef9c5b34eceb33bb6fc

3 files changed, 39 insertions, 11 deletions

diff --git a/Common.cpp b/Common.cpp
index 47f6e23..b6bb1b2 100644
--- a/Common.cpp
+++ b/Common.cpp
@@ -84,19 +84,32 @@ bool IsCorrectIdentifier(const std::string& name) {
   });
 }
 
-bool IsCorrectPropertyOrApiName(const std::string& name) {
+bool IsCorrectName(const std::string& name,
+                   const std::unordered_set<char>& allowed_chars) {
   if (name.empty()) return false;
+  if (!std::isalpha(*name.begin())) return false;
 
-  static std::unordered_set<char> allowed{'_', '-', '.'};
-
-  return std::all_of(name.begin(), name.end(), [](char ch) {
-    return std::isalnum(ch) != 0 || allowed.count(ch) != 0;
+  return std::all_of(name.begin(), name.end(), [allowed_chars](char ch) {
+    return std::isalnum(ch) != 0 || allowed_chars.count(ch) != 0;
   });
 }
 
+bool IsCorrectPropertyName(const std::string& name) {
+  std::unordered_set<char> allowed{'_', '-', '.'};
+  if (android::base::StartsWith(name, "ctl.")) {
+    allowed.emplace('$');
+  }
+  return IsCorrectName(name, allowed);
+}
+
+bool IsCorrectApiName(const std::string& name) {
+  static std::unordered_set<char> allowed{'_', '-'};
+  return IsCorrectName(name, allowed);
+}
+
 Result<void> ValidateProp(const sysprop::Properties& props,
                           const sysprop::Property& prop) {
-  if (!IsCorrectPropertyOrApiName(prop.api_name())) {
+  if (!IsCorrectApiName(prop.api_name())) {
     return Errorf("Invalid API name \"{}\"", prop.api_name());
   }
 
@@ -126,7 +139,7 @@ Result<void> ValidateProp(const sysprop::Properties& props,
   std::string prop_name = prop.prop_name();
   if (prop_name.empty()) prop_name = GenerateDefaultPropName(props, prop);
 
-  if (!IsCorrectPropertyOrApiName(prop_name)) {
+  if (!IsCorrectPropertyName(prop_name)) {
     return Errorf("Invalid prop name \"{}\"", prop.prop_name());
   }
 
diff --git a/tests/ApiCheckerTest.cpp b/tests/ApiCheckerTest.cpp
index 3f7b91f..af44644 100644
--- a/tests/ApiCheckerTest.cpp
+++ b/tests/ApiCheckerTest.cpp
@@ -60,7 +60,7 @@ props {
         type: Boolean
         scope: Public
         access: ReadWrite
-        prop_name: "prop3"
+        prop_name: "ctl.start$prop3"
     }
 }
 )";
@@ -89,7 +89,7 @@ props {
         type: Boolean
         scope: Public
         access: ReadWrite
-        prop_name: "prop3"
+        prop_name: "ctl.start$prop3"
     }
 }
 )";
@@ -112,7 +112,7 @@ props {
         scope: Internal
         access: Readonly
         integer_as_bool: true,
-        prop_name: "prop3"
+        prop_name: "ctl.start$prop3"
     }
 }
 )";
diff --git a/tests/InvalidSyspropTest.cpp b/tests/InvalidSyspropTest.cpp
index 199aedf..b03301c 100644
--- a/tests/InvalidSyspropTest.cpp
+++ b/tests/InvalidSyspropTest.cpp
@@ -62,6 +62,19 @@ prop {
 }
 )";
 
+constexpr const char* kInvalidPropName =
+    R"(
+owner: Vendor
+module: "vendor.module.name"
+prop {
+    api_name: "foo"
+    type: Integer
+    scope: Internal
+    access: Readonly
+    prop_name: "foo$bar"
+}
+)";
+
 constexpr const char* kEmptyEnumValues =
     R"(
 owner: Odm
@@ -104,7 +117,8 @@ constexpr const char* kInvalidNamespaceForPlatform =
 owner: Platform
 module: "android.PlatformProperties"
 prop {
-    api_name: "vendor.build.utc_long"
+    api_name: "vendor_build_utc_long"
+    prop_name: "vendor.build.utc_long"
     type: Long
     scope: Public
     access: ReadWrite
@@ -161,6 +175,7 @@ constexpr const char* kTestCasesAndExpectedErrors[][2] = {
     {kDuplicatedField, "Duplicated API name \"dup\""},
     {kEmptyProp, "There is no defined property"},
     {kInvalidApiName, "Invalid API name \"!@#$\""},
+    {kInvalidPropName, "Invalid prop name \"foo$bar\""},
     {kEmptyEnumValues, "Invalid enum value \"\" for API \"empty_enum_value\""},
     {kDuplicatedEnumValue, "Duplicated enum value \"On\" for API \"status\""},
     {kInvalidModuleName, "Invalid module name \"\""},