Ack, review only. Will push on Nguyen's behalf.
On Fri, 16 Feb 2018 at 7:29 pm, Ravi Sekhar Reddy Konda < ravisekhar.ko...@oracle.com> wrote: > Hi Nguyen, > > Ack, code review only > > Regards, > Ravi > > -----Original Message----- > From: Nguyen Luu [mailto:nguyen.tk....@dektech.com.au] > Sent: Tuesday, February 13, 2018 9:34 AM > To: hans.nordeb...@ericsson.com; gary....@dektech.com.au; > minh.c...@dektech.com.au; ravisekhar.ko...@oracle.com > Cc: opensaf-devel@lists.sourceforge.net; Nguyen Luu < > nguyen.tk....@dektech.com.au> > Subject: [PATCH 1/1] amf: Validate env variable format set in > comptype/comp objects [#2409] > > Valid environment variable should have the format 'var=value'. > This validation shall now be done by AMF during CCB operations on > SaAmfCompType (CREATE) and SaAmfComp (CREATE/MODIFY) objects regarding the > saAmfxxxCmdEnv attribute. > --- > src/amf/amfd/comp.cc | 59 > +++++++++++++++++++++++++++++++++++++++++++++++- > src/amf/amfd/comp.h | 2 ++ > src/amf/amfd/comptype.cc | 27 +++++++++++++++++++--- > 3 files changed, 84 insertions(+), 4 deletions(-) > > diff --git a/src/amf/amfd/comp.cc b/src/amf/amfd/comp.cc index > 482322d..10fb1c3 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,27 @@ 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()); > + /* NOTE: We shall only fail the env variable format validation at > CCB- > + * CREATE operation, but not during initial config read, so as to > avoid > + * breaking systems with invalid env variables pre-existing in > IMM */ > + if (opdata != nullptr) > + return 0; > + } > + } // 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 +1080,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..aeaeaae 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,28 @@ 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()); > + /* NOTE: We shall only fail the env variable format validation at > CCB- > + * CREATE operation, but not during initial config read, so as to > avoid > + * breaking systems with invalid env variables pre-existing in > IMM */ > + if (opdata != nullptr) > + return false; > + } > + } // for (...; i < num_of_cmd_env;...) } > + > return true; > } > > -- > 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 > ------------------------------------------------------------------------------ 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
