diff --git a/src/factory/FieldCreateFactory.cpp b/src/factory/FieldCreateFactory.cpp index 76eb7f1..7551315 100644 --- a/src/factory/FieldCreateFactory.cpp +++ b/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 \""< 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 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 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 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(); diff --git a/testApp/copy/testCreateRequest.cpp b/testApp/copy/testCreateRequest.cpp index d183bb1..616c6d1 100644 --- a/testApp/copy/testCreateRequest.cpp +++ b/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 <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]") diff --git a/testApp/pv/testPVData.cpp b/testApp/pv/testPVData.cpp index 1c83f88..6d061e8 100644 --- a/testApp/pv/testPVData.cpp +++ b/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();