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     | 54 +++++++++++++++++++++++++++++++++++++++++++++++-
 src/amf/amfd/comp.h      |  2 ++
 src/amf/amfd/comptype.cc | 22 +++++++++++++++++---
 src/amf/amfnd/clc.cc     |  7 +------
 src/amf/amfnd/compdb.cc  | 42 ++++++++++++++++++++++++++++++-------
 5 files changed, 110 insertions(+), 17 deletions(-)

diff --git a/src/amf/amfd/comp.cc b/src/amf/amfd/comp.cc
index 482322d..8805029 100644
--- a/src/amf/amfd/comp.cc
+++ b/src/amf/amfd/comp.cc
@@ -1,7 +1,7 @@
 /*      -*- OpenSAF  -*-
  *
  * (C) Copyright 2008 The OpenSAF Foundation
- * (C) Copyright 2017 Ericsson AB - All Rights Reserved.
+ * (C) Copyright 2017, 2018 Ericsson AB. All rights reserved.
  * Copyright (C) 2017, Oracle and/or its affiliates. All rights reserved.
  *
  * This program is distributed in the hope that it will be useful, but
@@ -328,6 +328,26 @@ done:
   TRACE_LEAVE();
 }
 
+/*
+ * Validate the component CmdEnv attribute
+ *
+ * @param const std::string
+ *
+ * @return bool
+ */
+bool is_cmd_env_valid(const std::string &cmd_env_var) {
+  /* following environment variable format is considered as invalid:
+   * - containing 'whitespace'
+   * - having none or more than one '='
+   */
+  if ((cmd_env_var.find_first_of(' ') != std::string::npos) ||
+      (std::count(cmd_env_var.begin(), cmd_env_var.end(), '=') != 1)) {
+    return false;
+  }
+
+  return true;
+}
+
 /**
  * Validate configuration attributes for an AMF Comp object
  * @param comp
@@ -339,6 +359,7 @@ static int is_config_valid(const std::string &dn,
                            CcbUtilOperationData_t *opdata) {
   SaAisErrorT rc;
   SaNameT aname;
+  unsigned int num_of_cmd_env;
   std::string::size_type pos;
   SaUint32T value;
 
@@ -399,6 +420,22 @@ 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++) {
+      std::string cmd_env = immutil_getStringAttr(attributes,
+                                                  "saAmfCompCmdEnv", i);
+
+      if (!is_cmd_env_valid(cmd_env)) {
+        report_ccb_validation_error(opdata, "Unknown environment variable 
format"
+                                    " '%s' for '%s'. Should be 'var=value'",
+                                    cmd_env.c_str(), dn.c_str());
+        return false;
+      }
+    } // for (...; i < num_of_cmd_env;...)
+  }
+
 #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");
@@ -1038,6 +1075,21 @@ 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]));
+
+        if (!is_cmd_env_valid(mod_comp_env)) {
+          report_ccb_validation_error(opdata, "Modification of saAmfCompCmdEnv"
+                                      " failed. Unknown environment 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/comp.h b/src/amf/amfd/comp.h
index 1493d71..3544a3a 100644
--- a/src/amf/amfd/comp.h
+++ b/src/amf/amfd/comp.h
@@ -1,6 +1,7 @@
 /*      -*- OpenSAF  -*-
  *
  * (C) Copyright 2008 The OpenSAF Foundation
+ * (C) Copyright 2017, 2018 Ericsson AB. All rights reserved.
  *
  * This program is distributed in the hope that it will be useful, but
  * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
@@ -295,4 +296,5 @@ extern SaAisErrorT check_comp_stability(const AVD_COMP *);
 extern AVD_CTCS_TYPE *get_ctcstype(const std::string &comptype_name,
                                    const std::string &cstype_name);
 extern void comp_ccb_apply_delete_hdlr(struct CcbUtilOperationData *opdata);
+extern bool is_cmd_env_valid(const std::string &cmd_env_var);
 #endif  // AMF_AMFD_COMP_H_
diff --git a/src/amf/amfd/comptype.cc b/src/amf/amfd/comptype.cc
index b6d4d6d..5be8f20 100644
--- a/src/amf/amfd/comptype.cc
+++ b/src/amf/amfd/comptype.cc
@@ -1,6 +1,7 @@
 /*      -*- OpenSAF  -*-
  *
  * (C) Copyright 2008 The OpenSAF Foundation
+ * (C) Copyright 2017, 2018 Ericsson AB. All rights reserved.
  * Copyright (C) 2017, Oracle and/or its affiliates. All rights reserved.
  *
  * This program is distributed in the hope that it will be useful, but
@@ -95,9 +96,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,6 +201,7 @@ 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::size_type pos;
@@ -394,6 +393,23 @@ 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++) {
+      std::string cmd_env = immutil_getStringAttr(attributes,
+                                                  "saAmfCtDefCmdEnv", i);
+
+      if (!is_cmd_env_valid(cmd_env)) {
+        report_ccb_validation_error(opdata, "Unknown environment variable 
format"
+                                    " '%s' for '%s'. Should be 'var=value'",
+                                    cmd_env.c_str(), dn.c_str());
+        return false;
+      }
+    } // for (...; i < num_of_cmd_env;...)
+  }
+
   return true;
 }
 
diff --git a/src/amf/amfnd/clc.cc b/src/amf/amfnd/clc.cc
index c8e60e6..72b7c82 100644
--- a/src/amf/amfnd/clc.cc
+++ b/src/amf/amfnd/clc.cc
@@ -1,6 +1,7 @@
 /*      -*- OpenSAF  -*-
  *
  * (C) Copyright 2008 The OpenSAF Foundation
+ * (C) Copyright 2017, 2018 Ericsson AB. All rights reserved.
  * Copyright (C) 2017, Oracle and/or its affiliates. All rights reserved.
  *
  * This program is distributed in the hope that it will be useful, but
@@ -3037,12 +3038,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 7f78194..aeb84d0 100644
--- a/src/amf/amfnd/compdb.cc
+++ b/src/amf/amfnd/compdb.cc
@@ -1,7 +1,7 @@
 /*      -*- OpenSAF  -*-
  *
  * (C) Copyright 2008 The OpenSAF Foundation
- * (C) Copyright 2017 Ericsson AB - All Rights Reserved.
+ * (C) Copyright 2017, 2018 Ericsson AB. All rights reserved.
  * Copyright (C) 2017, Oracle and/or its affiliates. All rights reserved.
  *
  * This program is distributed in the hope that it will be useful, but
@@ -1275,7 +1275,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};
@@ -1428,12 +1428,40 @@ 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);
+    /* 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];
+
+      /* If the same env variable is set in comp object, */
+      /* overwrite its default value set in comp type */
+      std::size_t equalPos = comp_env.find_first_of('=');
+
+      if (!comp_env.compare(0, equalPos + 1, compt_env, 0, equalPos + 1)) {
+        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