Merge remote-tracking branch 'md/validatefieldnames'

* md/validatefieldnames:
  fix testCreateRequest
  field names may not begin with a digit
  fail zero length field names
  validate field names

src/factory/FieldCreateFactory.cpp

@@ -947,9 +947,53 @@ StructureConstPtr FieldCreate::createStructure () const
       return createStructure(fieldNames,fields);
 }
 
+namespace {
+bool xisalnum(char c)
+{
+    return (c>='a' && c<='z') || (c>='A' && c<='Z') || (c>='0' && c<='9');
+}
+
+void validateFieldName(const std::string& n)
+{
+    // enforce [A-Za-z_][A-Za-z0-9_]*
+    if(n.size()==0)
+        throw std::invalid_argument("zero length field names not allowed");
+    if(n[0]>='0' && n[0]<='9') {
+        std::ostringstream msg;
+        msg<<"Field name \""<<n<<"\" must begin with A-Z, a-z, or '_'";
+        throw std::invalid_argument(msg.str());
+    }
+    for(size_t i=0, N=n.size(); i<N; i++)
+    {
+        char c = n[i];
+        if(xisalnum(c)) {}
+        else {
+            switch(c){
+            case '_':
+                break;
+            default:
+            {
+                std::ostringstream msg;
+                msg<<"Invalid charactor '"<<c<<"' ("<<(int)c<<") in field name \""<<n<<"\" "
+                     "must be A-Z, a-z, 0-9, or '_'";
+                throw std::invalid_argument(msg.str());
+            }
+            }
+        }
+    }
+}
+
+void validateFieldNames(const StringArray& l)
+{
+    for(StringArray::const_iterator it=l.begin(), end=l.end(); it!=end; ++it)
+        validateFieldName(*it);
+}
+}
+
 StructureConstPtr FieldCreate::createStructure (
     StringArray const & fieldNames,FieldConstPtrArray const & fields) const
 {
+      validateFieldNames(fieldNames);
       // TODO use std::make_shared
       std::tr1::shared_ptr<Structure> sp(new Structure(fieldNames,fields), Field::Deleter());
       StructureConstPtr structure = sp;
@@ -961,6 +1005,7 @@ StructureConstPtr FieldCreate::createStructure (
     StringArray const & fieldNames,
     FieldConstPtrArray const & fields) const
 {
+      validateFieldNames(fieldNames);
       // TODO use std::make_shared
       std::tr1::shared_ptr<Structure> sp(new Structure(fieldNames,fields,id), Field::Deleter());
       StructureConstPtr structure = sp;
@@ -979,6 +1024,7 @@ StructureArrayConstPtr FieldCreate::createStructureArray(
 UnionConstPtr FieldCreate::createUnion (
     StringArray const & fieldNames,FieldConstPtrArray const & fields) const
 {
+      validateFieldNames(fieldNames);
       // TODO use std::make_shared
       std::tr1::shared_ptr<Union> sp(new Union(fieldNames,fields), Field::Deleter());
       UnionConstPtr punion = sp;
@@ -990,6 +1036,7 @@ UnionConstPtr FieldCreate::createUnion (
     StringArray const & fieldNames,
     FieldConstPtrArray const & fields) const
 {
+      validateFieldNames(fieldNames);
       // TODO use std::make_shared
       std::tr1::shared_ptr<Union> sp(new Union(fieldNames,fields,id), Field::Deleter());
       UnionConstPtr punion = sp;
@@ -1039,6 +1086,7 @@ StructureConstPtr FieldCreate::appendFields(
     StringArray const & fieldNames,
     FieldConstPtrArray const & fields) const
 {
+    validateFieldNames(fieldNames);
     StringArray const & oldNames = structure->getFieldNames();
     FieldConstPtrArray const & oldFields = structure->getFields();
     size_t oldLen = oldNames.size();

testApp/copy/testCreateRequest.cpp

@@ -216,8 +216,8 @@ static void testCreateRequestInternal() {
     testPass("request %s",request.c_str());
 
     request = string("field(alarm,timeStamp,supply{")
-    	+ "0{voltage.value,current.value,power.value},"
-        + "1{voltage.value,current.value,power.value}"
+        + "zero{voltage.value,current.value,power.value},"
+        + "one{voltage.value,current.value,power.value}"
         + "})";
     if(debug) { cout  << "request " << request <<endl;}
     pvRequest = createRequest->createRequest(request);
@@ -226,12 +226,12 @@ static void testCreateRequestInternal() {
     testOk1(pvRequest.get()!=NULL);
     testOk1(pvRequest->getSubField("field.alarm").get()!=NULL);
     testOk1(pvRequest->getSubField("field.timeStamp").get()!=NULL);
-    testOk1(pvRequest->getSubField("field.supply.0.voltage.value").get()!=NULL);
-    testOk1(pvRequest->getSubField("field.supply.0.current.value").get()!=NULL);
-    testOk1(pvRequest->getSubField("field.supply.0.power.value").get()!=NULL);
-    testOk1(pvRequest->getSubField("field.supply.1.voltage.value").get()!=NULL);
-    testOk1(pvRequest->getSubField("field.supply.1.current.value").get()!=NULL);
-    testOk1(pvRequest->getSubField("field.supply.1.power.value").get()!=NULL);
+    testOk1(pvRequest->getSubField("field.supply.zero.voltage.value").get()!=NULL);
+    testOk1(pvRequest->getSubField("field.supply.zero.current.value").get()!=NULL);
+    testOk1(pvRequest->getSubField("field.supply.zero.power.value").get()!=NULL);
+    testOk1(pvRequest->getSubField("field.supply.one.voltage.value").get()!=NULL);
+    testOk1(pvRequest->getSubField("field.supply.one.current.value").get()!=NULL);
+    testOk1(pvRequest->getSubField("field.supply.one.power.value").get()!=NULL);
     testPass("request %s",request.c_str());
 
     request = string("record[process=true,xxx=yyy]")

testApp/pv/testPVData.cpp

@@ -65,6 +65,25 @@ static void testCreatePVStructure()
     std::cout << "testCreatePVStructure PASSED" << std::endl;
 }
 
+static void testCreatePVStructureWithInvalidName()
+{
+    testDiag("testCreatePVStructureWithInvalidName");
+    StringArray fieldNames;
+    fieldNames.push_back("ok");
+    fieldNames.push_back("this.is-wrong");
+    PVFieldPtrArray pvFields;
+    pvFields.push_back(pvDataCreate->createPVScalar(pvString));
+    pvFields.push_back(pvDataCreate->createPVScalar(pvInt));
+    try{
+        PVStructurePtr pvParent = pvDataCreate->createPVStructure(
+            fieldNames,pvFields);
+        testFail("Creation of invalid field name '%s' was allowed", fieldNames[1].c_str());
+    } catch(std::invalid_argument& e) {
+        testDiag("Exception: \"%s\"", e.what());
+        testPass("Creation of invalid field name '%s' fails as expected", fieldNames[1].c_str());
+    }
+}
+
 static void testPVScalarCommon(string /*fieldName*/,ScalarType stype)
 {
     PVScalarPtr pvScalar = pvDataCreate->createPVScalar(stype);
@@ -641,13 +660,14 @@ static void testFieldAccess()
 
 MAIN(testPVData)
 {
-    testPlan(242);
+    testPlan(243);
     fieldCreate = getFieldCreate();
     pvDataCreate = getPVDataCreate();
     standardField = getStandardField();
     standardPVField = getStandardPVField();
     convert = getConvert();
     testCreatePVStructure();
+    testCreatePVStructureWithInvalidName();
     testPVScalar();
     testScalarArray();
     testRequest();