Validate define node values (#1166)

soumeh01 · web-flow · commit 175941acd140 · 2025-02-24T16:21:53.000+01:00
diff --git a/tools/projmgr/include/ProjMgrYamlParser.h b/tools/projmgr/include/ProjMgrYamlParser.h
@@ -275,7 +275,7 @@ class ProjMgrYamlParser {
 protected:
   bool ParseCbuildPack(const std::string& input, CbuildPackItem& cbuildPack, bool checkSchema);
   void ParseMisc(const YAML::Node& parent, std::vector<MiscItem>& misc);
-  void ParseDefine(const YAML::Node& defineNode, std::vector<std::string>& define);
+  bool ParseDefine(const YAML::Node& defineNode, std::vector<std::string>& define);
   void ParsePacks(const YAML::Node& parent, const std::string& file, std::vector<PackItem>& packs);
   void ParseResolvedPacks(const YAML::Node& parent, std::vector<ResolvedPackItem>& resolvedPacks);
   void ParseProcessor(const YAML::Node& parent, ProcessorItem& processor);
diff --git a/tools/projmgr/schemas/common.schema.json b/tools/projmgr/schemas/common.schema.json
@@ -361,9 +361,19 @@
         "oneOf": [
           {
             "type": "object",
-            "properties": {
-              "key": { "type": "string" },
-              "value": { "type": ["number", "string"], "minLength": 1 }
+            "patternProperties": {
+              "^.*$": {
+                "oneOf": [
+                  {
+                    "type": "string",
+                    "pattern": "^([^=:]+)$",
+                    "minLength": 1
+                  },
+                  {
+                    "type": "number"
+                  }
+                ]
+              }
             }
           },
           {
@@ -383,9 +393,19 @@
         "oneOf": [
           {
             "type": "object",
-            "properties": {
-              "key": { "type": "string" },
-              "value": { "type": ["number", "string"], "minLength": 1 }
+            "patternProperties": {
+              "^.*$": {
+                "oneOf": [
+                  {
+                    "type": "string",
+                    "pattern": "^([^=:]+)$",
+                    "minLength": 1
+                  },
+                  {
+                    "type": "number"
+                  }
+                ]
+              }
             }
           },
           {
@@ -405,9 +425,19 @@
         "oneOf": [
           {
             "type": "object",
-            "properties": {
-              "key": { "type": "string" },
-              "value": { "type": ["number", "string"], "minLength": 1 }
+            "patternProperties": {
+              "^.*$": {
+                "oneOf": [
+                  {
+                    "type": "string",
+                    "pattern": "^([^=:]+)$",
+                    "minLength": 1
+                  },
+                  {
+                    "type": "number"
+                  }
+                ]
+              }
             }
           },
           {
diff --git a/tools/projmgr/src/ProjMgrYamlParser.cpp b/tools/projmgr/src/ProjMgrYamlParser.cpp
@@ -391,7 +391,43 @@ void ProjMgrYamlParser::ParseVector(const YAML::Node& parent, const string& key,
   }
 }
 
-void ProjMgrYamlParser::ParseDefine(const YAML::Node& defineNode, vector<string>& define) {
+bool ProjMgrYamlParser::ParseDefine(const YAML::Node& defineNode, vector<string>& define) {
+  auto ValidateDefineStr = [&](const std::string& defineValue) -> bool {
+    if (defineValue.empty()) {
+      return true;
+    }
+
+    const std::string escapedQuote = "\\\"";
+    int numQuotes = RteUtils::CountDelimiters(defineValue, "\"");
+    if (numQuotes == 0) {
+      return true;
+    }
+
+    bool isValid = true;
+    string errStr;
+    if (numQuotes == 2) {
+      if (defineValue.front() == '"') {
+        isValid = (defineValue.back() == '"');
+      }
+      else if (defineValue.substr(0, 2) == escapedQuote) {
+        isValid = (defineValue.substr(defineValue.size() - 2, 2) == escapedQuote);
+      }
+      else {
+        isValid = false;
+      }
+    }
+    else {
+      isValid = false;
+    }
+
+    if (!isValid) {
+      ProjMgrLogger::Get().Error("invalid define: " + defineValue + ", improper quotes");
+    }
+
+    return isValid;
+  };
+
+  bool success = true;
   if (defineNode.IsDefined() && defineNode.IsSequence()) {
     for (const auto& item : defineNode) {
       // accept map elements <string, string> or a string
@@ -402,15 +438,22 @@ void ProjMgrYamlParser::ParseDefine(const YAML::Node& defineNode, vector<string>
             if (YAML::IsNullString(element.second)) {
               element.second = "";
             }
-            define.push_back(element.first + "=" + element.second);
+            success = ValidateDefineStr(element.second);
+            if (success) {
+              define.push_back(element.first + "=" + element.second);
+            }
           }
         }
         else {
-          define.push_back(item.as<string>());
+          success = ValidateDefineStr(item.as<string>());
+          if (success) {
+            define.push_back(item.as<string>());
+          }
         }
       }
     }
   }
+  return success;
 }
 
 void ProjMgrYamlParser::ParseVectorOfStringPairs(const YAML::Node& parent, const string& key, vector<pair<string, string>>& value) {
@@ -869,7 +912,9 @@ bool ProjMgrYamlParser::ParseLinker(const YAML::Node& parent, const string& file
         return false;
       }
       linkerItem.autoGen = linkerEntry[YAML_AUTO].IsDefined();
-      ParseDefine(linkerEntry[YAML_DEFINE], linkerItem.defines);
+      if (!ParseDefine(linkerEntry[YAML_DEFINE], linkerItem.defines)) {
+        return false;
+      }
       ParseVectorOrString(linkerEntry, YAML_FORCOMPILER, linkerItem.forCompiler);
       ParsePortablePath(linkerEntry, file, YAML_REGIONS, linkerItem.regions);
       ParsePortablePath(linkerEntry, file, YAML_SCRIPT, linkerItem.script);
@@ -918,8 +963,12 @@ bool ProjMgrYamlParser::ParseBuildType(const YAML::Node& parent, const string& f
   }
   ParseProcessor(parent, buildType.processor);
   ParseMisc(parent, buildType.misc);
-  ParseDefine(parent[YAML_DEFINE], buildType.defines);
-  ParseDefine(parent[YAML_DEFINE_ASM], buildType.definesAsm);
+  if (!ParseDefine(parent[YAML_DEFINE], buildType.defines)) {
+    return false;
+  }
+  if (!ParseDefine(parent[YAML_DEFINE_ASM], buildType.definesAsm)) {
+    return false;
+  }
   ParseVector(parent, YAML_UNDEFINE, buildType.undefines);
   ParsePortablePaths(parent, file, YAML_ADDPATH, buildType.addpaths);
   ParsePortablePaths(parent, file, YAML_ADDPATH_ASM, buildType.addpathsAsm);
diff --git a/tools/projmgr/test/data/TestSolution/test_invalid_defines.csolution.yml b/tools/projmgr/test/data/TestSolution/test_invalid_defines.csolution.yml
@@ -0,0 +1,40 @@
+# yaml-language-server: $schema=https://raw.githubusercontent.com/Open-CMSIS-Pack/devtools/main/tools/projmgr/schemas/csolution.schema.json
+
+solution:
+  description: test description string
+  target-types:
+    - type: CM0
+      device: RteTest_ARMCM0
+
+  build-types:
+    - type: Debug
+      compiler: AC6
+
+  packs:
+    - pack: ARM::RteTest_DFP@0.2.0
+
+  projects:
+    - project: ./TestProject1/test1.cproject.yml
+
+  define:
+    - __SAME70Q21__
+    - __test__
+    - "Test 1"
+    - \"Test 3\"
+    - Test4
+    - KEY1: 1
+    - KEY2: "valid"
+    - KEY3: \"valid\"
+    - \"No_ending_escape_quotes
+    - Escape_quotes_in_\"middle\"
+    - \"Invalid_ending"\
+    - KEY4: \"No_ending_escape_quotes
+    - KEY5: \"sam.h\
+    - KEY6: Invalid_equal=character
+    - KEY7: Invalid_colon:character
+    - KEY8: \"Invalid_ending"\
+    - KEY9: No_Starting_escaped_quotes\"
+    - KEY10: \"Mixed_quotes"
+
+  define-asm:
+    - DEF_ASM_SOLUTION
diff --git a/tools/projmgr/test/src/ProjMgrUnitTests.cpp b/tools/projmgr/test/src/ProjMgrUnitTests.cpp
@@ -6679,3 +6679,39 @@ TEST_F(ProjMgrUnitTests, TestRunDebug) {
   ProjMgrTestEnv::CompareFile(testoutput_folder + "/run-debug+TestHW2.cbuild.yml",
     testinput_folder + "/TestRunDebug/ref/run-debug+TestHW2.cbuild.yml");
 }
+
+TEST_F(ProjMgrUnitTests, Test_Check_Define_Value_With_Quotes) {
+  StdStreamRedirect streamRedirect;
+  char* argv[6];
+  const string& csolution = testinput_folder + "/TestSolution/test_invalid_defines.csolution.yml";
+  argv[1] = (char*)"convert";
+  argv[2] = (char*)csolution.c_str();
+  argv[3] = (char*)"-o";
+  argv[4] = (char*)testoutput_folder.c_str();
+
+  // Test1: Check schema error
+  string expected = "\
+.*test_invalid_defines.csolution.yml:33:7 - error csolution: schema check failed, verify syntax\n\
+.*test_invalid_defines.csolution.yml:34:7 - error csolution: schema check failed, verify syntax\n\
+";
+  EXPECT_EQ(1, RunProjMgr(5, argv, m_envp));
+  string errStr = streamRedirect.GetErrorString();
+  EXPECT_TRUE(regex_search(errStr, regex(expected)));
+
+  //Test2: Check Parsing errors
+  streamRedirect.ClearStringStreams();
+  expected = "\
+error csolution: invalid define: \\\"No_ending_escape_quotes, improper quotes\n\
+error csolution: invalid define: Escape_quotes_in_\\\"middle\\\", improper quotes\n\
+error csolution: invalid define: \\\"Invalid_ending\"\\, improper quotes\n\
+error csolution: invalid define: \\\"No_ending_escape_quotes, improper quotes\n\
+error csolution: invalid define: \\\"sam.h\\, improper quotes\n\
+error csolution: invalid define: \\\"Invalid_ending\"\\, improper quotes\n\
+error csolution: invalid define: No_Starting_escaped_quotes\\\", improper quotes\n\
+error csolution: invalid define: \\\"Mixed_quotes\", improper quotes\n\
+";
+  argv[5] = (char*)"-n";
+  EXPECT_EQ(1, RunProjMgr(6, argv, m_envp));
+  errStr = streamRedirect.GetErrorString();
+  EXPECT_EQ(errStr, expected);
+}
