Valid environment variable should have the format 'var=value'.

AMF currently does not validate this format during CREATE CCBs
for comptype and comp objects (MODIFY allowed for comp after #2255)
related to saAmfxxxCmdEnv attribute.
Besides, the existing validation in avnd_comp_clc_cmd_execute()
is not good enough. This results in invalid env variables getting
printed with weird output.

Also, there is currently no check for duplicate env variables set
in both comptype and comp, so they get printed twice in trace.

This ticket fixes the above issues:
- Validate env variable format during comp[type] CREATE, MODIFY CCBs.
- Overwrite env variable set in comptype if it is also set in comp.
---
 src/amf/amfd/comp.cc     | 59 ++++++++++++++++++++++++++++++++++++++++++++++++
 src/amf/amfd/comptype.cc | 33 ++++++++++++++++++++++++---
 src/amf/amfnd/clc.cc     |  6 -----
 src/amf/amfnd/compdb.cc  | 44 +++++++++++++++++++++++++++++++-----
 4 files changed, 127 insertions(+), 15 deletions(-)

diff --git a/src/amf/amfd/comp.cc b/src/amf/amfd/comp.cc
index d4b51a6..2840e36 100644
--- a/src/amf/amfd/comp.cc
+++ b/src/amf/amfd/comp.cc
@@ -339,6 +339,8 @@ static int is_config_valid(const std::string &dn,
                            CcbUtilOperationData_t *opdata) {
   SaAisErrorT rc;
   SaNameT aname;
+  unsigned int num_of_cmd_env;
+  std::string cmd_env;
   std::string::size_type pos;
   SaUint32T value;
 
@@ -399,6 +401,36 @@ static int is_config_valid(const std::string &dn,
     return 0;
   }
 
+  if 
((immutil_getAttrValuesNumber(const_cast<SaImmAttrNameT>("saAmfCompCmdEnv"),
+                                   attributes, &num_of_cmd_env)) == SA_AIS_OK)
+  {
+    for (unsigned int i = 0; i < num_of_cmd_env; i++) {
+      cmd_env = immutil_getStringAttr(attributes, "saAmfCompCmdEnv", i);
+      osafassert(cmd_env.c_str());
+
+      /* env variable format with 'whitespace' is considered as invalid */
+      if (cmd_env.find_first_of(' ') != std::string::npos) {
+        report_ccb_validation_error(opdata, "Unknown enviroment variable 
format '%s' for '%s'."
+                                    " Should be 'var=value'", cmd_env.c_str(), 
dn.c_str());
+        return false;
+      }
+
+      std::size_t equalPos = cmd_env.find_first_of('=');
+      unsigned int equal_sign = 0;
+
+      while (equalPos != std::string::npos) {
+        equal_sign++;
+        equalPos = cmd_env.find_first_of('=', equalPos + 1);
+      }
+      /* env variable format with none or more than one '=' is considered as 
invalid */
+      if (equal_sign != 1) {
+        report_ccb_validation_error(opdata, "Unknown enviroment variable 
format '%s' for '%s'."
+                                    " Should be 'var=value'", cmd_env.c_str(), 
dn.c_str());
+        return false;
+      }
+         }
+  }
+
 #if 0
         if ((comp->comp_info.category == AVSV_COMP_TYPE_SA_AWARE) && 
(comp->comp_info.init_len == 0)) {
                 LOG_ER("Sa Aware Component: instantiation command not 
configured");
@@ -1035,6 +1067,33 @@ static SaAisErrorT 
ccb_completed_modify_hdlr(CcbUtilOperationData_t *opdata) {
             opdata, "Modification of saAmfCompCmdEnv failed, nullptr arg");
         goto done;
       }
+      for (unsigned index = 0; index < attribute->attrValuesNumber; index++) {
+        std::string mod_comp_env = *(static_cast<char 
**>(attribute->attrValues[index]));
+        osafassert(mod_comp_env.c_str());
+
+        /* env variable format with 'whitespace' is considered as invalid */
+        if (mod_comp_env.find_first_of(' ') != std::string::npos) {
+          report_ccb_validation_error(opdata, "Modification of saAmfCompCmdEnv 
failed."
+                                      " Unknown enviroment variable format 
'%s' for '%s'."
+                                      " Should be 'var=value'", 
mod_comp_env.c_str(),
+                                      
osaf_extended_name_borrow(&opdata->objectName));
+          goto done;
+        }
+        std::size_t equalPos = mod_comp_env.find_first_of('=');
+        unsigned int equal_sign = 0;
+        while (equalPos != std::string::npos) {
+          equal_sign++;
+          equalPos = mod_comp_env.find_first_of('=', equalPos + 1);
+        }
+        /* env variable format with none or more than one '=' is considered as 
invalid */
+        if (equal_sign != 1) {
+          report_ccb_validation_error(opdata, "Modification of saAmfCompCmdEnv 
failed."
+                                      " Unknown enviroment variable format 
'%s' for '%s'."
+                                      " Should be 'var=value'", 
mod_comp_env.c_str(),
+                                      
osaf_extended_name_borrow(&opdata->objectName));
+          goto done;
+        }
+      }
     } else if (!strcmp(attribute->attrName, "saAmfCompInstantiateCmdArgv")) {
       if (value_is_deleted == true) continue;
       char *param_val = *((char **)value);
diff --git a/src/amf/amfd/comptype.cc b/src/amf/amfd/comptype.cc
index 3d2636e..08e8b24 100644
--- a/src/amf/amfd/comptype.cc
+++ b/src/amf/amfd/comptype.cc
@@ -95,9 +95,6 @@ static AVD_COMP_TYPE *comptype_create(const std::string &dn,
   (void)immutil_getAttr(const_cast<SaImmAttrNameT>("saAmfCtSwBundle"),
                         attributes, 0, &ct_sw_bundle);
   compt->saAmfCtSwBundle = Amf::to_string(&ct_sw_bundle);
-  if ((str = immutil_getStringAttr(attributes, "saAmfCtDefCmdEnv", 0)) !=
-      nullptr)
-    strcpy(compt->saAmfCtDefCmdEnv, str);
   (void)immutil_getAttr(const_cast<SaImmAttrNameT>("saAmfCtDefClcCliTimeout"),
                         attributes, 0, &compt->saAmfCtDefClcCliTimeout);
   
(void)immutil_getAttr(const_cast<SaImmAttrNameT>("saAmfCtDefCallbackTimeout"),
@@ -203,8 +200,10 @@ static bool config_is_valid(const std::string &dn,
   SaUint32T value;
   SaTimeT time;
   SaAisErrorT rc;
+  unsigned int num_of_cmd_env;
   const char *cmd;
   const char *attr_name;
+  std::string cmd_env;
   std::string::size_type pos;
 
   if ((pos = dn.find(',')) == std::string::npos) {
@@ -394,6 +393,34 @@ static bool config_is_valid(const std::string &dn,
     return false;
   }
 
+  if 
((immutil_getAttrValuesNumber(const_cast<SaImmAttrNameT>("saAmfCtDefCmdEnv"),
+                                   attributes, &num_of_cmd_env)) == SA_AIS_OK)
+  {
+    for (unsigned int i = 0; i < num_of_cmd_env; i++) {
+      cmd_env = immutil_getStringAttr(attributes, "saAmfCtDefCmdEnv", i);
+      osafassert(cmd_env.c_str());
+
+      /* env variable format with 'whitespace' is considered as invalid */
+      if (cmd_env.find_first_of(' ') != std::string::npos) {
+        report_ccb_validation_error(opdata, "Unknown enviroment variable 
format '%s' for '%s'."
+                                    " Should be 'var=value'", cmd_env.c_str(), 
dn.c_str());
+        return false;
+      }
+      std::size_t equalPos = cmd_env.find_first_of('=');
+      unsigned int equal_sign = 0;
+      while (equalPos != std::string::npos) {
+        equal_sign++;
+        equalPos = cmd_env.find_first_of('=', equalPos + 1);
+      }
+      /* env variable format with none or more than one '=' is considered as 
invalid */
+      if (equal_sign != 1) {
+        report_ccb_validation_error(opdata, "Unknown enviroment variable 
format '%s' for '%s'."
+                                    " Should be 'var=value'", cmd_env.c_str(), 
dn.c_str());
+        return false;
+      }
+    }
+  }
+
   return true;
 }
 
diff --git a/src/amf/amfnd/clc.cc b/src/amf/amfnd/clc.cc
index c317f09..b3ba54d 100644
--- a/src/amf/amfnd/clc.cc
+++ b/src/amf/amfnd/clc.cc
@@ -3037,12 +3037,6 @@ uint32_t avnd_comp_clc_cmd_execute(AVND_CB *cb, 
AVND_COMP *comp,
   if (comp->saAmfCompCmdEnv != nullptr) {
     while ((env = comp->saAmfCompCmdEnv[env_counter]) != nullptr) {
       char *equalPos = strchr(env, '=');
-      if (equalPos == nullptr) {
-        LOG_ER("Unknown enviroment variable format '%s'. Should be 
'var=value'",
-               env);
-        env_counter++;
-        continue;
-      }
       env_set[env_counter].name = strndup(env, equalPos - env);
       env_set[env_counter].value = strdup(equalPos + 1);
       env_set[env_counter].overwrite = 1;
diff --git a/src/amf/amfnd/compdb.cc b/src/amf/amfnd/compdb.cc
index cf6e3ff..3663c35 100644
--- a/src/amf/amfnd/compdb.cc
+++ b/src/amf/amfnd/compdb.cc
@@ -1257,7 +1257,7 @@ static int comp_init(AVND_COMP *comp, const 
SaImmAttrValuesT_2 **attributes) {
   unsigned int num_of_comp_env = 0;
   unsigned int num_of_ct_env = 0;
   unsigned int env_cntr = 0;
-  const char *str;
+  unsigned int compt_env_cntr = 0;
   SaStringT env;
   SaImmHandleT immOmHandle;
   SaVersionT immVersion = {'A', 2, 15};
@@ -1410,12 +1410,44 @@ static int comp_init(AVND_COMP *comp, const 
SaImmAttrValuesT_2 **attributes) {
     env_cntr++;
   }
 
+  /* Get the last index in the saAmfCompCmdEnv array, */
+  /* up to which env variables from comp type are stored */
+  if (env_cntr > 0) {
+    compt_env_cntr = env_cntr;
+  }
+
   /* Get environment variables from our IMM comp object */
-  for (i = 0; i < num_of_comp_env; i++, env_cntr++) {
-    str = immutil_getStringAttr(attributes, "saAmfCompCmdEnv", i);
-    osafassert(str);
-    comp->saAmfCompCmdEnv[env_cntr] = StrDup(str);
-    osafassert(comp->saAmfCompCmdEnv[env_cntr]);
+  for (i = 0; i < num_of_comp_env; i++) {
+    std::string comp_env = immutil_getStringAttr(attributes,
+                                                 "saAmfCompCmdEnv", i);
+    osafassert(comp_env.c_str());
+    /* check for duplicate env variables set in both comp type and comp */
+    bool duplicate = false;
+
+    for (unsigned int j = 0; j < compt_env_cntr; j++) {
+      std::string compt_env = comp->saAmfCompCmdEnv[j];
+      std::size_t equalPos = compt_env.find_first_of('=');
+      std::string compt_env_name = compt_env.substr(0, equalPos);
+
+      /* If the same env variable is set in comp object, */
+      /* overwrite its default value set in comp type */
+      equalPos = comp_env.find_first_of('=');
+      std::string comp_env_name = comp_env.substr(0, equalPos);
+
+      if (comp_env_name.compare(compt_env_name) == 0) {
+        delete [] comp->saAmfCompCmdEnv[j];
+        comp->saAmfCompCmdEnv[j] = StrDup(comp_env.c_str());
+        (comp->numOfCompCmdEnv)--;
+        duplicate = true;
+        break;
+      }
+    }
+
+    if (!duplicate) {
+      comp->saAmfCompCmdEnv[env_cntr] = StrDup(comp_env.c_str());
+      osafassert(comp->saAmfCompCmdEnv[env_cntr]);
+      env_cntr++;
+    }
   }
 
   /* The env string array will be terminated by zero due to the c++
-- 
2.7.4


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to